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

[sashiko-bot@kernel.org]

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