[PATCH] watchdog: mtk_wdt: Stop a running watchdog over system sleep even if not opened

[PATCH] watchdog: mtk_wdt: Stop a running watchdog over system sleep even if not opened

[zoan37]

mtk_wdt_suspend() and mtk_wdt_resume() only stop and restart the
watchdog when it is active, that is, when userspace has opened it.

But a watchdog that the bootloader left enabled stays enabled at probe,
marked WDOG_HW_RUNNING, and the watchdog core pings it until userspace
takes over; that has been the case since
commit bbece05c0d3a ("watchdog: mtk_wdt: Remove mtk_wdt_stop() in probe()
to prevent the system freeze and it doesn't reboot by watchdog problem").
If the system suspends before userspace opens the device, or userspace
never does, nothing stops the watchdog and nothing pings it while the
system sleeps, so it resets the system one timeout (31 s by default)
into the sleep.

Also stop it on suspend and restart it on resume when it is running in
hardware, like sp805_wdt does.

Fixes: bbece05c0d3a ("watchdog: mtk_wdt: Remove mtk_wdt_stop() in probe() to prevent the system freeze and it doesn't reboot by watchdog problem")
Assisted-by: LLM
Signed-off-by: zoan37 <agentzoan@gmail.com>
---

Notes:
    Testing: on an MT8189 Chromebook (next-20261008 plus MT8189 support)
    whose firmware leaves the watchdog off; a local module parameter starts
    it in probe the same way a bootloader-enabled watchdog is picked up
    (WDOG_HW_RUNNING set, fed by the watchdog core, nobody opening
    /dev/watchdog). Without this patch the board reset about 30 s into
    s2idle; with it a 10-minute s2idle sleep came back without a reset.
    Not tested with a watchdog actually left enabled by a bootloader.
    Compile-tested (W=1) on next-20261008.

 drivers/watchdog/mtk_wdt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index 1630ab65d593..ff72e0fb1154 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -572,7 +572,8 @@ static int mtk_wdt_suspend(struct device *dev)
 {
 	struct mtk_wdt_dev *mtk_wdt = dev_get_drvdata(dev);
 
-	if (watchdog_active(&mtk_wdt->wdt_dev))
+	if (watchdog_active(&mtk_wdt->wdt_dev) ||
+	    watchdog_hw_running(&mtk_wdt->wdt_dev))
 		mtk_wdt_stop(&mtk_wdt->wdt_dev);
 
 	return 0;
@@ -582,7 +583,8 @@ static int mtk_wdt_resume(struct device *dev)
 {
 	struct mtk_wdt_dev *mtk_wdt = dev_get_drvdata(dev);
 
-	if (watchdog_active(&mtk_wdt->wdt_dev)) {
+	if (watchdog_active(&mtk_wdt->wdt_dev) ||
+	    watchdog_hw_running(&mtk_wdt->wdt_dev)) {
 		mtk_wdt_start(&mtk_wdt->wdt_dev);
 		mtk_wdt_ping(&mtk_wdt->wdt_dev);
 	}

base-commit: aac26bee2287c88af5be5a5ff96d783b19a28790
-- 
2.43.0

Re: [PATCH] watchdog: mtk_wdt: Stop a running watchdog over system sleep even if not opened

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: mtk_wdt: background ping worker accesses suspended device
--

commit 86c1d5dc273343e1ae7c163f9a06c48451758c35
Author: zoan37 <agentzoan@gmail.com>

watchdog: mtk_wdt: Stop a running watchdog over system sleep even if not opened

This commit updates mtk_wdt_suspend() and mtk_wdt_resume() to also stop and
restart the watchdog when it is running in hardware but hasn't been opened
by userspace yet.

> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 1630ab65d5932..ff72e0fb11541 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -572,7 +572,8 @@ static int mtk_wdt_suspend(struct device *dev)
>  {
>  	struct mtk_wdt_dev *mtk_wdt = dev_get_drvdata(dev);
>  
> -	if (watchdog_active(&mtk_wdt->wdt_dev))
> +	if (watchdog_active(&mtk_wdt->wdt_dev) ||
> +	    watchdog_hw_running(&mtk_wdt->wdt_dev))
>  		mtk_wdt_stop(&mtk_wdt->wdt_dev);

[Severity: High]
Does stopping the hardware here leave the software ping worker in an
inconsistent state?

The watchdog core's PM notifier is only registered if the driver explicitly
opts-in by setting the WDOG_NO_PING_ON_SUSPEND flag:

drivers/watchdog/watchdog_core.c:___watchdog_register_device() {
...
	if (test_bit(WDOG_NO_PING_ON_SUSPEND, &wdd->status)) {
		wdd->pm_nb.notifier_call = watchdog_pm_notifier;
...
}

Because mtk_wdt doesn't set this flag, watchdog_dev_suspend() is never called
to cancel the hrtimer and kthread_work for the background ping worker:

drivers/watchdog/watchdog_dev.c:watchdog_dev_suspend() {
...
	hrtimer_cancel(&wd_data->timer);
	kthread_cancel_work_sync(&wd_data->work);
...
}

During sleep, the hrtimer can expire and be deferred. Upon wake-up, timekeeping
resumes in syscore_resume and the overdue hrtimer fires in hardirq context,
waking the watchdogd thread.

If watchdogd preempts the suspend thread and executes mtk_wdt_ping() before
mtk_wdt_resume() runs, it will attempt to access device registers:

drivers/watchdog/mtk_wdt.c:mtk_wdt_ping() {
...
	iowrite32(WDT_RST_RELOAD, wdt_base + WDT_RST);
...
}

If the device's bus clocks or power domains are gated during suspend, this
early MMIO access can trigger a fatal bus fault. Even if clocks are always-on,
this causes the worker to ping a watchdog that the driver explicitly stopped.

Should WDOG_NO_PING_ON_SUSPEND be set in the driver to ensure the watchdog core
cancels the ping worker during suspend?

>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011004322.1118027-1-agentzoan@gmail.com?part=1