mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] watchdog: dw_wdt: fix reset line management
@ 2026-10-24 12:14 Artem Shimko
  2026-10-24 12:14 ` [PATCH v3 1/2] watchdog: dw_wdt: use devm_reset_control_get_optional_shared_deasserted Artem Shimko
  2026-10-24 12:14 ` [PATCH v3 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume Artem Shimko
  0 siblings, 2 replies; 4+ messages in thread
From: Artem Shimko @ 2026-10-24 12:14 UTC (permalink / raw)
  To: tzungbi, linux, p.zabel, Sergey.Semin, mika.westerberg, andi.shyti, iav
  Cc: Artem Shimko, wim, linux-watchdog, linux-kernel

Hello,

This is v3 of the series fixing reset line management in the dw_wdt
driver.

Testing:

$ ls /dev/watchdog*
/dev/watchdog   /dev/watchdog1  /dev/watchdog3
/dev/watchdog0  /dev/watchdog2

$ ls /sys/class/watchdog/
watchdog0  watchdog1  watchdog2  watchdog3

$ chmod +x watchdog-test
$ ./watchdog-test -f /dev/watchdog0 -i -s
watchdog_info:
 identity:              Synopsys DesignWare Watchdog
 firmware_version:      0
Support/Status: Set timeout (in seconds)
Support/Status: Supports magic close char
Support/Status: Pretimeout (in seconds), get/set
Support/Status: Keep alive ping reply
Support/Status: Keep alive ping reply

$ echo mem > /sys/power/state
...
...
[ 1560.147058] PM: suspend exit
[ 1560.371470] usb 1-1: new high-speed USB device number 3 using xhci-hcd
[ 1561.031825] cdc_ncm 1-1:2.0: MAC-Address: a4:2a:95:e8:fc:e4
[ 1561.037736] cdc_ncm 1-1:2.0: setting rx_max = 16384
[ 1561.054408] cdc_ncm 1-1:2.0: setting tx_max = 16384
[ 1561.084713] cdc_ncm 1-1:2.0 eth0: register 'cdc_ncm' at usb-xhci-hcd.2.auto-1, CDC NCM (NO ZLP), a4:2a:95:e8:fc:e4

$ ./watchdog-test -f /dev/watchdog0 -i -s
watchdog_info:
 identity:              Synopsys DesignWare Watchdog
 firmware_version:      0
Support/Status: Set timeout (in seconds)
Support/Status: Supports magic close char
Support/Status: Pretimeout (in seconds), get/set
Support/Status: Keep alive ping reply
Support/Status: Keep alive ping reply

$ ls /dev/watchdog*
/dev/watchdog   /dev/watchdog1  /dev/watchdog3
/dev/watchdog0  /dev/watchdog2

$ echo 1 > /dev/watchdog0
$ cat /sys/kernel/debug/1e012000.wdt/registers | grep ccvr
ccvr = 0x09a7baab
$ cat /sys/kernel/debug/1e012000.wdt/registers | grep ccvr
ccvr = 0x0599dce0
$ cat /sys/kernel/debug/1e012000.wdt/registers | grep ccvr
ccvr = 0x01926467
$ [  110.392536] watchdog0: pretimeout event

Thank you.

--
Best regards,
Artem

ChangeLog:
  v1:
    * https://lore.kernel.org/all/20260320135624.223863-1-a.shimko.dev@gmail.com/T/#t
  v2:
    * Reduced from 5 patches to 3: manual reset handling is replaced by
      the devm _deasserted variant from the start, so the intermediate
      states that could leak the deasserted reset line are gone and every
      commit is self-contained.
    * https://lore.kernel.org/all/20261022134354.534642-1-a.shimko.dev@gmail.com/T/#t
  v3:
    * Squashed the last two commits.
    * Add error check for reset_control_assert() in dw_wdt_suspend()

Artem Shimko (2):
  watchdog: dw_wdt: use
    devm_reset_control_get_optional_shared_deasserted
  watchdog: dw_wdt: manage reset line during system suspend/resume

 drivers/watchdog/dw_wdt.c | 40 +++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

-- 
2.43.0


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

* [PATCH v3 1/2] watchdog: dw_wdt: use devm_reset_control_get_optional_shared_deasserted
  2026-10-24 12:14 [PATCH v3 0/2] watchdog: dw_wdt: fix reset line management Artem Shimko
@ 2026-10-24 12:14 ` Artem Shimko
  2026-10-24 12:14 ` [PATCH v3 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume Artem Shimko
  1 sibling, 0 replies; 4+ messages in thread
From: Artem Shimko @ 2026-10-24 12:14 UTC (permalink / raw)
  To: tzungbi, linux, p.zabel, Sergey.Semin, mika.westerberg, andi.shyti, iav
  Cc: Artem Shimko, wim, linux-watchdog, linux-kernel

Replace devm_reset_control_get_optional_shared() with
devm_reset_control_get_optional_shared_deasserted() to let the
device-managed API handle reset line deassertion and assertion
automatically.

The driver currently manages the reset control manually: it calls
reset_control_deassert() during probe and reset_control_assert() in
error paths and remove. This is error-prone and requires explicit
cleanup in multiple places.

Switching to the _deasserted variant deasserts the reset line on
acquisition and asserts it again when the device is removed, which
allows removing the out_assert_rst label along with the related
assertions in the probe error paths and in dw_wdt_drv_remove().

Tested-by: Igor Velkov <iav@iav.lv>
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/watchdog/dw_wdt.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 39031445d754..2cffe337b4ea 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -588,9 +588,9 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 	if (IS_ERR(dw_wdt->pclk))
 		return PTR_ERR(dw_wdt->pclk);
 
-	dw_wdt->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
+	dw_wdt->rst = devm_reset_control_get_optional_shared_deasserted(dev, NULL);
 	if (IS_ERR(dw_wdt->rst))
-		return PTR_ERR(dw_wdt->rst);
+		return dev_err_probe(dev, PTR_ERR(dw_wdt->rst), "failed to acquire reset\n");
 
 	/* Enable normal reset without pre-timeout by default. */
 	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
@@ -617,11 +617,9 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 		dw_wdt->wdd.info = &dw_wdt_ident;
 	}
 
-	reset_control_deassert(dw_wdt->rst);
-
 	ret = dw_wdt_init_timeouts(dw_wdt, dev);
 	if (ret)
-		goto out_assert_rst;
+		return ret;
 
 	wdd = &dw_wdt->wdd;
 	wdd->ops = &dw_wdt_ops;
@@ -655,15 +653,11 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 
 	ret = watchdog_register_device(wdd);
 	if (ret)
-		goto out_assert_rst;
+		return ret;
 
 	dw_wdt_dbgfs_init(dw_wdt);
 
 	return 0;
-
-out_assert_rst:
-	reset_control_assert(dw_wdt->rst);
-	return ret;
 }
 
 static void dw_wdt_drv_remove(struct platform_device *pdev)
@@ -673,7 +667,6 @@ static void dw_wdt_drv_remove(struct platform_device *pdev)
 	dw_wdt_dbgfs_clear(dw_wdt);
 
 	watchdog_unregister_device(&dw_wdt->wdd);
-	reset_control_assert(dw_wdt->rst);
 }
 
 #ifdef CONFIG_OF
-- 
2.43.0


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

* [PATCH v3 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume
  2026-10-24 12:14 [PATCH v3 0/2] watchdog: dw_wdt: fix reset line management Artem Shimko
  2026-10-24 12:14 ` [PATCH v3 1/2] watchdog: dw_wdt: use devm_reset_control_get_optional_shared_deasserted Artem Shimko
@ 2026-10-24 12:14 ` Artem Shimko
  2026-10-24 13:19   ` Tzung-Bi Shih
  1 sibling, 1 reply; 4+ messages in thread
From: Artem Shimko @ 2026-10-24 12:14 UTC (permalink / raw)
  To: tzungbi, linux, p.zabel, Sergey.Semin, mika.westerberg, andi.shyti, iav
  Cc: Artem Shimko, wim, linux-watchdog, linux-kernel

The watchdog controller reset line is not currently managed during system
suspend/resume. This can lead to an inconsistent hardware state when
resuming from suspend, as the controller may not be properly reinitialized.

Assert the reset line before disabling clocks during suspend to ensure
the controller is held in reset while power is gated. During resume,
deassert the reset after clocks are enabled and before restoring register
state to guarantee the controller is properly initialized.

Tested-by: Igor Velkov <iav@iav.lv>
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/watchdog/dw_wdt.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 2cffe337b4ea..844a5f7738b6 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -378,10 +378,15 @@ static irqreturn_t dw_wdt_irq(int irq, void *devid)
 static int dw_wdt_suspend(struct device *dev)
 {
 	struct dw_wdt *dw_wdt = dev_get_drvdata(dev);
+	int err;
 
 	dw_wdt->control = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
 	dw_wdt->timeout = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
 
+	err = reset_control_assert(dw_wdt->rst);
+	if (err)
+		return err;
+
 	clk_disable_unprepare(dw_wdt->pclk);
 	clk_disable_unprepare(dw_wdt->clk);
 
@@ -391,16 +396,19 @@ static int dw_wdt_suspend(struct device *dev)
 static int dw_wdt_resume(struct device *dev)
 {
 	struct dw_wdt *dw_wdt = dev_get_drvdata(dev);
-	int err = clk_prepare_enable(dw_wdt->clk);
+	int err;
 
+	err = clk_prepare_enable(dw_wdt->clk);
 	if (err)
 		return err;
 
 	err = clk_prepare_enable(dw_wdt->pclk);
-	if (err) {
-		clk_disable_unprepare(dw_wdt->clk);
-		return err;
-	}
+	if (err)
+		goto unprepare_clk;
+
+	err = reset_control_deassert(dw_wdt->rst);
+	if (err)
+		goto unprepare_pclk;
 
 	writel(dw_wdt->timeout, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
 	writel(dw_wdt->control, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
@@ -408,6 +416,13 @@ static int dw_wdt_resume(struct device *dev)
 	dw_wdt_ping(&dw_wdt->wdd);
 
 	return 0;
+
+unprepare_pclk:
+	clk_disable_unprepare(dw_wdt->pclk);
+unprepare_clk:
+	clk_disable_unprepare(dw_wdt->clk);
+
+	return err;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(dw_wdt_pm_ops, dw_wdt_suspend, dw_wdt_resume);
-- 
2.43.0


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

* Re: [PATCH v3 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume
  2026-10-24 12:14 ` [PATCH v3 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume Artem Shimko
@ 2026-10-24 13:19   ` Tzung-Bi Shih
  0 siblings, 0 replies; 4+ messages in thread
From: Tzung-Bi Shih @ 2026-10-24 13:19 UTC (permalink / raw)
  To: Artem Shimko
  Cc: linux, p.zabel, Sergey.Semin, mika.westerberg, andi.shyti, iav,
	wim, linux-watchdog, linux-kernel

On Sat, Oct 24, 2026 at 03:14:24PM +0300, Artem Shimko wrote:
> The watchdog controller reset line is not currently managed during system
> suspend/resume. This can lead to an inconsistent hardware state when
> resuming from suspend, as the controller may not be properly reinitialized.
> 
> Assert the reset line before disabling clocks during suspend to ensure
> the controller is held in reset while power is gated. During resume,
> deassert the reset after clocks are enabled and before restoring register
> state to guarantee the controller is properly initialized.
> 
> Tested-by: Igor Velkov <iav@iav.lv>
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

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

end of thread, other threads:[~2026-10-24 13:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-24 12:14 [PATCH v3 0/2] watchdog: dw_wdt: fix reset line management Artem Shimko
2026-10-24 12:14 ` [PATCH v3 1/2] watchdog: dw_wdt: use devm_reset_control_get_optional_shared_deasserted Artem Shimko
2026-10-24 12:14 ` [PATCH v3 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume Artem Shimko
2026-10-24 13:19   ` Tzung-Bi Shih

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®