On 12/14/24 13:21, David Laight wrote:
From: Guenter Roeck
Sent: 12 December 2024 13:56
To: Phil Eichinger <phil@xxxxxxxxxxxxx>; wim@xxxxxxxxxxxxxxxxxx; joel@xxxxxxxxx;
andrew@xxxxxxxxxxxxxxxxxxxx; linux-watchdog@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
linux-aspeed@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] watchdog: aspeed: replace mdelay with msleep
On 12/12/24 03:30, Phil Eichinger wrote:
Since it is not called in an atomic context the mdelay function
can be replaced with msleep to avoid busy wait.
Signed-off-by: Phil Eichinger <phil@xxxxxxxxxxxxx>
---
drivers/watchdog/aspeed_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index b4773a6aaf8c..98ef341408f7 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -208,7 +208,7 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd,
wdt->ctrl &= ~WDT_CTRL_BOOT_SECONDARY;
aspeed_wdt_enable(wdt, 128 * WDT_RATE_1MHZ / 1000);
- mdelay(1000);
+ msleep(1000);
return 0;
}
This is a _restart_ handler. The only purpose of the delay is to wait
for the reset to trigger. It is not supposed to sleep.
With the recent scheduler changes isn't the code likely to get
pre-empted?
Which (effectively) converts is to a sleep?
This code is called from do_kernel_restart(), which in turn is called from
machine_restart(). I'd think that the kernel has a severe problem if it
decides to preempt that function.
Guenter
