[PATCH v5 00/12] handling all DM watchdogs in watchdog_reset()

[PATCH v5 00/12] handling all DM watchdogs in watchdog_reset()

[Rasmus Villemoes]

This series is an attempt at expanding the wdt-uclass provided
watchdog_reset() to handle all DM watchdogs, not just the first
one. Some of the ad hoc work done for the first DM watchdog in
initr_watchdog() is now moved to a .pre_probe hook so it is
automatically done for all devices.

It also includes a patch adding a driver for a gpio-petted watchdog
device (and a sandbox test of that) - it is included here because that
also gives a relatively easy way to have more than one (kind of)
watchdog device in the sandbox, which is then used at the end to test
that watchdog_reset() behaves as expected.

v2 here: https://lore.kernel.org/u-boot/20210527220017.1266765-1-rasmus.villemoes@prevas.dk/

v3 here: https://lore.kernel.org/u-boot/20210702124510.124401-1-rasmus.villemoes@prevas.dk/

v4 here: https://lore.kernel.org/u-boot/20210802150016.588750-1-rasmus.villemoes@prevas.dk/

Changes in v5:

- Handle leftover use of gd->watchdog_dev (new patches 7 and 8).

Rasmus Villemoes (12):
  watchdog: wdt-uclass.c: use wdt_start() in wdt_expire_now()
  watchdog: wdt-uclass.c: introduce struct wdt_priv
  watchdog: wdt-uclass.c: neaten UCLASS_DRIVER definition
  watchdog: wdt-uclass.c: refactor initr_watchdog()
  watchdog: wdt-uclass.c: keep track of each device's running state
  sandbox: disable CONFIG_WATCHDOG_AUTOSTART
  watchdog: wdt-uclass.c: add wdt_stop_all() helper
  board: x530: switch to wdt_stop_all()
  watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()
  watchdog: add gpio watchdog driver
  sandbox: add test of wdt_gpio driver
  sandbox: add test of wdt-uclass' watchdog_reset()

 arch/sandbox/dts/test.dts                     |   8 +
 board/alliedtelesis/x530/x530.c               |   5 +-
 configs/sandbox64_defconfig                   |   2 +
 configs/sandbox_defconfig                     |   2 +
 .../watchdog/gpio-wdt.txt                     |  19 ++
 drivers/watchdog/Kconfig                      |   9 +
 drivers/watchdog/Makefile                     |   1 +
 drivers/watchdog/gpio_wdt.c                   |  68 +++++++
 drivers/watchdog/wdt-uclass.c                 | 192 +++++++++++++-----
 include/asm-generic/global_data.h             |   6 -
 include/wdt.h                                 |   5 +
 test/dm/wdt.c                                 |  90 +++++++-
 12 files changed, 346 insertions(+), 61 deletions(-)
 create mode 100644 doc/device-tree-bindings/watchdog/gpio-wdt.txt
 create mode 100644 drivers/watchdog/gpio_wdt.c

-- 
2.31.1

[PATCH v5 01/12] watchdog: wdt-uclass.c: use wdt_start() in wdt_expire_now()

[Rasmus Villemoes]

wdt_start() does the "no ->start? return -ENOSYS" check, don't
open-code that in wdt_expire_now().

Also, wdt_start() maintains some global (and later some per-device)
state, which would get out of sync with this direct method call - not
that it matters much here since the board is supposed to reset very
soon.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 17334dbda6..df8164da2a 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -120,10 +120,8 @@ int wdt_expire_now(struct udevice *dev, ulong flags)
 	if (ops->expire_now) {
 		return ops->expire_now(dev, flags);
 	} else {
-		if (!ops->start)
-			return -ENOSYS;
+		ret = wdt_start(dev, 1, flags);
 
-		ret = ops->start(dev, 1, flags);
 		if (ret < 0)
 			return ret;
 
-- 
2.31.1

[PATCH v5 02/12] watchdog: wdt-uclass.c: introduce struct wdt_priv

[Rasmus Villemoes]

As preparation for having the wdt-uclass provided watchdog_reset()
function handle all DM watchdog devices, and not just the first such,
introduce a uclass-owned struct to hold the reset_period and
next_reset, so these become per-device instead of being static
variables.

No functional change intended.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c | 74 +++++++++++++++++++++++++----------
 1 file changed, 54 insertions(+), 20 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index df8164da2a..b29d214724 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -20,15 +20,24 @@ 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;
+struct wdt_priv {
+	/* Timeout, in seconds, to configure this device to. */
+	u32 timeout;
+	/*
+	 * Time, in milliseconds, between calling the device's ->reset()
+	 * method from watchdog_reset().
+	 */
+	ulong reset_period;
+	/*
+	 * Next time (as returned by get_timer(0)) to call
+	 * ->reset().
+	 */
+	ulong next_reset;
+};
 
 int initr_watchdog(void)
 {
-	u32 timeout = WATCHDOG_TIMEOUT_SECS;
+	struct wdt_priv *priv;
 	int ret;
 
 	/*
@@ -44,28 +53,21 @@ int initr_watchdog(void)
 			return 0;
 		}
 	}
-
-	if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
-		timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
-					       WATCHDOG_TIMEOUT_SECS);
-		reset_period = dev_read_u32_default(gd->watchdog_dev,
-						    "hw_margin_ms",
-						    4 * reset_period) / 4;
-	}
+	priv = dev_get_uclass_priv(gd->watchdog_dev);
 
 	if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
 		printf("WDT:   Not starting\n");
 		return 0;
 	}
 
-	ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0);
+	ret = wdt_start(gd->watchdog_dev, priv->timeout * 1000, 0);
 	if (ret != 0) {
 		printf("WDT:   Failed to start\n");
 		return 0;
 	}
 
 	printf("WDT:   Started with%s servicing (%ds timeout)\n",
-	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
+	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
 
 	return 0;
 }
@@ -139,18 +141,21 @@ int wdt_expire_now(struct udevice *dev, ulong flags)
  */
 void watchdog_reset(void)
 {
-	static ulong next_reset;
+	struct wdt_priv *priv;
+	struct udevice *dev;
 	ulong now;
 
 	/* Exit if GD is not ready or watchdog is not initialized yet */
 	if (!gd || !(gd->flags & GD_FLG_WDT_READY))
 		return;
 
+	dev = gd->watchdog_dev;
+	priv = dev_get_uclass_priv(dev);
 	/* Do not reset the watchdog too often */
 	now = get_timer(0);
-	if (time_after_eq(now, next_reset)) {
-		next_reset = now + reset_period;
-		wdt_reset(gd->watchdog_dev);
+	if (time_after_eq(now, priv->next_reset)) {
+		priv->next_reset = now + priv->reset_period;
+		wdt_reset(dev);
 	}
 }
 #endif
@@ -177,9 +182,38 @@ static int wdt_post_bind(struct udevice *dev)
 	return 0;
 }
 
+static int wdt_pre_probe(struct udevice *dev)
+{
+	u32 timeout = WATCHDOG_TIMEOUT_SECS;
+	/*
+	 * Reset every 1000ms, or however often is required as
+	 * indicated by a hw_margin_ms property.
+	 */
+	ulong reset_period = 1000;
+	struct wdt_priv *priv;
+
+	if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
+		timeout = dev_read_u32_default(dev, "timeout-sec", timeout);
+		reset_period = dev_read_u32_default(dev, "hw_margin_ms",
+						    4 * reset_period) / 4;
+	}
+	priv = dev_get_uclass_priv(dev);
+	priv->timeout = timeout;
+	priv->reset_period = reset_period;
+	/*
+	 * Pretend this device was last reset "long" ago so the first
+	 * watchdog_reset will actually call its ->reset method.
+	 */
+	priv->next_reset = get_timer(0);
+
+	return 0;
+}
+
 UCLASS_DRIVER(wdt) = {
 	.id		= UCLASS_WDT,
 	.name		= "watchdog",
 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
 	.post_bind	= wdt_post_bind,
+	.pre_probe		= wdt_pre_probe,
+	.per_device_auto	= sizeof(struct wdt_priv),
 };
-- 
2.31.1

[PATCH v5 03/12] watchdog: wdt-uclass.c: neaten UCLASS_DRIVER definition

[Rasmus Villemoes]

The addition of .pre_probe and .per_device_auto made this look
bad. Fix it.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index b29d214724..81287c759a 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -210,10 +210,10 @@ static int wdt_pre_probe(struct udevice *dev)
 }
 
 UCLASS_DRIVER(wdt) = {
-	.id		= UCLASS_WDT,
-	.name		= "watchdog",
-	.flags		= DM_UC_FLAG_SEQ_ALIAS,
-	.post_bind	= wdt_post_bind,
+	.id			= UCLASS_WDT,
+	.name			= "watchdog",
+	.flags			= DM_UC_FLAG_SEQ_ALIAS,
+	.post_bind		= wdt_post_bind,
 	.pre_probe		= wdt_pre_probe,
 	.per_device_auto	= sizeof(struct wdt_priv),
 };
-- 
2.31.1

[PATCH v5 04/12] watchdog: wdt-uclass.c: refactor initr_watchdog()

[Rasmus Villemoes]

In preparation for handling all DM watchdogs in watchdog_reset(), pull
out the code which handles starting (or not) the gd->watchdog_dev
device.

Include the device name in various printfs.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c | 37 ++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 81287c759a..0a1f43771c 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -35,11 +35,30 @@ struct wdt_priv {
 	ulong next_reset;
 };
 
-int initr_watchdog(void)
+static void init_watchdog_dev(struct udevice *dev)
 {
 	struct wdt_priv *priv;
 	int ret;
 
+	priv = dev_get_uclass_priv(dev);
+
+	if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
+		printf("WDT:   Not starting %s\n", dev->name);
+		return;
+	}
+
+	ret = wdt_start(dev, priv->timeout * 1000, 0);
+	if (ret != 0) {
+		printf("WDT:   Failed to start %s\n", dev->name);
+		return;
+	}
+
+	printf("WDT:   Started %s with%s servicing (%ds timeout)\n", dev->name,
+	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
+}
+
+int initr_watchdog(void)
+{
 	/*
 	 * Init watchdog: This will call the probe function of the
 	 * watchdog driver, enabling the use of the device
@@ -53,21 +72,7 @@ int initr_watchdog(void)
 			return 0;
 		}
 	}
-	priv = dev_get_uclass_priv(gd->watchdog_dev);
-
-	if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
-		printf("WDT:   Not starting\n");
-		return 0;
-	}
-
-	ret = wdt_start(gd->watchdog_dev, priv->timeout * 1000, 0);
-	if (ret != 0) {
-		printf("WDT:   Failed to start\n");
-		return 0;
-	}
-
-	printf("WDT:   Started with%s servicing (%ds timeout)\n",
-	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
+	init_watchdog_dev(gd->watchdog_dev);
 
 	return 0;
 }
-- 
2.31.1

[PATCH v5 05/12] watchdog: wdt-uclass.c: keep track of each device's running state

[Rasmus Villemoes]

As a step towards handling all DM watchdogs in watchdog_reset(), use a
per-device flag to keep track of whether the device has been started
instead of a bit in gd->flags.

We will still need that bit to know whether we are past
initr_watchdog() and hence have populated gd->watchdog_dev -
incidentally, that is how it was used prior to commit 9c44ff1c5f3c.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 0a1f43771c..358fc68e27 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -33,6 +33,8 @@ struct wdt_priv {
 	 * ->reset().
 	 */
 	ulong next_reset;
+	/* Whether watchdog_start() has been called on the device. */
+	bool running;
 };
 
 static void init_watchdog_dev(struct udevice *dev)
@@ -74,6 +76,7 @@ int initr_watchdog(void)
 	}
 	init_watchdog_dev(gd->watchdog_dev);
 
+	gd->flags |= GD_FLG_WDT_READY;
 	return 0;
 }
 
@@ -86,8 +89,11 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 		return -ENOSYS;
 
 	ret = ops->start(dev, timeout_ms, flags);
-	if (ret == 0)
-		gd->flags |= GD_FLG_WDT_READY;
+	if (ret == 0) {
+		struct wdt_priv *priv = dev_get_uclass_priv(dev);
+
+		priv->running = true;
+	}
 
 	return ret;
 }
@@ -101,8 +107,11 @@ int wdt_stop(struct udevice *dev)
 		return -ENOSYS;
 
 	ret = ops->stop(dev);
-	if (ret == 0)
-		gd->flags &= ~GD_FLG_WDT_READY;
+	if (ret == 0) {
+		struct wdt_priv *priv = dev_get_uclass_priv(dev);
+
+		priv->running = false;
+	}
 
 	return ret;
 }
@@ -156,6 +165,9 @@ void watchdog_reset(void)
 
 	dev = gd->watchdog_dev;
 	priv = dev_get_uclass_priv(dev);
+	if (!priv->running)
+		return;
+
 	/* Do not reset the watchdog too often */
 	now = get_timer(0);
 	if (time_after_eq(now, priv->next_reset)) {
-- 
2.31.1

[PATCH v5 06/12] sandbox: disable CONFIG_WATCHDOG_AUTOSTART

[Rasmus Villemoes]

For the unit tests, it is more convenient if the tests are in charge
of when the watchdog devices are started and stopped, so prevent
wdt-uclass from doing it automatically.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 configs/sandbox64_defconfig | 1 +
 configs/sandbox_defconfig   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index f7098b4969..3627a066b4 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -223,6 +223,7 @@ CONFIG_OSD=y
 CONFIG_SANDBOX_OSD=y
 CONFIG_SPLASH_SCREEN_ALIGN=y
 CONFIG_VIDEO_BMP_RLE8=y
+# CONFIG_WATCHDOG_AUTOSTART is not set
 CONFIG_WDT=y
 CONFIG_WDT_SANDBOX=y
 CONFIG_FS_CBFS=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index bcd82f76ff..34b749b47b 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -282,6 +282,7 @@ CONFIG_W1=y
 CONFIG_W1_GPIO=y
 CONFIG_W1_EEPROM=y
 CONFIG_W1_EEPROM_SANDBOX=y
+# CONFIG_WATCHDOG_AUTOSTART is not set
 CONFIG_WDT=y
 CONFIG_WDT_SANDBOX=y
 CONFIG_FS_CBFS=y
-- 
2.31.1

[PATCH v5 07/12] watchdog: wdt-uclass.c: add wdt_stop_all() helper

[Rasmus Villemoes]

Since the watchdog_dev member of struct global_data is going away in
favor of the wdt-uclass handling all watchdog devices, prepare for
that by adding a helper to call wdt_stop() on all known devices.

Initially, this will only be used in one single
place (board/alliedtelesis/x530/x530.c), and that user currently
ignores the return value of wdt_stop(). It's not clear what one should
do if we encounter an error (remember the error but still stop the
remaining ones? return immediately? try to unwind and
restart the devices already stopped?). Moreover, some watchdogs are by
design always-running (and don't have a ->stop method at all), so at
the very least some exception for -ENOSYS would be in order.

So for now, and until a real use case appears from which we can decide
the desired semantics, have the function return void and just emit a
log_debug() if an error is encountered.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c | 25 +++++++++++++++++++++++++
 include/wdt.h                 |  5 +++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 358fc68e27..75ff4c2a6c 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -116,6 +116,31 @@ int wdt_stop(struct udevice *dev)
 	return ret;
 }
 
+void wdt_stop_all(void)
+{
+	struct wdt_priv *priv;
+	struct udevice *dev;
+	struct uclass *uc;
+	int ret;
+
+	ret = uclass_get(UCLASS_WDT, &uc);
+	if (ret) {
+		log_debug("Error getting UCLASS_WDT: %d\n", ret);
+		return;
+	}
+
+	uclass_foreach_dev(dev, uc) {
+		if (!device_active(dev))
+			continue;
+		priv = dev_get_uclass_priv(dev);
+		if (!priv->running)
+			continue;
+		ret = wdt_stop(dev);
+		if (ret)
+			log_debug("wdt_stop(%s) failed: %d\n", dev->name, ret);
+	}
+}
+
 int wdt_reset(struct udevice *dev)
 {
 	const struct wdt_ops *ops = device_get_ops(dev);
diff --git a/include/wdt.h b/include/wdt.h
index bc242c2eb2..5ab5cecbe0 100644
--- a/include/wdt.h
+++ b/include/wdt.h
@@ -37,6 +37,11 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags);
  */
 int wdt_stop(struct udevice *dev);
 
+/*
+ * Stop all registered watchdog devices.
+ */
+void wdt_stop_all(void);
+
 /*
  * Reset the timer, typically restoring the counter to
  * the value configured by start()
-- 
2.31.1

[PATCH v5 08/12] board: x530: switch to wdt_stop_all()

[Rasmus Villemoes]

Since the gd->watchdog_dev member is going away, switch to using the
new wdt_stop_all() helper.

While here, clean up the preprocessor conditional: The ->watchdog_dev
member is actually guarded by CONFIG_WDT [disabling that in
x530_defconfig while keeping CONFIG_WATCHDOG breaks the build], and in
the new world order so is the existence of the wdt_stop_all()
function.

Actually, existence of wdt_stop_all() depends on CONFIG_${SPL_}WDT, so
really spell the condition using CONFIG_IS_ENABLED, and make it a C
rather than cpp if.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 board/alliedtelesis/x530/x530.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/board/alliedtelesis/x530/x530.c b/board/alliedtelesis/x530/x530.c
index 7bcfa828d7..8b31045a07 100644
--- a/board/alliedtelesis/x530/x530.c
+++ b/board/alliedtelesis/x530/x530.c
@@ -121,9 +121,8 @@ int board_init(void)
 
 void arch_preboot_os(void)
 {
-#ifdef CONFIG_WATCHDOG
-	wdt_stop(gd->watchdog_dev);
-#endif
+	if (CONFIG_IS_ENABLED(WDT))
+		wdt_stop_all();
 }
 
 static int led_7seg_init(unsigned int segments)
-- 
2.31.1

[PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

[Rasmus Villemoes]

A board can have and make use of more than one watchdog device, say
one built into the SOC and an external gpio-petted one. Having
wdt-uclass only handle the first is both a little arbitrary and
unexpected.

So change initr_watchdog() so we visit (probe) all DM watchdog
devices, and call the init_watchdog_dev helper for each.

Similarly let watchdog_reset() loop over the whole uclass - each
having their own ratelimiting metadata, and a separate "is this device
running" flag.

This gets rid of the watchdog_dev member of struct global_data.  We
do, however, still need the GD_FLG_WDT_READY set in
initr_watchdog(). This is because watchdog_reset() can get called
before DM is ready, and I don't think we can call uclass_get() that
early.

The current code just returns 0 if "getting" the first device fails -
that can of course happen because there are no devices, but it could
also happen if its ->probe call failed. In keeping with that, continue
with the handling of the remaining devices even if one fails to
probe. This is also why we cannot use uclass_probe_all().

If desired, it's possible to later add a per-device "u-boot,autostart"
boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
per-device.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c     | 56 ++++++++++++++++++++-----------
 include/asm-generic/global_data.h |  6 ----
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 75ff4c2a6c..33c08bdab2 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -61,20 +61,24 @@ static void init_watchdog_dev(struct udevice *dev)
 
 int initr_watchdog(void)
 {
-	/*
-	 * Init watchdog: This will call the probe function of the
-	 * watchdog driver, enabling the use of the device
-	 */
-	if (uclass_get_device_by_seq(UCLASS_WDT, 0,
-				     (struct udevice **)&gd->watchdog_dev)) {
-		debug("WDT:   Not found by seq!\n");
-		if (uclass_get_device(UCLASS_WDT, 0,
-				      (struct udevice **)&gd->watchdog_dev)) {
-			printf("WDT:   Not found!\n");
-			return 0;
+	struct udevice *dev;
+	struct uclass *uc;
+	int ret;
+
+	ret = uclass_get(UCLASS_WDT, &uc);
+	if (ret) {
+		log_debug("Error getting UCLASS_WDT: %d\n", ret);
+		return 0;
+	}
+
+	uclass_foreach_dev(dev, uc) {
+		ret = device_probe(dev);
+		if (ret) {
+			log_debug("Error probing %s: %d\n", dev->name, ret);
+			continue;
 		}
+		init_watchdog_dev(dev);
 	}
-	init_watchdog_dev(gd->watchdog_dev);
 
 	gd->flags |= GD_FLG_WDT_READY;
 	return 0;
@@ -182,22 +186,34 @@ void watchdog_reset(void)
 {
 	struct wdt_priv *priv;
 	struct udevice *dev;
+	struct uclass *uc;
 	ulong now;
 
 	/* Exit if GD is not ready or watchdog is not initialized yet */
 	if (!gd || !(gd->flags & GD_FLG_WDT_READY))
 		return;
 
-	dev = gd->watchdog_dev;
-	priv = dev_get_uclass_priv(dev);
-	if (!priv->running)
+	if (uclass_get(UCLASS_WDT, &uc))
 		return;
 
-	/* Do not reset the watchdog too often */
-	now = get_timer(0);
-	if (time_after_eq(now, priv->next_reset)) {
-		priv->next_reset = now + priv->reset_period;
-		wdt_reset(dev);
+	/*
+	 * All devices bound to the wdt uclass should have been probed
+	 * in initr_watchdog(). But just in case something went wrong,
+	 * check device_active() before accessing the uclass private
+	 * data.
+	 */
+	uclass_foreach_dev(dev, uc) {
+		if (!device_active(dev))
+			continue;
+		priv = dev_get_uclass_priv(dev);
+		if (!priv->running)
+			continue;
+		/* Do not reset the watchdog too often */
+		now = get_timer(0);
+		if (time_after_eq(now, priv->next_reset)) {
+			priv->next_reset = now + priv->reset_period;
+			wdt_reset(dev);
+		}
 	}
 }
 #endif
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index e55070303f..28d749538c 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -447,12 +447,6 @@ struct global_data {
 	 */
 	fdt_addr_t translation_offset;
 #endif
-#if CONFIG_IS_ENABLED(WDT)
-	/**
-	 * @watchdog_dev: watchdog device
-	 */
-	struct udevice *watchdog_dev;
-#endif
 #ifdef CONFIG_GENERATE_ACPI_TABLE
 	/**
 	 * @acpi_ctx: ACPI context pointer
-- 
2.31.1

Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

[Wolfgang Denk]

Dear Rasmus,

In message <20210811124800.2593226-10-rasmus.villemoes@prevas.dk> you wrote:
>
> +	ret = uclass_get(UCLASS_WDT, &uc);
> +	if (ret) {
> +		log_debug("Error getting UCLASS_WDT: %d\n", ret);
> +		return 0;
> +	}

Here the error goes silent, so we should fix the callers to report
it.

> +	uclass_foreach_dev(dev, uc) {
> +		ret = device_probe(dev);
> +		if (ret) {
> +			log_debug("Error probing %s: %d\n", dev->name, ret);
> +			continue;
>  		}

Here the situation is different.  The probing error is never
reported anywhere.  Is it really a normal condition that a
device_probe() fails here?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Here is an Appalachian version of management's answer  to  those  who
are  concerned  with  the fate of the project: "Don't worry about the
mule. Just load the wagon."         - Mike Dennison's hillbilly uncle

Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

[Rasmus Villemoes]

On 12/08/2021 08.50, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <20210811124800.2593226-10-rasmus.villemoes@prevas.dk> you wrote:
>>
>> +	ret = uclass_get(UCLASS_WDT, &uc);
>> +	if (ret) {
>> +		log_debug("Error getting UCLASS_WDT: %d\n", ret);
>> +		return 0;
>> +	}
> 
> Here the error goes silent, so we should fix the callers to report
> it.

The caller (singular) is the initr sequence, so returning an error is
effectively the same as halting the boot process, and as I've already
explained, I'm not going to change the semantics of initr_watchdog in
this regard.

Note that, as I've also already explained, the code which this is
replacing would also fail if uclass_get() fails (there are uclass_get()s
done inside the uclass_get_device_by_seq() and uclass_get_device()), and
would similarly just turn an error from one of those into "return 0".

Feel free to submit a patch if you feel a change in this area is in
order. That's completely unrelated to what these patches are trying to
achieve.

>> +	uclass_foreach_dev(dev, uc) {
>> +		ret = device_probe(dev);
>> +		if (ret) {
>> +			log_debug("Error probing %s: %d\n", dev->name, ret);
>> +			continue;
>>  		}
> 
> Here the situation is different.  The probing error is never
> reported anywhere.  Is it really a normal condition that a
> device_probe() fails here?

No, it is not a normal condition. I added the log_debug() after a
request from Simon.

Rasmus

Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

[Wolfgang Denk]

Dear Rasmus,

In message <4798abb5-07d9-fa88-931f-dbaff951e3fb@prevas.dk> you wrote:
> >>
> >> +	ret = uclass_get(UCLASS_WDT, &uc);
> >> +	if (ret) {
> >> +		log_debug("Error getting UCLASS_WDT: %d\n", ret);
> >> +		return 0;
> >> +	}
> > 
> > Here the error goes silent, so we should fix the callers to report
> > it.
>
> The caller (singular) is the initr sequence, so returning an error is
> effectively the same as halting the boot process, and as I've already
> explained, I'm not going to change the semantics of initr_watchdog in
> this regard.

In this case you must print an error message here.

> Feel free to submit a patch if you feel a change in this area is in
> order. That's completely unrelated to what these patches are trying to
> achieve.

You add new code here, so please make sure not to add known issues.

> >> +	uclass_foreach_dev(dev, uc) {
> >> +		ret = device_probe(dev);
> >> +		if (ret) {
> >> +			log_debug("Error probing %s: %d\n", dev->name, ret);
> >> +			continue;
> >>  		}
> > 
> > Here the situation is different.  The probing error is never
> > reported anywhere.  Is it really a normal condition that a
> > device_probe() fails here?
>
> No, it is not a normal condition. I added the log_debug() after a
> request from Simon.

But log_debug() is nothing any user will see in the field.  We need
an error message here, too.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
If the odds are a million to one against something occuring,  chances
are 50-50 it will.

Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]

On Thu, Aug 19, 2021 at 01:10:54PM +0200, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <4798abb5-07d9-fa88-931f-dbaff951e3fb@prevas.dk> you wrote:
> > >>
> > >> +	ret = uclass_get(UCLASS_WDT, &uc);
> > >> +	if (ret) {
> > >> +		log_debug("Error getting UCLASS_WDT: %d\n", ret);
> > >> +		return 0;
> > >> +	}
> > > 
> > > Here the error goes silent, so we should fix the callers to report
> > > it.
> >
> > The caller (singular) is the initr sequence, so returning an error is
> > effectively the same as halting the boot process, and as I've already
> > explained, I'm not going to change the semantics of initr_watchdog in
> > this regard.
> 
> In this case you must print an error message here.
> 
> > Feel free to submit a patch if you feel a change in this area is in
> > order. That's completely unrelated to what these patches are trying to
> > achieve.
> 
> You add new code here, so please make sure not to add known issues.
> 
> > >> +	uclass_foreach_dev(dev, uc) {
> > >> +		ret = device_probe(dev);
> > >> +		if (ret) {
> > >> +			log_debug("Error probing %s: %d\n", dev->name, ret);
> > >> +			continue;
> > >>  		}
> > > 
> > > Here the situation is different.  The probing error is never
> > > reported anywhere.  Is it really a normal condition that a
> > > device_probe() fails here?
> >
> > No, it is not a normal condition. I added the log_debug() after a
> > request from Simon.
> 
> But log_debug() is nothing any user will see in the field.  We need
> an error message here, too.

Wolfgang,

If you would like to come along afterwards with additional logging
changes, please do so.  As Rasmus has pointed out numerous times at this
point, what he's doing is consistent with everything else in these
areas.  Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v5 10/12] watchdog: add gpio watchdog driver

[Rasmus Villemoes]

A rather common kind of external watchdog circuit is one that is kept
alive by toggling a gpio. Add a driver for handling such a watchdog.

The corresponding linux driver apparently has support for some
watchdog circuits which can be disabled by tri-stating the gpio, but I
have never actually encountered such a chip in the wild; the whole
point of adding an external watchdog is usually that it is not in any
way under software control. For forward-compatibility, and to make DT
describe the hardware, the current driver only supports devices that
have the always-running property. I went a little back and forth on
whether I should fail ->probe or only ->start, and ended up deciding
->start was the right place.

The compatible string is probably a little odd as it has nothing to do
with linux per se - however, I chose that to make .dts snippets
reusable between device trees used with U-Boot and linux, and this is
the (only) compatible string that linux' corresponding driver and DT
binding accepts. I have asked whether one should/could add "wdt-gpio"
to that binding, but the answer was no:

  https://lore.kernel.org/lkml/CAL_JsqKEGaFpiFV_oAtE+S_bnHkg4qry+bhx2EDs=NSbVf_giA@mail.gmail.com/

If someone feels strongly about this, I can certainly remove the
"linux," part from the string - it probably wouldn't the only place where
one can't reuse a DT snippet as-is between linux and U-Boot.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 .../watchdog/gpio-wdt.txt                     | 19 ++++++
 drivers/watchdog/Kconfig                      |  9 +++
 drivers/watchdog/Makefile                     |  1 +
 drivers/watchdog/gpio_wdt.c                   | 68 +++++++++++++++++++
 4 files changed, 97 insertions(+)
 create mode 100644 doc/device-tree-bindings/watchdog/gpio-wdt.txt
 create mode 100644 drivers/watchdog/gpio_wdt.c

diff --git a/doc/device-tree-bindings/watchdog/gpio-wdt.txt b/doc/device-tree-bindings/watchdog/gpio-wdt.txt
new file mode 100644
index 0000000000..c9a8559a3e
--- /dev/null
+++ b/doc/device-tree-bindings/watchdog/gpio-wdt.txt
@@ -0,0 +1,19 @@
+GPIO watchdog timer
+
+Describes a simple watchdog timer which is reset by toggling a gpio.
+
+Required properties:
+
+- compatible: Must be "linux,wdt-gpio".
+- gpios: gpio to toggle when wdt driver reset method is called.
+- always-running: Boolean property indicating that the watchdog cannot
+  be disabled. At present, U-Boot only supports this kind of GPIO
+  watchdog.
+
+Example:
+
+	gpio-wdt {
+		gpios = <&gpio0 1 0>;
+		compatible = "linux,wdt-gpio";
+		always-running;
+	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f0ff2612a6..6fbb5c1b6d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -147,6 +147,15 @@ config WDT_CORTINA
 	  This driver support all CPU ISAs supported by Cortina
 	  Access CAxxxx SoCs.
 
+config WDT_GPIO
+	bool "External gpio watchdog support"
+	depends on WDT
+	depends on DM_GPIO
+	help
+	  Support for external watchdog fed by toggling a gpio. See
+	  doc/device-tree-bindings/watchdog/gpio-wdt.txt for
+	  information on how to describe the watchdog in device tree.
+
 config WDT_MPC8xx
 	bool "MPC8xx watchdog timer support"
 	depends on WDT && MPC8xx
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 5c7ef593fe..f14415bb8e 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_WDT_BOOKE) += booke_wdt.o
 obj-$(CONFIG_WDT_CORTINA) += cortina_wdt.o
 obj-$(CONFIG_WDT_ORION) += orion_wdt.o
 obj-$(CONFIG_WDT_CDNS) += cdns_wdt.o
+obj-$(CONFIG_WDT_GPIO) += gpio_wdt.o
 obj-$(CONFIG_WDT_MPC8xx) += mpc8xx_wdt.o
 obj-$(CONFIG_WDT_MT7620) += mt7620_wdt.o
 obj-$(CONFIG_WDT_MT7621) += mt7621_wdt.o
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
new file mode 100644
index 0000000000..982a66b3f9
--- /dev/null
+++ b/drivers/watchdog/gpio_wdt.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <dm.h>
+#include <dm/device_compat.h>
+#include <wdt.h>
+#include <asm/gpio.h>
+
+struct gpio_wdt_priv {
+	struct gpio_desc gpio;
+	bool always_running;
+	int state;
+};
+
+static int gpio_wdt_reset(struct udevice *dev)
+{
+	struct gpio_wdt_priv *priv = dev_get_priv(dev);
+
+	priv->state = !priv->state;
+
+	return dm_gpio_set_value(&priv->gpio, priv->state);
+}
+
+static int gpio_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
+{
+	struct gpio_wdt_priv *priv = dev_get_priv(dev);
+
+	if (priv->always_running)
+		return 0;
+
+	return -ENOSYS;
+}
+
+static int dm_probe(struct udevice *dev)
+{
+	struct gpio_wdt_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	priv->always_running = dev_read_bool(dev, "always-running");
+	ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio, GPIOD_IS_OUT);
+	if (ret < 0) {
+		dev_err(dev, "Request for wdt gpio failed: %d\n", ret);
+		return ret;
+	}
+
+	if (priv->always_running)
+		ret = gpio_wdt_reset(dev);
+
+	return ret;
+}
+
+static const struct wdt_ops gpio_wdt_ops = {
+	.start = gpio_wdt_start,
+	.reset = gpio_wdt_reset,
+};
+
+static const struct udevice_id gpio_wdt_ids[] = {
+	{ .compatible = "linux,wdt-gpio" },
+	{}
+};
+
+U_BOOT_DRIVER(wdt_gpio) = {
+	.name = "wdt_gpio",
+	.id = UCLASS_WDT,
+	.of_match = gpio_wdt_ids,
+	.ops = &gpio_wdt_ops,
+	.probe	= dm_probe,
+	.priv_auto = sizeof(struct gpio_wdt_priv),
+};
-- 
2.31.1

[PATCH v5 11/12] sandbox: add test of wdt_gpio driver

[Rasmus Villemoes]

It seems that no other test has claimed gpio_a:7 yet, so use that.

The only small wrinkle is modifying the existing wdt test to use
uclass_get_device_by_driver() since we now have two UCLASS_WDT
instances in play, so it's a little more robust to fetch the device by
driver and not merely uclass+index.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 arch/sandbox/dts/test.dts   |  6 ++++++
 configs/sandbox64_defconfig |  1 +
 configs/sandbox_defconfig   |  1 +
 test/dm/wdt.c               | 36 +++++++++++++++++++++++++++++++++++-
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index d5976318d1..fe5ac6ecd9 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -779,6 +779,12 @@
 		};
 	};
 
+	gpio-wdt {
+		gpios = <&gpio_a 7 0>;
+		compatible = "linux,wdt-gpio";
+		always-running;
+	};
+
 	mbox: mbox {
 		compatible = "sandbox,mbox";
 		#mbox-cells = <1>;
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 3627a066b4..6cad90b03e 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -225,6 +225,7 @@ CONFIG_SPLASH_SCREEN_ALIGN=y
 CONFIG_VIDEO_BMP_RLE8=y
 # CONFIG_WATCHDOG_AUTOSTART is not set
 CONFIG_WDT=y
+CONFIG_WDT_GPIO=y
 CONFIG_WDT_SANDBOX=y
 CONFIG_FS_CBFS=y
 CONFIG_FS_CRAMFS=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 34b749b47b..4a8b4f220d 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -284,6 +284,7 @@ CONFIG_W1_EEPROM=y
 CONFIG_W1_EEPROM_SANDBOX=y
 # CONFIG_WATCHDOG_AUTOSTART is not set
 CONFIG_WDT=y
+CONFIG_WDT_GPIO=y
 CONFIG_WDT_SANDBOX=y
 CONFIG_FS_CBFS=y
 CONFIG_FS_CRAMFS=y
diff --git a/test/dm/wdt.c b/test/dm/wdt.c
index 24b991dff6..abff853a02 100644
--- a/test/dm/wdt.c
+++ b/test/dm/wdt.c
@@ -6,6 +6,7 @@
 #include <common.h>
 #include <dm.h>
 #include <wdt.h>
+#include <asm/gpio.h>
 #include <asm/state.h>
 #include <asm/test.h>
 #include <dm/test.h>
@@ -19,7 +20,8 @@ static int dm_test_wdt_base(struct unit_test_state *uts)
 	struct udevice *dev;
 	const u64 timeout = 42;
 
-	ut_assertok(uclass_get_device(UCLASS_WDT, 0, &dev));
+	ut_assertok(uclass_get_device_by_driver(UCLASS_WDT,
+						DM_DRIVER_GET(wdt_sandbox), &dev));
 	ut_assertnonnull(dev);
 	ut_asserteq(0, state->wdt.counter);
 	ut_asserteq(false, state->wdt.running);
@@ -39,3 +41,35 @@ static int dm_test_wdt_base(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_wdt_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+static int dm_test_wdt_gpio(struct unit_test_state *uts)
+{
+	/*
+	 * The sandbox wdt gpio is "connected" to gpio bank a, offset
+	 * 7. Use the sandbox back door to verify that the gpio-wdt
+	 * driver behaves as expected.
+	 */
+	struct udevice *wdt, *gpio;
+	const u64 timeout = 42;
+	const int offset = 7;
+	int val;
+
+	ut_assertok(uclass_get_device_by_driver(UCLASS_WDT,
+						DM_DRIVER_GET(wdt_gpio), &wdt));
+	ut_assertnonnull(wdt);
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_GPIO, "base-gpios", &gpio));
+	ut_assertnonnull(gpio);
+	ut_assertok(wdt_start(wdt, timeout, 0));
+
+	val = sandbox_gpio_get_value(gpio, offset);
+	ut_assertok(wdt_reset(wdt));
+	ut_asserteq(!val, sandbox_gpio_get_value(gpio, offset));
+	ut_assertok(wdt_reset(wdt));
+	ut_asserteq(val, sandbox_gpio_get_value(gpio, offset));
+
+	ut_asserteq(-ENOSYS, wdt_stop(wdt));
+
+	return 0;
+}
+DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT);
-- 
2.31.1

[PATCH v5 12/12] sandbox: add test of wdt-uclass' watchdog_reset()

[Rasmus Villemoes]

Check that the watchdog_reset() implementation in wdt-uclass behaves
as expected:

- resets all activated watchdog devices
- leaves unactivated/stopped devices alone
- that the rate-limiting works, with a per-device threshold

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 arch/sandbox/dts/test.dts |  2 ++
 test/dm/wdt.c             | 54 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fe5ac6ecd9..bafc2e9494 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -782,6 +782,7 @@
 	gpio-wdt {
 		gpios = <&gpio_a 7 0>;
 		compatible = "linux,wdt-gpio";
+		hw_margin_ms = <100>;
 		always-running;
 	};
 
@@ -1264,6 +1265,7 @@
 
 	wdt0: wdt@0 {
 		compatible = "sandbox,wdt";
+		hw_margin_ms = <200>;
 	};
 
 	axi: axi@0 {
diff --git a/test/dm/wdt.c b/test/dm/wdt.c
index abff853a02..ee615f0e14 100644
--- a/test/dm/wdt.c
+++ b/test/dm/wdt.c
@@ -12,6 +12,8 @@
 #include <dm/test.h>
 #include <test/test.h>
 #include <test/ut.h>
+#include <linux/delay.h>
+#include <watchdog.h>
 
 /* Test that watchdog driver functions are called */
 static int dm_test_wdt_base(struct unit_test_state *uts)
@@ -73,3 +75,55 @@ static int dm_test_wdt_gpio(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT);
+
+static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts)
+{
+	struct sandbox_state *state = state_get_current();
+	struct udevice *gpio_wdt, *sandbox_wdt;
+	struct udevice *gpio;
+	const u64 timeout = 42;
+	const int offset = 7;
+	uint reset_count;
+	int val;
+
+	ut_assertok(uclass_get_device_by_driver(UCLASS_WDT,
+						DM_DRIVER_GET(wdt_gpio), &gpio_wdt));
+	ut_assertnonnull(gpio_wdt);
+	ut_assertok(uclass_get_device_by_driver(UCLASS_WDT,
+						DM_DRIVER_GET(wdt_sandbox), &sandbox_wdt));
+	ut_assertnonnull(sandbox_wdt);
+	ut_assertok(uclass_get_device_by_name(UCLASS_GPIO, "base-gpios", &gpio));
+	ut_assertnonnull(gpio);
+
+	/* Neither device should be "started", so watchdog_reset() should be a no-op. */
+	reset_count = state->wdt.reset_count;
+	val = sandbox_gpio_get_value(gpio, offset);
+	watchdog_reset();
+	ut_asserteq(reset_count, state->wdt.reset_count);
+	ut_asserteq(val, sandbox_gpio_get_value(gpio, offset));
+
+	/* Start both devices. */
+	ut_assertok(wdt_start(gpio_wdt, timeout, 0));
+	ut_assertok(wdt_start(sandbox_wdt, timeout, 0));
+
+	/* Make sure both devices have just been pinged. */
+	timer_test_add_offset(100);
+	watchdog_reset();
+	reset_count = state->wdt.reset_count;
+	val = sandbox_gpio_get_value(gpio, offset);
+
+	/* The gpio watchdog should be pinged, the sandbox one not. */
+	timer_test_add_offset(30);
+	watchdog_reset();
+	ut_asserteq(reset_count, state->wdt.reset_count);
+	ut_asserteq(!val, sandbox_gpio_get_value(gpio, offset));
+
+	/* After another ~30ms, both devices should get pinged. */
+	timer_test_add_offset(30);
+	watchdog_reset();
+	ut_asserteq(reset_count + 1, state->wdt.reset_count);
+	ut_asserteq(val, sandbox_gpio_get_value(gpio, offset));
+
+	return 0;
+}
+DM_TEST(dm_test_wdt_watchdog_reset, UT_TESTF_SCAN_FDT);
-- 
2.31.1