[PATCH] watchdog: npcm: fix clock prepare leak when setting timeout on a running watchdog

[PATCH] watchdog: npcm: fix clock prepare leak when setting timeout on a running watchdog

[Zhao Feng]

npcm_wdt_start() unconditionally prepares and enables the watchdog
clock, and npcm_wdt_stop() is the only place that disables it.
npcm_wdt_set_timeout() calls npcm_wdt_start() again whenever the
watchdog is already active, so every WDIOC_SETTIMEOUT ioctl issued
while the watchdog is running takes one extra clock prepare/enable
reference that is never dropped.

Split the register programming out of npcm_wdt_start() into a
__npcm_wdt_start() helper that assumes the clock is already running,
and call the helper from npcm_wdt_set_timeout() instead; only the
watchdog .start callback (and the restart path, which intentionally
keeps the clock) now acquires the clock.

Fixes: af084fdccfaf ("watchdog: npcm: Enable clock if provided")
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
---
Verified against current upstream master.  Not runtime-tested (no
Nuvoton NPCM hardware).

Note: this overlaps textually with Triet Hoang's pending
error-propagation patch
(https://marc.info/?l=linux-kernel&m=178955112320092)
but fixes a different bug; whichever lands second will need a
trivial rebase.

 drivers/watchdog/npcm_wdt.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -148,10 +148,8 @@
-static int npcm_wdt_start(struct watchdog_device *wdd)
+static void __npcm_wdt_start(struct watchdog_device *wdd)
 {
 	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
 	u32 val;
 
-	clk_prepare_enable(wdt->clk);
-
 	if (wdd->timeout < 2)
 		val = 0x800;
 	else if (wdd->timeout < 3)
@@ -176,10 +174,19 @@
 	val |= NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE;
 
 	writel(val, wdt->reg);
-
-	return 0;
 }
 
+static int npcm_wdt_start(struct watchdog_device *wdd)
+{
+	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
+
+	clk_prepare_enable(wdt->clk);
+
+	__npcm_wdt_start(wdd);
+
+	return 0;
+}
+
 static int npcm_wdt_stop(struct watchdog_device *wdd)
 {
 	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
@@ -216,7 +223,7 @@
 		wdd->timeout = 2750;
 
 	if (watchdog_active(wdd))
-		npcm_wdt_start(wdd);
+		__npcm_wdt_start(wdd);
 
 	return 0;
 }
-- 
2.39.2

Re: [PATCH] watchdog: npcm: fix clock prepare leak when setting timeout on a running watchdog

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] watchdog: npcm: sleep in atomic context in npcm_wdt_restart()
  [1]
--

commit 17842ca84b08ec6bbd438017f39f767a1910936b
Author: Zhao Feng <zhaofeng2@cmss.chinamobile.com>

watchdog: npcm: fix clock prepare leak when setting timeout on a running watchdog

This commit splits register programming out of npcm_wdt_start() into
__npcm_wdt_start() so that npcm_wdt_set_timeout() does not acquire an
extra clock reference when updating the timeout on an active watchdog.

> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> index 51348969bf492..31253fd7bad5b 100644
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c
> @@ -145,13 +145,11 @@ static int npcm_wdt_ping(struct watchdog_device *wdd)
>  	return 0;
>  }
>  
> -static int npcm_wdt_start(struct watchdog_device *wdd)
> +static void __npcm_wdt_start(struct watchdog_device *wdd)
>  {
>  	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
>  	u32 val;
>  
> -	clk_prepare_enable(wdt->clk);
> -
>  	if (wdd->timeout < 2)
>  		val = 0x800;
>  	else if (wdd->timeout < 3)
> @@ -176,6 +174,15 @@ static int npcm_wdt_start(struct watchdog_device *wdd)
>  	val |= NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE;
>  
>  	writel(val, wdt->reg);
> +}
> +
> +static int npcm_wdt_start(struct watchdog_device *wdd)
> +{
> +	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> +
> +	clk_prepare_enable(wdt->clk);
> +
> +	__npcm_wdt_start(wdd);
>  
>  	return 0;
>  }

[Severity: High]
This problem wasn't introduced by this patch, but can npcm_wdt_restart()
sleep in atomic context when calling clk_prepare_enable(wdt->clk)
[1]?

drivers/watchdog/npcm_wdt.c:npcm_wdt_restart() {
	struct npcm_wdt *wdt = to_npcm_wdt(wdd);

	/* For reset, we start the WDT clock and leave it running. */
	clk_prepare_enable(wdt->clk);
    ...
}

do_kernel_restart() invokes restart handlers via
atomic_notifier_call_chain() under an RCU read lock, whereas
clk_prepare_enable() calls clk_prepare() and takes the sleeping
prepare_lock mutex.  This triggers a scheduling-while-atomic bug and can
deadlock during reboot or panic recovery.

[1] https://sashiko.dev/#/bug/linux-d877d7dc-83e2-4c44-b88a-c3f7ea954cd4

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109071004.1506-1-zhaofeng2@cmss.chinamobile.com?part=1

Re: [PATCH] watchdog: npcm: fix clock prepare leak when setting timeout on a running watchdog

[Guenter Roeck]

On Mon, Nov 09, 2026 at 03:10:03PM +0800, Zhao Feng wrote:
> npcm_wdt_start() unconditionally prepares and enables the watchdog
> clock, and npcm_wdt_stop() is the only place that disables it.
> npcm_wdt_set_timeout() calls npcm_wdt_start() again whenever the
> watchdog is already active, so every WDIOC_SETTIMEOUT ioctl issued
> while the watchdog is running takes one extra clock prepare/enable
> reference that is never dropped.
> 
> Split the register programming out of npcm_wdt_start() into a
> __npcm_wdt_start() helper that assumes the clock is already running,
> and call the helper from npcm_wdt_set_timeout() instead; only the
> watchdog .start callback (and the restart path, which intentionally
> keeps the clock) now acquires the clock.
> 
> Fixes: af084fdccfaf ("watchdog: npcm: Enable clock if provided")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>

Applied.

Couple of notes:

The problem reported by Sashiko seems real, but I have any idea how to fix
it, and it is unrelated to this patch.

The error propagation patch has a number of issues (as reported by Sashiko)
and will have to be resubmitted anyway. When thet is done it will have
to be rebased to this patch.

Thanks,
Guenter

> ---
> Verified against current upstream master.  Not runtime-tested (no
> Nuvoton NPCM hardware).
> 
> Note: this overlaps textually with Triet Hoang's pending
> error-propagation patch
> (https://marc.info/?l=linux-kernel&m=178955112320092)
> but fixes a different bug; whichever lands second will need a
> trivial rebase.
> 
>  drivers/watchdog/npcm_wdt.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c
> @@ -148,10 +148,8 @@
> -static int npcm_wdt_start(struct watchdog_device *wdd)
> +static void __npcm_wdt_start(struct watchdog_device *wdd)
>  {
>  	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
>  	u32 val;
>  
> -	clk_prepare_enable(wdt->clk);
> -
>  	if (wdd->timeout < 2)
>  		val = 0x800;
>  	else if (wdd->timeout < 3)
> @@ -176,10 +174,19 @@
>  	val |= NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE;
>  
>  	writel(val, wdt->reg);
> -
> -	return 0;
>  }
>  
> +static int npcm_wdt_start(struct watchdog_device *wdd)
> +{
> +	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> +
> +	clk_prepare_enable(wdt->clk);
> +
> +	__npcm_wdt_start(wdd);
> +
> +	return 0;
> +}
> +
>  static int npcm_wdt_stop(struct watchdog_device *wdd)
>  {
>  	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> @@ -216,7 +223,7 @@
>  		wdd->timeout = 2750;
>  
>  	if (watchdog_active(wdd))
> -		npcm_wdt_start(wdd);
> +		__npcm_wdt_start(wdd);
>  
>  	return 0;
>  }