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

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);
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); 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); 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); return -EINVAL; }

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
Reviewed-by: Marek Vasut marex@denx.de
Thanks !

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

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@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@cherry.de
Thanks! Quentin
participants (4)
-
Marek Vasut
-
Patrice CHOTARD
-
Patrice Chotard
-
Quentin Schulz