[PATCH v2 0/3] watchdog: rzn1: Add support for direct hardware reset

[PATCH v2 0/3] watchdog: rzn1: Add support for direct hardware reset

[Herve Codina (Schneider Electric)]

Hi,

The current watchdog driver handling wachdogs of the RZ/N1 SoCs is based
on interrupt only to perform the reset. On the watchdog timeout, an
interrupt is triggered and the software initiates the reset.

The watchdogs available in the RZ/N1 SoCs can directly perform an
hardware reset using their dedicated reset line.

On timeout, the watchdog also asserts its dedicated reset line. This
reset line is connected to the reset controller (part of sysctrl) and,
if this line is enabled as a possible reset source at the reset
controller level, it initiates a system reset.

This series adds support for this feature allowing watchdogs to directly
reset the system with any software needs when a watchdog timeout occurs.

The first two patches are minor fixes and improvements without changing
the current functional behavior.

The third patch unconditionally enable watchdog reset sources at the
sysctrl level (sysctrl is handled by the RZ/N1 clock driver).

Compare to previous iteration, this v2 series reworked the support for
this feature moving from a DT property and a helper to enable the
watchdog reset sources to a simple unconditionally enable of watchdog
reset sources.

Best regards,
Hervé

Changes v1 -> v2:
  v1: https://lore.kernel.org/lkml/20260310173249.161354-1-herve.codina@bootlin.com/

  Patch 1 and 2:
    No changes

  Patch 3 (new in v2):
    Unconditionally enable watchdog reset sources

  Patch 3, 4 and 5 in v1:
    Removed

Herve Codina (Schneider Electric) (3):
  watchdog: rzn1: Fix reverse xmas tree declaration
  watchdog: rzn1: Use dev_err_probe()
  clk: renesas: r9a06g032: Enable watchdog reset sources

 drivers/clk/renesas/r9a06g032-clocks.c |  5 +++--
 drivers/watchdog/rzn1_wdt.c            | 22 ++++++++--------------
 2 files changed, 11 insertions(+), 16 deletions(-)

-- 
2.53.0

[PATCH v2 1/3] watchdog: rzn1: Fix reverse xmas tree declaration

[Herve Codina (Schneider Electric)]

Variables declared in probe() don't follow the reverse xmas
tree convention.

Fix the declaration in order to follow the convention.

Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
---
 drivers/watchdog/rzn1_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/rzn1_wdt.c b/drivers/watchdog/rzn1_wdt.c
index 96fd04fbc2a2..b7034eac91d0 100644
--- a/drivers/watchdog/rzn1_wdt.c
+++ b/drivers/watchdog/rzn1_wdt.c
@@ -101,10 +101,10 @@ static const struct watchdog_ops rzn1_wdt_ops = {
 static int rzn1_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct rzn1_watchdog *wdt;
 	struct device_node *np = dev->of_node;
-	struct clk *clk;
+	struct rzn1_watchdog *wdt;
 	unsigned long clk_rate;
+	struct clk *clk;
 	int ret;
 	int irq;
 
-- 
2.53.0

Re: [PATCH v2 1/3] watchdog: rzn1: Fix reverse xmas tree declaration

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On Fri, Mar 13, 2026 at 10:24:14AM +0100, Herve Codina (Schneider Electric) wrote:
> Variables declared in probe() don't follow the reverse xmas
> tree convention.
> 
> Fix the declaration in order to follow the convention.
> 
> Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>

Not a fan of such changes. But it works and I am not opposing it:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 2/3] watchdog: rzn1: Use dev_err_probe()

[Herve Codina (Schneider Electric)]

In the probe() function the following pattern is present several times:
	if (err) {
		dev_err(dev, ...);
		return err;
	}

Replace them by dev_err_probe() calls.

Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
---
 drivers/watchdog/rzn1_wdt.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/watchdog/rzn1_wdt.c b/drivers/watchdog/rzn1_wdt.c
index b7034eac91d0..98978b5cc5b4 100644
--- a/drivers/watchdog/rzn1_wdt.c
+++ b/drivers/watchdog/rzn1_wdt.c
@@ -122,22 +122,16 @@ static int rzn1_wdt_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(dev, irq, rzn1_wdt_irq, 0,
 			       np->name, wdt);
-	if (ret) {
-		dev_err(dev, "failed to request irq %d\n", irq);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to request irq %d\n", irq);
 
 	clk = devm_clk_get_enabled(dev, NULL);
-	if (IS_ERR(clk)) {
-		dev_err(dev, "failed to get the clock\n");
-		return PTR_ERR(clk);
-	}
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk), "failed to get the clock\n");
 
 	clk_rate = clk_get_rate(clk);
-	if (!clk_rate) {
-		dev_err(dev, "failed to get the clock rate\n");
-		return -EINVAL;
-	}
+	if (!clk_rate)
+		return dev_err_probe(dev, -EINVAL, "failed to get the clock\n");
 
 	wdt->clk_rate_khz = clk_rate / 1000;
 	wdt->wdtdev.info = &rzn1_wdt_info;
-- 
2.53.0

Re: [PATCH v2 2/3] watchdog: rzn1: Use dev_err_probe()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

On Fri, Mar 13, 2026 at 10:24:15AM +0100, Herve Codina (Schneider Electric) wrote:
> In the probe() function the following pattern is present several times:
> 	if (err) {
> 		dev_err(dev, ...);
> 		return err;
> 	}
> 
> Replace them by dev_err_probe() calls.
> 
> Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>

I am a fan of such changes, though :)

>  	clk_rate = clk_get_rate(clk);
> -	if (!clk_rate) {
> -		dev_err(dev, "failed to get the clock rate\n");
> -		return -EINVAL;
> -	}
> +	if (!clk_rate)
> +		return dev_err_probe(dev, -EINVAL, "failed to get the clock\n");

Wrong error string, we handle "clock rate" here.

With that fixed:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 3/3] clk: renesas: r9a06g032: Enable watchdog reset sources

[Herve Codina (Schneider Electric)]

The watchdog timeout is signaled using an interrupt and, on this
interrupt, a software initiated reset is performed.

This software initiated reset performs, in the end, a hardware system
reset using SWRST_REQ of RSTCTRL register.

The watchdog itself is able to control directly the hardware system
reset without any operation done by the interrupt handler. This feature
allows the watchdog to not depend on the software to reset the system
when a watchdog timeout occurs.

Indeed, when the watchdog timeout occurs, the watchdog requests a system
reset using its own hardware dedicated line but this reset source is
disabled at the reset controller level.

To benefit of this feature and be robust against software issues, enable
watchdogs reset sources.

Suggested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>
---
 drivers/clk/renesas/r9a06g032-clocks.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
index 7407a4183a6c..a0734182d112 100644
--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c
@@ -1342,8 +1342,9 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
 	/* Clear potentially pending resets */
 	writel(R9A06G032_SYSCTRL_WDA7RST_0 | R9A06G032_SYSCTRL_WDA7RST_1,
 	       clocks->reg + R9A06G032_SYSCTRL_RSTCTRL);
-	/* Allow software reset */
-	writel(R9A06G032_SYSCTRL_SWRST | R9A06G032_SYSCTRL_RSTEN_MRESET_EN,
+	/* Allow software reset and watchdog resets */
+	writel(R9A06G032_SYSCTRL_SWRST | R9A06G032_SYSCTRL_RSTEN_MRESET_EN |
+	       R9A06G032_SYSCTRL_WDA7RST_0 | R9A06G032_SYSCTRL_WDA7RST_1,
 	       clocks->reg + R9A06G032_SYSCTRL_RSTEN);
 
 	error = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART, SYS_OFF_PRIO_HIGH,
-- 
2.53.0

Re: [PATCH v2 3/3] clk: renesas: r9a06g032: Enable watchdog reset sources

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]

Hi Herve,

On Fri, Mar 13, 2026 at 10:24:16AM +0100, Herve Codina (Schneider Electric) wrote:
> The watchdog timeout is signaled using an interrupt and, on this
> interrupt, a software initiated reset is performed.
> 
> This software initiated reset performs, in the end, a hardware system
> reset using SWRST_REQ of RSTCTRL register.
> 
> The watchdog itself is able to control directly the hardware system
> reset without any operation done by the interrupt handler. This feature
> allows the watchdog to not depend on the software to reset the system
> when a watchdog timeout occurs.
> 
> Indeed, when the watchdog timeout occurs, the watchdog requests a system
> reset using its own hardware dedicated line but this reset source is
> disabled at the reset controller level.
> 
> To benefit of this feature and be robust against software issues, enable
> watchdogs reset sources.
> 
> Suggested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>

Yes, much more elegant than v1, I think:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

> +	/* Allow software reset and watchdog resets */
> +	writel(R9A06G032_SYSCTRL_SWRST | R9A06G032_SYSCTRL_RSTEN_MRESET_EN |

Super minor nit: I would swap this line...

> +	       R9A06G032_SYSCTRL_WDA7RST_0 | R9A06G032_SYSCTRL_WDA7RST_1,

... with this one. Feels more ordered if MRESET_EN is last. But I don't
insist.

This patch should have been sent seperately, though, IMHO. Mixing
watchdog and clock patches without a dependency only calls for unneeded
negotiations of involved subsystem maintainers.

Thanks for the series,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]