From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: u-boot@lists.denx.de
Cc: Stefan Roese <sr@denx.de>, Tom Rini <trini@konsulko.com>,
Marek Vasut <marek.vasut+renesas@mailbox.org>,
Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Subject: [PATCH 2/3] wdt-uclass: prevent multiple cyclic_register calls
Date: Thu, 9 May 2024 02:47:13 +0200 [thread overview]
Message-ID: <20240509004714.1394547-3-rasmus.villemoes@prevas.dk> (raw)
In-Reply-To: <20240509004714.1394547-1-rasmus.villemoes@prevas.dk>
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.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@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 417e8d7eef9..caa1567e0c3 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -122,10 +122,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,
@@ -140,6 +141,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));
@@ -160,6 +162,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
next prev parent reply other threads:[~2024-05-09 0:47 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-09 0:47 [PATCH 0/3] cyclic/watchdog patches Rasmus Villemoes
2024-05-09 0:47 ` [PATCH 1/3] cyclic: stop strdup'ing name in cyclic_register() Rasmus Villemoes
2024-05-13 6:03 ` Stefan Roese
2024-05-09 0:47 ` Rasmus Villemoes [this message]
2024-05-13 10:40 ` [PATCH 2/3] wdt-uclass: prevent multiple cyclic_register calls Stefan Roese
2024-05-13 11:09 ` Rasmus Villemoes
2024-05-13 14:24 ` Stefan Roese
2024-05-09 0:47 ` [PATCH 3/3] cyclic: make clients embed a struct cyclic_info in their own data structure Rasmus Villemoes
2024-05-16 6:43 ` Stefan Roese
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240509004714.1394547-3-rasmus.villemoes@prevas.dk \
--to=rasmus.villemoes@prevas.dk \
--cc=marek.vasut+renesas@mailbox.org \
--cc=sr@denx.de \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).