[PATCH 0/5] watchdog: dw_wdt: reset clean up and pm

[PATCH 0/5] watchdog: dw_wdt: reset clean up and pm

[Artem Shimko]

Hello maintainers and reviewers,

Patch 1 moves reset_control_deassert() before any register access to
ensure the watchdog controller is properly taken out of reset before
configuration, fixing a potential hardware access issue.

Patch 2 adds proper error handling for reset_control_deassert() to
prevent probe from continuing when reset deassertion fails, improving
diagnostics and reliability.

Patch 3 replaces manual reset control management with the
devm_reset_control_get_optional_shared_deasserted() helper, eliminating
explicit reset_control_assert() calls in error paths and remove.

Patch 4 adds reset line management during system suspend and resume,
asserting the reset before clocks are disabled and deasserting after
clocks are re-enabled to maintain consistent hardware state.

Patch 5 is a final cleanup of the resume function error handling.

Thank you!
--
Best regards,
Artem Shimko

Artem Shimko (5):
  watchdog: dw_wdt: move reset control deassertion before register
    access
  watchdog: dw_wdt: add error handling for reset control deassertion
  watchdog: dw_wdt: Use
    devm_reset_control_get_optional_shared_deasserted
  watchdog: dw_wdt: manage reset line during system suspend/resume
  watchdog: dw_wdt: clean up error paths in resume function

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

-- 
2.43.0

[PATCH 1/5] watchdog: dw_wdt: move reset control deassertion before register access

[Artem Shimko]

The watchdog controller must be taken out of reset before any register
access is attempted. Currently reset_control_deassert() is called after
dw_wdt_update_mode() which performs register read/write operations. If
the reset line is asserted, accessing registers may result in undefined
behavior or bus hangs.

Move reset_control_deassert() before dw_wdt_update_mode() to ensure the
controller is properly deasserted before the first register access. The
reset_control_assert() in the error path remains to properly revert the
reset state on probe failure.

Fixes: 46a1946314bf ("watchdog: dw_wdt: Add pre-timeouts support")
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/watchdog/dw_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index c3fbb6068c52..3450402b7707 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -592,6 +592,8 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 	if (IS_ERR(dw_wdt->rst))
 		return PTR_ERR(dw_wdt->rst);
 
+	reset_control_deassert(dw_wdt->rst);
+
 	/* Enable normal reset without pre-timeout by default. */
 	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
 
@@ -617,8 +619,6 @@ 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;
-- 
2.43.0

[PATCH 2/5] watchdog: dw_wdt: add error handling for reset control deassertion

[Artem Shimko]

The reset_control_deassert() call was performed without checking its
return value. This could lead to probe continuing even when the device
reset wasn't properly deasserted, potentially causing register access
failures or undefined behavior later.

Add proper error checking for reset_control_deassert() and return the
error code if deassertion fails. Also replace &pdev->dev with the local
dev pointer for consistency with the rest of the probe function.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/watchdog/dw_wdt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 3450402b7707..724ba435b3c8 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -588,11 +588,13 @@ 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(dev, NULL);
 	if (IS_ERR(dw_wdt->rst))
 		return PTR_ERR(dw_wdt->rst);
 
-	reset_control_deassert(dw_wdt->rst);
+	ret = reset_control_deassert(dw_wdt->rst);
+	if (ret)
+		return ret;
 
 	/* Enable normal reset without pre-timeout by default. */
 	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
-- 
2.43.0

[PATCH 3/5] watchdog: dw_wdt: Use devm_reset_control_get_optional_shared_deasserted

[Artem Shimko]

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

Replace devm_reset_control_get_optional_shared() with
devm_reset_control_get_optional_shared_deasserted() which automatically
deasserts the reset line on acquisition and ensures it will be asserted
again when the device is removed. This eliminates the need for manual
reset control in probe error paths, remove, and allows simplifying the
code by removing out_assert_rst label and related assertions. Use
dev_err_probe() for consistent error reporting.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/watchdog/dw_wdt.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 724ba435b3c8..27b5098832d5 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -588,13 +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(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);
-
-	ret = reset_control_deassert(dw_wdt->rst);
-	if (ret)
-		return ret;
+		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);
@@ -623,7 +619,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 
 	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;
@@ -657,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)
@@ -675,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 4/5] 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. Add proper
error handling for reset control operations during resume with appropriate
cleanup path on failure.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/watchdog/dw_wdt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 27b5098832d5..c2c5dc6e54e7 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -382,6 +382,8 @@ static int dw_wdt_suspend(struct device *dev)
 	dw_wdt->control = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
 	dw_wdt->timeout = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
 
+	reset_control_assert(dw_wdt->rst);
+
 	clk_disable_unprepare(dw_wdt->pclk);
 	clk_disable_unprepare(dw_wdt->clk);
 
@@ -402,12 +404,22 @@ static int dw_wdt_resume(struct device *dev)
 		return err;
 	}
 
+	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);
 
 	dw_wdt_ping(&dw_wdt->wdd);
 
 	return 0;
+
+unprepare_clk:
+	clk_disable_unprepare(dw_wdt->pclk);
+	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

[PATCH 5/5] watchdog: dw_wdt: clean up error paths in resume function

[Artem Shimko]

Clean up of dw_wdt_resume() function.

Rework the error handling to use properly named labels that match the
cleanup order. Use unprepare_clk for clock disable and unprepare_pclk
for pclk disable path.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/watchdog/dw_wdt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index c2c5dc6e54e7..c89e1fb91b1f 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -393,16 +393,15 @@ 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)
@@ -415,8 +414,9 @@ static int dw_wdt_resume(struct device *dev)
 
 	return 0;
 
-unprepare_clk:
+unprepare_pclk:
 	clk_disable_unprepare(dw_wdt->pclk);
+unprepare_clk:
 	clk_disable_unprepare(dw_wdt->clk);
 
 	return err;
-- 
2.43.0

Re: [PATCH 0/5] watchdog: dw_wdt: reset clean up and pm

[Guenter Roeck]

Hi,

On 3/20/26 06:56, Artem Shimko wrote:
> Hello maintainers and reviewers,
> 
> Patch 1 moves reset_control_deassert() before any register access to
> ensure the watchdog controller is properly taken out of reset before
> configuration, fixing a potential hardware access issue.
> 
> Patch 2 adds proper error handling for reset_control_deassert() to
> prevent probe from continuing when reset deassertion fails, improving
> diagnostics and reliability.
> 
> Patch 3 replaces manual reset control management with the
> devm_reset_control_get_optional_shared_deasserted() helper, eliminating
> explicit reset_control_assert() calls in error paths and remove.
> 
> Patch 4 adds reset line management during system suspend and resume,
> asserting the reset before clocks are disabled and deasserting after
> clocks are re-enabled to maintain consistent hardware state.
> 
> Patch 5 is a final cleanup of the resume function error handling.
> 
> Thank you!

Please check the AI review feedback at

https://sashiko.dev/#/patchset/20260320135624.223863-1-a.shimko.dev%40gmail.com

and address the concerns.

Thanks,
Guenter