| Message ID | 20220531183054.6476-1-quic_manafm@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND,1/2] power_supply: Register cooling device outside of probe | expand |
Hi, On Mon, Feb 27, 2023 at 01:46:52PM -0800, Subbaraman Narayanamurthy wrote: > On 6/9/22 3:12 PM, Sebastian Reichel wrote: > > Hi, > > > > On Wed, Jun 01, 2022 at 12:00:53AM +0530, Manaf Meethalavalappu Pallikunhi wrote: > >> Registering the cooling device from the probe can result in the > >> execution of get_property() function before it gets initialized. > >> > >> To avoid this, register the cooling device from a workqueue > >> instead of registering in the probe. > >> > >> Signed-off-by: Manaf Meethalavalappu Pallikunhi <quic_manafm@quicinc.com> > >> --- > > This removes error handling from the psy_register_cooler() call, so > > it introduces a new potential problem. If power_supply_get_property() > > is called to early -EAGAIN is returned. So can you elaborate the problem > > that you are seeing with the current code? > > > > -- Sebastian > > When the device boots up with all the vendor modules getting loaded, > here is what we're seeing when booting up with 6.1.11 recently. First > log is printed with adding a pr_err() in __power_supply_register(). > > [ 7.008938][ T682] power_supply battery: psy_register_cooler failed, rc=-11 > [ 7.030941][ T682] qti_battery_charger: probe of qcom,battery_charger failed with error -11 > > Here, our downstream qti_battery_charger driver exposes the following > power supply properties POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT and > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX under a power supply device. > > This is happening because of the following call sequence, > > battery_chg_probe() -> > power_supply_register() -> > psy_register_cooler() -> > thermal_cooling_device_register() -> > cdev->ops->get_max_state() -> > ps_get_max_charge_cntl_limit() -> > power_supply_get_property() > > ends up calling power_supply_get_property() to read CHARGE_CONTROL_LIMIT > property. > > However, it returns -EAGAIN because psy->initialized is set to true > later after psy_register_cooler() succeeds. So, this ends up in a > driver probe failure forever. This should be solved in 6.3: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/power/supply/power_supply_core.c?id=c85c191694cb1cf290b11059b3d2de8a2732ffd0 -- Sebastian
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c index 385814a14a0a..74623c4977db 100644 --- a/drivers/power/supply/power_supply_core.c +++ b/drivers/power/supply/power_supply_core.c @@ -132,6 +132,7 @@ void power_supply_changed(struct power_supply *psy) } EXPORT_SYMBOL_GPL(power_supply_changed); +static int psy_register_cooler(struct power_supply *psy); /* * Notify that power supply was registered after parent finished the probing. * @@ -139,6 +140,8 @@ EXPORT_SYMBOL_GPL(power_supply_changed); * calling power_supply_changed() directly from power_supply_register() * would lead to execution of get_property() function provided by the driver * too early - before the probe ends. + * Also, registering cooling device from the probe will execute the + * get_property() function. So register the cooling device after the probe. * * Avoid that by waiting on parent's mutex. */ @@ -156,6 +159,7 @@ static void power_supply_deferred_register_work(struct work_struct *work) } power_supply_changed(psy); + psy_register_cooler(psy); if (psy->dev.parent) mutex_unlock(&psy->dev.parent->mutex); @@ -1261,10 +1265,6 @@ __power_supply_register(struct device *parent, if (rc) goto register_thermal_failed; - rc = psy_register_cooler(psy); - if (rc) - goto register_cooler_failed; - rc = power_supply_create_triggers(psy); if (rc) goto create_triggers_failed; @@ -1294,8 +1294,6 @@ __power_supply_register(struct device *parent, add_hwmon_sysfs_failed: power_supply_remove_triggers(psy); create_triggers_failed: - psy_unregister_cooler(psy); -register_cooler_failed: psy_unregister_thermal(psy); register_thermal_failed: device_del(dev);
Registering the cooling device from the probe can result in the execution of get_property() function before it gets initialized. To avoid this, register the cooling device from a workqueue instead of registering in the probe. Signed-off-by: Manaf Meethalavalappu Pallikunhi <quic_manafm@quicinc.com> --- drivers/power/supply/power_supply_core.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)