[PATCH v2 0/9] Add watchdog driver support for RZ/T2H and RZ/N2H SoCs

[PATCH v2 0/9] Add watchdog driver support for RZ/T2H and RZ/N2H SoCs

[Prabhakar]

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

Hi All,

This patch series adds watchdog driver support for the Renesas RZ/T2H
(R9A09G077) and RZ/N2H (R9A09G087) SoCs. The necessary device tree
bindings and driver modifications are included.

v1->v2:
- Dropped items from clock-names and instead added maxItems: 1.
- Added reviewed-by from Rob.

Cheers,
Prabhakar

Lad Prabhakar (9):
  dt-bindings: watchdog: renesas,wdt: Add support for RZ/T2H and RZ/N2H
  watchdog: rzv2h_wdt: Obtain clock-divider ranges from OF match data
  watchdog: rzv2h_wdt: Obtain CKS divider via OF data
  watchdog: rzv2h_wdt: Make "oscclk" an optional clock
  watchdog: rzv2h_wdt: Add support for configurable count clock source
  watchdog: rzv2h_wdt: Make reset controller optional
  watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms
  watchdog: rzv2h: Add support for RZ/T2H
  watchdog: rzv2h_wdt: Improve error strings and add newlines

 .../bindings/watchdog/renesas,wdt.yaml        |  36 ++++-
 drivers/watchdog/rzv2h_wdt.c                  | 137 ++++++++++++++++--
 2 files changed, 154 insertions(+), 19 deletions(-)

-- 
2.50.1

[PATCH v2 1/9] dt-bindings: watchdog: renesas,wdt: Add support for RZ/T2H and RZ/N2H

[Prabhakar]

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

Extend the Renesas WDT device tree bindings to support the watchdog timer
found on the RZ/T2H (R9A09G077) and RZ/N2H (R9A09G087) SoCs.

The RZ/T2H WDT is mostly compatible with the one found on the RZ/V2H(P),
but includes an additional register and differs in the clock division
ratio settings for the WDTCR[CKS] field. To reflect these differences,
introduce a new compatible string, "renesas,r9a09g077-wdt".

The binding schema is updated accordingly. On RZ/T2H, the WDT does not
require the "resets" property. It also requires two register regions and
the presence of a "power-domains" property. The "clock-names" property is
limited to a single entry, "pclk", for this SoC.

The RZ/N2H SoC uses the same WDT IP as the RZ/T2H. It is supported by
using "renesas,r9a09g087-wdt" as the primary compatible string, with
"renesas,r9a09g077-wdt" listed as a fallback to describe the shared
hardware features.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
v1->v2:
- Dropped items from clock-names and instead added maxItems: 1.
- Added reviewed-by from Rob.
---
 .../bindings/watchdog/renesas,wdt.yaml        | 36 +++++++++++++++++--
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml b/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
index 78874b90c88c..b6e60162c263 100644
--- a/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
@@ -81,10 +81,17 @@ properties:
               - renesas,r9a09g056-wdt # RZ/V2N
           - const: renesas,r9a09g057-wdt # RZ/V2H(P)
 
-      - const: renesas,r9a09g057-wdt       # RZ/V2H(P)
+      - enum:
+          - renesas,r9a09g057-wdt    # RZ/V2H(P)
+          - renesas,r9a09g077-wdt    # RZ/T2H
+
+      - items:
+          - const: renesas,r9a09g087-wdt # RZ/N2H
+          - const: renesas,r9a09g077-wdt # RZ/T2H
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   interrupts:
     minItems: 1
@@ -132,6 +139,7 @@ allOf:
           compatible:
             contains:
               enum:
+                - renesas,r9a09g077-wdt
                 - renesas,rza-wdt
                 - renesas,rzn1-wdt
     then:
@@ -183,7 +191,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: renesas,r9a09g057-wdt
+            enum:
+              - renesas,r9a09g057-wdt
+              - renesas,r9a09g077-wdt
     then:
       properties:
         interrupts: false
@@ -192,6 +202,26 @@ allOf:
       required:
         - interrupts
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a09g077-wdt
+    then:
+      properties:
+        resets: false
+        clock-names:
+          maxItems: 1
+        reg:
+          minItems: 2
+      required:
+        - clock-names
+        - power-domains
+    else:
+      properties:
+        reg:
+          maxItems: 1
+
 additionalProperties: false
 
 examples:
-- 
2.50.1

[PATCH v2 2/9] watchdog: rzv2h_wdt: Obtain clock-divider ranges from OF match data

[Prabhakar]

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

Move the clock division ratio values into OF match data instead of
hardcoding them in the driver. Introduce `rzv2h_of_data` to hold `cks_min`
and `cks_max`, populated via the device tree match table. In probe, call
`of_device_get_match_data()` to retrieve these values for setting up the
watchdog.

This refactoring is transparent for existing RZ/V2H(P) usage and
facilitates adding RZ/T2H support with different divider ranges.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes.
---
 drivers/watchdog/rzv2h_wdt.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index 8defd0241213..d64d29709160 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -45,12 +45,18 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+struct rzv2h_of_data {
+	u8 cks_min;
+	u8 cks_max;
+};
+
 struct rzv2h_wdt_priv {
 	void __iomem *base;
 	struct clk *pclk;
 	struct clk *oscclk;
 	struct reset_control *rstc;
 	struct watchdog_device wdev;
+	const struct rzv2h_of_data *of_data;
 };
 
 static int rzv2h_wdt_ping(struct watchdog_device *wdev)
@@ -106,7 +112,7 @@ static int rzv2h_wdt_start(struct watchdog_device *wdev)
 	 * - RPES[9:8] - Window End Position Select - 11b: 0%
 	 * - TOPS[1:0] - Timeout Period Select - 11b: 16384 cycles (3FFFh)
 	 */
-	rzv2h_wdt_setup(wdev, WDTCR_CKS_CLK_256 | WDTCR_RPSS_100 |
+	rzv2h_wdt_setup(wdev, priv->of_data->cks_max | WDTCR_RPSS_100 |
 			WDTCR_RPES_0 | WDTCR_TOPS_16384);
 
 	/*
@@ -184,7 +190,7 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
 	 * - RPES[9:8] - Window End Position Select - 00b: 75%
 	 * - TOPS[1:0] - Timeout Period Select - 00b: 1024 cycles (03FFh)
 	 */
-	rzv2h_wdt_setup(wdev, WDTCR_CKS_CLK_1 | WDTCR_RPSS_25 |
+	rzv2h_wdt_setup(wdev, priv->of_data->cks_min | WDTCR_RPSS_25 |
 			WDTCR_RPES_75 | WDTCR_TOPS_1024);
 
 	rzv2h_wdt_ping(wdev);
@@ -213,6 +219,8 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	priv->of_data = of_device_get_match_data(dev);
+
 	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
@@ -254,8 +262,13 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 	return devm_watchdog_register_device(dev, &priv->wdev);
 }
 
+static const struct rzv2h_of_data rzv2h_wdt_of_data = {
+	.cks_min = WDTCR_CKS_CLK_1,
+	.cks_max = WDTCR_CKS_CLK_256,
+};
+
 static const struct of_device_id rzv2h_wdt_ids[] = {
-	{ .compatible = "renesas,r9a09g057-wdt", },
+	{ .compatible = "renesas,r9a09g057-wdt", .data = &rzv2h_wdt_of_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, rzv2h_wdt_ids);
-- 
2.50.1

[PATCH v2 3/9] watchdog: rzv2h_wdt: Obtain CKS divider via OF data

[Prabhakar]

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

Update the rzv2h_wdt driver to obtain the clock division ratio (`cks_div`)
from OF match data instead of using a hardcoded value. This allows the
driver to support SoCs where the clock divider differs from the default
value of 256.

This change is a preparatory step for supporting the RZ/T2H SoC, which
requires a divider value of 8192.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes.
---
 drivers/watchdog/rzv2h_wdt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index d64d29709160..c2f39dd56687 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -36,7 +36,6 @@
 #define WDTRCR_RSTIRQS		BIT(7)
 
 #define MAX_TIMEOUT_CYCLES	16384
-#define CLOCK_DIV_BY_256	256
 
 #define WDT_DEFAULT_TIMEOUT	60U
 
@@ -48,6 +47,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 struct rzv2h_of_data {
 	u8 cks_min;
 	u8 cks_max;
+	u16 cks_div;
 };
 
 struct rzv2h_wdt_priv {
@@ -238,7 +238,7 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(priv->rstc),
 				     "failed to get cpg reset");
 
-	priv->wdev.max_hw_heartbeat_ms = (MILLI * MAX_TIMEOUT_CYCLES * CLOCK_DIV_BY_256) /
+	priv->wdev.max_hw_heartbeat_ms = (MILLI * MAX_TIMEOUT_CYCLES * priv->of_data->cks_div) /
 					 clk_get_rate(priv->oscclk);
 	dev_dbg(dev, "max hw timeout of %dms\n", priv->wdev.max_hw_heartbeat_ms);
 
@@ -265,6 +265,7 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 static const struct rzv2h_of_data rzv2h_wdt_of_data = {
 	.cks_min = WDTCR_CKS_CLK_1,
 	.cks_max = WDTCR_CKS_CLK_256,
+	.cks_div = 256,
 };
 
 static const struct of_device_id rzv2h_wdt_ids[] = {
-- 
2.50.1

[PATCH v2 4/9] watchdog: rzv2h_wdt: Make "oscclk" an optional clock

[Prabhakar]

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

Update the driver to obtain the "oscclk" clock using
devm_clk_get_optional_prepared() instead of devm_clk_get_prepared().
This allows the driver to handle cases where the "oscclk" clock is not
present in the hardware or device tree.

This change is in preparation for adding support for the RZ/T2H SoC,
which does not provide the "oscclk" clock.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes.
---
 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 c2f39dd56687..baf9d64510b9 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -229,7 +229,7 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->pclk))
 		return dev_err_probe(dev, PTR_ERR(priv->pclk), "no pclk");
 
-	priv->oscclk = devm_clk_get_prepared(dev, "oscclk");
+	priv->oscclk = devm_clk_get_optional_prepared(dev, "oscclk");
 	if (IS_ERR(priv->oscclk))
 		return dev_err_probe(dev, PTR_ERR(priv->oscclk), "no oscclk");
 
-- 
2.50.1

[PATCH v2 5/9] watchdog: rzv2h_wdt: Add support for configurable count clock source

[Prabhakar]

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

Add support for selecting the count clock source used by the watchdog
timer. The RZ/V2H(P) SoC uses the LOCO as the count source, whereas on
RZ/T2H and RZ/N2H SoCs, the count source is the peripheral clock (PCLKL).

Introduce a `count_source` field in the SoC-specific data structure and
refactor the clock rate selection logic accordingly. This prepares the
driver for supporting the RZ/T2H and RZ/N2H SoCs, which differ in their
watchdog clocking architecture from RZ/V2H(P).

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes.
---
 drivers/watchdog/rzv2h_wdt.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index baf9d64510b9..cb584ac5860f 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -44,10 +44,16 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+enum rzv2h_wdt_count_source {
+	COUNT_SOURCE_LOCO,
+	COUNT_SOURCE_PCLK,
+};
+
 struct rzv2h_of_data {
 	u8 cks_min;
 	u8 cks_max;
 	u16 cks_div;
+	enum rzv2h_wdt_count_source count_source;
 };
 
 struct rzv2h_wdt_priv {
@@ -213,6 +219,7 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rzv2h_wdt_priv *priv;
+	struct clk *count_clk;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -238,8 +245,18 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(priv->rstc),
 				     "failed to get cpg reset");
 
+	switch (priv->of_data->count_source) {
+	case COUNT_SOURCE_LOCO:
+		count_clk = priv->oscclk;
+		break;
+	case COUNT_SOURCE_PCLK:
+		count_clk = priv->pclk;
+		break;
+	default:
+		return dev_err_probe(dev, -EINVAL, "Invalid count source\n");
+	}
 	priv->wdev.max_hw_heartbeat_ms = (MILLI * MAX_TIMEOUT_CYCLES * priv->of_data->cks_div) /
-					 clk_get_rate(priv->oscclk);
+					 clk_get_rate(count_clk);
 	dev_dbg(dev, "max hw timeout of %dms\n", priv->wdev.max_hw_heartbeat_ms);
 
 	ret = devm_pm_runtime_enable(dev);
@@ -266,6 +283,7 @@ static const struct rzv2h_of_data rzv2h_wdt_of_data = {
 	.cks_min = WDTCR_CKS_CLK_1,
 	.cks_max = WDTCR_CKS_CLK_256,
 	.cks_div = 256,
+	.count_source = COUNT_SOURCE_LOCO,
 };
 
 static const struct of_device_id rzv2h_wdt_ids[] = {
-- 
2.50.1

[PATCH v2 6/9] watchdog: rzv2h_wdt: Make reset controller optional

[Prabhakar]

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

Use devm_reset_control_get_optional_exclusive() instead of
devm_reset_control_get_exclusive() to allow the driver to operate
on platforms where a reset controller is not present.

This change is in preparation for supporting the RZ/T2H SoC, which
does not have reset.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes.
---
 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 cb584ac5860f..f0e2bf786acc 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -240,7 +240,7 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->oscclk))
 		return dev_err_probe(dev, PTR_ERR(priv->oscclk), "no oscclk");
 
-	priv->rstc = devm_reset_control_get_exclusive(dev, NULL);
+	priv->rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
 	if (IS_ERR(priv->rstc))
 		return dev_err_probe(dev, PTR_ERR(priv->rstc),
 				     "failed to get cpg reset");
-- 
2.50.1

[PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Prabhakar]

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

Update the watchdog minimum timeout value to be derived from
`max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
consistent minimum timeout in seconds.

This avoids hardcoding a value of `1` second and allows the driver to
adapt correctly to different hardware configurations that may set
`max_hw_heartbeat_ms` differently (e.g., based on the SoC clock source
and divider).

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes.
---
 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 f0e2bf786acc..9c11ce323c16 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -263,7 +263,7 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	priv->wdev.min_timeout = 1;
+	priv->wdev.min_timeout = DIV_ROUND_UP(priv->wdev.max_hw_heartbeat_ms, MSEC_PER_SEC);
 	priv->wdev.timeout = WDT_DEFAULT_TIMEOUT;
 	priv->wdev.info = &rzv2h_wdt_ident;
 	priv->wdev.ops = &rzv2h_wdt_ops;
-- 
2.50.1

[PATCH v2 8/9] watchdog: rzv2h: Add support for RZ/T2H

[Prabhakar]

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

Add support for the RZ/T2H watchdog timer. The RZ/T2H requires control of
the watchdog counter using the WDT Debug Control Register (WDTDCR), which
allows explicitly stopping and starting the counter. This behavior differs
from RZ/V2H, which doesn't use WDTDCR, so the driver is extended to handle
this requirement.

To support this, a new `wdtdcr` flag is introduced in the `rzv2h_of_data`
structure. When set, the driver maps the WDTDCR register and uses it to
control the watchdog counter in the start, stop, and restart callbacks.
Additionally, the clock divisor and count source for RZ/T2H are defined
to match its hardware configuration.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes.
---
 drivers/watchdog/rzv2h_wdt.c | 77 +++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index 9c11ce323c16..1b32bab87d67 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -21,11 +21,16 @@
 #define WDTSR			0x04	/* WDT Status Register RW, 16 */
 #define WDTRCR			0x06	/* WDT Reset Control Register RW, 8  */
 
+/* This register is only available on RZ/T2H and RZ/N2H SoCs */
+#define WDTDCR			0x00	/* WDT Debug Control Register RW, 32  */
+
 #define WDTCR_TOPS_1024		0x00
 #define WDTCR_TOPS_16384	0x03
 
 #define WDTCR_CKS_CLK_1		0x00
+#define WDTCR_CKS_CLK_4		0x10
 #define WDTCR_CKS_CLK_256	0x50
+#define WDTCR_CKS_CLK_8192	0x80
 
 #define WDTCR_RPES_0		0x300
 #define WDTCR_RPES_75		0x000
@@ -35,6 +40,8 @@
 
 #define WDTRCR_RSTIRQS		BIT(7)
 
+#define WDTDCR_WDTSTOPCTRL	BIT(0)
+
 #define MAX_TIMEOUT_CYCLES	16384
 
 #define WDT_DEFAULT_TIMEOUT	60U
@@ -54,10 +61,12 @@ struct rzv2h_of_data {
 	u8 cks_max;
 	u16 cks_div;
 	enum rzv2h_wdt_count_source count_source;
+	bool wdtdcr;
 };
 
 struct rzv2h_wdt_priv {
 	void __iomem *base;
+	void __iomem *wdtdcr;
 	struct clk *pclk;
 	struct clk *oscclk;
 	struct reset_control *rstc;
@@ -79,6 +88,20 @@ static int rzv2h_wdt_ping(struct watchdog_device *wdev)
 	return 0;
 }
 
+static void rzt2h_wdt_wdtdcr_count_stop(struct rzv2h_wdt_priv *priv)
+{
+	u32 reg = readl(priv->wdtdcr + WDTDCR);
+
+	writel(reg | WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
+}
+
+static void rzt2h_wdt_wdtdcr_count_start(struct rzv2h_wdt_priv *priv)
+{
+	u32 reg = readl(priv->wdtdcr + WDTDCR);
+
+	writel(reg & ~WDTDCR_WDTSTOPCTRL, priv->wdtdcr + WDTDCR);
+}
+
 static void rzv2h_wdt_setup(struct watchdog_device *wdev, u16 wdtcr)
 {
 	struct rzv2h_wdt_priv *priv = watchdog_get_drvdata(wdev);
@@ -113,7 +136,9 @@ static int rzv2h_wdt_start(struct watchdog_device *wdev)
 
 	/*
 	 * WDTCR
-	 * - CKS[7:4] - Clock Division Ratio Select - 0101b: oscclk/256
+	 * - CKS[7:4] - Clock Division Ratio Select
+	 *     - 0101b: oscclk/256 for RZ/V2H(P)
+	 *     - 1000b: pclkl/8192 for RZ/T2H
 	 * - RPSS[13:12] - Window Start Position Select - 11b: 100%
 	 * - RPES[9:8] - Window End Position Select - 11b: 0%
 	 * - TOPS[1:0] - Timeout Period Select - 11b: 16384 cycles (3FFFh)
@@ -121,6 +146,9 @@ static int rzv2h_wdt_start(struct watchdog_device *wdev)
 	rzv2h_wdt_setup(wdev, priv->of_data->cks_max | WDTCR_RPSS_100 |
 			WDTCR_RPES_0 | WDTCR_TOPS_16384);
 
+	if (priv->of_data->wdtdcr)
+		rzt2h_wdt_wdtdcr_count_start(priv);
+
 	/*
 	 * Down counting starts after writing the sequence 00h -> FFh to the
 	 * WDTRR register. Hence, call the ping operation after loading the counter.
@@ -139,6 +167,9 @@ static int rzv2h_wdt_stop(struct watchdog_device *wdev)
 	if (ret)
 		return ret;
 
+	if (priv->of_data->wdtdcr)
+		rzt2h_wdt_wdtdcr_count_stop(priv);
+
 	ret = pm_runtime_put(wdev->parent);
 	if (ret < 0)
 		return ret;
@@ -191,7 +222,9 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
 
 	/*
 	 * WDTCR
-	 * - CKS[7:4] - Clock Division Ratio Select - 0000b: oscclk/1
+	 * - CKS[7:4] - Clock Division Ratio Select
+	 *     - 0000b: oscclk/1 for RZ/V2H(P)
+	 *     - 0100b: pclkl/4 for RZ/T2H
 	 * - RPSS[13:12] - Window Start Position Select - 00b: 25%
 	 * - RPES[9:8] - Window End Position Select - 00b: 75%
 	 * - TOPS[1:0] - Timeout Period Select - 00b: 1024 cycles (03FFh)
@@ -199,6 +232,9 @@ static int rzv2h_wdt_restart(struct watchdog_device *wdev,
 	rzv2h_wdt_setup(wdev, priv->of_data->cks_min | WDTCR_RPSS_25 |
 			WDTCR_RPES_75 | WDTCR_TOPS_1024);
 
+	if (priv->of_data->wdtdcr)
+		rzt2h_wdt_wdtdcr_count_start(priv);
+
 	rzv2h_wdt_ping(wdev);
 
 	/* wait for underflow to trigger... */
@@ -215,6 +251,28 @@ static const struct watchdog_ops rzv2h_wdt_ops = {
 	.restart = rzv2h_wdt_restart,
 };
 
+static int rzt2h_wdt_wdtdcr_init(struct platform_device *pdev,
+				 struct rzv2h_wdt_priv *priv)
+{
+	int ret;
+
+	priv->wdtdcr = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(priv->wdtdcr))
+		return PTR_ERR(priv->wdtdcr);
+
+	ret = pm_runtime_resume_and_get(&pdev->dev);
+	if (ret)
+		return ret;
+
+	rzt2h_wdt_wdtdcr_count_stop(priv);
+
+	ret = pm_runtime_put(&pdev->dev);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int rzv2h_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -263,6 +321,12 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	if (priv->of_data->wdtdcr) {
+		ret = rzt2h_wdt_wdtdcr_init(pdev, priv);
+		if (ret)
+			return dev_err_probe(dev, ret, "WDTDCR init failed\n");
+	}
+
 	priv->wdev.min_timeout = DIV_ROUND_UP(priv->wdev.max_hw_heartbeat_ms, MSEC_PER_SEC);
 	priv->wdev.timeout = WDT_DEFAULT_TIMEOUT;
 	priv->wdev.info = &rzv2h_wdt_ident;
@@ -279,6 +343,14 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 	return devm_watchdog_register_device(dev, &priv->wdev);
 }
 
+static const struct rzv2h_of_data rzt2h_wdt_of_data = {
+	.cks_min = WDTCR_CKS_CLK_4,
+	.cks_max = WDTCR_CKS_CLK_8192,
+	.cks_div = 8192,
+	.count_source = COUNT_SOURCE_PCLK,
+	.wdtdcr = true,
+};
+
 static const struct rzv2h_of_data rzv2h_wdt_of_data = {
 	.cks_min = WDTCR_CKS_CLK_1,
 	.cks_max = WDTCR_CKS_CLK_256,
@@ -288,6 +360,7 @@ static const struct rzv2h_of_data rzv2h_wdt_of_data = {
 
 static const struct of_device_id rzv2h_wdt_ids[] = {
 	{ .compatible = "renesas,r9a09g057-wdt", .data = &rzv2h_wdt_of_data },
+	{ .compatible = "renesas,r9a09g077-wdt", .data = &rzt2h_wdt_of_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, rzv2h_wdt_ids);
-- 
2.50.1

[PATCH v2 9/9] watchdog: rzv2h_wdt: Improve error strings and add newlines

[Prabhakar]

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

Update rzv2h_wdt_probe() to provide clearer error strings when retrieving
the pclk, oscclk, and reset controller, and append missing newline
characters to dev_err_probe() and dev_warn() calls for proper log
formatting.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes.
---
 drivers/watchdog/rzv2h_wdt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/rzv2h_wdt.c b/drivers/watchdog/rzv2h_wdt.c
index 1b32bab87d67..db76a7c0e1d6 100644
--- a/drivers/watchdog/rzv2h_wdt.c
+++ b/drivers/watchdog/rzv2h_wdt.c
@@ -292,16 +292,16 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 
 	priv->pclk = devm_clk_get_prepared(dev, "pclk");
 	if (IS_ERR(priv->pclk))
-		return dev_err_probe(dev, PTR_ERR(priv->pclk), "no pclk");
+		return dev_err_probe(dev, PTR_ERR(priv->pclk), "Failed to get pclk\n");
 
 	priv->oscclk = devm_clk_get_optional_prepared(dev, "oscclk");
 	if (IS_ERR(priv->oscclk))
-		return dev_err_probe(dev, PTR_ERR(priv->oscclk), "no oscclk");
+		return dev_err_probe(dev, PTR_ERR(priv->oscclk), "Failed to get oscclk\n");
 
 	priv->rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
 	if (IS_ERR(priv->rstc))
 		return dev_err_probe(dev, PTR_ERR(priv->rstc),
-				     "failed to get cpg reset");
+				     "Failed to get cpg reset\n");
 
 	switch (priv->of_data->count_source) {
 	case COUNT_SOURCE_LOCO:
@@ -338,7 +338,7 @@ static int rzv2h_wdt_probe(struct platform_device *pdev)
 
 	ret = watchdog_init_timeout(&priv->wdev, 0, dev);
 	if (ret)
-		dev_warn(dev, "Specified timeout invalid, using default");
+		dev_warn(dev, "Specified timeout invalid, using default\n");
 
 	return devm_watchdog_register_device(dev, &priv->wdev);
 }
-- 
2.50.1

Re: [PATCH v2 0/9] Add watchdog driver support for RZ/T2H and RZ/N2H SoCs

[Wolfram Sang]

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

Hi,

>   dt-bindings: watchdog: renesas,wdt: Add support for RZ/T2H and RZ/N2H
>   watchdog: rzv2h_wdt: Obtain clock-divider ranges from OF match data
>   watchdog: rzv2h_wdt: Obtain CKS divider via OF data
>   watchdog: rzv2h_wdt: Make "oscclk" an optional clock
>   watchdog: rzv2h_wdt: Add support for configurable count clock source
>   watchdog: rzv2h_wdt: Make reset controller optional
>   watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms
>   watchdog: rzv2h: Add support for RZ/T2H
>   watchdog: rzv2h_wdt: Improve error strings and add newlines

Minor nit, but still: inconsistent prefix

I'd suggest to use "rzv2h" instead of "rzv2h_wdt" but it is ultimately
the WDT maintainers call...


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

Re: [PATCH v2 0/9] Add watchdog driver support for RZ/T2H and RZ/N2H SoCs

[Lad, Prabhakar]

Hi Wolfram,

On Tue, Jul 29, 2025 at 6:08 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> Hi,
>
> >   dt-bindings: watchdog: renesas,wdt: Add support for RZ/T2H and RZ/N2H
> >   watchdog: rzv2h_wdt: Obtain clock-divider ranges from OF match data
> >   watchdog: rzv2h_wdt: Obtain CKS divider via OF data
> >   watchdog: rzv2h_wdt: Make "oscclk" an optional clock
> >   watchdog: rzv2h_wdt: Add support for configurable count clock source
> >   watchdog: rzv2h_wdt: Make reset controller optional
> >   watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms
> >   watchdog: rzv2h: Add support for RZ/T2H
> >   watchdog: rzv2h_wdt: Improve error strings and add newlines
>
> Minor nit, but still: inconsistent prefix
>
> I'd suggest to use "rzv2h" instead of "rzv2h_wdt" but it is ultimately
> the WDT maintainers call...
>
I agree, I will switch to using "rzv2h".

Cheers,
Prabhakar

Re: [PATCH v2 2/9] watchdog: rzv2h_wdt: Obtain clock-divider ranges from OF match data

[Wolfram Sang]

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

On Tue, Jul 29, 2025 at 04:59:08PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Move the clock division ratio values into OF match data instead of
> hardcoding them in the driver. Introduce `rzv2h_of_data` to hold `cks_min`
> and `cks_max`, populated via the device tree match table. In probe, call
> `of_device_get_match_data()` to retrieve these values for setting up the
> watchdog.
> 
> This refactoring is transparent for existing RZ/V2H(P) usage and
> facilitates adding RZ/T2H support with different divider ranges.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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


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

Re: [PATCH v2 3/9] watchdog: rzv2h_wdt: Obtain CKS divider via OF data

[Wolfram Sang]

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

On Tue, Jul 29, 2025 at 04:59:09PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Update the rzv2h_wdt driver to obtain the clock division ratio (`cks_div`)
> from OF match data instead of using a hardcoded value. This allows the
> driver to support SoCs where the clock divider differs from the default
> value of 256.
> 
> This change is a preparatory step for supporting the RZ/T2H SoC, which
> requires a divider value of 8192.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

I'd think this should be merged with patch 2, but for the change itself:

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


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

Re: [PATCH v2 4/9] watchdog: rzv2h_wdt: Make "oscclk" an optional clock

[Wolfram Sang]

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

On Tue, Jul 29, 2025 at 04:59:10PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Update the driver to obtain the "oscclk" clock using
> devm_clk_get_optional_prepared() instead of devm_clk_get_prepared().
> This allows the driver to handle cases where the "oscclk" clock is not
> present in the hardware or device tree.
> 
> This change is in preparation for adding support for the RZ/T2H SoC,
> which does not provide the "oscclk" clock.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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


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

Re: [PATCH v2 5/9] watchdog: rzv2h_wdt: Add support for configurable count clock source

[Wolfram Sang]

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

On Tue, Jul 29, 2025 at 04:59:11PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Add support for selecting the count clock source used by the watchdog
> timer. The RZ/V2H(P) SoC uses the LOCO as the count source, whereas on
> RZ/T2H and RZ/N2H SoCs, the count source is the peripheral clock (PCLKL).
> 
> Introduce a `count_source` field in the SoC-specific data structure and
> refactor the clock rate selection logic accordingly. This prepares the
> driver for supporting the RZ/T2H and RZ/N2H SoCs, which differ in their
> watchdog clocking architecture from RZ/V2H(P).
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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


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

Re: [PATCH v2 6/9] watchdog: rzv2h_wdt: Make reset controller optional

[Wolfram Sang]

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

On Tue, Jul 29, 2025 at 04:59:12PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Use devm_reset_control_get_optional_exclusive() instead of
> devm_reset_control_get_exclusive() to allow the driver to operate
> on platforms where a reset controller is not present.
> 
> This change is in preparation for supporting the RZ/T2H SoC, which
> does not have reset.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

I'd suggest to merge this with patch 4.

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


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

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Wolfram Sang]

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

On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Update the watchdog minimum timeout value to be derived from
> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
> consistent minimum timeout in seconds.

I don't understand this change. Why is the _minimum_ timeout based on
the _maximum_ heartbeat?


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

Re: [PATCH v2 8/9] watchdog: rzv2h: Add support for RZ/T2H

[Wolfram Sang]

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

On Tue, Jul 29, 2025 at 04:59:14PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Add support for the RZ/T2H watchdog timer. The RZ/T2H requires control of
> the watchdog counter using the WDT Debug Control Register (WDTDCR), which
> allows explicitly stopping and starting the counter. This behavior differs
> from RZ/V2H, which doesn't use WDTDCR, so the driver is extended to handle
> this requirement.

Is it really required or is it an additional feature?

> To support this, a new `wdtdcr` flag is introduced in the `rzv2h_of_data`
> structure. When set, the driver maps the WDTDCR register and uses it to
> control the watchdog counter in the start, stop, and restart callbacks.
> Additionally, the clock divisor and count source for RZ/T2H are defined
> to match its hardware configuration.

Where is the register placed? We need a seperate resource for it? Can
you kindly give an example DT node for this case?


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

Re: [PATCH v2 9/9] watchdog: rzv2h_wdt: Improve error strings and add newlines

[Wolfram Sang]

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

On Tue, Jul 29, 2025 at 04:59:15PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Update rzv2h_wdt_probe() to provide clearer error strings when retrieving
> the pclk, oscclk, and reset controller, and append missing newline
> characters to dev_err_probe() and dev_warn() calls for proper log
> formatting.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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


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

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Lad, Prabhakar]

Hi Wolfram,

Thank you for the review.

On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Update the watchdog minimum timeout value to be derived from
> > `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
> > consistent minimum timeout in seconds.
>
> I don't understand this change. Why is the _minimum_ timeout based on
> the _maximum_ heartbeat?
>
The reason for deriving min_timeout from max_hw_heartbeat_ms is to
ensure the minimum watchdog period (in seconds) is compatible with the
underlying hardware.

max_hw_heartbeat_ms is calculated as:
max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;

This value varies by SoC:
 RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
 RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms

Since min_timeout is in seconds, setting it to:
min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);

ensures:
The minimum timeout period is never less than what the hardware can support.
- For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
- For V2H, it’s just 1s (174ms -> 1s).

Cheers,
Prabhakar

Re: [PATCH v2 8/9] watchdog: rzv2h: Add support for RZ/T2H

[Lad, Prabhakar]

Hi Wolfram,

Thank you for the review.

On Fri, Aug 1, 2025 at 5:12 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Tue, Jul 29, 2025 at 04:59:14PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add support for the RZ/T2H watchdog timer. The RZ/T2H requires control of
> > the watchdog counter using the WDT Debug Control Register (WDTDCR), which
> > allows explicitly stopping and starting the counter. This behavior differs
> > from RZ/V2H, which doesn't use WDTDCR, so the driver is extended to handle
> > this requirement.
>
> Is it really required or is it an additional feature?
>
Sorry for not being clear WDTDCR register is not present on the
RZ/V2H(P) SoC, and is required on RZ/T2H (and RZ/N2H) SoC to
start/stop down counting.

> > To support this, a new `wdtdcr` flag is introduced in the `rzv2h_of_data`
> > structure. When set, the driver maps the WDTDCR register and uses it to
> > control the watchdog counter in the start, stop, and restart callbacks.
> > Additionally, the clock divisor and count source for RZ/T2H are defined
> > to match its hardware configuration.
>
> Where is the register placed? We need a seperate resource for it? Can
> you kindly give an example DT node for this case?
>
The WDTDCR register is placed somewhere out and yes we need a separate
resource for it.

Below is the node for RZ/T2H SoC:
        wdt0: watchdog@80082000 {
            compatible = "renesas,r9a09g077-wdt";
            reg = <0 0x80082000 0 0x400>,
                  <0 0x81295100 0 0x04>;
            clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>;
            clock-names = "pclk";
            power-domains = <&cpg>;
            status = "disabled";
        };

Cheers,
Prabhakar

Re: [PATCH v2 3/9] watchdog: rzv2h_wdt: Obtain CKS divider via OF data

[Lad, Prabhakar]

Hi Wolfram,

Thank you for the review.

On Fri, Aug 1, 2025 at 5:04 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Tue, Jul 29, 2025 at 04:59:09PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Update the rzv2h_wdt driver to obtain the clock division ratio (`cks_div`)
> > from OF match data instead of using a hardcoded value. This allows the
> > driver to support SoCs where the clock divider differs from the default
> > value of 256.
> >
> > This change is a preparatory step for supporting the RZ/T2H SoC, which
> > requires a divider value of 8192.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> I'd think this should be merged with patch 2, but for the change itself:
>
Ok, I will squash it with patch 2/9.

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

Cheers,
Prabhakar

Re: [PATCH v2 6/9] watchdog: rzv2h_wdt: Make reset controller optional

[Lad, Prabhakar]

Hi Wolfram,

Thank you for the review.

On Fri, Aug 1, 2025 at 5:04 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Tue, Jul 29, 2025 at 04:59:12PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Use devm_reset_control_get_optional_exclusive() instead of
> > devm_reset_control_get_exclusive() to allow the driver to operate
> > on platforms where a reset controller is not present.
> >
> > This change is in preparation for supporting the RZ/T2H SoC, which
> > does not have reset.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> I'd suggest to merge this with patch 4.
>
Ok, I will squash it with patch 4/9.

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

Cheers,
Prabhakar

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Guenter Roeck]

On 8/1/25 04:05, Lad, Prabhakar wrote:
> Hi Wolfram,
> 
> Thank you for the review.
> 
> On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
>>
>> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>
>>> Update the watchdog minimum timeout value to be derived from
>>> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
>>> consistent minimum timeout in seconds.
>>
>> I don't understand this change. Why is the _minimum_ timeout based on
>> the _maximum_ heartbeat?
>>
> The reason for deriving min_timeout from max_hw_heartbeat_ms is to
> ensure the minimum watchdog period (in seconds) is compatible with the
> underlying hardware.
> 
> max_hw_heartbeat_ms is calculated as:
> max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;
> 
> This value varies by SoC:
>   RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
>   RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms
> 
> Since min_timeout is in seconds, setting it to:
> min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);
> 
> ensures:
> The minimum timeout period is never less than what the hardware can support.
> - For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
> - For V2H, it’s just 1s (174ms -> 1s).
> 

Sorry, I completely fail to understand the logic.

If the maximum timeout is, say, 2 seconds, why would the hardware
not be able to support a timeout of 1 second ?

Guenter

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Lad, Prabhakar]

Hi Guenter,

On Fri, Aug 1, 2025 at 2:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 8/1/25 04:05, Lad, Prabhakar wrote:
> > Hi Wolfram,
> >
> > Thank you for the review.
> >
> > On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
> > <wsa+renesas@sang-engineering.com> wrote:
> >>
> >> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
> >>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>
> >>> Update the watchdog minimum timeout value to be derived from
> >>> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
> >>> consistent minimum timeout in seconds.
> >>
> >> I don't understand this change. Why is the _minimum_ timeout based on
> >> the _maximum_ heartbeat?
> >>
> > The reason for deriving min_timeout from max_hw_heartbeat_ms is to
> > ensure the minimum watchdog period (in seconds) is compatible with the
> > underlying hardware.
> >
> > max_hw_heartbeat_ms is calculated as:
> > max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;
> >
> > This value varies by SoC:
> >   RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
> >   RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms
> >
> > Since min_timeout is in seconds, setting it to:
> > min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);
> >
> > ensures:
> > The minimum timeout period is never less than what the hardware can support.
> > - For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
> > - For V2H, it’s just 1s (174ms -> 1s).
> >
>
> Sorry, I completely fail to understand the logic.
>
> If the maximum timeout is, say, 2 seconds, why would the hardware
> not be able to support a timeout of 1 second ?
>
The watchdog timer on RZ/V2H (and RZ/T2H) is a 14 bit down counter. On
initialization the down counters on the SoCs are configured to the max
down counter. On RZ/V2H down counter value 4194304 (which evaluates to
174ms) is and on RZ/T2H is 134217728 (which evaluates to 2147ms). The
board will be reset when we get an underflow error.

So for example on T2H consider this example:
- down counter is 134217728
- min_timeout is set to 1 in the driver
- When set  WDIOC_SETTIMEOUT to 1
In this case the board will be reset after 2147ms, i.e. incorrect
behaviour as we expect the board to be reset after 1 sec. Hence the
min_timeout is set to 3s (2147ms -> 3s).

Please let me know if my understanding of min_timeout is incorrect here.

Cheers,
Prabhakar

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Guenter Roeck]

On 8/1/25 08:30, Lad, Prabhakar wrote:
> Hi Guenter,
> 
> On Fri, Aug 1, 2025 at 2:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 8/1/25 04:05, Lad, Prabhakar wrote:
>>> Hi Wolfram,
>>>
>>> Thank you for the review.
>>>
>>> On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
>>> <wsa+renesas@sang-engineering.com> wrote:
>>>>
>>>> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
>>>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>>>
>>>>> Update the watchdog minimum timeout value to be derived from
>>>>> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
>>>>> consistent minimum timeout in seconds.
>>>>
>>>> I don't understand this change. Why is the _minimum_ timeout based on
>>>> the _maximum_ heartbeat?
>>>>
>>> The reason for deriving min_timeout from max_hw_heartbeat_ms is to
>>> ensure the minimum watchdog period (in seconds) is compatible with the
>>> underlying hardware.
>>>
>>> max_hw_heartbeat_ms is calculated as:
>>> max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;
>>>
>>> This value varies by SoC:
>>>    RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
>>>    RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms
>>>
>>> Since min_timeout is in seconds, setting it to:
>>> min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);
>>>
>>> ensures:
>>> The minimum timeout period is never less than what the hardware can support.
>>> - For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
>>> - For V2H, it’s just 1s (174ms -> 1s).
>>>
>>
>> Sorry, I completely fail to understand the logic.
>>
>> If the maximum timeout is, say, 2 seconds, why would the hardware
>> not be able to support a timeout of 1 second ?
>>
> The watchdog timer on RZ/V2H (and RZ/T2H) is a 14 bit down counter. On
> initialization the down counters on the SoCs are configured to the max
> down counter. On RZ/V2H down counter value 4194304 (which evaluates to
> 174ms) is and on RZ/T2H is 134217728 (which evaluates to 2147ms). The
> board will be reset when we get an underflow error.
> 
> So for example on T2H consider this example:
> - down counter is 134217728
> - min_timeout is set to 1 in the driver
> - When set  WDIOC_SETTIMEOUT to 1
> In this case the board will be reset after 2147ms, i.e. incorrect
> behaviour as we expect the board to be reset after 1 sec. Hence the
> min_timeout is set to 3s (2147ms -> 3s).
> 
> Please let me know if my understanding of min_timeout is incorrect here.
> 

The driver is missing a set_timeout function. It should set RZ/T2H
to 62514079 if a timeout of 1 second is configured.

Guenter

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Lad, Prabhakar]

Hi Guenter,

On Fri, Aug 1, 2025 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 8/1/25 08:30, Lad, Prabhakar wrote:
> > Hi Guenter,
> >
> > On Fri, Aug 1, 2025 at 2:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On 8/1/25 04:05, Lad, Prabhakar wrote:
> >>> Hi Wolfram,
> >>>
> >>> Thank you for the review.
> >>>
> >>> On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
> >>> <wsa+renesas@sang-engineering.com> wrote:
> >>>>
> >>>> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
> >>>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>>>
> >>>>> Update the watchdog minimum timeout value to be derived from
> >>>>> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
> >>>>> consistent minimum timeout in seconds.
> >>>>
> >>>> I don't understand this change. Why is the _minimum_ timeout based on
> >>>> the _maximum_ heartbeat?
> >>>>
> >>> The reason for deriving min_timeout from max_hw_heartbeat_ms is to
> >>> ensure the minimum watchdog period (in seconds) is compatible with the
> >>> underlying hardware.
> >>>
> >>> max_hw_heartbeat_ms is calculated as:
> >>> max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;
> >>>
> >>> This value varies by SoC:
> >>>    RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
> >>>    RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms
> >>>
> >>> Since min_timeout is in seconds, setting it to:
> >>> min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);
> >>>
> >>> ensures:
> >>> The minimum timeout period is never less than what the hardware can support.
> >>> - For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
> >>> - For V2H, it’s just 1s (174ms -> 1s).
> >>>
> >>
> >> Sorry, I completely fail to understand the logic.
> >>
> >> If the maximum timeout is, say, 2 seconds, why would the hardware
> >> not be able to support a timeout of 1 second ?
> >>
> > The watchdog timer on RZ/V2H (and RZ/T2H) is a 14 bit down counter. On
> > initialization the down counters on the SoCs are configured to the max
> > down counter. On RZ/V2H down counter value 4194304 (which evaluates to
> > 174ms) is and on RZ/T2H is 134217728 (which evaluates to 2147ms). The
> > board will be reset when we get an underflow error.
> >
> > So for example on T2H consider this example:
> > - down counter is 134217728
> > - min_timeout is set to 1 in the driver
> > - When set  WDIOC_SETTIMEOUT to 1
> > In this case the board will be reset after 2147ms, i.e. incorrect
> > behaviour as we expect the board to be reset after 1 sec. Hence the
> > min_timeout is set to 3s (2147ms -> 3s).
> >
> > Please let me know if my understanding of min_timeout is incorrect here.
> >
>
> The driver is missing a set_timeout function. It should set RZ/T2H
> to 62514079 if a timeout of 1 second is configured.
>
Ok, you mean to handle the 1sec case, introduce the set_timeout for RZ/T2H SoC.

Although we cannot achieve the exact 1sec case as we can have only 4
timeout period options (number of cycles):

1] For TIMEOUT_CYCLES = 1024
 - (1000×1024×8192)/62500000 = 134.22 ms
2] For TIMEOUT_CYCLES = 4096
- (1000×4096×8192)/62500000 = 536.87 ms
3] For TIMEOUT_CYCLES = 8192
- (1000×8192×8192)/62500000 = 1,073.74 ms
4] For TIMEOUT_CYCLES = 16384
- (1000×16384×8192)/62500000 = 2,147.48 ms

So to handle the 1sec case I'll set the timeout period to 8192 with
which we get a timeout of 1,073.74 ms.

Cheers,
Prabhakar

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Guenter Roeck]

On 8/1/25 13:51, Lad, Prabhakar wrote:
> Hi Guenter,
> 
> On Fri, Aug 1, 2025 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 8/1/25 08:30, Lad, Prabhakar wrote:
>>> Hi Guenter,
>>>
>>> On Fri, Aug 1, 2025 at 2:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>>>
>>>> On 8/1/25 04:05, Lad, Prabhakar wrote:
>>>>> Hi Wolfram,
>>>>>
>>>>> Thank you for the review.
>>>>>
>>>>> On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
>>>>> <wsa+renesas@sang-engineering.com> wrote:
>>>>>>
>>>>>> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
>>>>>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>>>>>
>>>>>>> Update the watchdog minimum timeout value to be derived from
>>>>>>> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
>>>>>>> consistent minimum timeout in seconds.
>>>>>>
>>>>>> I don't understand this change. Why is the _minimum_ timeout based on
>>>>>> the _maximum_ heartbeat?
>>>>>>
>>>>> The reason for deriving min_timeout from max_hw_heartbeat_ms is to
>>>>> ensure the minimum watchdog period (in seconds) is compatible with the
>>>>> underlying hardware.
>>>>>
>>>>> max_hw_heartbeat_ms is calculated as:
>>>>> max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;
>>>>>
>>>>> This value varies by SoC:
>>>>>     RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
>>>>>     RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms
>>>>>
>>>>> Since min_timeout is in seconds, setting it to:
>>>>> min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);
>>>>>
>>>>> ensures:
>>>>> The minimum timeout period is never less than what the hardware can support.
>>>>> - For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
>>>>> - For V2H, it’s just 1s (174ms -> 1s).
>>>>>
>>>>
>>>> Sorry, I completely fail to understand the logic.
>>>>
>>>> If the maximum timeout is, say, 2 seconds, why would the hardware
>>>> not be able to support a timeout of 1 second ?
>>>>
>>> The watchdog timer on RZ/V2H (and RZ/T2H) is a 14 bit down counter. On
>>> initialization the down counters on the SoCs are configured to the max
>>> down counter. On RZ/V2H down counter value 4194304 (which evaluates to
>>> 174ms) is and on RZ/T2H is 134217728 (which evaluates to 2147ms). The
>>> board will be reset when we get an underflow error.
>>>
>>> So for example on T2H consider this example:
>>> - down counter is 134217728
>>> - min_timeout is set to 1 in the driver
>>> - When set  WDIOC_SETTIMEOUT to 1
>>> In this case the board will be reset after 2147ms, i.e. incorrect
>>> behaviour as we expect the board to be reset after 1 sec. Hence the
>>> min_timeout is set to 3s (2147ms -> 3s).
>>>
>>> Please let me know if my understanding of min_timeout is incorrect here.
>>>
>>
>> The driver is missing a set_timeout function. It should set RZ/T2H
>> to 62514079 if a timeout of 1 second is configured.
>>
> Ok, you mean to handle the 1sec case, introduce the set_timeout for RZ/T2H SoC.
> 
> Although we cannot achieve the exact 1sec case as we can have only 4
> timeout period options (number of cycles):
> 
> 1] For TIMEOUT_CYCLES = 1024
>   - (1000×1024×8192)/62500000 = 134.22 ms
> 2] For TIMEOUT_CYCLES = 4096
> - (1000×4096×8192)/62500000 = 536.87 ms
> 3] For TIMEOUT_CYCLES = 8192
> - (1000×8192×8192)/62500000 = 1,073.74 ms
> 4] For TIMEOUT_CYCLES = 16384
> - (1000×16384×8192)/62500000 = 2,147.48 ms
> 
> So to handle the 1sec case I'll set the timeout period to 8192 with
> which we get a timeout of 1,073.74 ms.
> 

Just four possible values to set the hardware timeout ? That is an odd
hardware. In that case, you could also set the period to 1024 or 4096
and set max_hw_heartbeat_ms accordingly. That would avoid the rounding
error.

Guenter

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Lad, Prabhakar]

Hi Guenter,

On Fri, Aug 1, 2025 at 10:04 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 8/1/25 13:51, Lad, Prabhakar wrote:
> > Hi Guenter,
> >
> > On Fri, Aug 1, 2025 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On 8/1/25 08:30, Lad, Prabhakar wrote:
> >>> Hi Guenter,
> >>>
> >>> On Fri, Aug 1, 2025 at 2:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>>>
> >>>> On 8/1/25 04:05, Lad, Prabhakar wrote:
> >>>>> Hi Wolfram,
> >>>>>
> >>>>> Thank you for the review.
> >>>>>
> >>>>> On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
> >>>>> <wsa+renesas@sang-engineering.com> wrote:
> >>>>>>
> >>>>>> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
> >>>>>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>>>>>
> >>>>>>> Update the watchdog minimum timeout value to be derived from
> >>>>>>> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
> >>>>>>> consistent minimum timeout in seconds.
> >>>>>>
> >>>>>> I don't understand this change. Why is the _minimum_ timeout based on
> >>>>>> the _maximum_ heartbeat?
> >>>>>>
> >>>>> The reason for deriving min_timeout from max_hw_heartbeat_ms is to
> >>>>> ensure the minimum watchdog period (in seconds) is compatible with the
> >>>>> underlying hardware.
> >>>>>
> >>>>> max_hw_heartbeat_ms is calculated as:
> >>>>> max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;
> >>>>>
> >>>>> This value varies by SoC:
> >>>>>     RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
> >>>>>     RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms
> >>>>>
> >>>>> Since min_timeout is in seconds, setting it to:
> >>>>> min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);
> >>>>>
> >>>>> ensures:
> >>>>> The minimum timeout period is never less than what the hardware can support.
> >>>>> - For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
> >>>>> - For V2H, it’s just 1s (174ms -> 1s).
> >>>>>
> >>>>
> >>>> Sorry, I completely fail to understand the logic.
> >>>>
> >>>> If the maximum timeout is, say, 2 seconds, why would the hardware
> >>>> not be able to support a timeout of 1 second ?
> >>>>
> >>> The watchdog timer on RZ/V2H (and RZ/T2H) is a 14 bit down counter. On
> >>> initialization the down counters on the SoCs are configured to the max
> >>> down counter. On RZ/V2H down counter value 4194304 (which evaluates to
> >>> 174ms) is and on RZ/T2H is 134217728 (which evaluates to 2147ms). The
> >>> board will be reset when we get an underflow error.
> >>>
> >>> So for example on T2H consider this example:
> >>> - down counter is 134217728
> >>> - min_timeout is set to 1 in the driver
> >>> - When set  WDIOC_SETTIMEOUT to 1
> >>> In this case the board will be reset after 2147ms, i.e. incorrect
> >>> behaviour as we expect the board to be reset after 1 sec. Hence the
> >>> min_timeout is set to 3s (2147ms -> 3s).
> >>>
> >>> Please let me know if my understanding of min_timeout is incorrect here.
> >>>
> >>
> >> The driver is missing a set_timeout function. It should set RZ/T2H
> >> to 62514079 if a timeout of 1 second is configured.
> >>
> > Ok, you mean to handle the 1sec case, introduce the set_timeout for RZ/T2H SoC.
> >
> > Although we cannot achieve the exact 1sec case as we can have only 4
> > timeout period options (number of cycles):
> >
> > 1] For TIMEOUT_CYCLES = 1024
> >   - (1000×1024×8192)/62500000 = 134.22 ms
> > 2] For TIMEOUT_CYCLES = 4096
> > - (1000×4096×8192)/62500000 = 536.87 ms
> > 3] For TIMEOUT_CYCLES = 8192
> > - (1000×8192×8192)/62500000 = 1,073.74 ms
> > 4] For TIMEOUT_CYCLES = 16384
> > - (1000×16384×8192)/62500000 = 2,147.48 ms
> >
> > So to handle the 1sec case I'll set the timeout period to 8192 with
> > which we get a timeout of 1,073.74 ms.
> >
>
> Just four possible values to set the hardware timeout ? That is an odd
> hardware. In that case, you could also set the period to 1024 or 4096
> and set max_hw_heartbeat_ms accordingly. That would avoid the rounding
> error.
>
Yes sadly we have four timeout periods only. To clarify, you mean to
set `max_hw_heartbeat_ms` in set_timeout?

Cheers,
Prabhakar

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Guenter Roeck]

On 8/2/25 12:26, Lad, Prabhakar wrote:
> Hi Guenter,
> 
> On Fri, Aug 1, 2025 at 10:04 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 8/1/25 13:51, Lad, Prabhakar wrote:
>>> Hi Guenter,
>>>
>>> On Fri, Aug 1, 2025 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>>>
>>>> On 8/1/25 08:30, Lad, Prabhakar wrote:
>>>>> Hi Guenter,
>>>>>
>>>>> On Fri, Aug 1, 2025 at 2:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>>>>>
>>>>>> On 8/1/25 04:05, Lad, Prabhakar wrote:
>>>>>>> Hi Wolfram,
>>>>>>>
>>>>>>> Thank you for the review.
>>>>>>>
>>>>>>> On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
>>>>>>> <wsa+renesas@sang-engineering.com> wrote:
>>>>>>>>
>>>>>>>> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
>>>>>>>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>>>>>>>
>>>>>>>>> Update the watchdog minimum timeout value to be derived from
>>>>>>>>> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
>>>>>>>>> consistent minimum timeout in seconds.
>>>>>>>>
>>>>>>>> I don't understand this change. Why is the _minimum_ timeout based on
>>>>>>>> the _maximum_ heartbeat?
>>>>>>>>
>>>>>>> The reason for deriving min_timeout from max_hw_heartbeat_ms is to
>>>>>>> ensure the minimum watchdog period (in seconds) is compatible with the
>>>>>>> underlying hardware.
>>>>>>>
>>>>>>> max_hw_heartbeat_ms is calculated as:
>>>>>>> max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;
>>>>>>>
>>>>>>> This value varies by SoC:
>>>>>>>      RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
>>>>>>>      RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms
>>>>>>>
>>>>>>> Since min_timeout is in seconds, setting it to:
>>>>>>> min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);
>>>>>>>
>>>>>>> ensures:
>>>>>>> The minimum timeout period is never less than what the hardware can support.
>>>>>>> - For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
>>>>>>> - For V2H, it’s just 1s (174ms -> 1s).
>>>>>>>
>>>>>>
>>>>>> Sorry, I completely fail to understand the logic.
>>>>>>
>>>>>> If the maximum timeout is, say, 2 seconds, why would the hardware
>>>>>> not be able to support a timeout of 1 second ?
>>>>>>
>>>>> The watchdog timer on RZ/V2H (and RZ/T2H) is a 14 bit down counter. On
>>>>> initialization the down counters on the SoCs are configured to the max
>>>>> down counter. On RZ/V2H down counter value 4194304 (which evaluates to
>>>>> 174ms) is and on RZ/T2H is 134217728 (which evaluates to 2147ms). The
>>>>> board will be reset when we get an underflow error.
>>>>>
>>>>> So for example on T2H consider this example:
>>>>> - down counter is 134217728
>>>>> - min_timeout is set to 1 in the driver
>>>>> - When set  WDIOC_SETTIMEOUT to 1
>>>>> In this case the board will be reset after 2147ms, i.e. incorrect
>>>>> behaviour as we expect the board to be reset after 1 sec. Hence the
>>>>> min_timeout is set to 3s (2147ms -> 3s).
>>>>>
>>>>> Please let me know if my understanding of min_timeout is incorrect here.
>>>>>
>>>>
>>>> The driver is missing a set_timeout function. It should set RZ/T2H
>>>> to 62514079 if a timeout of 1 second is configured.
>>>>
>>> Ok, you mean to handle the 1sec case, introduce the set_timeout for RZ/T2H SoC.
>>>
>>> Although we cannot achieve the exact 1sec case as we can have only 4
>>> timeout period options (number of cycles):
>>>
>>> 1] For TIMEOUT_CYCLES = 1024
>>>    - (1000×1024×8192)/62500000 = 134.22 ms
>>> 2] For TIMEOUT_CYCLES = 4096
>>> - (1000×4096×8192)/62500000 = 536.87 ms
>>> 3] For TIMEOUT_CYCLES = 8192
>>> - (1000×8192×8192)/62500000 = 1,073.74 ms
>>> 4] For TIMEOUT_CYCLES = 16384
>>> - (1000×16384×8192)/62500000 = 2,147.48 ms
>>>
>>> So to handle the 1sec case I'll set the timeout period to 8192 with
>>> which we get a timeout of 1,073.74 ms.
>>>
>>
>> Just four possible values to set the hardware timeout ? That is an odd
>> hardware. In that case, you could also set the period to 1024 or 4096
>> and set max_hw_heartbeat_ms accordingly. That would avoid the rounding
>> error.
>>
> Yes sadly we have four timeout periods only. To clarify, you mean to
> set `max_hw_heartbeat_ms` in set_timeout?
> 

No, during initialization, and have no set_timeout function. max_hw_heartbeat_ms
is not supposed to change during runtime. If you do change it, the results
are undefined.

Guenter

Re: [PATCH v2 8/9] watchdog: rzv2h: Add support for RZ/T2H

[Wolfram Sang]

On Tue, Jul 29, 2025 at 04:59:14PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Add support for the RZ/T2H watchdog timer. The RZ/T2H requires control of
> the watchdog counter using the WDT Debug Control Register (WDTDCR), which
> allows explicitly stopping and starting the counter. This behavior differs
> from RZ/V2H, which doesn't use WDTDCR, so the driver is extended to handle
> this requirement.
> 
> To support this, a new `wdtdcr` flag is introduced in the `rzv2h_of_data`
> structure. When set, the driver maps the WDTDCR register and uses it to
> control the watchdog counter in the start, stop, and restart callbacks.
> Additionally, the clock divisor and count source for RZ/T2H are defined
> to match its hardware configuration.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Well, this new DCR register is really strange, especially its location
in a seperate resource. But given that, your code handling it looks
okay to me, so:

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

Re: [PATCH v2 7/9] watchdog: rzv2h: Set min_timeout based on max_hw_heartbeat_ms

[Lad, Prabhakar]

Hi Guenter,

On Sun, Aug 3, 2025 at 1:16 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 8/2/25 12:26, Lad, Prabhakar wrote:
> > Hi Guenter,
> >
> > On Fri, Aug 1, 2025 at 10:04 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On 8/1/25 13:51, Lad, Prabhakar wrote:
> >>> Hi Guenter,
> >>>
> >>> On Fri, Aug 1, 2025 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>>>
> >>>> On 8/1/25 08:30, Lad, Prabhakar wrote:
> >>>>> Hi Guenter,
> >>>>>
> >>>>> On Fri, Aug 1, 2025 at 2:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>>>>>
> >>>>>> On 8/1/25 04:05, Lad, Prabhakar wrote:
> >>>>>>> Hi Wolfram,
> >>>>>>>
> >>>>>>> Thank you for the review.
> >>>>>>>
> >>>>>>> On Fri, Aug 1, 2025 at 5:10 AM Wolfram Sang
> >>>>>>> <wsa+renesas@sang-engineering.com> wrote:
> >>>>>>>>
> >>>>>>>> On Tue, Jul 29, 2025 at 04:59:13PM +0100, Prabhakar wrote:
> >>>>>>>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>>>>>>>
> >>>>>>>>> Update the watchdog minimum timeout value to be derived from
> >>>>>>>>> `max_hw_heartbeat_ms` using `DIV_ROUND_UP()` to ensure a valid and
> >>>>>>>>> consistent minimum timeout in seconds.
> >>>>>>>>
> >>>>>>>> I don't understand this change. Why is the _minimum_ timeout based on
> >>>>>>>> the _maximum_ heartbeat?
> >>>>>>>>
> >>>>>>> The reason for deriving min_timeout from max_hw_heartbeat_ms is to
> >>>>>>> ensure the minimum watchdog period (in seconds) is compatible with the
> >>>>>>> underlying hardware.
> >>>>>>>
> >>>>>>> max_hw_heartbeat_ms is calculated as:
> >>>>>>> max_hw_heartbeat_ms = (1000 * 16384 * cks_div) / clk_rate;
> >>>>>>>
> >>>>>>> This value varies by SoC:
> >>>>>>>      RZ/T2H: cks_div = 8192, clk ≈ 62.5 MHz -> max_hw_heartbeat_ms ~ 2147ms
> >>>>>>>      RZ/V2H: cks_div = 256, clk ≈ 240 MHz -> max_hw_heartbeat_ms ~ 174ms
> >>>>>>>
> >>>>>>> Since min_timeout is in seconds, setting it to:
> >>>>>>> min_timeout = DIV_ROUND_UP(max_hw_heartbeat_ms, 1000);
> >>>>>>>
> >>>>>>> ensures:
> >>>>>>> The minimum timeout period is never less than what the hardware can support.
> >>>>>>> - For T2H, this results in a min_timeout of 3s (2147ms -> 3s).
> >>>>>>> - For V2H, it’s just 1s (174ms -> 1s).
> >>>>>>>
> >>>>>>
> >>>>>> Sorry, I completely fail to understand the logic.
> >>>>>>
> >>>>>> If the maximum timeout is, say, 2 seconds, why would the hardware
> >>>>>> not be able to support a timeout of 1 second ?
> >>>>>>
> >>>>> The watchdog timer on RZ/V2H (and RZ/T2H) is a 14 bit down counter. On
> >>>>> initialization the down counters on the SoCs are configured to the max
> >>>>> down counter. On RZ/V2H down counter value 4194304 (which evaluates to
> >>>>> 174ms) is and on RZ/T2H is 134217728 (which evaluates to 2147ms). The
> >>>>> board will be reset when we get an underflow error.
> >>>>>
> >>>>> So for example on T2H consider this example:
> >>>>> - down counter is 134217728
> >>>>> - min_timeout is set to 1 in the driver
> >>>>> - When set  WDIOC_SETTIMEOUT to 1
> >>>>> In this case the board will be reset after 2147ms, i.e. incorrect
> >>>>> behaviour as we expect the board to be reset after 1 sec. Hence the
> >>>>> min_timeout is set to 3s (2147ms -> 3s).
> >>>>>
> >>>>> Please let me know if my understanding of min_timeout is incorrect here.
> >>>>>
> >>>>
> >>>> The driver is missing a set_timeout function. It should set RZ/T2H
> >>>> to 62514079 if a timeout of 1 second is configured.
> >>>>
> >>> Ok, you mean to handle the 1sec case, introduce the set_timeout for RZ/T2H SoC.
> >>>
> >>> Although we cannot achieve the exact 1sec case as we can have only 4
> >>> timeout period options (number of cycles):
> >>>
> >>> 1] For TIMEOUT_CYCLES = 1024
> >>>    - (1000×1024×8192)/62500000 = 134.22 ms
> >>> 2] For TIMEOUT_CYCLES = 4096
> >>> - (1000×4096×8192)/62500000 = 536.87 ms
> >>> 3] For TIMEOUT_CYCLES = 8192
> >>> - (1000×8192×8192)/62500000 = 1,073.74 ms
> >>> 4] For TIMEOUT_CYCLES = 16384
> >>> - (1000×16384×8192)/62500000 = 2,147.48 ms
> >>>
> >>> So to handle the 1sec case I'll set the timeout period to 8192 with
> >>> which we get a timeout of 1,073.74 ms.
> >>>
> >>
> >> Just four possible values to set the hardware timeout ? That is an odd
> >> hardware. In that case, you could also set the period to 1024 or 4096
> >> and set max_hw_heartbeat_ms accordingly. That would avoid the rounding
> >> error.
> >>
> > Yes sadly we have four timeout periods only. To clarify, you mean to
> > set `max_hw_heartbeat_ms` in set_timeout?
> >
>
> No, during initialization, and have no set_timeout function. max_hw_heartbeat_ms
> is not supposed to change during runtime. If you do change it, the results
> are undefined.
>
Thank you for the clarification. Ive done the changes as suggested. I
will send a v3 soon.

Cheers,
Prabhakar