[U-Boot] [PATCH 1/4 v4] watchdog: Implement generic watchdog_reset() version

[Chris Packham]

Hi Stefan,

Just noticed something when updating our internal u-boot repo

On Fri, Apr 12, 2019 at 2:00 AM Stefan Roese <sr at 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)