Re: [PATCH v2 4/8] watchdog: msc313e_wdt: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The msc313e_wdt_resume() callback ignores the return value of msc313e_wdt_start(). If clk_prepare_enable() fails, the clock remains disabled but the watchdog core state still considers it active/running. Subsequent watchdog operations (like ping) will perform blind writes to unclocked hardware registers (e.g., REG_WDT_CLR), leading to a system crash. Additionally, any subsequent stop() call will incorrectly decrement the clock's reference count.
--

commit e0ef26398a011a110179678902a905e77bb75d61
Author: Triet Hoang <triet.hoang.dev@gmail.com>

watchdog: msc313e_wdt: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

This commit converts the deprecated SIMPLE_DEV_PM_OPS() macro to
DEFINE_SIMPLE_DEV_PM_OPS() and uses pm_ptr(). It also removes the
__maybe_unused annotations from the suspend and resume callbacks.

> diff --git a/drivers/watchdog/msc313e_wdt.c b/drivers/watchdog/msc313e_wdt.c
> index 4a5cce2a16b18..a77ff57a44571 100644
> --- a/drivers/watchdog/msc313e_wdt.c
> +++ b/drivers/watchdog/msc313e_wdt.c

[ ... ]

> -static int __maybe_unused msc313e_wdt_resume(struct device *dev)
> +static int msc313e_wdt_resume(struct device *dev)
>  {
>  	struct msc313e_wdt_priv *priv = dev_get_drvdata(dev);
>  
>  	if (watchdog_active(&priv->wdev) || watchdog_hw_running(&priv->wdev))
>  		msc313e_wdt_start(&priv->wdev);
>  
>  	return 0;

[Severity: High]
This is a pre-existing issue, but does msc313e_wdt_resume() need to handle
the return value of msc313e_wdt_start()?

If clk_prepare_enable() fails inside msc313e_wdt_start():

msc313e_wdt_start() {
	...
	err = clk_prepare_enable(priv->clk);
	if (err)
		return err;
	...
}

The error is returned to msc313e_wdt_resume(), which discards it and returns
0. Because the watchdog core state still considers the hardware active,
could subsequent operations like watchdog ping perform writes to unclocked
hardware registers (such as REG_WDT_CLR) and trigger a bus fault or system
crash?

Additionally, if the clock remains disabled but the watchdog is still tracked
as running, will a subsequent stop() call incorrectly decrement the clock's
enable reference count?

>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914085451.891675-1-triet.hoang.dev@gmail.com?part=4