[PATCH v4 0/2] watchdog: dw_wdt: fix reset line management

[PATCH v4 0/2] watchdog: dw_wdt: fix reset line management

[Artem Shimko]

Hello,

This is v4 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().
    * https://lore.kernel.org/all/20261024121428.18862-1-a.shimko.dev@gmail.com/T/#t
  v4:
    * Dropped dev_err_probe() in probe to keep the driver consistent.

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 | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH v4 1/2] watchdog: dw_wdt: use devm_reset_control_get_optional_shared_deasserted

[Artem Shimko]

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 | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 39031445d754..ebd27ee684d1 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -588,7 +588,7 @@ 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);
 
@@ -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

[PATCH v4 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume

[Artem Shimko]

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>
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
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 ebd27ee684d1..684e0cb104c3 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

Re: [PATCH v4 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume

[Igor Velkov]

On Sat, Oct 24, 2026 at 05:08:51PM +0300, Artem Shimko wrote:
>  	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;

If clk_prepare_enable() fails in dw_wdt_resume(), we return with the reset
still asserted. The _deasserted devres then asserts it once more at unbind,
and on a shared control with deassert_count already at 0 that is a WARN_ON
and -EINVAL; the next suspend hits the same. Deasserting before the clocks
are enabled (or on the error paths) keeps every exit balanced.
Not tested on hardware: I can't make the clock enable fail.

v4 re-tested on ROCK Pi 4A (RK3399, resets = <&cru SRST_P_WDT0> added
locally): watchdog daemon at 60 s, 6 x s2idle for 90 s without a reset;
killing the daemon without magic close resets the board.

Re: [PATCH v4 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume

[Artem Shimko]

Hi Igor,


On Sat, Oct 24, 2026 at 9:46 PM Igor Velkov <iav@iav.lv> wrote:
>
> On Sat, Oct 24, 2026 at 05:08:51PM +0300, Artem Shimko wrote:
> >       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;
>
> If clk_prepare_enable() fails in dw_wdt_resume(), we return with the reset
> still asserted. The _deasserted devres then asserts it once more at unbind,
> and on a shared control with deassert_count already at 0 that is a WARN_ON
> and -EINVAL; the next suspend hits the same. Deasserting before the clocks
> are enabled (or on the error paths) keeps every exit balanced.
> Not tested on hardware: I can't make the clock enable fail.

Agreed.

I’ve examined several drivers [1][2], and it is indeed normal for the reset
to be deasserted before the clocks are enabled during the resume.

There is also no requirement that the reset be deasserted after the
clocks for dw_wdt.

I think we can move the reset deassert before the clock enables.

Let's wait a couple of days to get comments from other folks before sending v5.

[1] drivers/usb/dwc3/core.c
[2] drivers/gpu/drm/panfrost/panfrost_device.c

Thank you for testing and review =)

--
Best regards,
Artem

Re: [PATCH v4 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume

[Artem Shimko]

Hi Igor,

On Sat, Oct 24, 2026 at 9:46 PM Igor Velkov <iav@iav.lv> wrote:
>
> On Sat, Oct 24, 2026 at 05:08:51PM +0300, Artem Shimko wrote:
> >       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;
>
> If clk_prepare_enable() fails in dw_wdt_resume(), we return with the reset
> still asserted. The _deasserted devres then asserts it once more at unbind,
> and on a shared control with deassert_count already at 0 that is a WARN_ON
> and -EINVAL; the next suspend hits the same. Deasserting before the clocks
> are enabled (or on the error paths) keeps every exit balanced.
> Not tested on hardware: I can't make the clock enable fail.

The official datasheet DW_apb_wdt_databook says:
The reset must be de-asserted synchronously after the rising edge of tclk.

That means we can't switch the sequence of clk and reset in suspend/resume.

At the same time, we can't do a deassert reset in the error paths by this
reason as well (when we have errors in clocks).

Given this, i think the WARN_ON on a clock failure is a minor concern
comparing to violating the hardware synchronization requirement. I'd
prefer to keep v4 as it is and not to send v5.

Best regards,
Artem

Re: [PATCH v4 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume

[Igor Velkov]

Hi Artem,

On Sun, Oct 25, 2026 at 09:32:44PM +0300, Artem Shimko wrote:
> The official datasheet DW_apb_wdt_databook says:
> The reset must be de-asserted synchronously after the rising edge of tclk.

Fair enough, the databook requirement wins; I withdraw the comment.
v4 is fine with me as is, and my Tested-by stands.

Thanks for checking,
Igor

Re: [PATCH v4 1/2] watchdog: dw_wdt: use devm_reset_control_get_optional_shared_deasserted

[Guenter Roeck]

On Sat, Oct 24, 2026 at 05:08:50PM +0300, Artem Shimko wrote:
> 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>

Applied.

Thanks,
Guenter

Re: [PATCH v4 2/2] watchdog: dw_wdt: manage reset line during system suspend/resume

[Guenter Roeck]

On Sat, Oct 24, 2026 at 05:08:51PM +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>
> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>

Applied.

Thanks,
Guenter