* [PATCH AUTOSEL 6.7 15/39] power: supply: qcom_battmgr: Register the power supplies after PDR is up [not found] <20240128161130.200783-1-sashal@kernel.org> @ 2024-01-28 16:10 ` Sasha Levin 2024-01-29 13:03 ` Sebastian Reichel 0 siblings, 1 reply; 3+ messages in thread From: Sasha Levin @ 2024-01-28 16:10 UTC (permalink / raw) To: linux-kernel, stable Cc: Konrad Dybcio, Luca Weiss, Sebastian Reichel, Sasha Levin, andersson, sre, linux-arm-msm, linux-pm 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-6145745f34fe@linaro.org Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Signed-off-by: Sasha Levin <sashal@kernel.org> --- 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH AUTOSEL 6.7 15/39] power: supply: qcom_battmgr: Register the power supplies after PDR is up 2024-01-28 16:10 ` [PATCH AUTOSEL 6.7 15/39] power: supply: qcom_battmgr: Register the power supplies after PDR is up Sasha Levin @ 2024-01-29 13:03 ` Sebastian Reichel 2024-01-30 23:01 ` Sasha Levin 0 siblings, 1 reply; 3+ messages in thread From: Sebastian Reichel @ 2024-01-29 13:03 UTC (permalink / raw) To: Sasha Levin Cc: linux-kernel, stable, Konrad Dybcio, Luca Weiss, andersson, linux-arm-msm, linux-pm, Johan Hovold [-- Attachment #1: Type: text/plain, Size: 7824 bytes --] 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-6145745f34fe@linaro.org > 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 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH AUTOSEL 6.7 15/39] power: supply: qcom_battmgr: Register the power supplies after PDR is up 2024-01-29 13:03 ` Sebastian Reichel @ 2024-01-30 23:01 ` Sasha Levin 0 siblings, 0 replies; 3+ messages in thread From: Sasha Levin @ 2024-01-30 23:01 UTC (permalink / raw) To: Sebastian Reichel Cc: linux-kernel, stable, Konrad Dybcio, Luca Weiss, andersson, linux-arm-msm, linux-pm, Johan Hovold On Mon, Jan 29, 2024 at 02:03:14PM +0100, Sebastian Reichel wrote: >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-6145745f34fe@linaro.org >> 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. Dropped, thanks! -- Thanks, Sasha ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-01-30 23:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240128161130.200783-1-sashal@kernel.org>
2024-01-28 16:10 ` [PATCH AUTOSEL 6.7 15/39] power: supply: qcom_battmgr: Register the power supplies after PDR is up Sasha Levin
2024-01-29 13:03 ` Sebastian Reichel
2024-01-30 23:01 ` Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).