Re: [PATCH v6 05/12] watchdog: wdt-uclass.c: keep track of each device's running state

[Wolfgang Denk <wd@denx.de>]

Dear Rasmus,

please check your patches for proper error handling.

In message <20210819095706.3585923-6-rasmus.villemoes@prevas.dk> you wrote:
>
...
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 0a1f43771c..358fc68e27 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
...
> @@ -86,8 +89,11 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>  		return -ENOSYS;
>  
>  	ret = ops->start(dev, timeout_ms, flags);
> -	if (ret == 0)
> -		gd->flags |= GD_FLG_WDT_READY;
> +	if (ret == 0) {
> +		struct wdt_priv *priv = dev_get_uclass_priv(dev);
> +
> +		priv->running = true;

dev_get_uclass_priv() can return NULL, in which case you would be
dereferencing a NULL pointer...

> @@ -101,8 +107,11 @@ int wdt_stop(struct udevice *dev)
>  		return -ENOSYS;
>  
>  	ret = ops->stop(dev);
> -	if (ret == 0)
> -		gd->flags &= ~GD_FLG_WDT_READY;
> +	if (ret == 0) {
> +		struct wdt_priv *priv = dev_get_uclass_priv(dev);
> +
> +		priv->running = false;

Same here.

> @@ -156,6 +165,9 @@ void watchdog_reset(void)
>  
>  	dev = gd->watchdog_dev;
>  	priv = dev_get_uclass_priv(dev);
> +	if (!priv->running)
> +		return;
> +

And here again.

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
I used to be indecisive, now I'm not sure.