Re: [PATCH v4 1/2] power_supply: Register cooling device outside of probe

[Manaf Meethalavalappu Pallikunhi]

++

On 1/25/2022 1:04 AM, Manaf Meethalavalappu Pallikunhi wrote:
> Hi Sebastian Reichel,
>
> Just reminder for review for these patches
>
> Thanks,
>
> Manaf
>
> On 12/22/2021 2:11 PM, 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@xxxxxxxxxxx>
> > ---
> >   drivers/power/supply/power_supply_core.c | 10 ++++------
> >   1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/power/supply/power_supply_core.c 
> > b/drivers/power/supply/power_supply_core.c
> > index 668369b..ef6f290 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);
> > @@ -1238,10 +1242,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;
> > @@ -1271,8 +1271,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);