Re: [PATCH v3 2/4] power: regulator: Convert regulators_enable_boot_on/off() to regulator_post_probe

On 10/3/24 12:55 AM, Simon Glass wrote:
> Hi,
> 
> On Thu, 26 Sept 2024 at 17:15, Marek Vasut <[email protected]> wrote:
>>
>> Turn regulators_enable_boot_on() and regulators_enable_boot_off() into
>> empty functions. Implement matching functionality in regulator_post_probe()
>> instead. The regulator_post_probe() is called for all regulators after they
>> probe, and regulators that have regulator-always-on or regulator-boot-on DT
>> properties now always probe due to DM_FLAG_PROBE_AFTER_BIND being set on
>> such regulators in regulator_post_bind().
>>
>> Finally, fold regulator_unset() functionality into regulator_autoset().
>>
>> Signed-off-by: Marek Vasut <[email protected]>
>> ---
>> Cc: Ben Wolsieffer <[email protected]>
>> Cc: Caleb Connolly <[email protected]>
>> Cc: Chris Morgan <[email protected]>
>> Cc: Dragan Simic <[email protected]>
>> Cc: Eugen Hristev <[email protected]>
>> Cc: Francesco Dolcini <[email protected]>
>> Cc: Heinrich Schuchardt <[email protected]>
>> Cc: Jaehoon Chung <[email protected]>
>> Cc: Jagan Teki <[email protected]>
>> Cc: Jonas Karlman <[email protected]>
>> Cc: Kever Yang <[email protected]>
>> Cc: Matteo Lisi <[email protected]>
>> Cc: Mattijs Korpershoek <[email protected]>
>> Cc: Max Krummenacher <[email protected]>
>> Cc: Neil Armstrong <[email protected]>
>> Cc: Patrice Chotard <[email protected]>
>> Cc: Patrick Delaunay <[email protected]>
>> Cc: Philipp Tomsich <[email protected]>
>> Cc: Quentin Schulz <[email protected]>
>> Cc: Sam Day <[email protected]>
>> Cc: Simon Glass <[email protected]>
>> Cc: Sumit Garg <[email protected]>
>> Cc: Svyatoslav Ryhel <[email protected]>
>> Cc: Thierry Reding <[email protected]>
>> Cc: Tom Rini <[email protected]>
>> Cc: Volodymyr Babchuk <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> V2: Rebase on current u-boot/next
>> V3: No change
>> ---
>>   drivers/power/regulator/regulator-uclass.c | 60 +++++++---------------
>>   1 file changed, 19 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>> index 1a970004540..9fcc4bd85b9 100644
>> --- a/drivers/power/regulator/regulator-uclass.c
>> +++ b/drivers/power/regulator/regulator-uclass.c
>> @@ -314,6 +314,11 @@ int regulator_autoset(struct udevice *dev)
>>                          return ret;
>>          }
>>
>> +       if (uc_pdata->force_off) {
>> +               ret = regulator_set_enable(dev, false);
>> +               goto out;
>> +       }
>> +
>>          if (!uc_pdata->always_on && !uc_pdata->boot_on) {
>>                  ret = -EMEDIUMTYPE;
>>                  goto out;
>> @@ -518,56 +523,28 @@ static int regulator_pre_probe(struct udevice *dev)
>>          return 0;
>>   }
>>
>> -int regulators_enable_boot_on(bool verbose)
>> +static int regulator_post_probe(struct udevice *dev)
>>   {
>> -       struct udevice *dev;
>> -       struct uclass *uc;
>>          int ret;
>>
>> -       ret = uclass_get(UCLASS_REGULATOR, &uc);
>> -       if (ret)
>> +       ret = regulator_autoset(dev);
>> +       if (ret && ret != -EMEDIUMTYPE && ret != -EALREADY && ret != ENOSYS)
>>                  return ret;
>> -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
>> -            dev;
>> -            uclass_next_device(&dev)) {
>> -               ret = regulator_autoset(dev);
>> -               if (ret == -EMEDIUMTYPE || ret == -EALREADY) {
>> -                       ret = 0;
>> -                       continue;
>> -               }
>> -               if (verbose)
>> -                       regulator_show(dev, ret);
>> -               if (ret == -ENOSYS)
>> -                       ret = 0;
>> -       }
>>
>> -       return ret;
>> +       if (_DEBUG)
>> +               regulator_show(dev, ret);
>> +
>> +       return 0;
>>   }
>>
>> -int regulators_enable_boot_off(bool verbose)
>> +int regulators_enable_boot_on(bool verbose)
>>   {
>> -       struct udevice *dev;
>> -       struct uclass *uc;
>> -       int ret;
>> -
>> -       ret = uclass_get(UCLASS_REGULATOR, &uc);
>> -       if (ret)
>> -               return ret;
>> -       for (uclass_first_device(UCLASS_REGULATOR, &dev);
>> -            dev;
>> -            uclass_next_device(&dev)) {
>> -               ret = regulator_unset(dev);
>> -               if (ret == -EMEDIUMTYPE) {
>> -                       ret = 0;
>> -                       continue;
>> -               }
>> -               if (verbose)
>> -                       regulator_show(dev, ret);
>> -               if (ret == -ENOSYS)
>> -                       ret = 0;
>> -       }
>> +       return 0;
>> +}
>>
>> -       return ret;
>> +int regulators_enable_boot_off(bool verbose)
>> +{
>> +       return 0;
>>   }
>>
>>   UCLASS_DRIVER(regulator) = {
>> @@ -575,5 +552,6 @@ UCLASS_DRIVER(regulator) = {
>>          .name           = "regulator",
>>          .post_bind      = regulator_post_bind,
>>          .pre_probe      = regulator_pre_probe,
>> +       .post_probe     = regulator_post_probe,
>>          .per_device_plat_auto   = sizeof(struct dm_regulator_uclass_plat),
>>   };
>> --
>> 2.45.2
>>
> 
> I thought I objected to this patch, but it seems to be in -next? Does
> anyone know what has happened here?
> 
> I am seeing these errors now when running sandbox 'u-boot -D':
> 
>         i2c_emul_find() No emulators for device 'sandbox_pmic'
>    sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
>         out_set_value() PMIC write failed: -5
>         i2c_emul_find() No emulators for device 'sandbox_pmic'
>    sandbox_pmic_write() write error to device: 0000000018c49db0 register: 0x0!
>         out_set_value() PMIC write failed: -5
See Re: [PATCH] sandbox: i2c: Make sure phandle dependency is probed , 
let's continue it there.