[PATCH v6 0/8] Add syscon support for Renesas WDT driver

[PATCH v6 0/8] Add syscon support for Renesas WDT driver

[Prabhakar]

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

Hi all,

This series adds syscon support for the Renesas RZ/T2H and RZ/N2H SoCs in
the WDT driver. The WDTDCR register is part of the CPG/MSSR block for
which the CPG driver registers SYSC regmap. WDT driver now uses this
syscon to access the WDTDCR register.

These patches were part of two seprate series [0] and [1], out of which
certain patches have been queued separately, so I have included rest of
the patches into this single series.

Merge strategy, as the WDT driver and DTSI changes are interdependent
the driver patches can go via renesas-soc.

v5->v6:
- Used pm_runtime_put_sync() in rzv2h_wdt_stop() function.
- rebased on top of next-20260922

v4->v5:
- patch #1 is new addressing Sashiko's comments to avoid potential deadlock
- patch #4 is new addressing Sashiko's comments
- Dropped checking rzt2h_wdt_wdtdcr_count_start() return value in
  restart path.
- Handled the regmap locking in the driver and introduced nolock
  version to be used in the restart path.
- Added review-by tag.

[0] https://lore.kernel.org/all/20260817192540.423994-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
[1] https://lore.kernel.org/all/20260814191415.2110732-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (8):
  watchdog: rzv2h: Use pm_runtime_put_sync()
  watchdog: rzv2h: Drop enabling clocks in the restart handler
  watchdog: rzv2h: Drop WDTRCR_RSTIRQS define
  watchdog: rzv2h: Propagate WDTDCR access errors
  watchdog: rzv2h: Convert WDTDCR handling to regmap
  watchdog: rzv2h: Add syscon support for WDTDCR
  arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access
  arm64: dts: renesas: r9a09g087: Use CPG/MSSR syscon for WDTDCR access

 arch/arm64/boot/dts/renesas/r9a09g077.dtsi |  24 ++--
 arch/arm64/boot/dts/renesas/r9a09g087.dtsi |  24 ++--
 drivers/watchdog/Kconfig                   |   1 +
 drivers/watchdog/rzv2h_wdt.c               | 136 +++++++++++++++------
 4 files changed, 123 insertions(+), 62 deletions(-)

-- 
2.55.0

[PATCH v6 1/8] watchdog: rzv2h: Use pm_runtime_put_sync()

[Prabhakar]

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

pm_runtime_put() may trigger the idle check after pm_runtime_disable()
is run as part of devm_pm_runtime_enable()'s cleanup action, leaving
runtime PM active.

Use pm_runtime_put_sync() to ensure the idle check runs synchronously.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v5->v6:
- Used pm_runtime_put_sync() in rzv2h_wdt_stop() function.

v4->v5:
- New patch
---
 drivers/watchdog/rzv2h_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index 83540dd9a37b..057b890de0b0 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -174,7 +174,7 @@ static int rzv2h_wdt_stop(struct watchdog_device *wdev)
 	if (priv->of_data->wdtdcr)
 		rzt2h_wdt_wdtdcr_count_stop(priv);
 
-	pm_runtime_put(wdev->parent);
+	pm_runtime_put_sync(wdev->parent);
 
 	return 0;
 }
@@ -268,7 +268,7 @@ static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
 
 	rzt2h_wdt_wdtdcr_count_stop(priv);
 
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 
 	return 0;
 }
-- 
2.55.0

[PATCH v6 2/8] watchdog: rzv2h: Drop enabling clocks in the restart handler

[Prabhakar]

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

The watchdog restart handler runs from do_kernel_restart() with
interrupts disabled and after smp_send_stop() has halted the other CPUs.
Calling clk_enable() from the !watchdog_active() path takes the clock
framework's global enable_lock. If a stopped CPU held this lock, the
CPU executing the restart handler can spin indefinitely and prevent the
watchdog from resetting the system.

Keep pclk and oscclk enabled for the lifetime of the watchdog device by
using devm_clk_get_enabled() and devm_clk_get_optional_enabled().
Remove the conditional clock enable/disable operations from the restart
handler so it does not acquire any clock framework locks.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v5->v6:
- No change

v4->v5:
- New patch
---
 drivers/watchdog/rzv2h_wdt.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index 057b890de0b0..ee1649e0edae 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -191,22 +191,9 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
 	int ret;
 
 	if (!watchdog_active(wdev)) {
-		ret = clk_enable(priv->pclk);
-		if (ret)
-			return ret;
-
-		ret = clk_enable(priv->oscclk);
-		if (ret) {
-			clk_disable(priv->pclk);
-			return ret;
-		}
-
 		ret = reset_control_deassert(priv->rstc);
-		if (ret) {
-			clk_disable(priv->oscclk);
-			clk_disable(priv->pclk);
+		if (ret)
 			return ret;
-		}
 	} else {
 		/*
 		 * Writing to the WDT Control Register (WDTCR) or WDT Reset
@@ -291,11 +278,11 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	priv->pclk = devm_clk_get_prepared(dev, "pclk");
+	priv->pclk = devm_clk_get_enabled(dev, "pclk");
 	if (IS_ERR(priv->pclk))
 		return dev_err_probe(dev, PTR_ERR(priv->pclk), "Failed to get pclk\n");
 
-	priv->oscclk = devm_clk_get_optional_prepared(dev, "oscclk");
+	priv->oscclk = devm_clk_get_optional_enabled(dev, "oscclk");
 	if (IS_ERR(priv->oscclk))
 		return dev_err_probe(dev, PTR_ERR(priv->oscclk), "Failed to get oscclk\n");
 
-- 
2.55.0

[PATCH v6 3/8] 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>
---
v5->v6:
- No change

v4->v5:
- No change

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 ee1649e0edae..9774fe8e441e 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -39,8 +39,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.55.0

[PATCH v6 4/8] watchdog: rzv2h: Propagate WDTDCR access errors

[Prabhakar]

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

The WDTDCR helpers 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 and stop paths and unwind the
resources acquired before the WDTDCR access, restoring the reset state.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v5->v6:
- No change

v4->v5:
- Dropped checking rzt2h_wdt_wdtdcr_count_start() return value in
  restart path.

v2->v3:
- New patch, split from v2 patch #1 to make thing easier to review.
---
 drivers/watchdog/rzv2h_wdt.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index 9774fe8e441e..a65e5d6c48ad 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -87,18 +87,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)
@@ -148,8 +152,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
@@ -169,8 +179,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_sync(wdev->parent);
 
@@ -219,8 +234,10 @@ 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)
+	if (priv->of_data->wdtdcr) {
+		/* best effort, ignore ret */
 		rzt2h_wdt_wdtdcr_count_start(priv);
+	}
 
 	rzv2h_wdt_ping(wdev);
 
@@ -251,11 +268,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_sync(&pdev->dev);
 
-	return 0;
+	return ret;
 }
 
 static int rzv2h_wdt_probe(struct platform_device *pdev)
-- 
2.55.0

Re: [PATCH v6 4/8] watchdog: rzv2h: Propagate WDTDCR access errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: rzv2h: unconfigured hardware reset on stop error path
--

commit b536187da1b8888ce4af583c2cf2a8479cfaaa53
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

watchdog: rzv2h: Propagate WDTDCR access errors

This commit updates the watchdog start and stop paths to handle WDTDCR
register access errors. It propagates failures back to the caller and
unwinds acquired resources, attempting to restore the reset state.

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c

[ ... ]

> @@ -169,8 +179,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;
> +		}
> +	}

[Severity: High]
Does this error path unwind leave the hardware in an unconfigured state?

Earlier in rzv2h_wdt_stop(), reset_control_assert() places the watchdog IP
into reset, which clears its configuration registers to default values. If
rzt2h_wdt_wdtdcr_count_stop() fails, this path deasserts the reset line but
does not restore the previous running configuration, missing the required
udelay(3) and rzv2h_wdt_setup() initialization.

Since rzv2h_wdt_stop() returns an error, won't the watchdog core assume the
device is still active and continue to issue keepalive pings?

If the core pings the newly-deasserted but unconfigured watchdog, it seems
this would start the down-counter with default reset register values,
triggering an immediate unintended hardware reset.

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

Re: [PATCH v6 4/8] watchdog: rzv2h: Propagate WDTDCR access errors

[Guenter Roeck]

On Wed, Sep 23, 2026 at 11:40:50AM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The WDTDCR helpers 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 and stop paths and unwind the
> resources acquired before the WDTDCR access, restoring the reset state.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v5->v6:
> - No change
> 
> v4->v5:
> - Dropped checking rzt2h_wdt_wdtdcr_count_start() return value in
>   restart path.
> 
> v2->v3:
> - New patch, split from v2 patch #1 to make thing easier to review.
> ---
>  drivers/watchdog/rzv2h_wdt.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index 9774fe8e441e..a65e5d6c48ad 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c
> @@ -87,18 +87,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)
> @@ -148,8 +152,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);

It seems to me this does way more than handling the error:
it puts the module into reset and calls pm_runtime_put()
on the parent. This is a substantial functional change
that is not explained.

> +			pm_runtime_put(wdev->parent);
> +			return ret;
> +		}
> +	}
>  
>  	/*
>  	 * Down counting starts after writing the sequence 00h -> FFh to the
> @@ -169,8 +179,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);

Same here, and the logic behind taking the module out of reset
in the stop function and putting it into reset in the start function
isn't obvious to me.

Frankly, I think it would be much better and risk fewer problems
to just ignore the theoretic errors from the regmap functions.

> +			return ret;
> +		}
> +	}
>  
>  	pm_runtime_put_sync(wdev->parent);
>  
> @@ -219,8 +234,10 @@ 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)
> +	if (priv->of_data->wdtdcr) {
> +		/* best effort, ignore ret */
>  		rzt2h_wdt_wdtdcr_count_start(priv);
> +	}
>  
>  	rzv2h_wdt_ping(wdev);
>  
> @@ -251,11 +268,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_sync(&pdev->dev);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int rzv2h_wdt_probe(struct platform_device *pdev)
> -- 
> 2.55.0
> 

Re: [PATCH v6 4/8] watchdog: rzv2h: Propagate WDTDCR access errors

[Lad, Prabhakar]

Hi Guenter,

Thank you for the review.

On Wed, Sep 23, 2026 at 10:17 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Wed, Sep 23, 2026 at 11:40:50AM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The WDTDCR helpers 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 and stop paths and unwind the
> > resources acquired before the WDTDCR access, restoring the reset state.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v5->v6:
> > - No change
> >
> > v4->v5:
> > - Dropped checking rzt2h_wdt_wdtdcr_count_start() return value in
> >   restart path.
> >
> > v2->v3:
> > - New patch, split from v2 patch #1 to make thing easier to review.
> > ---
> >  drivers/watchdog/rzv2h_wdt.c | 35 ++++++++++++++++++++++++++---------
> >  1 file changed, 26 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> > index 9774fe8e441e..a65e5d6c48ad 100644
> > --- a/drivers/watchdog/rzv2h_wdt.c
> > +++ b/drivers/watchdog/rzv2h_wdt.c
> > @@ -87,18 +87,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)
> > @@ -148,8 +152,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);
>
> It seems to me this does way more than handling the error:
> it puts the module into reset and calls pm_runtime_put()
> on the parent. This is a substantial functional change
> that is not explained.
>
The intention was only to unwind what rzv2h_wdt_start() had already
done before the WDTDCR access (the runtime PM get and the reset
deassert), so that a failure would leave the module in the same state
as before the call. I agree this should have been explained, and that
it adds more churn than the change is worth.

> > +                     pm_runtime_put(wdev->parent);
> > +                     return ret;
> > +             }
> > +     }
> >
> >       /*
> >        * Down counting starts after writing the sequence 00h -> FFh to the
> > @@ -169,8 +179,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);
>
> Same here, and the logic behind taking the module out of reset
> in the stop function and putting it into reset in the start function
> isn't obvious to me.
>
Same reasoning as above.

> Frankly, I think it would be much better and risk fewer problems
> to just ignore the theoretic errors from the regmap functions.
>
Agreed. For the MMIO-backed regmap these accesses cannot fail in
practice, so in the next version I'll keep
rzt2h_wdt_wdtdcr_count_start() and rzt2h_wdt_wdtdcr_count_stop()
returning void, drop the error unwinding from the start/stop paths,
and ignore the return values of the regmap calls.

Cheers,
Prabhakar

> > +                     return ret;
> > +             }
> > +     }
> >
> >       pm_runtime_put_sync(wdev->parent);
> >
> > @@ -219,8 +234,10 @@ 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)
> > +     if (priv->of_data->wdtdcr) {
> > +             /* best effort, ignore ret */
> >               rzt2h_wdt_wdtdcr_count_start(priv);
> > +     }
> >
> >       rzv2h_wdt_ping(wdev);
> >
> > @@ -251,11 +268,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_sync(&pdev->dev);
> >
> > -     return 0;
> > +     return ret;
> >  }
> >
> >  static int rzv2h_wdt_probe(struct platform_device *pdev)
> > --
> > 2.55.0
> >

Re: [PATCH v6 4/8] watchdog: rzv2h: Propagate WDTDCR access errors

[Guenter Roeck]

On Wed, Sep 23, 2026 at 11:40:50AM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The WDTDCR helpers 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.

For real or just theoretically ? If for real, warranting the quite
substantial and invasive error handling code in this patch, this
really needs to be explained further.

Thanks,
Guenter

[PATCH v6 5/8] 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>
---
v5->v6:
- No change

v4->v5:
- Handled the regmap locking in the driver and introduced nolock
  version to be used in the restart path.

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

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index a65e5d6c48ad..bf46e239751c 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2024 Renesas Electronics Corporation.
  */
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/io.h>
@@ -12,7 +13,9 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
+#include <linux/spinlock.h>
 #include <linux/units.h>
 #include <linux/watchdog.h>
 
@@ -65,7 +68,8 @@ struct rzv2h_of_data {
 
 struct rzv2h_wdt_priv {
 	void __iomem *base;
-	void __iomem *wdtdcr;
+	struct regmap *wdtdcr_regmap;
+	spinlock_t regmap_lock; /* Protects access to the wdtdcr_regmap */
 	struct clk *pclk;
 	struct clk *oscclk;
 	struct reset_control *rstc;
@@ -89,20 +93,19 @@ 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);
+	guard(spinlock_irqsave)(&priv->regmap_lock);
+	return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
+}
 
-	return 0;
+static inline int rzt2h_wdt_wdtdcr_count_start_nolock(struct rzv2h_wdt_priv *priv)
+{
+	return regmap_clear_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;
+	guard(spinlock_irqsave)(&priv->regmap_lock);
+	return rzt2h_wdt_wdtdcr_count_start_nolock(priv);
 }
 
 static void rzv2h_wdt_setup(struct watchdog_device *wdev, u16 wdtcr)
@@ -235,8 +238,11 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
 			WDTCR_RPES_75 | WDTCR_TOPS_1024);
 
 	if (priv->of_data->wdtdcr) {
-		/* best effort, ignore ret */
-		rzt2h_wdt_wdtdcr_count_start(priv);
+		/*
+		 * Best effort, ignore return value and use the unlocked
+		 * variant of the function to avoid potential deadlocks.
+		 */
+		rzt2h_wdt_wdtdcr_count_start_nolock(priv);
 	}
 
 	rzv2h_wdt_ping(wdev);
@@ -255,14 +261,32 @@ 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,
+	.max_register_is_0 = true,
+	.disable_locking = 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);
+
+	spin_lock_init(&priv->regmap_lock);
+
+	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.55.0

[PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Prabhakar]

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

On RZ/T2H and RZ/N2H SoCs, the WDTDCR (WDT Debug Control Register) resides
in the second register region of the CPG/MSSR block. Since this
multi-function block is shared with other peripherals, it is exposed via a
syscon regmap interface.

Look up the SYSC regmap using the optional "renesas,sysc" property and
derive the WDTDCR offset from the watchdog instance index.

Retain the existing MMIO-based access method when the "renesas,sysc"
property is absent to preserve compatibility with existing DT's.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v5->v6:
- No change

v4->v5:
- Code changes due to regmap_lock introduction.

v3->v4:
- No change

v2->v3:
- Made use of the new "renesas,sysc" phandle-array property to access the
  WDTDCR register via the CPG/MSSR syscon node.
- Updated commit message

v1->v2:
- No change.
---
 drivers/watchdog/Kconfig     |  1 +
 drivers/watchdog/rzv2h_wdt.c | 64 +++++++++++++++++++++++++++---------
 2 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f7d0a0c2c0ef..3c4fc9916d8c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1046,6 +1046,7 @@ config RENESAS_RZV2HWDT
 	depends on ARCH_RENESAS || COMPILE_TEST
 	depends on PM || COMPILE_TEST
 	select WATCHDOG_CORE
+	select MFD_SYSCON
 	help
 	  This driver adds watchdog support for the integrated watchdogs in the
 	  Renesas RZ/{G3E,V2H(P)} SoCs. These watchdogs can be used to reset a
diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index bf46e239751c..9d00e54b823e 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -9,6 +9,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -46,6 +47,11 @@
 
 #define WDT_DEFAULT_TIMEOUT	60U
 
+#define RZT2H_WDT_MAX_INSTANCES	6
+
+#define RZT2H_SYS_BLOCK1(n)	(BIT(16) | (0x5100 + (n) * 4))
+#define RZT2H_WDTDCR_OFFSET(n)	RZT2H_SYS_BLOCK1(n)
+
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
@@ -66,15 +72,20 @@ struct rzv2h_of_data {
 	bool wdtdcr;
 };
 
+struct rzv2h_sysc_wdtdcr {
+	struct regmap *regmap;
+	spinlock_t regmap_lock; /* Protects access to the regmap */
+	unsigned int offset;
+};
+
 struct rzv2h_wdt_priv {
 	void __iomem *base;
-	struct regmap *wdtdcr_regmap;
-	spinlock_t regmap_lock; /* Protects access to the wdtdcr_regmap */
 	struct clk *pclk;
 	struct clk *oscclk;
 	struct reset_control *rstc;
 	struct watchdog_device wdev;
 	const struct rzv2h_of_data *of_data;
+	struct rzv2h_sysc_wdtdcr sysc;
 };
 
 static int rzv2h_wdt_ping(struct watchdog_device *wdev)
@@ -93,18 +104,22 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
 
 static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
 {
-	guard(spinlock_irqsave)(&priv->regmap_lock);
-	return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
+	struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
+
+	guard(spinlock_irqsave)(&sysc->regmap_lock);
+	return regmap_set_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
 }
 
 static inline int rzt2h_wdt_wdtdcr_count_start_nolock(struct rzv2h_wdt_priv *priv)
 {
-	return regmap_clear_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
+	struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
+
+	return regmap_clear_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
 }
 
 static int rzt2h_wdt_wdtdcr_count_start(struct rzv2h_wdt_priv *priv)
 {
-	guard(spinlock_irqsave)(&priv->regmap_lock);
+	guard(spinlock_irqsave)(&priv->sysc.regmap_lock);
 	return rzt2h_wdt_wdtdcr_count_start_nolock(priv);
 }
 
@@ -274,19 +289,38 @@ static const struct regmap_config rzv2h_wdtdcr_regmap_config = {
 static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
 				 struct rzv2h_wdt_priv *priv)
 {
-	void __iomem *wdtdcr;
+	struct device_node *np = pdev->dev.of_node;
+	bool syscon_present = of_property_present(np, "renesas,sysc");
+	struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
 	int ret;
 
-	wdtdcr = devm_platform_ioremap_resource(pdev, 1);
-	if (IS_ERR(wdtdcr))
-		return PTR_ERR(wdtdcr);
+	spin_lock_init(&sysc->regmap_lock);
 
-	spin_lock_init(&priv->regmap_lock);
+	if (syscon_present) {
+		unsigned int wdt_index;
 
-	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);
+		sysc->regmap = syscon_regmap_lookup_by_phandle_args(np, "renesas,sysc",
+								    1, &wdt_index);
+		if (IS_ERR(sysc->regmap))
+			return PTR_ERR(sysc->regmap);
+
+		if (wdt_index >= RZT2H_WDT_MAX_INSTANCES)
+			return -EINVAL;
+
+		sysc->offset = RZT2H_WDTDCR_OFFSET(wdt_index);
+	} else {
+		void __iomem *wdtdcr;
+
+		wdtdcr = devm_platform_ioremap_resource(pdev, 1);
+		if (IS_ERR(wdtdcr))
+			return PTR_ERR(wdtdcr);
+
+		sysc->regmap = devm_regmap_init_mmio(&pdev->dev, wdtdcr,
+						     &rzv2h_wdtdcr_regmap_config);
+		if (IS_ERR(sysc->regmap))
+			return PTR_ERR(sysc->regmap);
+		sysc->offset = WDTDCR;
+	}
 
 	ret = pm_runtime_resume_and_get(&pdev->dev);
 	if (ret)
-- 
2.55.0

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: rzv2h: Deadlock in restart handler via syscon regmap locking
--

commit 1e1b26069f70735a537750b22a51257bb422c2e4
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

watchdog: rzv2h: Add syscon support for WDTDCR

The commit adds support for accessing the WDTDCR register via a syscon
regmap interface for RZ/T2H and RZ/N2H SoCs. It looks up the SYSC regmap
using the "renesas,sysc" property or falls back to MMIO access if absent.

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index 76c1d315b0e77..30b0b70fecaa9 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c

[ ... ]

> @@ -93,18 +104,22 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
>  
>  static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
>  {
> -	guard(spinlock_irqsave)(&priv->regmap_lock);
> -	return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> +	struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> +
> +	guard(spinlock_irqsave)(&sysc->regmap_lock);
> +	return regmap_set_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
>  }

[Severity: High]
Is the external sysc->regmap_lock still necessary here?

When using the syscon regmap returned by syscon_regmap_lookup_by_phandle_args(),
the regmap has internal locking enabled by default. Since regmap_set_bits()
already performs an atomic read-modify-write using the syscon's internal lock,
this external wrapper lock appears redundant for the syscon path.

>  
>  static inline int rzt2h_wdt_wdtdcr_count_start_nolock(struct rzv2h_wdt_priv *priv)
>  {
> -	return regmap_clear_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> +	struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> +
> +	return regmap_clear_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
>  }

[Severity: High]
Could this lead to a deadlock during a system panic or restart?

This function is called by the rzv2h_wdt_restart() handler, which executes in
atomic or panic context and must remain strictly lockless to prevent deadlocks.

While bypassing the driver's local sysc->regmap_lock was safe in the legacy
MMIO fallback case (where rzv2h_wdtdcr_regmap_config disables locking), the
syscon regmap has internal locking enabled.

Calling regmap_clear_bits() unconditionally attempts to acquire the syscon
regmap's internal spinlock. If the system panics while another CPU is accessing
the shared syscon block, the restart handler could hang trying to acquire this
lock, defeating the purpose of the watchdog restart recovery.

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

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Lad, Prabhakar]

Hi Guenter,

On Wed, Sep 23, 2026 at 11:52 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] watchdog: rzv2h: Deadlock in restart handler via syscon regmap locking
> --
>
> commit 1e1b26069f70735a537750b22a51257bb422c2e4
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> watchdog: rzv2h: Add syscon support for WDTDCR
>
> The commit adds support for accessing the WDTDCR register via a syscon
> regmap interface for RZ/T2H and RZ/N2H SoCs. It looks up the SYSC regmap
> using the "renesas,sysc" property or falls back to MMIO access if absent.
>
> > diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> > index 76c1d315b0e77..30b0b70fecaa9 100644
> > --- a/drivers/watchdog/rzv2h_wdt.c
> > +++ b/drivers/watchdog/rzv2h_wdt.c
>
> [ ... ]
>
> > @@ -93,18 +104,22 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
> >
> >  static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
> >  {
> > -     guard(spinlock_irqsave)(&priv->regmap_lock);
> > -     return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> > +     struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> > +
> > +     guard(spinlock_irqsave)(&sysc->regmap_lock);
> > +     return regmap_set_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
> >  }
>
> [Severity: High]
> Is the external sysc->regmap_lock still necessary here?
>
> When using the syscon regmap returned by syscon_regmap_lookup_by_phandle_args(),
> the regmap has internal locking enabled by default. Since regmap_set_bits()
> already performs an atomic read-modify-write using the syscon's internal lock,
> this external wrapper lock appears redundant for the syscon path.
>
> >
> >  static inline int rzt2h_wdt_wdtdcr_count_start_nolock(struct rzv2h_wdt_priv *priv)
> >  {
> > -     return regmap_clear_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> > +     struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> > +
> > +     return regmap_clear_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
> >  }
>
> [Severity: High]
> Could this lead to a deadlock during a system panic or restart?
>
> This function is called by the rzv2h_wdt_restart() handler, which executes in
> atomic or panic context and must remain strictly lockless to prevent deadlocks.
>
> While bypassing the driver's local sysc->regmap_lock was safe in the legacy
> MMIO fallback case (where rzv2h_wdtdcr_regmap_config disables locking), the
> syscon regmap has internal locking enabled.
>
> Calling regmap_clear_bits() unconditionally attempts to acquire the syscon
> regmap's internal spinlock. If the system panics while another CPU is accessing
> the shared syscon block, the restart handler could hang trying to acquire this
> lock, defeating the purpose of the watchdog restart recovery.
>
Agreed, the syscon regmap lock cannot be bypassed from the watchdog
driver, so the _nolock variant does not achieve anything for the
syscon case.

Rather than working around this, I think the right fix is to not
register a watchdog restart handler on RZ/T2H at all. On RZ/T2H,
system reset is implemented by TF-A through PSCI SYSTEM_RESET, which
triggers a software system reset via the SWRSYS register. The PSCI
driver registers its restart handler at priority 129, while the
watchdog restart handler is registered at the default priority of 0,
so PSCI already resets the system first and the watchdog restart
handler is never reached on this SoC.

With the restart handler gone on RZ/T2H, WDTDCR is only accessed
during probe and from the start/stop callbacks.

Is this approach OK?

Cheers,
Prabhakar

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Guenter Roeck]

On Thu, Sep 24, 2026 at 02:44:07PM +0100, Lad, Prabhakar wrote:
> Hi Guenter,
> 
> On Wed, Sep 23, 2026 at 11:52 AM <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] watchdog: rzv2h: Deadlock in restart handler via syscon regmap locking
> > --
> >
> > commit 1e1b26069f70735a537750b22a51257bb422c2e4
> > Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > watchdog: rzv2h: Add syscon support for WDTDCR
> >
> > The commit adds support for accessing the WDTDCR register via a syscon
> > regmap interface for RZ/T2H and RZ/N2H SoCs. It looks up the SYSC regmap
> > using the "renesas,sysc" property or falls back to MMIO access if absent.
> >
> > > diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> > > index 76c1d315b0e77..30b0b70fecaa9 100644
> > > --- a/drivers/watchdog/rzv2h_wdt.c
> > > +++ b/drivers/watchdog/rzv2h_wdt.c
> >
> > [ ... ]
> >
> > > @@ -93,18 +104,22 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
> > >
> > >  static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
> > >  {
> > > -     guard(spinlock_irqsave)(&priv->regmap_lock);
> > > -     return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> > > +     struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> > > +
> > > +     guard(spinlock_irqsave)(&sysc->regmap_lock);
> > > +     return regmap_set_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
> > >  }
> >
> > [Severity: High]
> > Is the external sysc->regmap_lock still necessary here?
> >
> > When using the syscon regmap returned by syscon_regmap_lookup_by_phandle_args(),
> > the regmap has internal locking enabled by default. Since regmap_set_bits()
> > already performs an atomic read-modify-write using the syscon's internal lock,
> > this external wrapper lock appears redundant for the syscon path.
> >
> > >
> > >  static inline int rzt2h_wdt_wdtdcr_count_start_nolock(struct rzv2h_wdt_priv *priv)
> > >  {
> > > -     return regmap_clear_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> > > +     struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> > > +
> > > +     return regmap_clear_bits(sysc->regmap, sysc->offset, WDTDCR_WDTSTOPCTRL);
> > >  }
> >
> > [Severity: High]
> > Could this lead to a deadlock during a system panic or restart?
> >
> > This function is called by the rzv2h_wdt_restart() handler, which executes in
> > atomic or panic context and must remain strictly lockless to prevent deadlocks.
> >
> > While bypassing the driver's local sysc->regmap_lock was safe in the legacy
> > MMIO fallback case (where rzv2h_wdtdcr_regmap_config disables locking), the
> > syscon regmap has internal locking enabled.
> >
> > Calling regmap_clear_bits() unconditionally attempts to acquire the syscon
> > regmap's internal spinlock. If the system panics while another CPU is accessing
> > the shared syscon block, the restart handler could hang trying to acquire this
> > lock, defeating the purpose of the watchdog restart recovery.
> >
> Agreed, the syscon regmap lock cannot be bypassed from the watchdog
> driver, so the _nolock variant does not achieve anything for the
> syscon case.
> 
> Rather than working around this, I think the right fix is to not
> register a watchdog restart handler on RZ/T2H at all. On RZ/T2H,
> system reset is implemented by TF-A through PSCI SYSTEM_RESET, which
> triggers a software system reset via the SWRSYS register. The PSCI
> driver registers its restart handler at priority 129, while the
> watchdog restart handler is registered at the default priority of 0,
> so PSCI already resets the system first and the watchdog restart
> handler is never reached on this SoC.
> 
> With the restart handler gone on RZ/T2H, WDTDCR is only accessed
> during probe and from the start/stop callbacks.
> 
> Is this approach OK?
> 
Makes sense to me.

Thanks,
Guenter

RE: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Biju Das]

Hi Geunter/Prabhakar,

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: 24 September 2026 22:49
> Subject: Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR
> 
> On Thu, Sep 24, 2026 at 02:44:07PM +0100, Lad, Prabhakar wrote:
> > Hi Guenter,
> >
> > On Wed, Sep 23, 2026 at 11:52 AM <sashiko-bot@kernel.org> wrote:
> > >
> > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > - [High] watchdog: rzv2h: Deadlock in restart handler via syscon
> > > regmap locking
> > > --
> > >
> > > commit 1e1b26069f70735a537750b22a51257bb422c2e4
> > > Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > watchdog: rzv2h: Add syscon support for WDTDCR
> > >
> > > The commit adds support for accessing the WDTDCR register via a
> > > syscon regmap interface for RZ/T2H and RZ/N2H SoCs. It looks up the
> > > SYSC regmap using the "renesas,sysc" property or falls back to MMIO access if absent.
> > >
> > > > diff --git a/drivers/watchdog/rzv2h_wdt.c
> > > > b/drivers/watchdog/rzv2h_wdt.c index 76c1d315b0e77..30b0b70fecaa9
> > > > 100644
> > > > --- a/drivers/watchdog/rzv2h_wdt.c
> > > > +++ b/drivers/watchdog/rzv2h_wdt.c
> > >
> > > [ ... ]
> > >
> > > > @@ -93,18 +104,22 @@ static int rzv2h_wdt_ping(struct
> > > > watchdog_device *wdev)
> > > >
> > > >  static int rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv
> > > > *priv)  {
> > > > -     guard(spinlock_irqsave)(&priv->regmap_lock);
> > > > -     return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> > > > +     struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> > > > +
> > > > +     guard(spinlock_irqsave)(&sysc->regmap_lock);
> > > > +     return regmap_set_bits(sysc->regmap, sysc->offset,
> > > > + WDTDCR_WDTSTOPCTRL);
> > > >  }
> > >
> > > [Severity: High]
> > > Is the external sysc->regmap_lock still necessary here?
> > >
> > > When using the syscon regmap returned by
> > > syscon_regmap_lookup_by_phandle_args(),
> > > the regmap has internal locking enabled by default. Since
> > > regmap_set_bits() already performs an atomic read-modify-write using
> > > the syscon's internal lock, this external wrapper lock appears redundant for the syscon path.
> > >
> > > >
> > > >  static inline int rzt2h_wdt_wdtdcr_count_start_nolock(struct
> > > > rzv2h_wdt_priv *priv)  {
> > > > -     return regmap_clear_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
> > > > +     struct rzv2h_sysc_wdtdcr *sysc = &priv->sysc;
> > > > +
> > > > +     return regmap_clear_bits(sysc->regmap, sysc->offset,
> > > > + WDTDCR_WDTSTOPCTRL);
> > > >  }
> > >
> > > [Severity: High]
> > > Could this lead to a deadlock during a system panic or restart?
> > >
> > > This function is called by the rzv2h_wdt_restart() handler, which
> > > executes in atomic or panic context and must remain strictly lockless to prevent deadlocks.
> > >
> > > While bypassing the driver's local sysc->regmap_lock was safe in the
> > > legacy MMIO fallback case (where rzv2h_wdtdcr_regmap_config disables
> > > locking), the syscon regmap has internal locking enabled.
> > >
> > > Calling regmap_clear_bits() unconditionally attempts to acquire the
> > > syscon regmap's internal spinlock. If the system panics while
> > > another CPU is accessing the shared syscon block, the restart
> > > handler could hang trying to acquire this lock, defeating the purpose of the watchdog restart
> recovery.
> > >
> > Agreed, the syscon regmap lock cannot be bypassed from the watchdog
> > driver, so the _nolock variant does not achieve anything for the
> > syscon case.
> >
> > Rather than working around this, I think the right fix is to not
> > register a watchdog restart handler on RZ/T2H at all. On RZ/T2H,
> > system reset is implemented by TF-A through PSCI SYSTEM_RESET, which
> > triggers a software system reset via the SWRSYS register. The PSCI
> > driver registers its restart handler at priority 129, while the
> > watchdog restart handler is registered at the default priority of 0,
> > so PSCI already resets the system first and the watchdog restart
> > handler is never reached on this SoC.
> >
> > With the restart handler gone on RZ/T2H, WDTDCR is only accessed
> > during probe and from the start/stop callbacks.
> >
> > Is this approach OK?
> >
> Makes sense to me.

Just a question,
For automatic software upgrade, if the reboot using PSCI fails, what is the fallback
for reboot, reset the board?

Cheers,
Biju

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Guenter Roeck]

On Fri, Sep 25, 2026 at 04:13:24AM +0000, Biju Das wrote:
> Hi Geunter/Prabhakar,
> 
...
> Just a question,
> For automatic software upgrade, if the reboot using PSCI fails, what is the fallback
> for reboot, reset the board?

That seems to be a bit theoretic. The reason for having the ability to
register multiple restart handlers is not to have failure fallbacks,
but to have fallbacks if preferred restart handlers are not available.
If the PSCI restart handler is present, it is expected to work.
That is why it has higher priority. The idea is to use the highest
priority restart handler, not to try one after another.

Sure, the failure fallback works as well, but that doesn't mean
that fallbacks should be provided just to provide fallbacks.

If you claim that the PSCI restart handler does not work on this
system, provide evicence, and we can discuss this further.

Thanks,
Guenter

RE: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Biju Das]

Hi Guenter,

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: 25 September 2026 05:51
> Subject: Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR
> 
> On Fri, Sep 25, 2026 at 04:13:24AM +0000, Biju Das wrote:
> > Hi Geunter/Prabhakar,
> >
> ...
> > Just a question,
> > For automatic software upgrade, if the reboot using PSCI fails, what
> > is the fallback for reboot, reset the board?
> 
> That seems to be a bit theoretic. The reason for having the ability to register multiple restart handlers
> is not to have failure fallbacks, but to have fallbacks if preferred restart handlers are not available.
> If the PSCI restart handler is present, it is expected to work.
> That is why it has higher priority. The idea is to use the highest priority restart handler, not to try
> one after another.
> 
> Sure, the failure fallback works as well, but that doesn't mean that fallbacks should be provided just to
> provide fallbacks.
> 
> If you claim that the PSCI restart handler does not work on this system, provide evicence, and we can
> discuss this further.

I agree normal cases there won't be any failures

I never heard about PSCI restart fails. But have seen a case due to a bug in
serial driver that made the PSCI restart to fail as it hangs the systems.

So, if this is the case, a memory corruption can cause the same.

Long back I worked on femto based product(single core), where we have normal restart
and a fallback watchdog restart. But it does not have PSCI system handlers.
On some cases, we have seen automatic upgrade failed because of failure in
normal restart.

So, I just throw a question based on that's all.

Cheers,
Biju

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Lad, Prabhakar]

Hi Biju,

On Fri, Sep 25, 2026 at 6:11 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Guenter,
>
> > -----Original Message-----
> > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > Sent: 25 September 2026 05:51
> > Subject: Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR
> >
> > On Fri, Sep 25, 2026 at 04:13:24AM +0000, Biju Das wrote:
> > > Hi Geunter/Prabhakar,
> > >
> > ...
> > > Just a question,
> > > For automatic software upgrade, if the reboot using PSCI fails, what
> > > is the fallback for reboot, reset the board?
> >
> > That seems to be a bit theoretic. The reason for having the ability to register multiple restart handlers
> > is not to have failure fallbacks, but to have fallbacks if preferred restart handlers are not available.
> > If the PSCI restart handler is present, it is expected to work.
> > That is why it has higher priority. The idea is to use the highest priority restart handler, not to try
> > one after another.
> >
> > Sure, the failure fallback works as well, but that doesn't mean that fallbacks should be provided just to
> > provide fallbacks.
> >
> > If you claim that the PSCI restart handler does not work on this system, provide evicence, and we can
> > discuss this further.
>
> I agree normal cases there won't be any failures
>
> I never heard about PSCI restart fails. But have seen a case due to a bug in
> serial driver that made the PSCI restart to fail as it hangs the systems.
>
> So, if this is the case, a memory corruption can cause the same.
>
> Long back I worked on femto based product(single core), where we have normal restart
> and a fallback watchdog restart. But it does not have PSCI system handlers.
> On some cases, we have seen automatic upgrade failed because of failure in
> normal restart.
>
If you think the PSCI might fail and not restart the machine the other
alternative would be to export functions to configure WDTDCR using
readl/writel.

Geert/Guenter - are you OK with the above approach.

Cheers,
Prabhakar

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Geert Uytterhoeven]

Hi Prabhakar,

On Fri, 25 Sept 2026 at 11:06, Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Fri, Sep 25, 2026 at 6:11 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > > On Fri, Sep 25, 2026 at 04:13:24AM +0000, Biju Das wrote:
> > > > Just a question,
> > > > For automatic software upgrade, if the reboot using PSCI fails, what
> > > > is the fallback for reboot, reset the board?
> > >
> > > That seems to be a bit theoretic. The reason for having the ability to register multiple restart handlers
> > > is not to have failure fallbacks, but to have fallbacks if preferred restart handlers are not available.
> > > If the PSCI restart handler is present, it is expected to work.
> > > That is why it has higher priority. The idea is to use the highest priority restart handler, not to try
> > > one after another.
> > >
> > > Sure, the failure fallback works as well, but that doesn't mean that fallbacks should be provided just to
> > > provide fallbacks.
> > >
> > > If you claim that the PSCI restart handler does not work on this system, provide evicence, and we can
> > > discuss this further.
> >
> > I agree normal cases there won't be any failures
> >
> > I never heard about PSCI restart fails. But have seen a case due to a bug in

Oh yes, if PSCI is not really implemented ;-)

> > serial driver that made the PSCI restart to fail as it hangs the systems.
> >
> > So, if this is the case, a memory corruption can cause the same.
> >
> > Long back I worked on femto based product(single core), where we have normal restart
> > and a fallback watchdog restart. But it does not have PSCI system handlers.
> > On some cases, we have seen automatic upgrade failed because of failure in
> > normal restart.
> >
> If you think the PSCI might fail and not restart the machine the other
> alternative would be to export functions to configure WDTDCR using
> readl/writel.

Well, if PSCI resets fails: that's exactly why you have a watchdog
(or two) in the system, right?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Lad, Prabhakar]

Hi Geert,

On Fri, Sep 25, 2026 at 10:09 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, 25 Sept 2026 at 11:06, Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Fri, Sep 25, 2026 at 6:11 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > > > On Fri, Sep 25, 2026 at 04:13:24AM +0000, Biju Das wrote:
> > > > > Just a question,
> > > > > For automatic software upgrade, if the reboot using PSCI fails, what
> > > > > is the fallback for reboot, reset the board?
> > > >
> > > > That seems to be a bit theoretic. The reason for having the ability to register multiple restart handlers
> > > > is not to have failure fallbacks, but to have fallbacks if preferred restart handlers are not available.
> > > > If the PSCI restart handler is present, it is expected to work.
> > > > That is why it has higher priority. The idea is to use the highest priority restart handler, not to try
> > > > one after another.
> > > >
> > > > Sure, the failure fallback works as well, but that doesn't mean that fallbacks should be provided just to
> > > > provide fallbacks.
> > > >
> > > > If you claim that the PSCI restart handler does not work on this system, provide evicence, and we can
> > > > discuss this further.
> > >
> > > I agree normal cases there won't be any failures
> > >
> > > I never heard about PSCI restart fails. But have seen a case due to a bug in
>
> Oh yes, if PSCI is not really implemented ;-)
>
> > > serial driver that made the PSCI restart to fail as it hangs the systems.
> > >
> > > So, if this is the case, a memory corruption can cause the same.
> > >
> > > Long back I worked on femto based product(single core), where we have normal restart
> > > and a fallback watchdog restart. But it does not have PSCI system handlers.
> > > On some cases, we have seen automatic upgrade failed because of failure in
> > > normal restart.
> > >
> > If you think the PSCI might fail and not restart the machine the other
> > alternative would be to export functions to configure WDTDCR using
> > readl/writel.
>
> Well, if PSCI resets fails: that's exactly why you have a watchdog
> (or two) in the system, right?
>
Agreed, To avoid taking the syscon lock from restart context, how
about exporting helpers from the CPG/MSSR driver to start/stop the
WDTDCR count using plain readl/writel? Each WDTDCR register belongs to
a single watchdog instance, so no locking is needed.

  /* include/linux/soc/renesas/renesas.h */
  int rzt2h_cpg_wdtdcr_count_start(unsigned int wdt_index);
  int rzt2h_cpg_wdtdcr_count_stop(unsigned int wdt_index);

Cheers,
Prabhakar

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Lad, Prabhakar]

Hi Geert,

On Fri, Sep 25, 2026 at 10:20 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Geert,
>
> On Fri, Sep 25, 2026 at 10:09 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> >
> > Hi Prabhakar,
> >
> > On Fri, 25 Sept 2026 at 11:06, Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > > On Fri, Sep 25, 2026 at 6:11 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > > > > On Fri, Sep 25, 2026 at 04:13:24AM +0000, Biju Das wrote:
> > > > > > Just a question,
> > > > > > For automatic software upgrade, if the reboot using PSCI fails, what
> > > > > > is the fallback for reboot, reset the board?
> > > > >
> > > > > That seems to be a bit theoretic. The reason for having the ability to register multiple restart handlers
> > > > > is not to have failure fallbacks, but to have fallbacks if preferred restart handlers are not available.
> > > > > If the PSCI restart handler is present, it is expected to work.
> > > > > That is why it has higher priority. The idea is to use the highest priority restart handler, not to try
> > > > > one after another.
> > > > >
> > > > > Sure, the failure fallback works as well, but that doesn't mean that fallbacks should be provided just to
> > > > > provide fallbacks.
> > > > >
> > > > > If you claim that the PSCI restart handler does not work on this system, provide evicence, and we can
> > > > > discuss this further.
> > > >
> > > > I agree normal cases there won't be any failures
> > > >
> > > > I never heard about PSCI restart fails. But have seen a case due to a bug in
> >
> > Oh yes, if PSCI is not really implemented ;-)
> >
> > > > serial driver that made the PSCI restart to fail as it hangs the systems.
> > > >
> > > > So, if this is the case, a memory corruption can cause the same.
> > > >
> > > > Long back I worked on femto based product(single core), where we have normal restart
> > > > and a fallback watchdog restart. But it does not have PSCI system handlers.
> > > > On some cases, we have seen automatic upgrade failed because of failure in
> > > > normal restart.
> > > >
> > > If you think the PSCI might fail and not restart the machine the other
> > > alternative would be to export functions to configure WDTDCR using
> > > readl/writel.
> >
> > Well, if PSCI resets fails: that's exactly why you have a watchdog
> > (or two) in the system, right?
> >
> Agreed, To avoid taking the syscon lock from restart context, how
> about exporting helpers from the CPG/MSSR driver to start/stop the
> WDTDCR count using plain readl/writel? Each WDTDCR register belongs to
> a single watchdog instance, so no locking is needed.
>
>   /* include/linux/soc/renesas/renesas.h */
>   int rzt2h_cpg_wdtdcr_count_start(unsigned int wdt_index);
>   int rzt2h_cpg_wdtdcr_count_stop(unsigned int wdt_index);
>
The DT binding patch that adds the renesas,sysc property has now been
merged [0].

While keeping syscon support, I propose adding
rzt2h_cpg_wdtdcr_count_start(), which can be used safely in atomic
contexts. For non-atomic cases, we can continue using syscon.

What do you think?

[0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/watchdog/renesas,r9a09g057-wdt.yaml?h=next-20260924#n51

Cheers,
Prabhakar

Re: [PATCH v6 6/8] watchdog: rzv2h: Add syscon support for WDTDCR

[Guenter Roeck]

On Fri, Sep 25, 2026 at 01:24:32PM +0100, Lad, Prabhakar wrote:
> The DT binding patch that adds the renesas,sysc property has now been
> merged [0].
> 
> While keeping syscon support, I propose adding
> rzt2h_cpg_wdtdcr_count_start(), which can be used safely in atomic
> contexts. For non-atomic cases, we can continue using syscon.
> 
> What do you think?

Whatever you folks think will work. Personally I think this is
over-engineering. If there is memory corruption or some serial
access failure (is that system really that fragile ?), you'll
have much more severe problems to deal with than reset not working.
I would not even try to "handle" that by mandating a second reset
handler, but it is your code, after all, so I'll accept it as long
as it works.

Guenter

[PATCH v6 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access

[Prabhakar]

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

The WDTDCR registers for wdt0-wdt5 reside in the second register region of
the CPG/MSSR block. This multi-function block is now exposed via a unified
syscon regmap interface.

Replace the direct mapping of the individual WDTDCR registers with the
new "renesas,sysc" phandle property pointing to the CPG/MSSR block syscon
node.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5->v6:
- No change

v4->v5:
- Added review-by tag.

v3->v4:
- No change

v2->v3:
- Renamed the "renesas,sys" property to "renesas,sysc"
- Updated commit message

v1->v2:
- No change.
---
 arch/arm64/boot/dts/renesas/r9a09g077.dtsi | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
index 674e35e4f242..18d748661a60 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
@@ -327,61 +327,61 @@ channel1 {
 
 		wdt0: watchdog@80082000 {
 			compatible = "renesas,r9a09g077-wdt";
-			reg = <0 0x80082000 0 0x400>,
-			      <0 0x81295100 0 0x04>;
+			reg = <0 0x80082000 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 0>;
 			status = "disabled";
 		};
 
 		wdt1: watchdog@80082400 {
 			compatible = "renesas,r9a09g077-wdt";
-			reg = <0 0x80082400 0 0x400>,
-			      <0 0x81295104 0 0x04>;
+			reg = <0 0x80082400 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 1>;
 			status = "disabled";
 		};
 
 		wdt2: watchdog@80082800 {
 			compatible = "renesas,r9a09g077-wdt";
-			reg = <0 0x80082800 0 0x400>,
-			      <0 0x81295108 0 0x04>;
+			reg = <0 0x80082800 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 2>;
 			status = "disabled";
 		};
 
 		wdt3: watchdog@80082c00 {
 			compatible = "renesas,r9a09g077-wdt";
-			reg = <0 0x80082c00 0 0x400>,
-			      <0 0x8129510c 0 0x04>;
+			reg = <0 0x80082c00 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 3>;
 			status = "disabled";
 		};
 
 		wdt4: watchdog@80083000 {
 			compatible = "renesas,r9a09g077-wdt";
-			reg = <0 0x80083000 0 0x400>,
-			      <0 0x81295110 0 0x04>;
+			reg = <0 0x80083000 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 4>;
 			status = "disabled";
 		};
 
 		wdt5: watchdog@80083400 {
 			compatible = "renesas,r9a09g077-wdt";
-			reg = <0 0x80083400 0 0x400>,
-			      <0 0x81295114 0 0x04>;
+			reg = <0 0x80083400 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 5>;
 			status = "disabled";
 		};
 
-- 
2.55.0

Re: [PATCH v6 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] probe: watchdog driver perpetually fails to probe due to invalid syscon target
--

commit 4e5d24eec602a0526d0eac3d1f59b54aa0dd4798
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access

The WDTDCR registers for wdt0-wdt5 reside in the second register region of
the CPG/MSSR block. This multi-function block is now exposed via a unified
syscon regmap interface.

> diff --git a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
> index 40494159831d8..ad5189d55cef2 100644
> --- a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
> @@ -327,61 +327,61 @@ channel1 {
>  
>  		wdt0: watchdog@80082000 {
>  			compatible = "renesas,r9a09g077-wdt";
> -			reg = <0 0x80082000 0 0x400>,
> -			      <0 0x81295100 0 0x04>;
> +			reg = <0 0x80082000 0 0x400>;
>  			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>;
>  			clock-names = "pclk";
>  			power-domains = <&cpg>;
> +			renesas,sysc = <&cpg 0>;

[Severity: High]
Does the cpg node have the "syscon" compatible string?

Looking at the cpg node definition in arch/arm64/boot/dts/renesas/r9a09g077.dtsi:

		cpg: clock-controller@80280000 {
			compatible = "renesas,r9a09g077-cpg-mssr";
			reg = <0 0x80280000 0 0x10000>,
			      <0 0x81280000 0 0x10000>;

Because it lacks the "syscon" compatible string, and its driver does not
manually register a syscon regmap, won't syscon_regmap_lookup_by_phandle_args()
perpetually fail and return -EPROBE_DEFER during the watchdog driver probe?

Furthermore, if it were treated as a generic syscon, the generic driver only
maps the first memory region (size 0x10000), meaning max_register is 0xFFFC.
Since the WDTDCR offset is calculated as 0x15100 + n*4, won't all register
accesses be rejected by the regmap subsystem as out-of-bounds?

>  			status = "disabled";
>  		};

[ ... ]

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

[PATCH v6 8/8] arm64: dts: renesas: r9a09g087: Use CPG/MSSR syscon for WDTDCR access

[Prabhakar]

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

The WDTDCR registers for wdt0-wdt5 reside in the second register region of
the CPG/MSSR block. This multi-function block is now exposed via a unified
syscon regmap interface.

Replace the direct mapping of the individual WDTDCR registers with the
new "renesas,sysc" phandle property pointing to the CPG/MSSR block syscon
node.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5->v6:
- No change

v4->v5:
- Added review-by tag.

v3->v4:
- No change

v2->v3:
- Renamed the "renesas,sys" property to "renesas,sysc"
- Updated commit message

v1->v2:
- No change.
---
 arch/arm64/boot/dts/renesas/r9a09g087.dtsi | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g087.dtsi b/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
index 68c8daed557b..abca7aa89458 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
@@ -327,61 +327,61 @@ channel1 {
 
 		wdt0: watchdog@80082000 {
 			compatible = "renesas,r9a09g087-wdt", "renesas,r9a09g077-wdt";
-			reg = <0 0x80082000 0 0x400>,
-			      <0 0x81295100 0 0x04>;
+			reg = <0 0x80082000 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 0>;
 			status = "disabled";
 		};
 
 		wdt1: watchdog@80082400 {
 			compatible = "renesas,r9a09g087-wdt", "renesas,r9a09g077-wdt";
-			reg = <0 0x80082400 0 0x400>,
-			      <0 0x81295104 0 0x04>;
+			reg = <0 0x80082400 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 1>;
 			status = "disabled";
 		};
 
 		wdt2: watchdog@80082800 {
 			compatible = "renesas,r9a09g087-wdt", "renesas,r9a09g077-wdt";
-			reg = <0 0x80082800 0 0x400>,
-			      <0 0x81295108 0 0x04>;
+			reg = <0 0x80082800 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 2>;
 			status = "disabled";
 		};
 
 		wdt3: watchdog@80082c00 {
 			compatible = "renesas,r9a09g087-wdt", "renesas,r9a09g077-wdt";
-			reg = <0 0x80082c00 0 0x400>,
-			      <0 0x8129510c 0 0x04>;
+			reg = <0 0x80082c00 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 3>;
 			status = "disabled";
 		};
 
 		wdt4: watchdog@80083000 {
 			compatible = "renesas,r9a09g087-wdt", "renesas,r9a09g077-wdt";
-			reg = <0 0x80083000 0 0x400>,
-			      <0 0x81295110 0 0x04>;
+			reg = <0 0x80083000 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 4>;
 			status = "disabled";
 		};
 
 		wdt5: watchdog@80083400 {
 			compatible = "renesas,r9a09g087-wdt", "renesas,r9a09g077-wdt";
-			reg = <0 0x80083400 0 0x400>,
-			      <0 0x81295114 0 0x04>;
+			reg = <0 0x80083400 0 0x400>;
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
 			clock-names = "pclk";
 			power-domains = <&cpg>;
+			renesas,sysc = <&cpg 5>;
 			status = "disabled";
 		};
 
-- 
2.55.0

Re: [PATCH v6 8/8] arm64: dts: renesas: r9a09g087: Use CPG/MSSR syscon for WDTDCR access

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: syscon regmap lookup fails for WDT due to non-syscon CPG node
--

commit 965ed970b62c83d1a303f6e2583dfa7e66ebde5d
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

arm64: dts: renesas: r9a09g087: Use CPG/MSSR syscon for WDTDCR access

This commit replaces direct register mappings for watchdog timers
wdt0-wdt5 with a syscon phandle pointing to the CPG/MSSR block. The WDTDCR
registers are noted to reside in the second register region of the CPG.

> diff --git a/arch/arm64/boot/dts/renesas/r9a09g087.dtsi b/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
> index e8d4f76949ccb..40ffe56a40353 100644
> --- a/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a09g087.dtsi

[ ... ]

> @@ -327,61 +327,61 @@ channel1 {
>  
>  		wdt0: watchdog@80082000 {
>  			compatible = "renesas,r9a09g087-wdt", "renesas,r9a09g077-wdt";
> -			reg = <0 0x80082000 0 0x400>,
> -			      <0 0x81295100 0 0x04>;
> +			reg = <0 0x80082000 0 0x400>;
>  			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
>  			clock-names = "pclk";
>  			power-domains = <&cpg>;
> +			renesas,sysc = <&cpg 0>;

[Severity: High]
Does this property correctly resolve the syscon regmap?

Looking at the driver implementation, it appears to use
syscon_regmap_lookup_by_phandle_args() on the cpg node. However, the cpg
node lacks the "syscon" compatible string, and the CPG driver does not
manually register a regmap via of_syscon_register_regmap(). Will this cause
the lookup to perpetually fail with -EPROBE_DEFER and leave the watchdog
unavailable?

Additionally, if the cpg node were defined as a standard syscon, it would
only map its first reg region (size 0x10000). The watchdog driver generates
an offset of 0x15100. Writing to this offset would exceed the regmap's size
limit and cause an out-of-bounds -EIO failure.

Applying the offset to the first region also entirely misses the physically
correct second region address of 0x81295100.

Are there additional patches that update the CPG node or driver to handle
these secondary regions and avoid the regmap lookup failures?

>  			status = "disabled";
>  		};

[ ... ]

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