[PATCH] power: regulator: replace some debug() by dev_dbg/err()

[Patrice CHOTARD]

On 12/2/24 10:32, Quentin Schulz wrote:
> Hi Patrice,
> 
> On 11/29/24 1:44 PM, Patrice Chotard wrote:
>> Replace some debug() by dev_dbg() when dev variable
>> is available/valid.
>>
>> To ease debugging, use dev_err() instead of dev_dbg() for
>> alerting when regulator has nonunique value.
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard at foss.st.com>
>> ---
>>
>>   drivers/power/regulator/regulator-uclass.c | 15 ++++++++-------
>>   1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>> index decd0802c84..aa6918ef50a 100644
>> --- a/drivers/power/regulator/regulator-uclass.c
>> +++ b/drivers/power/regulator/regulator-uclass.c
>> @@ -9,6 +9,7 @@
>>   #include <errno.h>
>>   #include <dm.h>
>>   #include <log.h>
>> +#include <dm/device_compat.h>
>>   #include <dm/uclass-internal.h>
>>   #include <linux/delay.h>
>>   #include <power/pmic.h>
>> @@ -43,8 +44,8 @@ static void regulator_set_value_ramp_delay(struct udevice *dev, int old_uV,
>>   {
>>       int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
>>   -    debug("regulator %s: delay %u us (%d uV -> %d uV)\n", dev->name, delay,
>> -          old_uV, new_uV);
>> +    dev_dbg(dev, "regulator %s: delay %u us (%d uV -> %d uV)\n", dev->name, delay,
>> +        old_uV, new_uV);
> 
> Isn't dev_dbg already printing dev->name?

You are right, i will rewwork this part

> 
>>         udelay(delay);
>>   }
>> @@ -263,7 +264,7 @@ int regulator_get_by_platname(const char *plat_name, struct udevice **devp)
>>       for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev;
>>            ret = uclass_find_next_device(&dev)) {
>>           if (ret) {
>> -            debug("regulator %s, ret=%d\n", dev->name, ret);
>> +            dev_dbg(dev, "regulator %s, ret=%d\n", dev->name, ret);
> 
> Ditto.

ok
> 
>>               continue;
>>           }
>>   @@ -439,16 +440,16 @@ static int regulator_post_bind(struct udevice *dev)
>>       /* Regulator's mandatory constraint */
>>       uc_pdata->name = dev_read_string(dev, property);
>>       if (!uc_pdata->name) {
>> -        debug("%s: dev '%s' has no property '%s'\n",
>> -              __func__, dev->name, property);
>> +        dev_dbg(dev, "%s: dev '%s' has no property '%s'\n",
>> +            __func__, dev->name, property);
> 
> As well.

ok
> 
>>           uc_pdata->name = dev_read_name(dev);
>>           if (!uc_pdata->name)
>>               return -EINVAL;
>>       }
>>         if (!regulator_name_is_unique(dev, uc_pdata->name)) {
>> -        debug("'%s' of dev: '%s', has nonunique value: '%s\n",
>> -              property, dev->name, uc_pdata->name);
>> +        dev_err(dev, "'%s' of dev: '%s', has nonunique value: '%s\n",
>> +            property, dev->name, uc_pdata->name);
> 
> Similarly.
> 
> So, do we not print the same info twice in the message? If so, then we should rework the debug message to remove it.
> 
> Additionally, split in two commits, one for migratin to dev_dbg and one for migrating to dev_err so we can revert one or the other and the change is explicit. (I've done a mixed find-replace a few releases ago that made some people unhappy and it would have been easier to revert just the commit that was problematic than patching things up manually :) ).

Ok i will split it into 2 different patchs

Thanks

> 
> The change itself is fine otherwise, so
> 
> Reviewed-by: Quentin Schulz <quentin.schulz at cherry.de>
> 
> Thanks!
> Quentin