Re: [PATCH v6 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

Wolfgang Denk
2021-08-19T11:43:57Z

Dear Rasmus,

In message <20210819095706.3585923-10-rasmus.villemoes@prevas.dk> you wrote:
>
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 5b1c0df5d6..7570710c4d 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -61,20 +61,24 @@ static void init_watchdog_dev(struct udevice *dev)
...
> + ret = uclass_get(UCLASS_WDT, &uc);
> + if (ret) {
> + log_debug("Error getting UCLASS_WDT: %d\n", ret);
> + return 0;
> + }
> +
> + uclass_foreach_dev(dev, uc) {
> + ret = device_probe(dev);
> + if (ret) {
> + log_debug("Error probing %s: %d\n", dev->name, ret);
> + continue;
> }

As discussed - errors need to be shown to the user, and not only in
images with debugging enabled.

> @@ -182,22 +186,34 @@ void watchdog_reset(void)
> {
> struct wdt_priv *priv;
> struct udevice *dev;
> + struct uclass *uc;
> ulong now;
>
> /* Exit if GD is not ready or watchdog is not initialized yet */
> if (!gd || !(gd->flags & GD_FLG_WDT_READY))
> return;
>
> - dev = gd->watchdog_dev;
> - priv = dev_get_uclass_priv(dev);
> - if (!priv->running)
> + if (uclass_get(UCLASS_WDT, &uc))
> return;

Do I see this crrectly that you remove here the code which you just
added in patch 02 of this series?

Why not doing it right from the beginning?

> + uclass_foreach_dev(dev, uc) {
> + if (!device_active(dev))
> + continue;
> + priv = dev_get_uclass_priv(dev);
> + if (!priv->running)
> + continue;

Potential NULL pointer dereference.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
An Ada exception is when a routine gets in trouble and says
'Beam me up, Scotty'.