| Message ID | 20230101174342.58351-1-caleb.connolly@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] power: supply: fix circular dependency with cooling device | expand |
On Sonntag, 1. Jänner 2023 18:43:40 CET Caleb Connolly wrote: > A recent change in thermal/core means it now calls the cooling device > ->get_max_state() callback during __thermal_cooling_device_register(). > This creates a circular dependency as it attempts to fetch a power > supply property before the psy is initialised. Move this call later to > break the dependency. > > Fixes: c408b3d1d9bb ("thermal: Validate new state in cur_state_store()") > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> Fixes USB on qcom-apq8026-lg-lenok (with smbb driver) Tested-by: Luca Weiss <luca@z3ntu.xyz> Regards Luca > --- > drivers/power/supply/power_supply_core.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/power/supply/power_supply_core.c > b/drivers/power/supply/power_supply_core.c index 7c790c41e2fe..c921111ff26a > 100644 > --- a/drivers/power/supply/power_supply_core.c > +++ b/drivers/power/supply/power_supply_core.c > @@ -1354,10 +1354,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; > @@ -1378,17 +1374,27 @@ __power_supply_register(struct device *parent, > atomic_inc(&psy->use_cnt); > psy->initialized = true; > > + /* This has to be done after updating use_cnt and initialized > + * otherwise when __thermal_cooling_device_register calls back > + * to ->get_max_state() the psy core will return -EAGAIN.. > + */ > + rc = psy_register_cooler(psy); > + if (rc) > + goto register_cooler_failed; > + > queue_delayed_work(system_power_efficient_wq, > &psy->deferred_register_work, > POWER_SUPPLY_DEFERRED_REGISTER_TIME); > > return psy; > > +register_cooler_failed: > + power_supply_remove_hwmon_sysfs(psy); > + psy->initialized = false; > + atomic_dec(&psy->use_cnt); > 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: > wakeup_init_failed:
Hi, On Thu, Jan 12, 2023 at 10:47:37PM +0100, Luca Weiss wrote: > On Sonntag, 1. Jänner 2023 18:43:40 CET Caleb Connolly wrote: > > A recent change in thermal/core means it now calls the cooling device > > ->get_max_state() callback during __thermal_cooling_device_register(). > > This creates a circular dependency as it attempts to fetch a power > > supply property before the psy is initialised. Move this call later to > > break the dependency. > > > > Fixes: c408b3d1d9bb ("thermal: Validate new state in cur_state_store()") > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > > Fixes USB on qcom-apq8026-lg-lenok (with smbb driver) > > Tested-by: Luca Weiss <luca@z3ntu.xyz> > > Regards > Luca I queued the following patch instead, which rips out the broken cooling support: https://lore.kernel.org/all/20230121111621.2821558-1-andreas@kemnade.info/ -- Sebastian > > > --- > > drivers/power/supply/power_supply_core.c | 18 ++++++++++++------ > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/power/supply/power_supply_core.c > > b/drivers/power/supply/power_supply_core.c index 7c790c41e2fe..c921111ff26a > > 100644 > > --- a/drivers/power/supply/power_supply_core.c > > +++ b/drivers/power/supply/power_supply_core.c > > @@ -1354,10 +1354,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; > > @@ -1378,17 +1374,27 @@ __power_supply_register(struct device *parent, > > atomic_inc(&psy->use_cnt); > > psy->initialized = true; > > > > + /* This has to be done after updating use_cnt and initialized > > + * otherwise when __thermal_cooling_device_register calls back > > + * to ->get_max_state() the psy core will return -EAGAIN.. > > + */ > > + rc = psy_register_cooler(psy); > > + if (rc) > > + goto register_cooler_failed; > > + > > queue_delayed_work(system_power_efficient_wq, > > &psy->deferred_register_work, > > POWER_SUPPLY_DEFERRED_REGISTER_TIME); > > > > return psy; > > > > +register_cooler_failed: > > + power_supply_remove_hwmon_sysfs(psy); > > + psy->initialized = false; > > + atomic_dec(&psy->use_cnt); > > 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: > > wakeup_init_failed:
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c index 7c790c41e2fe..c921111ff26a 100644 --- a/drivers/power/supply/power_supply_core.c +++ b/drivers/power/supply/power_supply_core.c @@ -1354,10 +1354,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; @@ -1378,17 +1374,27 @@ __power_supply_register(struct device *parent, atomic_inc(&psy->use_cnt); psy->initialized = true; + /* This has to be done after updating use_cnt and initialized + * otherwise when __thermal_cooling_device_register calls back + * to ->get_max_state() the psy core will return -EAGAIN.. + */ + rc = psy_register_cooler(psy); + if (rc) + goto register_cooler_failed; + queue_delayed_work(system_power_efficient_wq, &psy->deferred_register_work, POWER_SUPPLY_DEFERRED_REGISTER_TIME); return psy; +register_cooler_failed: + power_supply_remove_hwmon_sysfs(psy); + psy->initialized = false; + atomic_dec(&psy->use_cnt); 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: wakeup_init_failed:
A recent change in thermal/core means it now calls the cooling device ->get_max_state() callback during __thermal_cooling_device_register(). This creates a circular dependency as it attempts to fetch a power supply property before the psy is initialised. Move this call later to break the dependency. Fixes: c408b3d1d9bb ("thermal: Validate new state in cur_state_store()") Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> --- drivers/power/supply/power_supply_core.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)