[PATCH] watchdog: gpio_wdt: use __udelay() to avoid recursion

The udelay() function in lib/time.c contains a WATCHDOG_RESET() call. The only reason this doesn't lead to a catastrophic infinite recursion is due to the rate-limiting in wdt-uclass.c:
if (time_after_eq(now, priv->next_reset)) { priv->next_reset = now + priv->reset_period; wdt_reset(dev); }
But this would fall apart if ->next_reset was updated after calling the device's reset method.
This is needlessly fragile, and it's easy enough to avoid that recursion in the first place by just using __udelay() directly.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- drivers/watchdog/gpio_wdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c index fe06ec8cc9..2920c2c751 100644 --- a/drivers/watchdog/gpio_wdt.c +++ b/drivers/watchdog/gpio_wdt.c @@ -31,7 +31,7 @@ static int gpio_wdt_reset(struct udevice *dev) case HW_ALGO_LEVEL: /* Pulse */ dm_gpio_set_value(&priv->gpio, 1); - udelay(1); + __udelay(1); dm_gpio_set_value(&priv->gpio, 0); break; }

Hi Rasmus,
(add Pali to Cc)
On 27.09.22 09:45, Rasmus Villemoes wrote:
The udelay() function in lib/time.c contains a WATCHDOG_RESET() call. The only reason this doesn't lead to a catastrophic infinite recursion is due to the rate-limiting in wdt-uclass.c:
if (time_after_eq(now, priv->next_reset)) { priv->next_reset = now + priv->reset_period; wdt_reset(dev); }
But this would fall apart if ->next_reset was updated after calling the device's reset method.
This is needlessly fragile, and it's easy enough to avoid that recursion in the first place by just using __udelay() directly.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
drivers/watchdog/gpio_wdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c index fe06ec8cc9..2920c2c751 100644 --- a/drivers/watchdog/gpio_wdt.c +++ b/drivers/watchdog/gpio_wdt.c @@ -31,7 +31,7 @@ static int gpio_wdt_reset(struct udevice *dev) case HW_ALGO_LEVEL: /* Pulse */ dm_gpio_set_value(&priv->gpio, 1);
udelay(1);
dm_gpio_set_value(&priv->gpio, 0); break; }__udelay(1);
Yes, good catch:
Reviewed-by: Stefan Roese sr@denx.de
The max6370_wdt.c driver has the same problem AFAICT. Pali, could you please take a look a this?
Thanks, Stefan

On Tuesday 27 September 2022 10:00:00 Stefan Roese wrote:
The max6370_wdt.c driver has the same problem AFAICT. Pali, could you please take a look a this?
Done, here is patch: https://patchwork.ozlabs.org/project/uboot/patch/20220927101919.6999-1-pali@...

On 27.09.22 09:45, Rasmus Villemoes wrote:
The udelay() function in lib/time.c contains a WATCHDOG_RESET() call. The only reason this doesn't lead to a catastrophic infinite recursion is due to the rate-limiting in wdt-uclass.c:
if (time_after_eq(now, priv->next_reset)) { priv->next_reset = now + priv->reset_period; wdt_reset(dev); }
But this would fall apart if ->next_reset was updated after calling the device's reset method.
This is needlessly fragile, and it's easy enough to avoid that recursion in the first place by just using __udelay() directly.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Applied to u-boot-watchdog/master
Thanks, Stefan
drivers/watchdog/gpio_wdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c index fe06ec8cc9..2920c2c751 100644 --- a/drivers/watchdog/gpio_wdt.c +++ b/drivers/watchdog/gpio_wdt.c @@ -31,7 +31,7 @@ static int gpio_wdt_reset(struct udevice *dev) case HW_ALGO_LEVEL: /* Pulse */ dm_gpio_set_value(&priv->gpio, 1);
udelay(1);
dm_gpio_set_value(&priv->gpio, 0); break; }__udelay(1);
Viele Grüße, Stefan Roese
participants (3)
-
Pali Rohár
-
Rasmus Villemoes
-
Stefan Roese