From df2b3829c67fc059fb4b6d8d7dfdc5946d199674 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 21 May 2024 10:46:51 +0200 Subject: [PATCH] wdt-uclass: prevent multiple cyclic_register calls 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@20bc000, cpu-time: 22 us, frequency: 1.01 times/s => wdt list watchdog@20bc000 (imx_wdt) => wdt dev watchdog@20bc000 => wdt start 50000 WDT: Started watchdog@20bc000 with servicing every 1000ms (50s timeout) => cyclic list function: watchdog@20bc000, cpu-time: 37 us, frequency: 1.03 times/s function: watchdog@20bc000, cpu-time: 241 us, frequency: 1.01 times/s => wdt start 12345 WDT: Started watchdog@20bc000 with servicing every 1000ms (12s timeout) => cyclic list function: watchdog@20bc000, cpu-time: 36 us, frequency: 1.03 times/s function: watchdog@20bc000, cpu-time: 100 us, frequency: 1.04 times/s function: watchdog@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 Signed-off-by: Rasmus Villemoes --- 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 c88312ec72..12850016c9 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.39.5