Hi Stefan, Just noticed something when updating our internal u-boot repo
On Fri, Apr 12, 2019 at 2:00 AM Stefan Roese <s...@denx.de> wrote: > > This patch tries to implement a generic watchdog_reset() function that > can be used by all boards that want to service the watchdog device in > U-Boot. This watchdog servicing is enabled via CONFIG_WATCHDOG. > > Without this approach, new boards or platforms needed to implement a > board specific version of this functionality, mostly copy'ing the same > code over and over again into their board or platforms code base. > > With this new generic function, the scattered other functions are now > removed to be replaced by the generic one. The new version also enables > the configuration of the watchdog timeout via the DT "timeout-sec" > property (if enabled via CONFIG_OF_CONTROL). > > This patch also adds a new flag to the GD flags, to flag that the > watchdog is ready to use and adds the pointer to the watchdog device > to the GD. This enables us to remove the global "watchdog_dev" > variable, which was prone to cause problems because of its potentially > very early use in watchdog_reset(), even before the BSS is cleared. <snip> > diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c > index 23b7e3360d..bbfac4f0f9 100644 > --- a/drivers/watchdog/wdt-uclass.c > +++ b/drivers/watchdog/wdt-uclass.c > @@ -10,6 +10,8 @@ > #include <dm/device-internal.h> > #include <dm/lists.h> > > +DECLARE_GLOBAL_DATA_PTR; > + > int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) > { > const struct wdt_ops *ops = device_get_ops(dev); > @@ -63,6 +65,30 @@ int wdt_expire_now(struct udevice *dev, ulong flags) > return ret; > } > > +#if defined(CONFIG_WATCHDOG) > +/* > + * Called by macro WATCHDOG_RESET. This function be called *very* early, > + * so we need to make sure, that the watchdog driver is ready before using > + * it in this function. > + */ > +void watchdog_reset(void) > +{ > + static ulong next_reset; > + ulong 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 (now > next_reset) { If the counter wraps we stop tickling the watchdog and the board will reset. On our 32-bit arm boards this is only about 72 minutes so it got noticed in production testing. The fix we had in our board code was: +#ifndef time_after +#define time_after(a,b) \ + ((long)((b) - (a)) < 0) +#endif @@ -202,7 +208,7 @@ void watchdog_reset(void) now = timer_get_us(); /* Do not reset the watchdog too often */ - if (now > next_reset) { + if (time_after(now, next_reset)) { wdt_reset(watchdog_dev); next_reset = now + 1000; } I'll send a proper patch (gmail will eat the whitespace in the inline diff). > + next_reset = now + 1000; /* reset every 1000ms */ > + wdt_reset(gd->watchdog_dev); > + } > +} > +#endif > + > static int wdt_post_bind(struct udevice *dev) > { > #if defined(CONFIG_NEEDS_MANUAL_RELOC)