Re: [PATCH 3/3] watchdog: honour hw_margin_ms DT property

[Stefan Roese]

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.villem...@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 <s...@denx.de>

Thanks,
Stefan