[PATCH v2 1/5] rockchip: io-domain: Add debug logging for regulators during probe

[Justin Klaassen]

 Hi Quentin,

On Feb 4, 2025 at 09:16:29, Quentin Schulz <quentin.schulz at cherry.de> wrote:

> Hi Justin,
>
> On 1/28/25 10:37 PM, Justin Klaassen wrote:
>
> [You don't often get email from justin at tidylabs.net. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
>
> Log the value of the regulators during initialization of the IO-domain
>
> driver to aid in debugging GPIO voltage configuration problems.
>
>
> Signed-off-by: Justin Klaassen <justin at tidylabs.net>
>
> ---
>
>
> Changes in v2:
>
> - Made added IO-domain driver logs verbose and consistent
>
>
>   drivers/misc/rockchip-io-domain.c | 7 ++++++-
>
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
>
> diff --git a/drivers/misc/rockchip-io-domain.c
> b/drivers/misc/rockchip-io-domain.c
>
> index 025b6049a9f..fdefc9c5fe3 100644
>
> --- a/drivers/misc/rockchip-io-domain.c
>
> +++ b/drivers/misc/rockchip-io-domain.c
>
> @@ -5,6 +5,8 @@
>
>    * Ported from linux drivers/soc/rockchip/io-domain.c
>
>    */
>
>
> +#define LOG_CATEGORY UCLASS_NOP
>
> +
>
>   #include <dm.h>
>
>   #include <dm/device_compat.h>
>
>   #include <regmap.h>
>
> @@ -344,8 +346,10 @@ static int rockchip_iodomain_probe(struct udevice
> *dev)
>
>                          continue;
>
>
>                  ret = device_get_supply_regulator(dev, supply_name, &reg);
>
> -               if (ret)
>
> +               if (ret) {
>
> +                       log_debug("%s: Regulator not found\n",
> supply_name);
>
>
> Can you use dev_dbg() instead?
>

Done.


> This would match what we use for other messages in the probe function?
>
> Specifically, it could add the device name in front, which would be
> useful when debugging to highlight the IO domain device is the one
> requesting this regulator that isn't found.
>
>                          continue;
>
> +               }
>
>
>                  ret = regulator_autoset(reg);
>
>                  if (ret && ret != -EALREADY && ret != -EMEDIUMTYPE &&
>
> @@ -353,6 +357,7 @@ static int rockchip_iodomain_probe(struct udevice *dev)
>
>                          continue;
>
>
>                  uV = regulator_get_value(reg);
>
> +               log_debug("%s: Regulator %s at %d uV\n", supply_name,
> reg->name, uV);
>
>
> Ditto.
>

Done.


> Not sure the reg->name brings a lot since this information could be
> derived from supply_name if I'm not mistaken, but I'm not too bothered
> by it 🙂
>

I found reg->name to be helpful when debugging as it made it slightly
easier to find the correct regulator without having to go through a few
levels of redirection in the device tree.


>
> Cheers,
> Quentin
>

>
Thanks,
Justin