[PATCH] watchdog: dw_wdt: Stop ping worker on system suspend

[PATCH] watchdog: dw_wdt: Stop ping worker on system suspend

[Guenter Roeck]

When the Synopsys DesignWare watchdog is active with the watchdog core
heartbeat worker running (such as when WDOG_HW_RUNNING is set at boot,
when the watchdog is left running after /dev/watchdog is closed, or when
configured with a timeout exceeding wdd->max_hw_heartbeat_ms), the
watchdog core arms its hrtimer so the non-freezable watchdogd kthread
worker periodically pings the hardware.

Because dw_wdt_drv_probe() does not call watchdog_stop_ping_on_suspend(),
WDOG_NO_PING_ON_SUSPEND is not set and the watchdog core leaves its
hrtimer and ping work active across system suspend. After
dw_wdt_suspend() asserts dw_wdt->rst and disables dw_wdt->pclk and
dw_wdt->clk, an expiring timer queues watchdog_ping_work(), invoking
dw_wdt_ping() to write WDOG_COUNTER_RESTART_REG_OFFSET while the
controller is unclocked and in reset. This may trigger a synchronous
external abort, bus error, or system hang.

This did not matter and actually worked as expected since the watchdog
can not be stopped directly. However, commit 1bfe88893808 ("watchdog:
dw_wdt: add stop watchdog operation") implemented stop functionality by
resetting the watchdog core, and commit 0b0ff67cbc2aa ("watchdog: dw_wdt:
manage reset line during system suspend/resume") puts the watchdog hardware
into reset during suspend, causing the potential problem described above.

Call watchdog_stop_ping_on_suspend() in dw_wdt_drv_probe() so the
watchdog core cancels its ping timer and work in PM_SUSPEND_PREPARE
before clocks are gated and resumes pinging in PM_POST_SUSPEND after
clocks and reset are restored.

Fixes: f29a72c24ad4 ("watchdog: dw_wdt: Convert to use watchdog infrastructure")
Fixes: 1bfe88893808 ("watchdog: dw_wdt: add stop watchdog operation")
Fixes: 0b0ff67cbc2aa ("watchdog: dw_wdt: manage reset line during system suspend/resume")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/bug/linux-4cde70aa-2cd0-405d-a626-7b41bfe6bbb7
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/dw_wdt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index c1d40d7b2165..7c1c2378f475 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -653,6 +653,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 
 	watchdog_set_restart_priority(wdd, 128);
 	watchdog_stop_on_reboot(wdd);
+	watchdog_stop_ping_on_suspend(wdd);
 
 	ret = watchdog_register_device(wdd);
 	if (ret)
-- 
2.45.2

Re: [PATCH] watchdog: dw_wdt: Stop ping worker on system suspend

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] watchdog: dw_wdt: unresolvable SHA 0b0ff67cbc2aa in Fixes
  trailer
--

commit dfd3154d77d7cf35efcd8342d85817dd9459cc8c
Author: Guenter Roeck <linux@roeck-us.net>

> watchdog: dw_wdt: Stop ping worker on system suspend
>
> When the Synopsys DesignWare watchdog is active with the watchdog core
> heartbeat worker running (such as when WDOG_HW_RUNNING is set at boot,
> when the watchdog is left running after /dev/watchdog is closed, or when
> configured with a timeout exceeding wdd->max_hw_heartbeat_ms), the
> watchdog core arms its hrtimer so the non-freezable watchdogd kthread
> worker periodically pings the hardware.
>
> Because dw_wdt_drv_probe() does not call watchdog_stop_ping_on_suspend(),
> WDOG_NO_PING_ON_SUSPEND is not set and the watchdog core leaves its
> hrtimer and ping work active across system suspend. After
> dw_wdt_suspend() asserts dw_wdt->rst and disables dw_wdt->pclk and
> dw_wdt->clk, an expiring timer queues watchdog_ping_work(), invoking
> dw_wdt_ping() to write WDOG_COUNTER_RESTART_REG_OFFSET while the
> controller is unclocked and in reset. This may trigger a synchronous
> external abort, bus error, or system hang.
>
> This did not matter and actually worked as expected since the watchdog
> can not be stopped directly. However, commit 1bfe88893808 ("watchdog:
> dw_wdt: add stop watchdog operation") implemented stop functionality by
> resetting the watchdog core, and commit 0b0ff67cbc2aa ("watchdog: dw_wdt:
> manage reset line during system suspend/resume") puts the watchdog hardware
> into reset during suspend, causing the potential problem described above.
>
> Call watchdog_stop_ping_on_suspend() in dw_wdt_drv_probe() so the
> watchdog core cancels its ping timer and work in PM_SUSPEND_PREPARE
> before clocks are gated and resumes pinging in PM_POST_SUSPEND after
> clocks and reset are restored.
>
> Fixes: f29a72c24ad4 ("watchdog: dw_wdt: Convert to use watchdog infrastructure")
> Fixes: 1bfe88893808 ("watchdog: dw_wdt: add stop watchdog operation")
> Fixes: 0b0ff67cbc2aa ("watchdog: dw_wdt: manage reset line during system suspend/resume")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/bug/linux-4cde70aa-2cd0-405d-a626-7b41bfe6bbb7
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

[Severity: Low]
The commit SHA 0b0ff67cbc2aa ("watchdog: dw_wdt: manage reset line during
system suspend/resume") cited in the commit description and third Fixes
trailer cannot be resolved in the repository. Should this reference
cbe95d1faf57 ("watchdog: dw_wdt: manage reset line during system
suspend/resume") instead:

  Fixes: cbe95d1faf57 ("watchdog: dw_wdt: manage reset line during system suspend/resume")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108233329.1059273-1-linux@roeck-us.net?part=1