| Message ID | 20240228182723.12855-1-ben.wolsieffer@hefring.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | watchdog: stm32_iwdg: initialize default timeout | expand |
On 2/28/24 10:27, Ben Wolsieffer wrote: > The driver never sets a default timeout value, therefore it is > initialized to zero. When CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is > enabled, the watchdog is started during probe. The kernel is supposed to > automatically ping the watchdog from this point until userspace takes > over, but this does not happen if the configured timeout is zero. A zero > timeout causes watchdog_need_worker() to return false, so the heartbeat > worker does not run and the system therefore resets soon after the > driver is probed. > > This patch fixes this by setting an arbitrary non-zero default timeout. > The default could be read from the hardware instead, but I didn't see > any reason to add this complexity. > > This has been tested on an STM32F746. > > Fixes: 85fdc63fe256 ("drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe") > Signed-off-by: Ben Wolsieffer <ben.wolsieffer@hefring.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/stm32_iwdg.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c > index d9fd50df9802..5404e0387620 100644 > --- a/drivers/watchdog/stm32_iwdg.c > +++ b/drivers/watchdog/stm32_iwdg.c > @@ -20,6 +20,8 @@ > #include <linux/platform_device.h> > #include <linux/watchdog.h> > > +#define DEFAULT_TIMEOUT 10 > + > /* IWDG registers */ > #define IWDG_KR 0x00 /* Key register */ > #define IWDG_PR 0x04 /* Prescaler Register */ > @@ -248,6 +250,7 @@ static int stm32_iwdg_probe(struct platform_device *pdev) > wdd->parent = dev; > wdd->info = &stm32_iwdg_info; > wdd->ops = &stm32_iwdg_ops; > + wdd->timeout = DEFAULT_TIMEOUT; > wdd->min_timeout = DIV_ROUND_UP((RLR_MIN + 1) * PR_MIN, wdt->rate); > wdd->max_hw_heartbeat_ms = ((RLR_MAX + 1) * wdt->data->max_prescaler * > 1000) / wdt->rate;
diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index d9fd50df9802..5404e0387620 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -20,6 +20,8 @@ #include <linux/platform_device.h> #include <linux/watchdog.h> +#define DEFAULT_TIMEOUT 10 + /* IWDG registers */ #define IWDG_KR 0x00 /* Key register */ #define IWDG_PR 0x04 /* Prescaler Register */ @@ -248,6 +250,7 @@ static int stm32_iwdg_probe(struct platform_device *pdev) wdd->parent = dev; wdd->info = &stm32_iwdg_info; wdd->ops = &stm32_iwdg_ops; + wdd->timeout = DEFAULT_TIMEOUT; wdd->min_timeout = DIV_ROUND_UP((RLR_MIN + 1) * PR_MIN, wdt->rate); wdd->max_hw_heartbeat_ms = ((RLR_MAX + 1) * wdt->data->max_prescaler * 1000) / wdt->rate;
The driver never sets a default timeout value, therefore it is initialized to zero. When CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED is enabled, the watchdog is started during probe. The kernel is supposed to automatically ping the watchdog from this point until userspace takes over, but this does not happen if the configured timeout is zero. A zero timeout causes watchdog_need_worker() to return false, so the heartbeat worker does not run and the system therefore resets soon after the driver is probed. This patch fixes this by setting an arbitrary non-zero default timeout. The default could be read from the hardware instead, but I didn't see any reason to add this complexity. This has been tested on an STM32F746. Fixes: 85fdc63fe256 ("drivers: watchdog: stm32_iwdg: set WDOG_HW_RUNNING at probe") Signed-off-by: Ben Wolsieffer <ben.wolsieffer@hefring.com> --- drivers/watchdog/stm32_iwdg.c | 3 +++ 1 file changed, 3 insertions(+)