mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] watchdog: npcm: fix sleeping clock call in restart handler
@ 2026-11-11  6:41 zhaofeng2
  2026-11-11  8:32 ` [PATCH v2] " zhaofeng2
  0 siblings, 1 reply; 3+ messages in thread
From: zhaofeng2 @ 2026-11-11  6:41 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck
  Cc: Zhao Feng, linux-watchdog, linux-kernel, Tomer Maimon,
	Joel Stanley, j.neuschaefer, Triet Hoang, stable

From: Zhao Feng <zhaofeng2@cmss.chinamobile.com>

The watchdog .restart callback runs in atomic context: it is invoked
from the restart handler chain via atomic_notifier_call_chain(), under
an RCU read-side critical section, on every reboot or restart.  Since
commit af084fdccfaf ("watchdog: npcm: Enable clock if provided"),
npcm_wdt_restart() calls clk_prepare_enable(), and clk_prepare()
takes a mutex unconditionally, so kernels built with
CONFIG_DEBUG_ATOMIC_SLEEP=y report

  BUG: sleeping function called from invalid context

on every reboot.  Worse, a panic-time restart can block on
prepare_lock and never issue the reset at all.

Prepare the clock once at probe time and keep it prepared for the
lifetime of the driver (balanced by a devm action), so the restart
handler only needs clk_enable(), which is safe in atomic context.
clk_prepare()/clk_unprepare() accept a NULL clock, so the
optional-clock case is unaffected.

Fixes: af084fdccfaf ("watchdog: npcm: Enable clock if provided")
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
---
Issue reported by Sashiko's review of Triet Hoang's pending
error-propagation patch; maintainer confirmed the problem in
https://lore.kernel.org/linux-kernel/19258f94-ec01-48f4-9d04-22e5ecb5cac2@roeck-us.net/

Based on linux-next, which already carries
"watchdog: npcm: fix clock prepare leak when setting timeout on a
running watchdog".

Verified against linux-next.  Not runtime-tested (no Nuvoton NPCM
hardware).

 drivers/watchdog/npcm_wdt.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -242,8 +242,12 @@
 {
 	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
 
-	/* For reset, we start the WDT clock and leave it running. */
-	clk_prepare_enable(wdt->clk);
+	/*
+	 * For reset, we start the WDT clock and leave it running.
+	 * The clock was prepared at probe time; only clk_enable() is
+	 * safe in this atomic (restart notifier) context.
+	 */
+	clk_enable(wdt->clk);
 
 	writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, wdt->reg);
 	udelay(1000);
@@ -393,6 +397,11 @@
 	.restart = npcm_wdt_restart,
 };
 
+static void npcm_clk_unprepare(void *data)
+{
+	clk_unprepare(data);
+}
+
 static int npcm_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -421,6 +430,20 @@
 	if (IS_ERR(wdt->clk))
 		return PTR_ERR(wdt->clk);
 
+	/*
+	 * Prepare the clock once and keep it prepared for the lifetime
+	 * of the driver: npcm_wdt_restart() runs in atomic context and
+	 * may only call clk_enable().
+	 */
+	ret = clk_prepare(wdt->clk);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&pdev->dev, npcm_clk_unprepare,
+				       wdt->clk);
+	if (ret)
+		return ret;
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;



^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2] watchdog: npcm: fix sleeping clock call in restart handler
  2026-11-11  6:41 [PATCH] watchdog: npcm: fix sleeping clock call in restart handler zhaofeng2
@ 2026-11-11  8:32 ` zhaofeng2
  2026-11-14 17:14   ` Guenter Roeck
  0 siblings, 1 reply; 3+ messages in thread
From: zhaofeng2 @ 2026-11-11  8:32 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck
  Cc: Zhao Feng, linux-watchdog, linux-kernel, Tomer Maimon,
	Joel Stanley, j.neuschaefer, Triet Hoang, stable

From: Zhao Feng <zhaofeng2@cmss.chinamobile.com>

The watchdog .restart callback runs in atomic context: it is invoked
from the restart handler chain via atomic_notifier_call_chain(), under
an RCU read-side critical section, on every reboot or restart.  Since
commit af084fdccfaf ("watchdog: npcm: Enable clock if provided"),
npcm_wdt_restart() calls clk_prepare_enable(), and clk_prepare()
takes a mutex unconditionally, so kernels built with
CONFIG_DEBUG_ATOMIC_SLEEP=y report

  BUG: sleeping function called from invalid context

on every reboot.  Worse, a panic-time restart can block on
prepare_lock and never issue the reset at all.

Get the clock with devm_clk_get_optional_prepared() at probe time and
keep it prepared for the lifetime of the driver, so the restart
handler only needs clk_enable(), which is safe in atomic context.
With the clock permanently prepared, npcm_wdt_start() and
npcm_wdt_stop() are simplified to clk_enable()/clk_disable() as well.

Fixes: af084fdccfaf ("watchdog: npcm: Enable clock if provided")
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
---
v2: use devm_clk_get_optional_prepared() instead of open-coding
    clk_prepare() + devm_add_action_or_reset(), and simplify
    start/stop to clk_enable()/clk_disable(), per Sashiko's review.

Issue reported by Sashiko's review of Triet Hoang's pending
error-propagation patch; maintainer confirmed the problem in
https://lore.kernel.org/linux-kernel/19258f94-ec01-48f4-9d04-22e5ecb5cac2@roeck-us.net/

Based on linux-next, which already carries
"watchdog: npcm: fix clock prepare leak when setting timeout on a
running watchdog".

Verified against linux-next.  Not runtime-tested (no Nuvoton NPCM
hardware).

 drivers/watchdog/npcm_wdt.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -180,7 +180,7 @@
 {
 	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
 
-	clk_prepare_enable(wdt->clk);
+	clk_enable(wdt->clk);
 
 	__npcm_wdt_start(wdd);
 
@@ -193,7 +193,7 @@
 
 	writel(0, wdt->reg);
 
-	clk_disable_unprepare(wdt->clk);
+	clk_disable(wdt->clk);
 
 	return 0;
 }
@@ -242,8 +242,12 @@
 {
 	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
 
-	/* For reset, we start the WDT clock and leave it running. */
-	clk_prepare_enable(wdt->clk);
+	/*
+	 * For reset, we start the WDT clock and leave it running.
+	 * The clock was prepared at probe time; only clk_enable() is
+	 * safe in this atomic (restart notifier) context.
+	 */
+	clk_enable(wdt->clk);
 
 	writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, wdt->reg);
 	udelay(1000);
@@ -417,7 +421,12 @@
 
 	start = res->start;
 
-	wdt->clk = devm_clk_get_optional(&pdev->dev, NULL);
+	/*
+	 * Keep the clock prepared for the lifetime of the driver:
+	 * npcm_wdt_restart() runs in atomic context and may only call
+	 * clk_enable().
+	 */
+	wdt->clk = devm_clk_get_optional_prepared(&pdev->dev, NULL);
 	if (IS_ERR(wdt->clk))
 		return PTR_ERR(wdt->clk);
 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] watchdog: npcm: fix sleeping clock call in restart handler
  2026-11-11  8:32 ` [PATCH v2] " zhaofeng2
@ 2026-11-14 17:14   ` Guenter Roeck
  0 siblings, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2026-11-14 17:14 UTC (permalink / raw)
  To: zhaofeng2
  Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Tomer Maimon,
	Joel Stanley, j.neuschaefer, Triet Hoang, stable

On Wed, Nov 11, 2026 at 04:32:36PM +0800, zhaofeng2@cmss.chinamobile.com wrote:
> From: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
> 
> The watchdog .restart callback runs in atomic context: it is invoked
> from the restart handler chain via atomic_notifier_call_chain(), under
> an RCU read-side critical section, on every reboot or restart.  Since
> commit af084fdccfaf ("watchdog: npcm: Enable clock if provided"),
> npcm_wdt_restart() calls clk_prepare_enable(), and clk_prepare()
> takes a mutex unconditionally, so kernels built with
> CONFIG_DEBUG_ATOMIC_SLEEP=y report
> 
>   BUG: sleeping function called from invalid context
> 
> on every reboot.  Worse, a panic-time restart can block on
> prepare_lock and never issue the reset at all.
> 
> Get the clock with devm_clk_get_optional_prepared() at probe time and
> keep it prepared for the lifetime of the driver, so the restart
> handler only needs clk_enable(), which is safe in atomic context.
> With the clock permanently prepared, npcm_wdt_start() and
> npcm_wdt_stop() are simplified to clk_enable()/clk_disable() as well.
> 
> Fixes: af084fdccfaf ("watchdog: npcm: Enable clock if provided")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>

Applied.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-14 17:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-11  6:41 [PATCH] watchdog: npcm: fix sleeping clock call in restart handler zhaofeng2
2026-11-11  8:32 ` [PATCH v2] " zhaofeng2
2026-11-14 17:14   ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®