devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/8] Add syscon support for Renesas WDT driver
@ 2026-09-11 11:17 Prabhakar
  2026-09-11 11:17 ` [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler Prabhakar
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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 startegy, as the WDT driver and DTSI changes are interdependent
maybe the driver patches can go via renesas-soc.

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: Drop enabling clocks in the restart handler
  watchdog: rzv2h: Drop WDTRCR_RSTIRQS define
  watchdog: rzv2h: Propagate WDTDCR access errors
  watchdog: rzv2h: Use pm_runtime_put_sync()
  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               | 134 +++++++++++++++------
 4 files changed, 122 insertions(+), 61 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler
  2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
@ 2026-09-11 11:17 ` Prabhakar
  2026-09-11 11:32   ` sashiko-bot
  2026-09-11 11:17 ` [PATCH v5 2/8] watchdog: rzv2h: Drop WDTRCR_RSTIRQS define Prabhakar
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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>
---
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 3b6abb66a1da..bd86da71b0eb 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
@@ -290,11 +277,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 2/8] watchdog: rzv2h: Drop WDTRCR_RSTIRQS define
  2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
  2026-09-11 11:17 ` [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler Prabhakar
@ 2026-09-11 11:17 ` Prabhakar
  2026-09-11 11:17 ` [PATCH v5 3/8] watchdog: rzv2h: Propagate WDTDCR access errors Prabhakar
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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>
---
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 bd86da71b0eb..7c6a4855bd8a 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 3/8] watchdog: rzv2h: Propagate WDTDCR access errors
  2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
  2026-09-11 11:17 ` [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler Prabhakar
  2026-09-11 11:17 ` [PATCH v5 2/8] watchdog: rzv2h: Drop WDTRCR_RSTIRQS define Prabhakar
@ 2026-09-11 11:17 ` Prabhakar
  2026-09-11 11:26   ` sashiko-bot
  2026-09-11 11:17 ` [PATCH v5 4/8] watchdog: rzv2h: Use pm_runtime_put_sync() Prabhakar
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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>
---
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 7c6a4855bd8a..cbf414e0c9a3 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(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(&pdev->dev);
 
-	return 0;
+	return ret;
 }
 
 static int rzv2h_wdt_probe(struct platform_device *pdev)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 4/8] watchdog: rzv2h: Use pm_runtime_put_sync()
  2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
                   ` (2 preceding siblings ...)
  2026-09-11 11:17 ` [PATCH v5 3/8] watchdog: rzv2h: Propagate WDTDCR access errors Prabhakar
@ 2026-09-11 11:17 ` Prabhakar
  2026-09-11 11:27   ` sashiko-bot
  2026-09-11 11:17 ` [PATCH v5 5/8] watchdog: rzv2h: Convert WDTDCR handling to regmap Prabhakar
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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>
---
v4->v5:
- New patch
---
 drivers/watchdog/rzv2h_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index cbf414e0c9a3..4a5a856060e9 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -270,7 +270,7 @@ static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
 
 	ret = rzt2h_wdt_wdtdcr_count_stop(priv);
 
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 
 	return ret;
 }
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 5/8] watchdog: rzv2h: Convert WDTDCR handling to regmap
  2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
                   ` (3 preceding siblings ...)
  2026-09-11 11:17 ` [PATCH v5 4/8] watchdog: rzv2h: Use pm_runtime_put_sync() Prabhakar
@ 2026-09-11 11:17 ` Prabhakar
  2026-09-11 11:17 ` [PATCH v5 6/8] watchdog: rzv2h: Add syscon support for WDTDCR Prabhakar
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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>
---
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 4a5a856060e9..550112446db4 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);
+	guard(spinlock_irqsave)(&priv->regmap_lock);
+	return regmap_set_bits(priv->wdtdcr_regmap, WDTDCR, WDTDCR_WDTSTOPCTRL);
+}
 
-	writel(reg | WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
-
-	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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 6/8] watchdog: rzv2h: Add syscon support for WDTDCR
  2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
                   ` (4 preceding siblings ...)
  2026-09-11 11:17 ` [PATCH v5 5/8] watchdog: rzv2h: Convert WDTDCR handling to regmap Prabhakar
@ 2026-09-11 11:17 ` Prabhakar
  2026-09-11 11:28   ` sashiko-bot
  2026-09-11 11:17 ` [PATCH v5 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access Prabhakar
  2026-09-11 11:17 ` [PATCH v5 8/8] arm64: dts: renesas: r9a09g087: " Prabhakar
  7 siblings, 1 reply; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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>
---
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 550112446db4..fcd1ff99c0ed 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access
  2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
                   ` (5 preceding siblings ...)
  2026-09-11 11:17 ` [PATCH v5 6/8] watchdog: rzv2h: Add syscon support for WDTDCR Prabhakar
@ 2026-09-11 11:17 ` Prabhakar
  2026-09-11 11:32   ` sashiko-bot
  2026-09-11 11:17 ` [PATCH v5 8/8] arm64: dts: renesas: r9a09g087: " Prabhakar
  7 siblings, 1 reply; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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>
---
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 de20bed2797b..80910e0bd977 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v5 8/8] arm64: dts: renesas: r9a09g087: Use CPG/MSSR syscon for WDTDCR access
  2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
                   ` (6 preceding siblings ...)
  2026-09-11 11:17 ` [PATCH v5 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access Prabhakar
@ 2026-09-11 11:17 ` Prabhakar
  2026-09-11 11:33   ` sashiko-bot
  7 siblings, 1 reply; 21+ messages in thread
From: Prabhakar @ 2026-09-11 11:17 UTC (permalink / raw)
  To: Geert Uytterhoeven, Guenter Roeck, Wim Van Sebroeck, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
  Cc: linux-renesas-soc, devicetree, linux-kernel, linux-watchdog,
	Prabhakar, Biju Das, Fabrizio Castro, Lad 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>
---
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 813bb0cb021c..3ede2e89cf67 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 3/8] watchdog: rzv2h: Propagate WDTDCR access errors
  2026-09-11 11:17 ` [PATCH v5 3/8] watchdog: rzv2h: Propagate WDTDCR access errors Prabhakar
@ 2026-09-11 11:26   ` sashiko-bot
  2026-09-11 11:30     ` Lad, Prabhakar
  0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 11:26 UTC (permalink / raw)
  To: Prabhakar; +Cc: conor+dt, robh, linux-watchdog, devicetree, wsa+renesas

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The error path in rzv2h_wdt_stop() leaves the watchdog unconfigured and missing a required delay after reset deassertion.
--

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

watchdog: rzv2h: Propagate WDTDCR access errors

The WDTDCR helpers currently access the register directly, but this will
change to use regmap, which can fail. This commit propagates errors from
rzt2h_wdt_wdtdcr_count_start() and rzt2h_wdt_wdtdcr_count_stop() and unwinds
the resources if an error occurs.

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index 7c6a4855bd8ad..cbf414e0c9a3c 100644
> --- 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;
> +		}
> +	}
>  
>  	pm_runtime_put(wdev->parent);

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

When rzt2h_wdt_wdtdcr_count_stop() fails in rzv2h_wdt_stop(), the code calls
reset_control_deassert() to bring the hardware out of reset. However, it
does not include the mandatory udelay(3) wait for clock stabilization or
restore the watchdog register configuration via rzv2h_wdt_setup().

This leaves the hardware out of reset but unconfigured, which might trigger an
immediate spurious reboot if default timeouts are active, or fail to reboot
the system during a hang if the watchdog remains dormant.

Could this error handling be updated to fully restore the previous watchdog
state?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 4/8] watchdog: rzv2h: Use pm_runtime_put_sync()
  2026-09-11 11:17 ` [PATCH v5 4/8] watchdog: rzv2h: Use pm_runtime_put_sync() Prabhakar
@ 2026-09-11 11:27   ` sashiko-bot
  2026-09-11 12:11     ` Lad, Prabhakar
  0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 11:27 UTC (permalink / raw)
  To: Prabhakar; +Cc: conor+dt, linux-watchdog, wsa+renesas, robh, devicetree

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

Pre-existing issues:
- [High] Incomplete fix: rzv2h_wdt_stop() still uses asynchronous pm_runtime_put() during driver unbind, risking a race with pm_runtime_disable().
- [High] Kernel panic from division by zero when the optional 'oscclk' is missing from the device tree.
- [High] Missing NULL check for of_device_get_match_data() return value leads to a NULL pointer dereference.
--

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

watchdog: rzv2h: Use pm_runtime_put_sync()

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.

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index cbf414e0c9a3c..4a5a856060e91 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c
> @@ -270,7 +270,7 @@ static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
>  
>  	ret = rzt2h_wdt_wdtdcr_count_stop(priv);
>  
> -	pm_runtime_put(&pdev->dev);
> +	pm_runtime_put_sync(&pdev->dev);
>  
>  	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but it seems there is another
instance of this same issue in rzv2h_wdt_stop(). During module unbind, the
devres cleanup triggers watchdog_unregister_device(), which calls
rzv2h_wdt_stop(). This function issues an asynchronous pm_runtime_put():

drivers/watchdog/rzv2h_wdt.c:rzv2h_wdt_stop() {
	...
	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);
	...
}

Should this also use pm_runtime_put_sync() to ensure the idle check completes
before the devres cleanup calls pm_runtime_disable()?

[Severity: High]
This is a pre-existing issue, but I noticed a potential NULL pointer
dereference in rzv2h_wdt_probe(). The return value of
of_device_get_match_data() is not checked:

drivers/watchdog/rzv2h_wdt.c:rzv2h_wdt_probe() {
	...
	priv->of_data = of_device_get_match_data(dev);
	...
	switch (priv->of_data->count_source) {
	...
}

If the driver is bound via sysfs without device tree match data, could
priv->of_data be NULL here and cause a crash?

[Severity: High]
This is a pre-existing issue, but could the clock rate calculation result in a
division by zero?

drivers/watchdog/rzv2h_wdt.c:rzv2h_wdt_probe() {
	...
	priv->oscclk = devm_clk_get_optional_enabled(dev, "oscclk");
	...
	switch (priv->of_data->count_source) {
	case COUNT_SOURCE_LOCO:
		count_clk = priv->oscclk;
		break;
	...
	priv->wdev.max_hw_heartbeat_ms = (MILLI * priv->of_data->timeout_cycles *
					  priv->of_data->cks_div) / clk_get_rate(count_clk);
	...
}

If the optional 'oscclk' is omitted in the device tree, priv->oscclk will be
NULL. When count_clk is assigned this NULL clock, clk_get_rate(count_clk)
safely returns 0. Will this cause a kernel panic due to division by zero?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 6/8] watchdog: rzv2h: Add syscon support for WDTDCR
  2026-09-11 11:17 ` [PATCH v5 6/8] watchdog: rzv2h: Add syscon support for WDTDCR Prabhakar
@ 2026-09-11 11:28   ` sashiko-bot
  2026-09-11 11:39     ` Lad, Prabhakar
  0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 11:28 UTC (permalink / raw)
  To: Prabhakar; +Cc: conor+dt, devicetree, wsa+renesas, linux-watchdog, robh

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Watchdog restart handler may deadlock during system panic because it executes a locked regmap operation on a syscon regmap.
--

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

watchdog: rzv2h: Add syscon support for WDTDCR

This commit introduces syscon regmap support for the WDTDCR (WDT Debug Control
Register) on RZ/T2H and RZ/N2H SoCs. It retrieves the SYSC regmap using the
"renesas,sysc" property and derives the WDTDCR offset from the watchdog
instance index, while preserving MMIO compatibility when the property is absent.

> diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> index 550112446db49..9dee410ab7f31 100644
> --- a/drivers/watchdog/rzv2h_wdt.c
> +++ b/drivers/watchdog/rzv2h_wdt.c

[ ... ]

>  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]
Does this introduce a deadlock risk during a system panic?

The watchdog restart handler, rzv2h_wdt_restart(), expects an unlocked
execution path to avoid deadlocks and specifically calls this nolock variant:

	if (priv->of_data->wdtdcr) {
		/*
		 * Best effort, ignore return value and use the unlocked
		 * variant of the function to avoid potential deadlocks.
		 */
		rzt2h_wdt_wdtdcr_count_start_nolock(priv);
	}

Previously, the driver used a dedicated MMIO regmap with
.disable_locking = true. Now that sysc->regmap can be a syscon regmap,
which is shared and uses internal spinlocks, regmap_clear_bits() will attempt
to acquire the syscon lock.

If the system panics while another CPU holds this shared syscon spinlock,
will attempting to acquire it here cause the panic handler to hang instead of
resetting the system?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 3/8] watchdog: rzv2h: Propagate WDTDCR access errors
  2026-09-11 11:26   ` sashiko-bot
@ 2026-09-11 11:30     ` Lad, Prabhakar
  0 siblings, 0 replies; 21+ messages in thread
From: Lad, Prabhakar @ 2026-09-11 11:30 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: conor+dt, robh, linux-watchdog, devicetree, wsa+renesas

Hi Sashiko,

On Fri, Sep 11, 2026 at 12:26 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The error path in rzv2h_wdt_stop() leaves the watchdog unconfigured and missing a required delay after reset deassertion.
> --
>
> commit 3f7bad2568920b818b4bdb62349364febc443827
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> watchdog: rzv2h: Propagate WDTDCR access errors
>
> The WDTDCR helpers currently access the register directly, but this will
> change to use regmap, which can fail. This commit propagates errors from
> rzt2h_wdt_wdtdcr_count_start() and rzt2h_wdt_wdtdcr_count_stop() and unwinds
> the resources if an error occurs.
>
> > diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> > index 7c6a4855bd8ad..cbf414e0c9a3c 100644
> > --- 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;
> > +             }
> > +     }
> >
> >       pm_runtime_put(wdev->parent);
>
> [Severity: High]
> Does this error path leave the hardware in an uninitialized state?
>
> When rzt2h_wdt_wdtdcr_count_stop() fails in rzv2h_wdt_stop(), the code calls
> reset_control_deassert() to bring the hardware out of reset. However, it
> does not include the mandatory udelay(3) wait for clock stabilization or
> restore the watchdog register configuration via rzv2h_wdt_setup().
>
The udelay(3)is not required as we are not reprogramming the WDT in
the rzv2h_wdt_stop().

> This leaves the hardware out of reset but unconfigured, which might trigger an
> immediate spurious reboot if default timeouts are active, or fail to reboot
> the system during a hang if the watchdog remains dormant.
>
> Could this error handling be updated to fully restore the previous watchdog
> state?
>
Yes it does, if the rzt2h_wdt_wdtdcr_count_stop() fails in
rzv2h_wdt_stop() we revert back things so that further pings from the
WDT core continue without any issues.

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access
  2026-09-11 11:17 ` [PATCH v5 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access Prabhakar
@ 2026-09-11 11:32   ` sashiko-bot
  2026-09-11 12:08     ` Lad, Prabhakar
  0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 11:32 UTC (permalink / raw)
  To: Prabhakar; +Cc: linux-watchdog, robh, conor+dt, devicetree, wsa+renesas

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The transition to using a syscon regmap for WDTDCR access is broken, preventing the watchdog from probing and containing an out-of-bounds regmap access flaw.
--

commit 0e87a2089f722e3ac7bcab46cff76d445536d9fd
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access

This patch replaces the direct mapping of individual WDTDCR registers for
watchdogs wdt0-wdt5 with a new "renesas,sysc" phandle property pointing
to the CPG/MSSR block. The watchdog driver is expected to use a unified
syscon regmap interface for these accesses.

> 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 this node successfully acquire a syscon regmap from the cpg node?

The cpg node lacks the "syscon" compatible string in its definition:

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

Without the "syscon" compatible string, syscon_node_to_regmap() will return
-EPROBE_DEFER indefinitely, preventing the watchdog driver from probing.

Furthermore, will the generic syscon regmap allow access to the WDTDCR
registers in the second memory region?

The generic syscon framework only maps the first reg resource region, which
the cpg node defines as 0x10000 bytes. The watchdog driver calculates a
WDTDCR offset of 0x15100 via:

drivers/watchdog/rzv2h_wdt.c:RZT2H_SYS_BLOCK1()
#define RZT2H_SYS_BLOCK1(n)	(BIT(16) | (0x5100 + (n) * 4))

This offset exceeds the max_register limit (0xFFFC) of the generic syscon
regmap. When rzt2h_wdt_wdtdcr_count_stop() passes this offset to
regmap_set_bits(), the regmap core will reject the write with -EINVAL.

>  			status = "disabled";
>  		};

[ ... ]

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler
  2026-09-11 11:17 ` [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler Prabhakar
@ 2026-09-11 11:32   ` sashiko-bot
  2026-09-11 11:36     ` Lad, Prabhakar
  0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 11:32 UTC (permalink / raw)
  To: Prabhakar; +Cc: linux-watchdog, devicetree, wsa+renesas, robh, conor+dt

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

Pre-existing issues:
- [High] Unchecked division by zero during probe if the optional 'oscclk' is missing from the device tree.
--

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

watchdog: rzv2h: Drop enabling clocks in the restart handler

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, which can cause a spin if another CPU holds it.
This commit keeps pclk and oscclk enabled for the lifetime of the watchdog
device by using devm_clk_get_enabled() and devm_clk_get_optional_enabled().

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

[ ... ]

> @@ -290,11 +277,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");

[Severity: High]
This is a pre-existing issue, but can this optional clock fetch result in a
divide by zero oops?

If the optional "oscclk" is legitimately omitted from the device tree (or the
device tree is malformed), priv->oscclk will be successfully returned as NULL.

Later in rzv2h_wdt_probe(), if the chip variant uses COUNT_SOURCE_LOCO, this
NULL pointer is assigned to count_clk and evaluated:

	switch (priv->of_data->count_source) {
	case COUNT_SOURCE_LOCO:
		count_clk = priv->oscclk;
		break;
	...

	priv->wdev.max_hw_heartbeat_ms = (MILLI * priv->of_data->timeout_cycles *
					  priv->of_data->cks_div) / clk_get_rate(count_clk);

Because clk_get_rate(NULL) safely returns 0, the unchecked division will
cause a divide error exception during device probe. Should this verify
count_clk or its rate before the division?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 8/8] arm64: dts: renesas: r9a09g087: Use CPG/MSSR syscon for WDTDCR access
  2026-09-11 11:17 ` [PATCH v5 8/8] arm64: dts: renesas: r9a09g087: " Prabhakar
@ 2026-09-11 11:33   ` sashiko-bot
  2026-09-11 12:09     ` Lad, Prabhakar
  0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-09-11 11:33 UTC (permalink / raw)
  To: Prabhakar; +Cc: robh, devicetree, linux-watchdog, conor+dt, wsa+renesas

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Watchdog probe failure due to missing syscon compatible string in CPG/MSSR node and invalid physical memory bounds for the disjoint second register region.
--

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

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

This commit replaces the direct mapping of WDTDCR registers for watchdogs with
a renesas,sysc phandle to the CPG/MSSR node. It updates wdt0-wdt5 to remove
the second register region and use the syscon interface instead.

> 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>;

[Severity: High]
Does this change cause a silent mismatch with the actual hardware topology?

Generic syscons map only the first contiguous reg block. The driver computes
the WDTDCR offset as 0x15100. Since the first CPG block mapped is typically
0x80280000 with size 0x10000, accessing 0x15100 either fails the regmap bounds
check or mistakenly accesses 0x80295100 instead of the previously configured
0x81295100 located in the second CPG block.

>  			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
>  			clock-names = "pclk";
>  			power-domains = <&cpg>;
> +			renesas,sysc = <&cpg 0>;

[Severity: High]
Will the watchdog driver probe fail indefinitely here?

When the rzv2h_wdt driver uses syscon_regmap_lookup_by_phandle_args() to parse
this renesas,sysc phandle, it evaluates syscon_node_to_regmap() on the cpg
node.

Because the cpg node in r9a09g087.dtsi lacks the syscon compatible string,
and the renesas-cpg-mssr driver doesn't register a custom regmap (e.g., via
of_syscon_register_regmap()), the lookup fails and returns
ERR_PTR(-EPROBE_DEFER). This prevents the watchdog hardware from functioning.

>  			status = "disabled";
>  		};

[ ... ]

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler
  2026-09-11 11:32   ` sashiko-bot
@ 2026-09-11 11:36     ` Lad, Prabhakar
  0 siblings, 0 replies; 21+ messages in thread
From: Lad, Prabhakar @ 2026-09-11 11:36 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-watchdog, devicetree, wsa+renesas, robh, conor+dt

Hi sashiko,

On Fri, Sep 11, 2026 at 12:32 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Unchecked division by zero during probe if the optional 'oscclk' is missing from the device tree.
> --
>
> commit e61522c3f4f781e0b8cf17bd6e22135a7d714358
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> watchdog: rzv2h: Drop enabling clocks in the restart handler
>
> 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, which can cause a spin if another CPU holds it.
> This commit keeps pclk and oscclk enabled for the lifetime of the watchdog
> device by using devm_clk_get_enabled() and devm_clk_get_optional_enabled().
>
> > diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> > index 3b6abb66a1da0..bd86da71b0eb8 100644
> > --- a/drivers/watchdog/rzv2h_wdt.c
> > +++ b/drivers/watchdog/rzv2h_wdt.c
>
> [ ... ]
>
> > @@ -290,11 +277,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");
>
> [Severity: High]
> This is a pre-existing issue, but can this optional clock fetch result in a
> divide by zero oops?
>
> If the optional "oscclk" is legitimately omitted from the device tree (or the
> device tree is malformed), priv->oscclk will be successfully returned as NULL.
>
> Later in rzv2h_wdt_probe(), if the chip variant uses COUNT_SOURCE_LOCO, this
> NULL pointer is assigned to count_clk and evaluated:
>
>         switch (priv->of_data->count_source) {
>         case COUNT_SOURCE_LOCO:
>                 count_clk = priv->oscclk;
>                 break;
>         ...
>
>         priv->wdev.max_hw_heartbeat_ms = (MILLI * priv->of_data->timeout_cycles *
>                                           priv->of_data->cks_div) / clk_get_rate(count_clk);
>
> Because clk_get_rate(NULL) safely returns 0, the unchecked division will
> cause a divide error exception during device probe. Should this verify
> count_clk or its rate before the division?
>
oscclk is optional for T2H SoCs and is handled in the DT binding, to
be fully proof, I'll add a check for this case.

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 6/8] watchdog: rzv2h: Add syscon support for WDTDCR
  2026-09-11 11:28   ` sashiko-bot
@ 2026-09-11 11:39     ` Lad, Prabhakar
  0 siblings, 0 replies; 21+ messages in thread
From: Lad, Prabhakar @ 2026-09-11 11:39 UTC (permalink / raw)
  To: sashiko-reviews, Geert Uytterhoeven
  Cc: conor+dt, devicetree, wsa+renesas, linux-watchdog, robh

Hi sashiko,

On Fri, Sep 11, 2026 at 12:28 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Watchdog restart handler may deadlock during system panic because it executes a locked regmap operation on a syscon regmap.
> --
>
> commit a3a7e98fdd4da0cb2994accaf697498b31aba4d4
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> watchdog: rzv2h: Add syscon support for WDTDCR
>
> This commit introduces syscon regmap support for the WDTDCR (WDT Debug Control
> Register) on RZ/T2H and RZ/N2H SoCs. It retrieves the SYSC regmap using the
> "renesas,sysc" property and derives the WDTDCR offset from the watchdog
> instance index, while preserving MMIO compatibility when the property is absent.
>
> > diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> > index 550112446db49..9dee410ab7f31 100644
> > --- a/drivers/watchdog/rzv2h_wdt.c
> > +++ b/drivers/watchdog/rzv2h_wdt.c
>
> [ ... ]
>
> >  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]
> Does this introduce a deadlock risk during a system panic?
>
> The watchdog restart handler, rzv2h_wdt_restart(), expects an unlocked
> execution path to avoid deadlocks and specifically calls this nolock variant:
>
>         if (priv->of_data->wdtdcr) {
>                 /*
>                  * Best effort, ignore return value and use the unlocked
>                  * variant of the function to avoid potential deadlocks.
>                  */
>                 rzt2h_wdt_wdtdcr_count_start_nolock(priv);
>         }
>
> Previously, the driver used a dedicated MMIO regmap with
> .disable_locking = true. Now that sysc->regmap can be a syscon regmap,
> which is shared and uses internal spinlocks, regmap_clear_bits() will attempt
> to acquire the syscon lock.
>
Agreed, I will update the syscon regmap in the CPG to drop the locking
and let consumers handle locking.

@Geert, would this approach of consumers handling the locking be OK?

Cheers,
Prabhakar

> If the system panics while another CPU holds this shared syscon spinlock,
> will attempting to acquire it here cause the panic handler to hang instead of
> resetting the system?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260911111731.3404308-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=6

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access
  2026-09-11 11:32   ` sashiko-bot
@ 2026-09-11 12:08     ` Lad, Prabhakar
  0 siblings, 0 replies; 21+ messages in thread
From: Lad, Prabhakar @ 2026-09-11 12:08 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-watchdog, robh, conor+dt, devicetree, wsa+renesas

Hi sashiko,

On Fri, Sep 11, 2026 at 12:32 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The transition to using a syscon regmap for WDTDCR access is broken, preventing the watchdog from probing and containing an out-of-bounds regmap access flaw.
> --
>
> commit 0e87a2089f722e3ac7bcab46cff76d445536d9fd
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access
>
> This patch replaces the direct mapping of individual WDTDCR registers for
> watchdogs wdt0-wdt5 with a new "renesas,sysc" phandle property pointing
> to the CPG/MSSR block. The watchdog driver is expected to use a unified
> syscon regmap interface for these accesses.
>
> > 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 this node successfully acquire a syscon regmap from the cpg node?
>
> The cpg node lacks the "syscon" compatible string in its definition:
>
>                 cpg: clock-controller@80280000 {
>                         compatible = "renesas,r9a09g077-cpg-mssr";
>                         reg = <0 0x80280000 0 0x10000>,
>                               <0 0x81280000 0 0x10000>;
>                         ...
>                 };
>
> Without the "syscon" compatible string, syscon_node_to_regmap() will return
> -EPROBE_DEFER indefinitely, preventing the watchdog driver from probing.
>
> Furthermore, will the generic syscon regmap allow access to the WDTDCR
> registers in the second memory region?
>
> The generic syscon framework only maps the first reg resource region, which
> the cpg node defines as 0x10000 bytes. The watchdog driver calculates a
> WDTDCR offset of 0x15100 via:
>
> drivers/watchdog/rzv2h_wdt.c:RZT2H_SYS_BLOCK1()
> #define RZT2H_SYS_BLOCK1(n)     (BIT(16) | (0x5100 + (n) * 4))
>
> This offset exceeds the max_register limit (0xFFFC) of the generic syscon
> regmap. When rzt2h_wdt_wdtdcr_count_stop() passes this offset to
> regmap_set_bits(), the regmap core will reject the write with -EINVAL.
>
The syscon is registered in the CPG driver and the issue reported is a
false positive.

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 8/8] arm64: dts: renesas: r9a09g087: Use CPG/MSSR syscon for WDTDCR access
  2026-09-11 11:33   ` sashiko-bot
@ 2026-09-11 12:09     ` Lad, Prabhakar
  0 siblings, 0 replies; 21+ messages in thread
From: Lad, Prabhakar @ 2026-09-11 12:09 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: robh, devicetree, linux-watchdog, conor+dt, wsa+renesas

Hi Sashiko,

On Fri, Sep 11, 2026 at 12:33 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Watchdog probe failure due to missing syscon compatible string in CPG/MSSR node and invalid physical memory bounds for the disjoint second register region.
> --
>
> commit 026369134b14659a11e1791a537a1142aec5c877
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> arm64: dts: renesas: r9a09g087: Use CPG/MSSR syscon for WDTDCR access
>
> This commit replaces the direct mapping of WDTDCR registers for watchdogs with
> a renesas,sysc phandle to the CPG/MSSR node. It updates wdt0-wdt5 to remove
> the second register region and use the syscon interface instead.
>
> > 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>;
>
> [Severity: High]
> Does this change cause a silent mismatch with the actual hardware topology?
>
> Generic syscons map only the first contiguous reg block. The driver computes
> the WDTDCR offset as 0x15100. Since the first CPG block mapped is typically
> 0x80280000 with size 0x10000, accessing 0x15100 either fails the regmap bounds
> check or mistakenly accesses 0x80295100 instead of the previously configured
> 0x81295100 located in the second CPG block.
>
The syscon is registered in the CPG driver and the reported issue is a
false positive.

> >                       clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>;
> >                       clock-names = "pclk";
> >                       power-domains = <&cpg>;
> > +                     renesas,sysc = <&cpg 0>;
>
> [Severity: High]
> Will the watchdog driver probe fail indefinitely here?
>
> When the rzv2h_wdt driver uses syscon_regmap_lookup_by_phandle_args() to parse
> this renesas,sysc phandle, it evaluates syscon_node_to_regmap() on the cpg
> node.
>
> Because the cpg node in r9a09g087.dtsi lacks the syscon compatible string,
> and the renesas-cpg-mssr driver doesn't register a custom regmap (e.g., via
> of_syscon_register_regmap()), the lookup fails and returns
> ERR_PTR(-EPROBE_DEFER). This prevents the watchdog hardware from functioning.
>
The syscon is registered in the CPG driver and the issue reported is a
false positive.

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 4/8] watchdog: rzv2h: Use pm_runtime_put_sync()
  2026-09-11 11:27   ` sashiko-bot
@ 2026-09-11 12:11     ` Lad, Prabhakar
  0 siblings, 0 replies; 21+ messages in thread
From: Lad, Prabhakar @ 2026-09-11 12:11 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: conor+dt, linux-watchdog, wsa+renesas, robh, devicetree

Hi Sashiko,

On Fri, Sep 11, 2026 at 12:27 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Incomplete fix: rzv2h_wdt_stop() still uses asynchronous pm_runtime_put() during driver unbind, risking a race with pm_runtime_disable().
> - [High] Kernel panic from division by zero when the optional 'oscclk' is missing from the device tree.
> - [High] Missing NULL check for of_device_get_match_data() return value leads to a NULL pointer dereference.
> --
>
> commit a446e675679ece36ef9554b71239f1b2c12cce73
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> watchdog: rzv2h: Use pm_runtime_put_sync()
>
> 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.
>
> > diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
> > index cbf414e0c9a3c..4a5a856060e91 100644
> > --- a/drivers/watchdog/rzv2h_wdt.c
> > +++ b/drivers/watchdog/rzv2h_wdt.c
> > @@ -270,7 +270,7 @@ static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
> >
> >       ret = rzt2h_wdt_wdtdcr_count_stop(priv);
> >
> > -     pm_runtime_put(&pdev->dev);
> > +     pm_runtime_put_sync(&pdev->dev);
> >
> >       return ret;
> >  }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but it seems there is another
> instance of this same issue in rzv2h_wdt_stop(). During module unbind, the
> devres cleanup triggers watchdog_unregister_device(), which calls
> rzv2h_wdt_stop(). This function issues an asynchronous pm_runtime_put():
>
> drivers/watchdog/rzv2h_wdt.c:rzv2h_wdt_stop() {
>         ...
>         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);
>         ...
> }
>
> Should this also use pm_runtime_put_sync() to ensure the idle check completes
> before the devres cleanup calls pm_runtime_disable()?
>
Agreed, I will update rzv2h_wdt_stop() to use pm_runtime_put_sync() call.

> [Severity: High]
> This is a pre-existing issue, but I noticed a potential NULL pointer
> dereference in rzv2h_wdt_probe(). The return value of
> of_device_get_match_data() is not checked:
>
> drivers/watchdog/rzv2h_wdt.c:rzv2h_wdt_probe() {
>         ...
>         priv->of_data = of_device_get_match_data(dev);
>         ...
>         switch (priv->of_data->count_source) {
>         ...
> }
>
> If the driver is bound via sysfs without device tree match data, could
> priv->of_data be NULL here and cause a crash?
>
> [Severity: High]
> This is a pre-existing issue, but could the clock rate calculation result in a
> division by zero?
>
> drivers/watchdog/rzv2h_wdt.c:rzv2h_wdt_probe() {
>         ...
>         priv->oscclk = devm_clk_get_optional_enabled(dev, "oscclk");
>         ...
>         switch (priv->of_data->count_source) {
>         case COUNT_SOURCE_LOCO:
>                 count_clk = priv->oscclk;
>                 break;
>         ...
>         priv->wdev.max_hw_heartbeat_ms = (MILLI * priv->of_data->timeout_cycles *
>                                           priv->of_data->cks_div) / clk_get_rate(count_clk);
>         ...
> }
>
> If the optional 'oscclk' is omitted in the device tree, priv->oscclk will be
> NULL. When count_clk is assigned this NULL clock, clk_get_rate(count_clk)
> safely returns 0. Will this cause a kernel panic due to division by zero?
>
As replied in the previous patch, I will add a new patch to check the
clock rate to avoid division by zero.

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-09-11 12:12 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-11 11:17 [PATCH v5 0/8] Add syscon support for Renesas WDT driver Prabhakar
2026-09-11 11:17 ` [PATCH v5 1/8] watchdog: rzv2h: Drop enabling clocks in the restart handler Prabhakar
2026-09-11 11:32   ` sashiko-bot
2026-09-11 11:36     ` Lad, Prabhakar
2026-09-11 11:17 ` [PATCH v5 2/8] watchdog: rzv2h: Drop WDTRCR_RSTIRQS define Prabhakar
2026-09-11 11:17 ` [PATCH v5 3/8] watchdog: rzv2h: Propagate WDTDCR access errors Prabhakar
2026-09-11 11:26   ` sashiko-bot
2026-09-11 11:30     ` Lad, Prabhakar
2026-09-11 11:17 ` [PATCH v5 4/8] watchdog: rzv2h: Use pm_runtime_put_sync() Prabhakar
2026-09-11 11:27   ` sashiko-bot
2026-09-11 12:11     ` Lad, Prabhakar
2026-09-11 11:17 ` [PATCH v5 5/8] watchdog: rzv2h: Convert WDTDCR handling to regmap Prabhakar
2026-09-11 11:17 ` [PATCH v5 6/8] watchdog: rzv2h: Add syscon support for WDTDCR Prabhakar
2026-09-11 11:28   ` sashiko-bot
2026-09-11 11:39     ` Lad, Prabhakar
2026-09-11 11:17 ` [PATCH v5 7/8] arm64: dts: renesas: r9a09g077: Use CPG/MSSR syscon for WDTDCR access Prabhakar
2026-09-11 11:32   ` sashiko-bot
2026-09-11 12:08     ` Lad, Prabhakar
2026-09-11 11:17 ` [PATCH v5 8/8] arm64: dts: renesas: r9a09g087: " Prabhakar
2026-09-11 11:33   ` sashiko-bot
2026-09-11 12:09     ` Lad, Prabhakar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).