[RFC PATCH] watchdog: base rate-limiting on get_ticks() rather than get_timer()

Rasmus Villemoes rasmus.villemoes at prevas.dk
Fri Jun 5 13:16:57 CEST 2020


On powerpc, get_timer() is implemented using a volatile variable that
gets incremented from the decrementer interrupt handler. Hence, when
interrupts are disabled, time as measured by get_timer() ceases to
pass.

Interrupts are necessarily disabled during bootm (see the comment in
bootm_disable_interrupts() for why). But after interrupts are
disabled, there's still lots of work to do - e.g. decompressing the
kernel image to the right load address. Unfortunately, the
rate-limiting logic in wdt_uclass.c's watchdog_reset function means
that WATCHDOG_RESET() becomes a no-op, since get_timer() never
progresses past the next_reset.

This, combined with an external gpio-triggered watchdog that must be
petted at least every 800ms, means our board gets reset before booting
into linux.

Now, at least on powerpc, get_ticks() continues to return sensible
results whether or not interrupts are enabled. So this fixes the above
problem for our board - but I don't know if get_ticks() can be assumed
to always work.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
---
This is what I had in mind. I also considered making it a config knob
(possibly just auto-selected based on $ARCH) whether to use
get_timer() or get_ticks(), but that becomes quite ugly.

 drivers/watchdog/wdt-uclass.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 4cdb7bd64c..7be4e9b5bc 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -16,14 +16,15 @@ 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;
+static u64 ratelimit_ticks;
 
 int initr_watchdog(void)
 {
+	/*
+	 * Reset every 1000ms, or however often is required as indicated by a
+	 * hw_margin_ms property.
+	 */
+	u32 reset_period = 1000;
 	u32 timeout = WATCHDOG_TIMEOUT_SECS;
 
 	/*
@@ -48,6 +49,7 @@ int initr_watchdog(void)
 						    4 * reset_period) / 4;
 	}
 
+	ratelimit_ticks = usec_to_tick(reset_period * 1000);
 	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
 	gd->flags |= GD_FLG_WDT_READY;
 	printf("WDT:   Started with%s servicing (%ds timeout)\n",
@@ -117,17 +119,17 @@ int wdt_expire_now(struct udevice *dev, ulong flags)
  */
 void watchdog_reset(void)
 {
-	static ulong next_reset;
-	ulong now;
+	static u64 last_reset;
+	u64 now;
 
 	/* Exit if GD is not ready or watchdog is not initialized yet */
 	if (!gd || !(gd->flags & GD_FLG_WDT_READY))
 		return;
 
 	/* Do not reset the watchdog too often */
-	now = get_timer(0);
-	if (time_after(now, next_reset)) {
-		next_reset = now + reset_period;
+	now = get_ticks();
+	if (now - last_reset >= ratelimit_ticks) {
+		last_reset = now;
 		wdt_reset(gd->watchdog_dev);
 	}
 }
-- 
2.23.0



More information about the U-Boot mailing list