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? David > > NACK. > > Guenter > > > - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
