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

2 Dec 2024

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@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?
...
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.
...
	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.
...
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 :) ).
The change itself is fine otherwise, so
Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Thanks!
Quentin

    