Re: [PATCH AUTOSEL 6.7 15/39] power: supply: qcom_battmgr: Register the power supplies after PDR is up

Hi,

On Sun, Jan 28, 2024 at 11:10:35AM -0500, Sasha Levin wrote:

From: Konrad Dybcio konrad.dybcio@linaro.org

[ Upstream commit b43f7ddc2b7a5a90447d96cb4d3c6d142dd4a810 ]

Currently, a not-yet-entirely-initialized battmgr (e.g. with pd-mapper not having yet started or ADSP not being up etc.) results in a couple of zombie power supply devices hanging around.

This is particularly noticeable when trying to suspend the device (even s2idle): the PSY-internal thermal zone is inaccessible and returns -ENODEV, which causes log spam.

Register the power supplies only after we received some notification indicating battmgr is ready to take off.

Signed-off-by: Konrad Dybcio konrad.dybcio@linaro.org Tested-by: Luca Weiss luca.weiss@fairphone.com Link: https://lore.kernel.org/r/20231218-topic-battmgr_fixture_attempt-v1-1-614574... Signed-off-by: Sebastian Reichel sebastian.reichel@collabora.com Signed-off-by: Sasha Levin sashal@kernel.org

---

Please drop it, I have a patch queued reverting this patch.

-- Sebastian

drivers/power/supply/qcom_battmgr.c | 109 +++++++++++++++------------- 1 file changed, 60 insertions(+), 49 deletions(-)

diff --git a/drivers/power/supply/qcom_battmgr.c b/drivers/power/supply/qcom_battmgr.c index ec163d1bcd18..a12e2a66d516 100644 --- a/drivers/power/supply/qcom_battmgr.c +++ b/drivers/power/supply/qcom_battmgr.c @@ -282,6 +282,7 @@ struct qcom_battmgr_wireless { struct qcom_battmgr { struct device *dev;

• struct auxiliary_device *adev; struct pmic_glink_client *client;

enum qcom_battmgr_variant variant; @@ -1293,11 +1294,69 @@ static void qcom_battmgr_enable_worker(struct work_struct *work) dev_err(battmgr->dev, "failed to request power notifications\n"); } +static char *qcom_battmgr_battery[] = { "battery" };

+static void qcom_battmgr_register_psy(struct qcom_battmgr *battmgr) +{

• struct power_supply_config psy_cfg_supply = {};
• struct auxiliary_device *adev = battmgr->adev;
• struct power_supply_config psy_cfg = {};
• struct device *dev = &adev->dev;
• psy_cfg.drv_data = battmgr;
• psy_cfg.of_node = adev->dev.of_node;
• psy_cfg_supply.drv_data = battmgr;
• psy_cfg_supply.of_node = adev->dev.of_node;
• psy_cfg_supply.supplied_to = qcom_battmgr_battery;
• psy_cfg_supply.num_supplicants = 1;
• if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {

• battmgr->bat_psy = devm_power_supply_register(dev, &sc8280xp_bat_psy_desc, &psy_cfg);
  

• if (IS_ERR(battmgr->bat_psy))
  

• 	dev_err(dev, "failed to register battery power supply (%ld)\n",
  

• 		PTR_ERR(battmgr->bat_psy));
  

• battmgr->ac_psy = devm_power_supply_register(dev, &sc8280xp_ac_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->ac_psy))
  

• 	dev_err(dev, "failed to register AC power supply (%ld)\n",
  

• 		PTR_ERR(battmgr->ac_psy));
  

• battmgr->usb_psy = devm_power_supply_register(dev, &sc8280xp_usb_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->usb_psy))
  

• 	dev_err(dev, "failed to register USB power supply (%ld)\n",
  

• 		PTR_ERR(battmgr->usb_psy));
  

• battmgr->wls_psy = devm_power_supply_register(dev, &sc8280xp_wls_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->wls_psy))
  

• 	dev_err(dev, "failed to register wireless charing power supply (%ld)\n",
  

• 		PTR_ERR(battmgr->wls_psy));
  

• } else {

• battmgr->bat_psy = devm_power_supply_register(dev, &sm8350_bat_psy_desc, &psy_cfg);
  

• if (IS_ERR(battmgr->bat_psy))
  

• 	dev_err(dev, "failed to register battery power supply (%ld)\n",
  

• 		PTR_ERR(battmgr->bat_psy));
  

• battmgr->usb_psy = devm_power_supply_register(dev, &sm8350_usb_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->usb_psy))
  

• 	dev_err(dev, "failed to register USB power supply (%ld)\n",
  

• 		PTR_ERR(battmgr->usb_psy));
  

• battmgr->wls_psy = devm_power_supply_register(dev, &sm8350_wls_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->wls_psy))
  

• 	dev_err(dev, "failed to register wireless charing power supply (%ld)\n",
  

• 		PTR_ERR(battmgr->wls_psy));
  

• }

+}

static void qcom_battmgr_pdr_notify(void *priv, int state) { struct qcom_battmgr *battmgr = priv; if (state == SERVREG_SERVICE_STATE_UP) {

• if (!battmgr->bat_psy)
  

• 	qcom_battmgr_register_psy(battmgr);
  

• battmgr->service_up = true; schedule_work(&battmgr->enable_work); } else {

@@ -1312,13 +1371,9 @@ static const struct of_device_id qcom_battmgr_of_variants[] = { {} }; -static char *qcom_battmgr_battery[] = { "battery" };

static int qcom_battmgr_probe(struct auxiliary_device *adev, const struct auxiliary_device_id *id) {

• struct power_supply_config psy_cfg_supply = {};
• struct power_supply_config psy_cfg = {}; const struct of_device_id *match; struct qcom_battmgr *battmgr; struct device *dev = &adev->dev;

@@ -1328,14 +1383,7 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev, return -ENOMEM; battmgr->dev = dev;

• psy_cfg.drv_data = battmgr;
• psy_cfg.of_node = adev->dev.of_node;
• psy_cfg_supply.drv_data = battmgr;
• psy_cfg_supply.of_node = adev->dev.of_node;
• psy_cfg_supply.supplied_to = qcom_battmgr_battery;
• psy_cfg_supply.num_supplicants = 1;

• battmgr->adev = adev;

INIT_WORK(&battmgr->enable_work, qcom_battmgr_enable_worker); mutex_init(&battmgr->lock); @@ -1347,43 +1395,6 @@ static int qcom_battmgr_probe(struct auxiliary_device *adev, else battmgr->variant = QCOM_BATTMGR_SM8350;

• if (battmgr->variant == QCOM_BATTMGR_SC8280XP) {

• battmgr->bat_psy = devm_power_supply_register(dev, &sc8280xp_bat_psy_desc, &psy_cfg);
  

• if (IS_ERR(battmgr->bat_psy))
  

• 	return dev_err_probe(dev, PTR_ERR(battmgr->bat_psy),
  

• 			     "failed to register battery power supply\n");
  

• battmgr->ac_psy = devm_power_supply_register(dev, &sc8280xp_ac_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->ac_psy))
  

• 	return dev_err_probe(dev, PTR_ERR(battmgr->ac_psy),
  

• 			     "failed to register AC power supply\n");
  

• battmgr->usb_psy = devm_power_supply_register(dev, &sc8280xp_usb_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->usb_psy))
  

• 	return dev_err_probe(dev, PTR_ERR(battmgr->usb_psy),
  

• 			     "failed to register USB power supply\n");
  

• battmgr->wls_psy = devm_power_supply_register(dev, &sc8280xp_wls_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->wls_psy))
  

• 	return dev_err_probe(dev, PTR_ERR(battmgr->wls_psy),
  

• 			     "failed to register wireless charing power supply\n");
  

• } else {

• battmgr->bat_psy = devm_power_supply_register(dev, &sm8350_bat_psy_desc, &psy_cfg);
  

• if (IS_ERR(battmgr->bat_psy))
  

• 	return dev_err_probe(dev, PTR_ERR(battmgr->bat_psy),
  

• 			     "failed to register battery power supply\n");
  

• battmgr->usb_psy = devm_power_supply_register(dev, &sm8350_usb_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->usb_psy))
  

• 	return dev_err_probe(dev, PTR_ERR(battmgr->usb_psy),
  

• 			     "failed to register USB power supply\n");
  

• battmgr->wls_psy = devm_power_supply_register(dev, &sm8350_wls_psy_desc, &psy_cfg_supply);
  

• if (IS_ERR(battmgr->wls_psy))
  

• 	return dev_err_probe(dev, PTR_ERR(battmgr->wls_psy),
  

• 			     "failed to register wireless charing power supply\n");
  

• }
• battmgr->client = devm_pmic_glink_register_client(dev, PMIC_GLINK_OWNER_BATTMGR, qcom_battmgr_callback,

-- 2.43.0