[PATCH] watchdog: npcm: Propagate errors from clock enable

[PATCH] watchdog: npcm: Propagate errors from clock enable

[Triet Hoang]

Check and propagate errors returned by clk_prepare_enable() when
starting or restarting the watchdog. Also propagate errors from
npcm_wdt_start() through npcm_wdt_set_timeout() and probe.

This prevents the driver from silently continuing when the watchdog
clock cannot be enabled.

Fixes: 975b7f0fe669 ("watchdog: Add Nuvoton NPCM watchdog driver")
Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
---
 drivers/watchdog/npcm_wdt.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
index 51348969bf49..0976a580b9f2 100644
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -149,8 +149,11 @@ static int npcm_wdt_start(struct watchdog_device *wdd)
 {
 	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
 	u32 val;
+	int err;
 
-	clk_prepare_enable(wdt->clk);
+	err = clk_prepare_enable(wdt->clk);
+	if (err)
+		return err;
 
 	if (wdd->timeout < 2)
 		val = 0x800;
@@ -216,7 +219,7 @@ static int npcm_wdt_set_timeout(struct watchdog_device *wdd,
 		wdd->timeout = 2750;
 
 	if (watchdog_active(wdd))
-		npcm_wdt_start(wdd);
+		return npcm_wdt_start(wdd);
 
 	return 0;
 }
@@ -234,9 +237,12 @@ static int npcm_wdt_restart(struct watchdog_device *wdd,
 			    unsigned long action, void *data)
 {
 	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
+	int err;
 
 	/* For reset, we start the WDT clock and leave it running. */
-	clk_prepare_enable(wdt->clk);
+	err = clk_prepare_enable(wdt->clk);
+	if (err)
+		return err;
 
 	writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, wdt->reg);
 	udelay(1000);
@@ -428,13 +434,18 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 	watchdog_init_timeout(&wdt->wdd, 0, dev);
 
 	/* Ensure timeout is able to be represented by the hardware */
-	npcm_wdt_set_timeout(&wdt->wdd, wdt->wdd.timeout);
+	ret = npcm_wdt_set_timeout(&wdt->wdd, wdt->wdd.timeout);
+	if (ret)
+		return ret;
 
 	npcm_get_reset_status(wdt, dev, data, start);
 
 	if (npcm_is_running(&wdt->wdd)) {
 		/* Restart with the default or device-tree specified timeout */
-		npcm_wdt_start(&wdt->wdd);
+		ret = npcm_wdt_start(&wdt->wdd);
+		if (ret)
+			return ret;
+
 		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
 	}
 
-- 
2.53.0

Re: [PATCH] watchdog: npcm: Propagate errors from clock enable

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] watchdog: npcm: software timeout desync on start error causes unintended reset
- [High] watchdog: npcm: aborting restart handler on clock error hangs system reboot
--

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

watchdog: npcm: Propagate errors from clock enable

Check and propagate errors returned by clk_prepare_enable() when starting or
restarting the watchdog. Also propagate errors from npcm_wdt_start() through
npcm_wdt_set_timeout() and probe to prevent the driver from silently
continuing when the watchdog clock cannot be enabled.

> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> index 51348969bf492..0976a580b9f21 100644
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c
> @@ -149,8 +149,11 @@ static int npcm_wdt_start(struct watchdog_device *wdd)
>  {
>  	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
>  	u32 val;
> +	int err;
>  
> -	clk_prepare_enable(wdt->clk);
> +	err = clk_prepare_enable(wdt->clk);
> +	if (err)
> +		return err;
>  
>  	if (wdd->timeout < 2)
>  		val = 0x800;
[ ... ]
> @@ -216,7 +219,7 @@ static int npcm_wdt_set_timeout(struct watchdog_device *wdd,
>  		wdd->timeout = 2750;
>  
>  	if (watchdog_active(wdd))
> -		npcm_wdt_start(wdd);
> +		return npcm_wdt_start(wdd);

[Severity: High]
Does returning an error here cause a state mismatch between software and
hardware?

Earlier in npcm_wdt_set_timeout(), wdd->timeout is modified before
npcm_wdt_start() is called:

	if (timeout < 2)
		wdd->timeout = 1;
	else if (timeout < 3)
		...

If npcm_wdt_start() returns the error from clk_prepare_enable(), the new
timeout value remains in wdd->timeout while the hardware continues running
with the old timeout.

The watchdog core's keepalive worker will recalculate the ping interval
using the new (potentially much longer) wdd->timeout. The hardware
watchdog, running with a shorter timeout, could then expire and
unexpectedly reset the system.

>  
>  	return 0;
>  }
> @@ -234,9 +237,12 @@ static int npcm_wdt_restart(struct watchdog_device *wdd,
>  			    unsigned long action, void *data)
>  {
>  	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> +	int err;
>  
>  	/* For reset, we start the WDT clock and leave it running. */
> -	clk_prepare_enable(wdt->clk);
> +	err = clk_prepare_enable(wdt->clk);
> +	if (err)
> +		return err;

[Severity: High]
Could returning an error here hang the system during reboot?

npcm_wdt_restart() is invoked via watchdog_restart_notifier() in the
watchdog core to handle system reboots. The watchdog core maps any non-zero
return from a restart handler to a notification value that aborts the
notifier chain.

This causes atomic_notifier_call_chain() to immediately break out of the
restart_handler_list loop in kernel/reboot.c.

As a result, not only does the NPCM watchdog skip its attempt to write to
the reset registers (which might succeed if the clock was already enabled),
but all lower-priority fallback restart handlers are preempted. The system
will hang indefinitely instead of rebooting.

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