[U-Boot] u-boot: at91: wdt broken for smartweb and taurus board

[Heiko Schocher]

Hello all,

just found out, that since patch:

commit 1473f6ac882fde8078826ca828aa3494ff98bf08
Author: Prasanthi Chellakumar <Prasanthi.Chellakumar at microchip.com>
Date:   Tue Oct 9 11:46:40 2018 -0700

     arm: at91: wdt: Convert watchdog driver to dm/dt

     Convert the Watchdog driver for AT91SAM9x processors to support
     the driver model and device tree. Changes "CONFIG_AT91SAM9_WATCHDOG"
     to new "CONFIG_WDT_AT91" Kconfig option.

     Signed-off-by: Prasanthi Chellakumar <prasanthi.chellakumar at microchip.com>

wdt always triggers really fast on smartweb and taurus board...

A fast look into it, and I see, that at91_wdt_probe() seems not called.

Prasanthi already mentioned to me (offlist), to add

#include <wdt.h>

static struct udevice *watchdog_dev;

#ifdef CONFIG_WDT_AT91
          if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) {
                  printf("Cannot find watchdog!\n");
          } else {
                  printf("Enabling Watchdog\n");
                  wdt_start(watchdog_dev, 15, 0);
          }
#endif

into board code. With this patch at91_wdt_probe() is called now.

Digging deeper in it and it seems there are more things broken:

http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/watchdog/at91sam9_wdt.c;h=13f8772e4130ac1e273c920d211eefc4491f5eff;hb=ee168783ae889cf449cee36cc1e51e108a210ed4#l48

timeout_s in at91_wdt_start() is assumed in seconds, but wdt_start in include/wdt.h
states the timeout is in milliseconds :-(

So code goes into the if:

         if (timeout_s > WDT_MAX_TIMEOUT || timeout_s < WDT_MIN_TIMEOUT)
                  timeout = priv->timeout;

and sets timeout new, but never converts the new value with WDT_SEC2TICKS()
macro, so the wrong value is written into AT91_WDT_MR ... following
patch fixes this:

diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index 81f448322d..c67bed705f 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -48,11 +48,12 @@ struct at91_wdt_priv {
  static int at91_wdt_start(struct udevice *dev, u64 timeout_s, ulong flags)
  {
         struct at91_wdt_priv *priv = dev_get_priv(dev);
-       u32 timeout = WDT_SEC2TICKS(timeout_s);
+       u32 timeout;

         if (timeout_s > WDT_MAX_TIMEOUT || timeout_s < WDT_MIN_TIMEOUT)
-               timeout = priv->timeout;
+               timeout_s = priv->timeout;

+       timeout = WDT_SEC2TICKS(timeout_s);
         /* Check if disabled */
         if (readl(priv->regs + AT91_WDT_MR) & AT91_WDT_MR_WDDIS) {
                 printf("sorry, watchdog is disabled\n");

With this 2 patches, same value as before commit 1473f6ac882fd is written
into AT91_WDT_MR now ... but board reset thorugh WDT still remains ...

Seems with DM_WDT support, wdt reset() gets not called ...

Any ideas where to look at ?

And is it really necessary to call wdt_start() from board code to
start wdt ? DTS entries for wdt seem correct to me.

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de