[PATCH v3 0/3] watchdog: rzv2h: Convert WDTDCR handling to regmap

[PATCH v3 0/3] watchdog: rzv2h: Convert WDTDCR handling to regmap

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series converts the WDTDCR register access from raw
readl/writel variants over to the regmap framework using
devm_regmap_init_mmio(). This conversion serves as a preparatory
refactoring step. It allows the driver to subsequently support
syscon-based system controllers natively by passing along alternative
regmap handles without forcing messy architectural branching at runtime.

Note,
 - This change was suggested [0]
   [0] https://lore.kernel.org/all/c3ef312a-4c26-498c-90c8-118c6dc035a0@roeck-us.net/ 
- Patches are rebased on top of next-20260813

v2->v3:
- Patch#1, is split from v2 patch #1 to make thing easier to review.
- Moved unwinding changes to patch#1.
- Set use_raw_spinlock to true in regmap config as certain watchdog operations
  runs in an atomic panic context.
- Updated commit message to reflect the change for patch#3.

v1->v2:
 - Dropped patch #1 from v1 series.
 - Switched to use regmap_set/clear_bits()

v2: https://lore.kernel.org/all/20260715133922.2129340-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
v1: https://lore.kernel.org/all/20260702160457.1884345-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (3):
  watchdog: rzv2h_wdt: Propagate WDTDCR access errors
  watchdog: rzv2h: Convert WDTDCR handling to regmap
  watchdog: rzv2h: Drop WDTRCR_RSTIRQS define

 drivers/watchdog/rzv2h_wdt.c | 80 +++++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 23 deletions(-)

-- 
2.43.0

[PATCH v3 1/3] watchdog: rzv2h_wdt: Propagate WDTDCR access errors

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The WDTDCR helpers currently access the register directly using
`readl()`/`writel()` and therefore cannot report failures to their callers.
WDTDCR is located in a shared syscon region and will be accessed through
regmap in a subsequent change, where register accesses can fail.

Make `rzt2h_wdt_wdtdcr_count_start()` and `rzt2h_wdt_wdtdcr_count_stop()`
return an error so their callers can propagate failures.

Handle these errors in the watchdog start, stop and restart paths and
unwind resources acquired before the WDTDCR access. In particular, restore
the reset and clock state and release the runtime PM reference as
appropriate when an access fails.

The helpers still return zero with the current `readl()`/`writel()`
implementation, so this change does not alter the driver's behaviour by
itself. It prepares the error handling required for the subsequent regmap
conversion of WDTDCR access.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2->v3:
- New patch, split from v2 patch #1 to make thing easier to review.
---
 drivers/watchdog/rzv2h_wdt.c | 49 ++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index 3b6abb66a1da..b770226f785e 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -89,18 +89,22 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
 	return 0;
 }
 
-static void rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
+static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
 {
 	u32 reg = readl(priv->wdtdcr + WDTDCR);
 
 	writel(reg | WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
+
+	return 0;
 }
 
-static void rzt2h_wdt_wdtdcr_count_start(struct rzv2h_wdt_priv *priv)
+static int rzt2h_wdt_wdtdcr_count_start(struct rzv2h_wdt_priv *priv)
 {
 	u32 reg = readl(priv->wdtdcr + WDTDCR);
 
 	writel(reg & ~WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
+
+	return 0;
 }
 
 static void rzv2h_wdt_setup(struct watchdog_device *wdev, u16 wdtcr)
@@ -150,8 +154,14 @@ static int rzv2h_wdt_start(struct watchdog_device *wdev)
 	rzv2h_wdt_setup(wdev, of_data->cks_max | WDTCR_RPSS_100 |
 			WDTCR_RPES_0 | of_data->tops);
 
-	if (priv->of_data->wdtdcr)
-		rzt2h_wdt_wdtdcr_count_start(priv);
+	if (priv->of_data->wdtdcr) {
+		ret = rzt2h_wdt_wdtdcr_count_start(priv);
+		if (ret) {
+			reset_control_assert(priv->rstc);
+			pm_runtime_put(wdev->parent);
+			return ret;
+		}
+	}
 
 	/*
 	 * Down counting starts after writing the sequence 00h -> FFh to the
@@ -171,8 +181,13 @@ static int rzv2h_wdt_stop(struct watchdog_device *wdev)
 	if (ret)
 		return ret;
 
-	if (priv->of_data->wdtdcr)
-		rzt2h_wdt_wdtdcr_count_stop(priv);
+	if (priv->of_data->wdtdcr) {
+		ret = rzt2h_wdt_wdtdcr_count_stop(priv);
+		if (ret) {
+			reset_control_deassert(priv->rstc);
+			return ret;
+		}
+	}
 
 	pm_runtime_put(wdev->parent);
 
@@ -188,9 +203,10 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
 			     unsigned long action, void *data)
 {
 	struct rzv2h_wdt_priv *priv = watchdog_get_drvdata(wdev);
+	bool active = watchdog_active(wdev);
 	int ret;
 
-	if (!watchdog_active(wdev)) {
+	if (!active) {
 		ret = clk_enable(priv->pclk);
 		if (ret)
 			return ret;
@@ -234,8 +250,19 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
 	rzv2h_wdt_setup(wdev, priv->of_data->cks_min | WDTCR_RPSS_25 |
 			WDTCR_RPES_75 | WDTCR_TOPS_1024);
 
-	if (priv->of_data->wdtdcr)
-		rzt2h_wdt_wdtdcr_count_start(priv);
+	if (priv->of_data->wdtdcr) {
+		ret = rzt2h_wdt_wdtdcr_count_start(priv);
+		if (ret) {
+			if (!active) {
+				reset_control_assert(priv->rstc);
+				clk_disable(priv->oscclk);
+				clk_disable(priv->pclk);
+			} else {
+				reset_control_reset(priv->rstc);
+			}
+			return ret;
+		}
+	}
 
 	rzv2h_wdt_ping(wdev);
 
@@ -266,11 +293,11 @@ static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
 	if (ret)
 		return ret;
 
-	rzt2h_wdt_wdtdcr_count_stop(priv);
+	ret = rzt2h_wdt_wdtdcr_count_stop(priv);
 
 	pm_runtime_put(&pdev->dev);
 
-	return 0;
+	return ret;
 }
 
 static int rzv2h_wdt_probe(struct platform_device *pdev)
-- 
2.43.0

[PATCH v3 2/3] watchdog: rzv2h: Convert WDTDCR handling to regmap

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Convert the WDTDCR register access from raw readl/writel variants over to
the regmap framework using devm_regmap_init_mmio().

This conversion serves as a preparatory refactoring step. It allows the
driver to subsequently support syscon-based system controllers natively
by passing along alternative regmap handles without forcing messy
architectural branching at runtime.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2->v3:
- Dropped unwinding changes to patch#1.
- Set use_raw_spinlock to true in regmap config as certain watchdog operations
  runs in an atomic panic context.
---
 drivers/watchdog/rzv2h_wdt.c | 37 ++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index b770226f785e..cf28caef31e8 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
 #include <linux/units.h>
 #include <linux/watchdog.h>
@@ -67,7 +68,7 @@ struct rzv2h_of_data {
 
 struct rzv2h_wdt_priv {
 	void __iomem *base;
-	void __iomem *wdtdcr;
+	struct regmap *wdtdcr_regmap;
 	struct clk *pclk;
 	struct clk *oscclk;
 	struct reset_control *rstc;
@@ -91,20 +92,12 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
 
 static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
 {
-	u32 reg = readl(priv->wdtdcr + WDTDCR);
-
-	writel(reg | WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
-
-	return 0;
+	return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
 }
 
 static int rzt2h_wdt_wdtdcr_count_start(struct rzv2h_wdt_priv *priv)
 {
-	u32 reg = readl(priv->wdtdcr + WDTDCR);
-
-	writel(reg & ~WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
-
-	return 0;
+	return regmap_clear_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
 }
 
 static void rzv2h_wdt_setup(struct watchdog_device *wdev, u16 wdtcr)
@@ -280,14 +273,30 @@ static const struct watchdog_ops rzv2h_wdt_ops = {
 	.restart = rzv2h_wdt_restart,
 };
 
+static const struct regmap_config rzv2h_wdtdcr_regmap_config = {
+	.name = "wdtdcr",
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = WDTDCR,
+	.fast_io = true,
+	.use_raw_spinlock = true,
+};
+
 static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
 				 struct rzv2h_wdt_priv *priv)
 {
+	void __iomem *wdtdcr;
 	int ret;
 
-	priv->wdtdcr = devm_platform_ioremap_resource(pdev, 1);
-	if (IS_ERR(priv->wdtdcr))
-		return PTR_ERR(priv->wdtdcr);
+	wdtdcr = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(wdtdcr))
+		return PTR_ERR(wdtdcr);
+
+	priv->wdtdcr_regmap = devm_regmap_init_mmio(&pdev->dev, wdtdcr,
+						    &rzv2h_wdtdcr_regmap_config);
+	if (IS_ERR(priv->wdtdcr_regmap))
+		return PTR_ERR(priv->wdtdcr_regmap);
 
 	ret = pm_runtime_resume_and_get(&pdev->dev);
 	if (ret)
-- 
2.43.0

[PATCH v3 3/3] watchdog: rzv2h: Drop WDTRCR_RSTIRQS define

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

WDTRCR_RSTIRQS is unused, so drop it.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2->v3:
- Updated commit message to reflect the change in v3.
---
 drivers/watchdog/rzv2h_wdt.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index cf28caef31e8..8af2f33b5680 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -40,8 +40,6 @@
 #define WDTCR_RPSS_25		0x00
 #define WDTCR_RPSS_100		0x3000
 
-#define WDTRCR_RSTIRQS		BIT(7)
-
 #define WDTDCR_WDTSTOPCTRL	BIT(0)
 
 #define WDT_DEFAULT_TIMEOUT	60U
-- 
2.43.0

Re: [PATCH v3 2/3] watchdog: rzv2h: Convert WDTDCR handling to regmap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Using a regmap configured with `use_raw_spinlock = true` in the watchdog restart handler introduces a deadlock if a panic occurs while another CPU holds the regmap lock.
- [High] The watchdog restart handler inappropriately aborts on regmap errors, silently preventing system reboot.

Pre-existing issues:
- [High] Calling `clk_enable()` from `rzv2h_wdt_restart()` can cause deadlocks if another CPU holds the corresponding framework spinlocks during a panic.
--

--- Patch [2]: [PATCH v3 2/3] watchdog: rzv2h: Convert WDTDCR handling to regmap ---
commit 81d528488e87b317129eaa95c0076e2e770f3265
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: watchdog: rzv2h: Convert WDTDCR handling to regmap

This commit converts the WDTDCR register access from raw readl/writel
variants over to the regmap framework using devm_regmap_init_mmio(). This
refactoring allows the driver to subsequently support syscon-based system
controllers natively.

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index b770226f785ea..cf28caef31e8e 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c
[ ... ]
> @@ -91,20 +92,12 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
>  
>  static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
>  {
> -	u32 reg = readl(priv->wdtdcr + WDTDCR);
> -
> -	writel(reg | WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
> -
> -	return 0;
> +	return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
>  }
>  
>  static int rzt2h_wdt_wdtdcr_count_start(struct rzv2h_wdt_priv *priv)
>  {
> -	u32 reg = readl(priv->wdtdcr + WDTDCR);
> -
> -	writel(reg & ~WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
> -
> -	return 0;
> +	return regmap_clear_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
>  }

[Severity: High]
Since rzt2h_wdt_wdtdcr_count_start() is called by rzv2h_wdt_restart() during
a system panic, returning regmap_clear_bits() here means the restart handler
can now fail if the regmap access fails.

If rzt2h_wdt_wdtdcr_count_start() returns an error, the restart handler aborts
early:

    rzv2h_wdt_restart()
        if (priv->of_data->wdtdcr) {
            ret = rzt2h_wdt_wdtdcr_count_start(priv);
            if (ret) {
                ...
                return ret;
            }
        }
        rzv2h_wdt_ping(wdev);

Does this silently prevent the watchdog ping and hang the system indefinitely
during a panic instead of rebooting? Should the restart handler ignore the
error to ensure the watchdog ping always occurs?

[Severity: High]
This is a pre-existing issue, but I noticed the restart handler calls
clk_enable() when the watchdog is not active:

    rzv2h_wdt_restart()
        if (!active) {
            ret = clk_enable(priv->pclk);
            if (ret)
                return ret;
        }

Since rzv2h_wdt_restart() is called in an atomic panic context where other CPUs
are halted via smp_send_stop(), can this cause a deadlock? If another CPU is
halted while holding the clock framework's global enable_lock spinlock, the
panicking CPU will spin indefinitely waiting for it.

[ ... ]
> @@ -280,14 +273,30 @@ static const struct watchdog_ops rzv2h_wdt_ops = {
>  	.restart = rzv2h_wdt_restart,
>  };
>  
> +static const struct regmap_config rzv2h_wdtdcr_regmap_config = {
> +	.name = "wdtdcr",
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = WDTDCR,
> +	.fast_io = true,
> +	.use_raw_spinlock = true,
> +};

[Severity: High]
Since rzt2h_wdt_wdtdcr_count_start() uses regmap_clear_bits() and is called
by the panic restart handler, can this raw spinlock introduce a deadlock?

If a panic occurs while another CPU is executing rzv2h_wdt_start() or
rzv2h_wdt_stop() and holds this regmap's raw spinlock, the other CPU is
halted by smp_send_stop(). When the panicking CPU executes rzv2h_wdt_restart(),
it will attempt to acquire the exact same raw spinlock and hang forever
instead of rebooting.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814191415.2110732-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=2

Re: [PATCH v3 1/3] watchdog: rzv2h_wdt: Propagate WDTDCR access errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Error path unwinding in `rzv2h_wdt_stop()` leaves the watchdog hardware unconfigured, causing an unintended immediate system reboot.
- [High] Aborting the `restart` handler upon register access failure prevents system reboot and hangs the system.
- [Medium] Using asynchronous `pm_runtime_put()` on the probe error path leaves the device permanently powered on.
--

--- Patch [1]: [PATCH v3 1/3] watchdog: rzv2h_wdt: Propagate WDTDCR access errors ---
commit ffabc27a1b8e9074dcd4cdaba7bc802a6d661fcc
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

watchdog: rzv2h_wdt: Propagate WDTDCR access errors

The WDTDCR helpers currently access the register directly using readl()/writel()
and therefore cannot report failures to their callers. Make the helpers return
an error so their callers can propagate failures.

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index 3b6abb66a1da0..b770226f785ea 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c

[ ... ]

> @@ -171,8 +181,13 @@ static int rzv2h_wdt_stop(struct watchdog_device *wdev)
>  	if (ret)
>  		return ret;
>  
> -	if (priv->of_data->wdtdcr)
> -		rzt2h_wdt_wdtdcr_count_stop(priv);
> +	if (priv->of_data->wdtdcr) {
> +		ret = rzt2h_wdt_wdtdcr_count_stop(priv);
> +		if (ret) {
> +			reset_control_deassert(priv->rstc);
> +			return ret;
> +		}
> +	}
>  
>  	pm_runtime_put(wdev->parent);

[Severity: High]
If rzt2h_wdt_wdtdcr_count_stop() fails, this unwinding leaves the hardware
out of reset but unconfigured. 

If the stop operation returns an error, the watchdog core assumes the
hardware is still running and immediately issues a fallback ping. Will this
ping against zeroed, unconfigured registers start a zero-length timeout and
cause an unintended immediate system reboot?

[ ... ]

> @@ -234,8 +250,19 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
>  	rzv2h_wdt_setup(wdev, priv->of_data->cks_min | WDTCR_RPSS_25 |
>  			WDTCR_RPES_75 | WDTCR_TOPS_1024);
>  
> -	if (priv->of_data->wdtdcr)
> -		rzt2h_wdt_wdtdcr_count_start(priv);
> +	if (priv->of_data->wdtdcr) {
> +		ret = rzt2h_wdt_wdtdcr_count_start(priv);
> +		if (ret) {
> +			if (!active) {
> +				reset_control_assert(priv->rstc);
> +				clk_disable(priv->oscclk);
> +				clk_disable(priv->pclk);
> +			} else {
> +				reset_control_reset(priv->rstc);
> +			}
> +			return ret;
> +		}
> +	}
>  
>  	rzv2h_wdt_ping(wdev);

[Severity: High]
Does returning an error here prevent the system from resetting?

The watchdog core's restart notifier wrapper translates an error return into
NOTIFY_BAD. Since NOTIFY_BAD includes the NOTIFY_STOP_MASK bit, this aborts
the restart notifier chain entirely, skipping any fallback handlers and
causing the system to hang indefinitely.

Should this restart handler make a best-effort attempt (perhaps ignoring the
WDTDCR start failure) instead of returning an error?

[ ... ]

> @@ -266,11 +293,11 @@ static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
>  	if (ret)
>  		return ret;
>  
> -	rzt2h_wdt_wdtdcr_count_stop(priv);
> +	ret = rzt2h_wdt_wdtdcr_count_stop(priv);
>  
>  	pm_runtime_put(&pdev->dev);
>  
> -	return 0;
> +	return ret;
>  }

[Severity: Medium]
When rzt2h_wdt_wdtdcr_count_stop() fails, this returns an error that fails
the probe function.

Does using the asynchronous pm_runtime_put() here leave the device powered
on?

Since probe failed, devres immediately calls pm_runtime_disable(), which
cancels any pending asynchronous power-management work. Should this be
pm_runtime_put_sync() instead, to ensure the device powers down before the
disable occurs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814191415.2110732-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1