The wdt_start() function may be called with a timeout greater
than the hardware-supported maximum. This in turn can result
in a bogus print, for example, if the device clamps the timeout
to 10s but wdt_start() is called with a 60s timeout:
Started <watchdog@> with servicing every 1000ms (60s timeout)

Add a max_timeout_ms field to uclass-plat data, so drivers can
set the hardware-limited max timeout value there during probing.
For drivers that support this feature, the startup print now
shows both the actual and requested timeout values, e.g.
Started <watchdog@> with servicing every 1000ms (10s timeout, requested 60s)

The timeout comparison is done in whole seconds to avoid noise
from sub-second rounding.

The value defaults to 0. For drivers that do not support
this feature, the resulting print remains the same as before.

While at it, rename the string str in wdt_start() to svc_str for
cleaner distinction from the new req_str, replace its memset with
svc_str[0] = '\0', and use sizeof(svc_str) instead of the magic 16
in the snprintf.

Signed-off-by: Juuso Rinta <[email protected]>
---
 drivers/watchdog/wdt-uclass.c | 34 +++++++++++++++++++++++++++++-----
 include/wdt.h                 | 11 +++++++++++
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 438833b2245..1a58256217a 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -127,9 +127,12 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong 
flags)
        ret = ops->start(dev, timeout_ms, flags);
        if (ret == 0) {
                struct wdt_priv *priv = dev_get_uclass_priv(dev);
-               char str[16];
+               struct wdt_uc_plat *plat = dev_get_uclass_plat(dev);
+               u64 effective_ms = timeout_ms;
+               char svc_str[16];
+               char req_str[32];
 
-               memset(str, 0, 16);
+               svc_str[0] = '\0';
                if (IS_ENABLED(CONFIG_WATCHDOG)) {
                        if (priv->running)
                                cyclic_unregister(&priv->cyclic);
@@ -139,13 +142,33 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong 
flags)
                                        priv->reset_period * 1000,
                                        dev->name);
 
-                       snprintf(str, 16, "every %ldms", priv->reset_period);
+                       snprintf(svc_str, sizeof(svc_str), "every %ldms",
+                                priv->reset_period);
                }
 
                priv->running = true;
-               printf("WDT:   Started %s with%s servicing %s (%ds timeout)\n",
+
+               /*
+                * If the driver reported a maximum timeout value,
+                * check if the requested timeout is clamped
+                */
+               req_str[0] = '\0';
+               if (plat->max_timeout_ms) {
+                       if (timeout_ms > plat->max_timeout_ms)
+                               effective_ms = plat->max_timeout_ms;
+
+                       /*
+                        * Note the requested value when it differs at 
whole-second
+                        * resolution. Sub-second rounding is ignored to avoid 
noise.
+                        */
+                       if ((u32)lldiv(effective_ms, 1000) != 
(u32)lldiv(timeout_ms, 1000))
+                               snprintf(req_str, sizeof(req_str), ", requested 
%ds",
+                                        (u32)lldiv(timeout_ms, 1000));
+               }
+
+               printf("WDT:   Started %s with%s servicing %s (%ds 
timeout%s)\n",
                       dev->name, IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out",
-                      str, (u32)lldiv(timeout_ms, 1000));
+                      svc_str, (u32)lldiv(effective_ms, 1000), req_str);
        }
 
        return ret;
@@ -268,4 +291,5 @@ UCLASS_DRIVER(wdt) = {
        .flags                  = DM_UC_FLAG_SEQ_ALIAS,
        .pre_probe              = wdt_pre_probe,
        .per_device_auto        = sizeof(struct wdt_priv),
+       .per_device_plat_auto   = sizeof(struct wdt_uc_plat),
 };
diff --git a/include/wdt.h b/include/wdt.h
index 1ef656585c4..db1faa321b3 100644
--- a/include/wdt.h
+++ b/include/wdt.h
@@ -74,6 +74,17 @@ int wdt_reset(struct udevice *dev);
  */
 int wdt_expire_now(struct udevice *dev, ulong flags);
 
+/**
+ * struct wdt_uc_plat - uclass platform data for a watchdog device
+ *
+ * @max_timeout_ms: Maximum timeout (in ms) that the hardware can honour.
+ *  A driver should set this typically at probe time. Default value 0
+ *  means no limit set by the driver.
+ */
+struct wdt_uc_plat {
+       u32 max_timeout_ms;
+};
+
 /*
  * struct wdt_ops - Driver model wdt operations
  *

-- 
2.39.2

Reply via email to