| Message ID | 20200313160458.19727-4-rasmus.villemoes@prevas.dk |
|---|---|
| State | Accepted |
| Commit | 40d7f3c8d111d4e4b6c1df105d5a4cedf5b7d6c1 |
| Headers | show |
| Series | [1/3] watchdog: remove stale ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS from wdt.h | expand |
On 13.03.20 17:04, Rasmus Villemoes wrote: > Some watchdog devices, e.g. external gpio-triggered ones, must be > reset more often than once per second, which means that the current > rate-limiting logic in watchdog_reset() fails to keep the board alive. > > gpio-wdt.txt in the linux source tree defines a "hw_margin_ms" > property used to specifiy the maximum time allowed between resetting > the device. Allow any watchdog device to specify such a property, and > then use a reset period of one quarter of that. We keep the current > default of resetting once every 1000ms. > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk> > --- > drivers/watchdog/wdt-uclass.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c > index fb3e247c5f..436a52fd08 100644 > --- a/drivers/watchdog/wdt-uclass.c > +++ b/drivers/watchdog/wdt-uclass.c > @@ -15,6 +15,12 @@ DECLARE_GLOBAL_DATA_PTR; > > #define WATCHDOG_TIMEOUT_SECS (CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000) > > +/* > + * Reset every 1000ms, or however often is required as indicated by a > + * hw_margin_ms property. > + */ > +static ulong reset_period = 1000; > + > int initr_watchdog(void) > { > u32 timeout = WATCHDOG_TIMEOUT_SECS; > @@ -36,6 +42,8 @@ int initr_watchdog(void) > if (CONFIG_IS_ENABLED(OF_CONTROL)) { > timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec", > WATCHDOG_TIMEOUT_SECS); > + reset_period = dev_read_u32_default(gd->watchdog_dev, "hw_margin_ms", > + 4*reset_period)/4; Nitpicking: checkpatch will most likely complain about missing spaces here. > } > > wdt_start(gd->watchdog_dev, timeout * 1000, 0); > @@ -117,7 +125,7 @@ void watchdog_reset(void) > /* Do not reset the watchdog too often */ > now = get_timer(0); > if (time_after(now, next_reset)) { > - next_reset = now + 1000; /* reset every 1000ms */ > + next_reset = now + reset_period; > wdt_reset(gd->watchdog_dev); > } > } > Other than that: Reviewed-by: Stefan Roese <sr at denx.de> Thanks, Stefan
On 14/03/2020 12.57, Stefan Roese wrote: > On 13.03.20 17:04, Rasmus Villemoes wrote: >> ? int initr_watchdog(void) >> ? { >> ????? u32 timeout = WATCHDOG_TIMEOUT_SECS; >> @@ -36,6 +42,8 @@ int initr_watchdog(void) >> ????? if (CONFIG_IS_ENABLED(OF_CONTROL)) { >> ????????? timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec", >> ???????????????????????????? WATCHDOG_TIMEOUT_SECS); >> +??????? reset_period = dev_read_u32_default(gd->watchdog_dev, >> "hw_margin_ms", >> +??????????????????????????? 4*reset_period)/4; > > Nitpicking: checkpatch will most likely complain about missing spaces > here. Right. Want me to resend, or can you fix if/when applying? Rasmus
On 16.03.20 16:39, Rasmus Villemoes wrote: > On 14/03/2020 12.57, Stefan Roese wrote: >> On 13.03.20 17:04, Rasmus Villemoes wrote: >>> ? int initr_watchdog(void) >>> ? { >>> ????? u32 timeout = WATCHDOG_TIMEOUT_SECS; >>> @@ -36,6 +42,8 @@ int initr_watchdog(void) >>> ????? if (CONFIG_IS_ENABLED(OF_CONTROL)) { >>> ????????? timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec", >>> ???????????????????????????? WATCHDOG_TIMEOUT_SECS); >>> +??????? reset_period = dev_read_u32_default(gd->watchdog_dev, >>> "hw_margin_ms", >>> +??????????????????????????? 4*reset_period)/4; >> >> Nitpicking: checkpatch will most likely complain about missing spaces >> here. > > Right. Want me to resend, or can you fix if/when applying? If there will be no other changes requiring a v2, then I can fix this while applying. Thanks, Stefan
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index fb3e247c5f..436a52fd08 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -15,6 +15,12 @@ DECLARE_GLOBAL_DATA_PTR; #define WATCHDOG_TIMEOUT_SECS (CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000) +/* + * Reset every 1000ms, or however often is required as indicated by a + * hw_margin_ms property. + */ +static ulong reset_period = 1000; + int initr_watchdog(void) { u32 timeout = WATCHDOG_TIMEOUT_SECS; @@ -36,6 +42,8 @@ int initr_watchdog(void) if (CONFIG_IS_ENABLED(OF_CONTROL)) { timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec", WATCHDOG_TIMEOUT_SECS); + reset_period = dev_read_u32_default(gd->watchdog_dev, "hw_margin_ms", + 4*reset_period)/4; } wdt_start(gd->watchdog_dev, timeout * 1000, 0); @@ -117,7 +125,7 @@ void watchdog_reset(void) /* Do not reset the watchdog too often */ now = get_timer(0); if (time_after(now, next_reset)) { - next_reset = now + 1000; /* reset every 1000ms */ + next_reset = now + reset_period; wdt_reset(gd->watchdog_dev); } }
Some watchdog devices, e.g. external gpio-triggered ones, must be reset more often than once per second, which means that the current rate-limiting logic in watchdog_reset() fails to keep the board alive. gpio-wdt.txt in the linux source tree defines a "hw_margin_ms" property used to specifiy the maximum time allowed between resetting the device. Allow any watchdog device to specify such a property, and then use a reset period of one quarter of that. We keep the current default of resetting once every 1000ms. Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk> --- drivers/watchdog/wdt-uclass.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)