[PATCH v2 2/3] wdt-uclass: prevent multiple cyclic_register calls

Rasmus Villemoes rasmus.villemoes at prevas.dk
Thu May 16 09:53:13 CEST 2024


Currently, the cyclic_register() done in wdt_start() is not undone in
wdt_stop(). Moreover, calling wdt_start multiple times (which is
perfectly allowed on an already started device, e.g. to change the
timeout value) will result in another struct cyclic_info being
registered, referring to the same watchdog device.

This can easily be seen on e.g. a wandboard:

=> cyclic list
function: watchdog at 20bc000, cpu-time: 22 us, frequency: 1.01 times/s
=> wdt list
watchdog at 20bc000 (imx_wdt)
=> wdt dev watchdog at 20bc000
=> wdt start 50000
WDT:   Started watchdog at 20bc000 with servicing every 1000ms (50s timeout)
=> cyclic list
function: watchdog at 20bc000, cpu-time: 37 us, frequency: 1.03 times/s
function: watchdog at 20bc000, cpu-time: 241 us, frequency: 1.01 times/s
=> wdt start 12345
WDT:   Started watchdog at 20bc000 with servicing every 1000ms (12s timeout)
=> cyclic list
function: watchdog at 20bc000, cpu-time: 36 us, frequency: 1.03 times/s
function: watchdog at 20bc000, cpu-time: 100 us, frequency: 1.04 times/s
function: watchdog at 20bc000, cpu-time: 299 us, frequency: 1.00 times/s

So properly unregister the watchdog device from the cyclic framework
in wdt_stop(). In wdt_start(), we cannot just skip the registration,
as the (new) timeout value may mean that we have to ask the cyclic
framework to call us more often. So if we're already running,
first unregister the old cyclic instance.

Reviewed-by: Stefan Roese <sr at denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
---
 drivers/watchdog/wdt-uclass.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index c88312ec721..12850016c93 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -121,10 +121,11 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 		struct wdt_priv *priv = dev_get_uclass_priv(dev);
 		char str[16];
 
-		priv->running = true;
-
 		memset(str, 0, 16);
 		if (IS_ENABLED(CONFIG_WATCHDOG)) {
+			if (priv->running)
+				cyclic_unregister(priv->cyclic);
+
 			/* Register the watchdog driver as a cyclic function */
 			priv->cyclic = cyclic_register(wdt_cyclic,
 						       priv->reset_period * 1000,
@@ -139,6 +140,7 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 			}
 		}
 
+		priv->running = true;
 		printf("WDT:   Started %s with%s servicing %s (%ds timeout)\n",
 		       dev->name, IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out",
 		       str, (u32)lldiv(timeout_ms, 1000));
@@ -159,6 +161,9 @@ int wdt_stop(struct udevice *dev)
 	if (ret == 0) {
 		struct wdt_priv *priv = dev_get_uclass_priv(dev);
 
+		if (IS_ENABLED(CONFIG_WATCHDOG) && priv->running)
+			cyclic_unregister(priv->cyclic);
+
 		priv->running = false;
 	}
 
-- 
2.40.1.1.g1c60b9335d



More information about the U-Boot mailing list