[PATCH v3 0/2] renesas: Renesas R-Car Gen4 watchdog driver

[PATCH v3 0/2] renesas: Renesas R-Car Gen4 watchdog driver

[Shmuel Leib Melamud]

These series add support of Renesas R-Car Gen4 watchdog timer.
Timeouts up to 8184.0s are supported (CKS1 register is not involved).
The watchdog uses the clock type CLK_TYPE_GEN4_MDSEL, so a separate
patch adds handling of this constant to gen3_clk_get_rate64() function.

The series were tested on real Renesas R8A779F0 hardware. If the
watchdog driver is enabled at the build time, the watchdog timer is
initialized when U-Boot starts. Under normal circumstances, U-Boot loads
the kernel, it starts systemd and systemd continues to pet the watchdog.
If systemd is not started before the timeout expires, the watchdog
resets the board.

Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>

---
Changes in v3:
- Reference to the Linux driver added.
- Clock driver change moved to a separate patch.
- rwdt_ prefix used everywhere instead of renesas_wdt_.
- Disable the clock if rwdt_probe() fails.
- List of compatibles updated.
- Link to v2: https://lore.kernel.org/r/20250530-us-renesas-watchdog-v2-1-b0d8f96c64dc@redhat.com

---
Shmuel Leib Melamud (2):
      renesas: Handle CLK_TYPE_GEN4_MDSEL in gen3_clk_get_rate64()
      renesas: Renesas R-Car Gen4 watchdog driver

 drivers/clk/renesas/clk-rcar-gen3.c |   4 +-
 drivers/watchdog/Kconfig            |   8 ++
 drivers/watchdog/Makefile           |   1 +
 drivers/watchdog/renesas_wdt.c      | 182 ++++++++++++++++++++++++++++++++++++
 4 files changed, 194 insertions(+), 1 deletion(-)
---
base-commit: 3b6760ddeb4ef940226921017cd9088c89784b01
change-id: 20250530-us-renesas-watchdog-2c79dbbd5cd2

Best regards,
-- 
Shmuel Leib Melamud <smelamud@redhat.com>

[PATCH v3 0/2] renesas: Renesas R-Car Gen4 watchdog driver

[Shmuel Leib Melamud via B4 Relay]

These series add support of Renesas R-Car Gen4 watchdog timer.
Timeouts up to 8184.0s are supported (CKS1 register is not involved).
The watchdog uses the clock type CLK_TYPE_GEN4_MDSEL, so a separate
patch adds handling of this constant to gen3_clk_get_rate64() function.

The series were tested on real Renesas R8A779F0 hardware. If the
watchdog driver is enabled at the build time, the watchdog timer is
initialized when U-Boot starts. Under normal circumstances, U-Boot loads
the kernel, it starts systemd and systemd continues to pet the watchdog.
If systemd is not started before the timeout expires, the watchdog
resets the board.

Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>

---
Changes in v3:
- Reference to the Linux driver added.
- Clock driver change moved to a separate patch.
- rwdt_ prefix used everywhere instead of renesas_wdt_.
- Disable the clock if rwdt_probe() fails.
- List of compatibles updated.
- Link to v2: https://lore.kernel.org/r/20250530-us-renesas-watchdog-v2-1-b0d8f96c64dc@redhat.com

---
Shmuel Leib Melamud (2):
      renesas: Handle CLK_TYPE_GEN4_MDSEL in gen3_clk_get_rate64()
      renesas: Renesas R-Car Gen4 watchdog driver

 drivers/clk/renesas/clk-rcar-gen3.c |   4 +-
 drivers/watchdog/Kconfig            |   8 ++
 drivers/watchdog/Makefile           |   1 +
 drivers/watchdog/renesas_wdt.c      | 182 ++++++++++++++++++++++++++++++++++++
 4 files changed, 194 insertions(+), 1 deletion(-)
---
base-commit: 3b6760ddeb4ef940226921017cd9088c89784b01
change-id: 20250530-us-renesas-watchdog-2c79dbbd5cd2

Best regards,
-- 
Shmuel Leib Melamud <smelamud@redhat.com>

[PATCH v3 1/2] renesas: Handle CLK_TYPE_GEN4_MDSEL in gen3_clk_get_rate64()

[Shmuel Leib Melamud]

Add support of CLK_TYPE_GEN4_MDSEL clock type to gen3_clk_get_rate64()
function. In particular, this type of clock is used by Renesas R-Car
Gen4 watchdog. It operates similarly to CLK_TYPE_GEN3_MDSEL clock.

Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>
---
 drivers/clk/renesas/clk-rcar-gen3.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/clk-rcar-gen3.c b/drivers/clk/renesas/clk-rcar-gen3.c
index 375cc4a4930873ad0d5509c19ad04a0ea5545aa0..5745acf4023c9114f6fa13b5e4baa306c5b57d33 100644
--- a/drivers/clk/renesas/clk-rcar-gen3.c
+++ b/drivers/clk/renesas/clk-rcar-gen3.c
@@ -68,7 +68,7 @@ static int gen3_clk_get_parent(struct gen3_clk_priv *priv, struct clk *clk,
 		if (ret)
 			return ret;
 
-		if (core->type == CLK_TYPE_GEN3_MDSEL) {
+		if (core->type == CLK_TYPE_GEN3_MDSEL || core->type == CLK_TYPE_GEN4_MDSEL) {
 			shift = priv->cpg_mode & BIT(core->offset) ? 0 : 16;
 			parent->dev = clk->dev;
 			parent->id = core->parent >> shift;
@@ -318,6 +318,8 @@ static u64 gen3_clk_get_rate64(struct clk *clk)
 						"FIXED");
 
 	case CLK_TYPE_GEN3_MDSEL:
+		fallthrough;
+	case CLK_TYPE_GEN4_MDSEL:
 		shift = priv->cpg_mode & BIT(core->offset) ? 0 : 16;
 		div = (core->div >> shift) & 0xffff;
 		rate = gen3_clk_get_rate64(&parent) / div;

-- 
2.49.0

[PATCH v3 1/2] renesas: Handle CLK_TYPE_GEN4_MDSEL in gen3_clk_get_rate64()

[Shmuel Leib Melamud via B4 Relay]

From: Shmuel Leib Melamud <smelamud@redhat.com>

Add support of CLK_TYPE_GEN4_MDSEL clock type to gen3_clk_get_rate64()
function. In particular, this type of clock is used by Renesas R-Car
Gen4 watchdog. It operates similarly to CLK_TYPE_GEN3_MDSEL clock.

Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>
---
 drivers/clk/renesas/clk-rcar-gen3.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/clk-rcar-gen3.c b/drivers/clk/renesas/clk-rcar-gen3.c
index 375cc4a4930873ad0d5509c19ad04a0ea5545aa0..5745acf4023c9114f6fa13b5e4baa306c5b57d33 100644
--- a/drivers/clk/renesas/clk-rcar-gen3.c
+++ b/drivers/clk/renesas/clk-rcar-gen3.c
@@ -68,7 +68,7 @@ static int gen3_clk_get_parent(struct gen3_clk_priv *priv, struct clk *clk,
 		if (ret)
 			return ret;
 
-		if (core->type == CLK_TYPE_GEN3_MDSEL) {
+		if (core->type == CLK_TYPE_GEN3_MDSEL || core->type == CLK_TYPE_GEN4_MDSEL) {
 			shift = priv->cpg_mode & BIT(core->offset) ? 0 : 16;
 			parent->dev = clk->dev;
 			parent->id = core->parent >> shift;
@@ -318,6 +318,8 @@ static u64 gen3_clk_get_rate64(struct clk *clk)
 						"FIXED");
 
 	case CLK_TYPE_GEN3_MDSEL:
+		fallthrough;
+	case CLK_TYPE_GEN4_MDSEL:
 		shift = priv->cpg_mode & BIT(core->offset) ? 0 : 16;
 		div = (core->div >> shift) & 0xffff;
 		rate = gen3_clk_get_rate64(&parent) / div;

-- 
2.49.0

[PATCH v3 2/2] renesas: Renesas R-Car Gen4 watchdog driver

[Shmuel Leib Melamud]

Add support of Renesas R-Car Gen4 watchdog timer. Timeouts up to
8184.0s are supported (CKS1 register is not involved). The watchdog
uses the clock of type CLK_TYPE_GEN4_MDSEL.

The timeout is set in
dts/upstream/src/arm64/renesas/r8a779f0-spider-cpu.dtsi section &rwdt.

This driver is based on upstream linux commit:
e70140ba0d2b("Get rid of 'remove_new' relic from platform driver struct")

Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>
---
 drivers/watchdog/Kconfig       |   8 ++
 drivers/watchdog/Makefile      |   1 +
 drivers/watchdog/renesas_wdt.c | 182 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 191 insertions(+)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1bb67f5035231df9f6ce01adb08d074855393143..a0f2948335f9c3b74b7823f039e578ff4030e97c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -335,6 +335,14 @@ config WDT_K3_RTI_FW_FILE
 
 endif
 
+config WDT_RENESAS
+	bool "Renesas watchdog timer support"
+	depends on WDT && R8A779F0
+	select CLK
+	select CLK_RENESAS
+	help
+	  Enables Renesas SoC R8A779F0 watchdog timer support.
+
 config WDT_SANDBOX
 	bool "Enable Watchdog Timer support for Sandbox"
 	depends on SANDBOX && WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index e6bd4c587af6133c405dde6dbada8050debc781c..c4467d6e126d0c3e119bf8014b302e5b1432f541 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_WDT_MTK) += mtk_wdt.o
 obj-$(CONFIG_WDT_NPCM) += npcm_wdt.o
 obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o
 obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
+obj-$(CONFIG_WDT_RENESAS) += renesas_wdt.o
 obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.o
 obj-$(CONFIG_WDT_K3_RTI) += rti_wdt.o
 obj-$(CONFIG_WDT_SIEMENS_PMIC) += siemens_pmic_wdt.o
diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
new file mode 100644
index 0000000000000000000000000000000000000000..5dc5b0b51e0d19589ea96e7a66d4b790d377e7f8
--- /dev/null
+++ b/drivers/watchdog/renesas_wdt.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2025 Red Hat, Inc., Shmuel Leib Melamud <smelamud@redhat.com>
+
+#include <clk.h>
+#include <dm.h>
+#include <wdt.h>
+#include <asm/io.h>
+#include <dm/device_compat.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+
+#define usleep_range(a, b) udelay((b))
+
+struct rwdt {
+	u32 cnt;
+	u32 csra;
+	u32 csrb;
+};
+
+#define RWTCSRA_WOVF	BIT(4)
+#define RWTCSRA_WRFLG	BIT(5)
+#define RWTCSRA_TME		BIT(7)
+
+#define CSR_MASK	0xA5A5A500
+#define CNT_MASK	0x5A5A0000
+
+/*
+ * In probe, clk_rate is checked to be not more than 16 bit * biggest clock
+ * divider (12 bits). d is only a factor to fully utilize the WDT counter and
+ * will not exceed its 16 bits. Thus, no overflow, we stay below 32 bits.
+ */
+#define MUL_BY_CLKS_PER_SEC(p, d) \
+	DIV_ROUND_UP((d) * (p)->clk_rate, clk_divs[(p)->cks])
+
+/* d is 16 bit, clk_divs 12 bit -> no 32 bit overflow */
+#define DIV_BY_CLKS_PER_SEC(p, d) ((d) * clk_divs[(p)->cks] / (p)->clk_rate)
+
+static const unsigned int clk_divs[] = { 1, 4, 16, 32, 64, 128, 1024, 4096 };
+
+struct rwdt_priv {
+	struct rwdt __iomem *wdt;
+	unsigned long clk_rate;
+	u8 cks;
+	struct clk clk;
+};
+
+static void rwdt_wait_cycles(struct rwdt_priv *priv, unsigned int cycles)
+{
+	unsigned int delay;
+
+	delay = DIV_ROUND_UP(cycles * 1000000, priv->clk_rate);
+
+	usleep_range(delay, 2 * delay);
+}
+
+static int rwdt_start(struct udevice *dev, u64 timeout, ulong flags)
+{
+	struct rwdt_priv *priv = dev_get_priv(dev);
+	u64 max_timeout;
+	u8 val;
+
+	max_timeout = DIV_BY_CLKS_PER_SEC(priv, 65536);
+	timeout = min(max_timeout, timeout / 1000);
+
+	/* Stop the timer before we modify any register */
+	val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
+	writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
+	/* Delay 2 cycles before setting watchdog counter */
+	rwdt_wait_cycles(priv, 2);
+
+	while (readb_relaxed(&priv->wdt->csra) & RWTCSRA_WRFLG)
+		cpu_relax();
+
+	writel_relaxed((65536 - MUL_BY_CLKS_PER_SEC(priv, timeout)) | CNT_MASK,
+		       &priv->wdt->cnt);
+
+	writel_relaxed(priv->cks | RWTCSRA_TME | CSR_MASK, &priv->wdt->csra);
+
+	return 0;
+}
+
+static int rwdt_stop(struct udevice *dev)
+{
+	struct rwdt_priv *priv = dev_get_priv(dev);
+
+	writel_relaxed(priv->cks | CSR_MASK, &priv->wdt->csra);
+
+	return 0;
+}
+
+static int rwdt_reset(struct udevice *dev)
+{
+	struct rwdt_priv *priv = dev_get_priv(dev);
+	u8 val;
+
+	/* Stop the timer before we modify any register */
+	val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
+	writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
+	/* Delay 2 cycles before setting watchdog counter */
+	rwdt_wait_cycles(priv, 2);
+
+	writel_relaxed(0xffff | CNT_MASK, &priv->wdt->cnt);
+	/* smallest divider to reboot soon */
+	writel_relaxed(0 | CSR_MASK, &priv->wdt->csra);
+
+	readb_poll_timeout(&priv->wdt->csra, val, !(val & RWTCSRA_WRFLG), 100);
+
+	writel_relaxed(RWTCSRA_TME | CSR_MASK, &priv->wdt->csra);
+
+	/* wait 2 cycles, so watchdog will trigger */
+	rwdt_wait_cycles(priv, 2);
+
+	return 0;
+}
+
+static int rwdt_probe(struct udevice *dev)
+{
+	struct rwdt_priv *priv = dev_get_priv(dev);
+	unsigned long clks_per_sec;
+	int ret, i;
+
+	priv->wdt = dev_remap_addr(dev);
+	if (!priv->wdt)
+		return -EINVAL;
+
+	ret = clk_get_by_index(dev, 0, &priv->clk);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_enable(&priv->clk);
+	if (ret)
+		return ret;
+
+	priv->clk_rate = clk_get_rate(&priv->clk);
+	if (!priv->clk_rate) {
+		ret = -ENOENT;
+		goto err_clk_disable;
+	}
+
+	for (i = ARRAY_SIZE(clk_divs) - 1; i >= 0; i--) {
+		clks_per_sec = priv->clk_rate / clk_divs[i];
+		if (clks_per_sec && clks_per_sec < 65536) {
+			priv->cks = i;
+			break;
+		}
+	}
+
+	/* can't find a suitable clock divider */
+	if (i < 0) {
+		ret = -ERANGE;
+		goto err_clk_disable;
+	}
+
+	return 0;
+
+err_clk_disable:
+	clk_disable(&priv->clk);
+
+	return ret;
+}
+
+static const struct wdt_ops rwdt_ops = {
+	.start = rwdt_start,
+	.reset = rwdt_reset,
+	.stop = rwdt_stop,
+};
+
+static const struct udevice_id rwdt_ids[] = {
+	{ .compatible = "renesas,rcar-gen2-wdt" },
+	{ .compatible = "renesas,rcar-gen3-wdt" },
+	{ .compatible = "renesas,rcar-gen4-wdt" },
+	{}
+};
+
+U_BOOT_DRIVER(wdt_renesas) = {
+	.name = "wdt_renesas",
+	.id = UCLASS_WDT,
+	.of_match = rwdt_ids,
+	.ops = &rwdt_ops,
+	.probe	= rwdt_probe,
+	.priv_auto = sizeof(struct rwdt_priv),
+};

-- 
2.49.0

[PATCH v3 2/2] renesas: Renesas R-Car Gen4 watchdog driver

[Shmuel Leib Melamud via B4 Relay]

From: Shmuel Leib Melamud <smelamud@redhat.com>

Add support of Renesas R-Car Gen4 watchdog timer. Timeouts up to
8184.0s are supported (CKS1 register is not involved). The watchdog
uses the clock of type CLK_TYPE_GEN4_MDSEL.

The timeout is set in
dts/upstream/src/arm64/renesas/r8a779f0-spider-cpu.dtsi section &rwdt.

This driver is based on upstream linux commit:
e70140ba0d2b("Get rid of 'remove_new' relic from platform driver struct")

Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>
---
 drivers/watchdog/Kconfig       |   8 ++
 drivers/watchdog/Makefile      |   1 +
 drivers/watchdog/renesas_wdt.c | 182 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 191 insertions(+)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1bb67f5035231df9f6ce01adb08d074855393143..a0f2948335f9c3b74b7823f039e578ff4030e97c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -335,6 +335,14 @@ config WDT_K3_RTI_FW_FILE
 
 endif
 
+config WDT_RENESAS
+	bool "Renesas watchdog timer support"
+	depends on WDT && R8A779F0
+	select CLK
+	select CLK_RENESAS
+	help
+	  Enables Renesas SoC R8A779F0 watchdog timer support.
+
 config WDT_SANDBOX
 	bool "Enable Watchdog Timer support for Sandbox"
 	depends on SANDBOX && WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index e6bd4c587af6133c405dde6dbada8050debc781c..c4467d6e126d0c3e119bf8014b302e5b1432f541 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_WDT_MTK) += mtk_wdt.o
 obj-$(CONFIG_WDT_NPCM) += npcm_wdt.o
 obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o
 obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
+obj-$(CONFIG_WDT_RENESAS) += renesas_wdt.o
 obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.o
 obj-$(CONFIG_WDT_K3_RTI) += rti_wdt.o
 obj-$(CONFIG_WDT_SIEMENS_PMIC) += siemens_pmic_wdt.o
diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
new file mode 100644
index 0000000000000000000000000000000000000000..5dc5b0b51e0d19589ea96e7a66d4b790d377e7f8
--- /dev/null
+++ b/drivers/watchdog/renesas_wdt.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2025 Red Hat, Inc., Shmuel Leib Melamud <smelamud@redhat.com>
+
+#include <clk.h>
+#include <dm.h>
+#include <wdt.h>
+#include <asm/io.h>
+#include <dm/device_compat.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+
+#define usleep_range(a, b) udelay((b))
+
+struct rwdt {
+	u32 cnt;
+	u32 csra;
+	u32 csrb;
+};
+
+#define RWTCSRA_WOVF	BIT(4)
+#define RWTCSRA_WRFLG	BIT(5)
+#define RWTCSRA_TME		BIT(7)
+
+#define CSR_MASK	0xA5A5A500
+#define CNT_MASK	0x5A5A0000
+
+/*
+ * In probe, clk_rate is checked to be not more than 16 bit * biggest clock
+ * divider (12 bits). d is only a factor to fully utilize the WDT counter and
+ * will not exceed its 16 bits. Thus, no overflow, we stay below 32 bits.
+ */
+#define MUL_BY_CLKS_PER_SEC(p, d) \
+	DIV_ROUND_UP((d) * (p)->clk_rate, clk_divs[(p)->cks])
+
+/* d is 16 bit, clk_divs 12 bit -> no 32 bit overflow */
+#define DIV_BY_CLKS_PER_SEC(p, d) ((d) * clk_divs[(p)->cks] / (p)->clk_rate)
+
+static const unsigned int clk_divs[] = { 1, 4, 16, 32, 64, 128, 1024, 4096 };
+
+struct rwdt_priv {
+	struct rwdt __iomem *wdt;
+	unsigned long clk_rate;
+	u8 cks;
+	struct clk clk;
+};
+
+static void rwdt_wait_cycles(struct rwdt_priv *priv, unsigned int cycles)
+{
+	unsigned int delay;
+
+	delay = DIV_ROUND_UP(cycles * 1000000, priv->clk_rate);
+
+	usleep_range(delay, 2 * delay);
+}
+
+static int rwdt_start(struct udevice *dev, u64 timeout, ulong flags)
+{
+	struct rwdt_priv *priv = dev_get_priv(dev);
+	u64 max_timeout;
+	u8 val;
+
+	max_timeout = DIV_BY_CLKS_PER_SEC(priv, 65536);
+	timeout = min(max_timeout, timeout / 1000);
+
+	/* Stop the timer before we modify any register */
+	val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
+	writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
+	/* Delay 2 cycles before setting watchdog counter */
+	rwdt_wait_cycles(priv, 2);
+
+	while (readb_relaxed(&priv->wdt->csra) & RWTCSRA_WRFLG)
+		cpu_relax();
+
+	writel_relaxed((65536 - MUL_BY_CLKS_PER_SEC(priv, timeout)) | CNT_MASK,
+		       &priv->wdt->cnt);
+
+	writel_relaxed(priv->cks | RWTCSRA_TME | CSR_MASK, &priv->wdt->csra);
+
+	return 0;
+}
+
+static int rwdt_stop(struct udevice *dev)
+{
+	struct rwdt_priv *priv = dev_get_priv(dev);
+
+	writel_relaxed(priv->cks | CSR_MASK, &priv->wdt->csra);
+
+	return 0;
+}
+
+static int rwdt_reset(struct udevice *dev)
+{
+	struct rwdt_priv *priv = dev_get_priv(dev);
+	u8 val;
+
+	/* Stop the timer before we modify any register */
+	val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
+	writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
+	/* Delay 2 cycles before setting watchdog counter */
+	rwdt_wait_cycles(priv, 2);
+
+	writel_relaxed(0xffff | CNT_MASK, &priv->wdt->cnt);
+	/* smallest divider to reboot soon */
+	writel_relaxed(0 | CSR_MASK, &priv->wdt->csra);
+
+	readb_poll_timeout(&priv->wdt->csra, val, !(val & RWTCSRA_WRFLG), 100);
+
+	writel_relaxed(RWTCSRA_TME | CSR_MASK, &priv->wdt->csra);
+
+	/* wait 2 cycles, so watchdog will trigger */
+	rwdt_wait_cycles(priv, 2);
+
+	return 0;
+}
+
+static int rwdt_probe(struct udevice *dev)
+{
+	struct rwdt_priv *priv = dev_get_priv(dev);
+	unsigned long clks_per_sec;
+	int ret, i;
+
+	priv->wdt = dev_remap_addr(dev);
+	if (!priv->wdt)
+		return -EINVAL;
+
+	ret = clk_get_by_index(dev, 0, &priv->clk);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_enable(&priv->clk);
+	if (ret)
+		return ret;
+
+	priv->clk_rate = clk_get_rate(&priv->clk);
+	if (!priv->clk_rate) {
+		ret = -ENOENT;
+		goto err_clk_disable;
+	}
+
+	for (i = ARRAY_SIZE(clk_divs) - 1; i >= 0; i--) {
+		clks_per_sec = priv->clk_rate / clk_divs[i];
+		if (clks_per_sec && clks_per_sec < 65536) {
+			priv->cks = i;
+			break;
+		}
+	}
+
+	/* can't find a suitable clock divider */
+	if (i < 0) {
+		ret = -ERANGE;
+		goto err_clk_disable;
+	}
+
+	return 0;
+
+err_clk_disable:
+	clk_disable(&priv->clk);
+
+	return ret;
+}
+
+static const struct wdt_ops rwdt_ops = {
+	.start = rwdt_start,
+	.reset = rwdt_reset,
+	.stop = rwdt_stop,
+};
+
+static const struct udevice_id rwdt_ids[] = {
+	{ .compatible = "renesas,rcar-gen2-wdt" },
+	{ .compatible = "renesas,rcar-gen3-wdt" },
+	{ .compatible = "renesas,rcar-gen4-wdt" },
+	{}
+};
+
+U_BOOT_DRIVER(wdt_renesas) = {
+	.name = "wdt_renesas",
+	.id = UCLASS_WDT,
+	.of_match = rwdt_ids,
+	.ops = &rwdt_ops,
+	.probe	= rwdt_probe,
+	.priv_auto = sizeof(struct rwdt_priv),
+};

-- 
2.49.0

Re: [PATCH v3 1/2] renesas: Handle CLK_TYPE_GEN4_MDSEL in gen3_clk_get_rate64()

[Mattijs Korpershoek]

Hi Shmuel,

Thank you for the patch.

On Tue, Jun 03, 2025 at 06:06, Shmuel Leib Melamud via B4 Relay <devnull+smelamud.redhat.com@kernel.org> wrote:

> From: Shmuel Leib Melamud <smelamud@redhat.com>
>
> Add support of CLK_TYPE_GEN4_MDSEL clock type to gen3_clk_get_rate64()
> function. In particular, this type of clock is used by Renesas R-Car
> Gen4 watchdog. It operates similarly to CLK_TYPE_GEN3_MDSEL clock.
>
> Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

nitpick: for a future submission, consider looking at the commit title
for a particular file to have the same subject.

$ git log --oneline -- drivers/clk/renesas/clk-rcar-gen3.c

Shows that most titles start with "clk: renesas", not "renesas:"

> ---
>  drivers/clk/renesas/clk-rcar-gen3.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/renesas/clk-rcar-gen3.c b/drivers/clk/renesas/clk-rcar-gen3.c
> index 375cc4a4930873ad0d5509c19ad04a0ea5545aa0..5745acf4023c9114f6fa13b5e4baa306c5b57d33 100644
> --- a/drivers/clk/renesas/clk-rcar-gen3.c
> +++ b/drivers/clk/renesas/clk-rcar-gen3.c
> @@ -68,7 +68,7 @@ static int gen3_clk_get_parent(struct gen3_clk_priv *priv, struct clk *clk,
>  		if (ret)
>  			return ret;
>  
> -		if (core->type == CLK_TYPE_GEN3_MDSEL) {
> +		if (core->type == CLK_TYPE_GEN3_MDSEL || core->type == CLK_TYPE_GEN4_MDSEL) {
>  			shift = priv->cpg_mode & BIT(core->offset) ? 0 : 16;
>  			parent->dev = clk->dev;
>  			parent->id = core->parent >> shift;
> @@ -318,6 +318,8 @@ static u64 gen3_clk_get_rate64(struct clk *clk)
>  						"FIXED");
>  
>  	case CLK_TYPE_GEN3_MDSEL:
> +		fallthrough;
> +	case CLK_TYPE_GEN4_MDSEL:
>  		shift = priv->cpg_mode & BIT(core->offset) ? 0 : 16;
>  		div = (core->div >> shift) & 0xffff;
>  		rate = gen3_clk_get_rate64(&parent) / div;
>
> -- 
> 2.49.0

Re: [PATCH v3 2/2] renesas: Renesas R-Car Gen4 watchdog driver

[Mattijs Korpershoek]

Hi Shmuel,

Thank you for the patch.

On Tue, Jun 03, 2025 at 06:06, Shmuel Leib Melamud via B4 Relay <devnull+smelamud.redhat.com@kernel.org> wrote:

> From: Shmuel Leib Melamud <smelamud@redhat.com>
>
> Add support of Renesas R-Car Gen4 watchdog timer. Timeouts up to
> 8184.0s are supported (CKS1 register is not involved). The watchdog
> uses the clock of type CLK_TYPE_GEN4_MDSEL.
>
> The timeout is set in
> dts/upstream/src/arm64/renesas/r8a779f0-spider-cpu.dtsi section &rwdt.
>
> This driver is based on upstream linux commit:
> e70140ba0d2b("Get rid of 'remove_new' relic from platform driver struct")
>
> Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>
> ---
>  drivers/watchdog/Kconfig       |   8 ++
>  drivers/watchdog/Makefile      |   1 +
>  drivers/watchdog/renesas_wdt.c | 182 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 191 insertions(+)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 1bb67f5035231df9f6ce01adb08d074855393143..a0f2948335f9c3b74b7823f039e578ff4030e97c 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -335,6 +335,14 @@ config WDT_K3_RTI_FW_FILE
>  
>  endif
>  
> +config WDT_RENESAS
> +	bool "Renesas watchdog timer support"
> +	depends on WDT && R8A779F0
> +	select CLK
> +	select CLK_RENESAS
> +	help
> +	  Enables Renesas SoC R8A779F0 watchdog timer support.
> +
>  config WDT_SANDBOX
>  	bool "Enable Watchdog Timer support for Sandbox"
>  	depends on SANDBOX && WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index e6bd4c587af6133c405dde6dbada8050debc781c..c4467d6e126d0c3e119bf8014b302e5b1432f541 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_WDT_MTK) += mtk_wdt.o
>  obj-$(CONFIG_WDT_NPCM) += npcm_wdt.o
>  obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o
>  obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
> +obj-$(CONFIG_WDT_RENESAS) += renesas_wdt.o
>  obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.o
>  obj-$(CONFIG_WDT_K3_RTI) += rti_wdt.o
>  obj-$(CONFIG_WDT_SIEMENS_PMIC) += siemens_pmic_wdt.o
> diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5dc5b0b51e0d19589ea96e7a66d4b790d377e7f8
> --- /dev/null
> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2025 Red Hat, Inc., Shmuel Leib Melamud <smelamud@redhat.com>
> +
> +#include <clk.h>
> +#include <dm.h>
> +#include <wdt.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +
> +#define usleep_range(a, b) udelay((b))
> +
> +struct rwdt {
> +	u32 cnt;
> +	u32 csra;
> +	u32 csrb;
> +};
> +
> +#define RWTCSRA_WOVF	BIT(4)
> +#define RWTCSRA_WRFLG	BIT(5)
> +#define RWTCSRA_TME		BIT(7)
> +
> +#define CSR_MASK	0xA5A5A500
> +#define CNT_MASK	0x5A5A0000
> +
> +/*
> + * In probe, clk_rate is checked to be not more than 16 bit * biggest clock
> + * divider (12 bits). d is only a factor to fully utilize the WDT counter and
> + * will not exceed its 16 bits. Thus, no overflow, we stay below 32 bits.
> + */
> +#define MUL_BY_CLKS_PER_SEC(p, d) \
> +	DIV_ROUND_UP((d) * (p)->clk_rate, clk_divs[(p)->cks])
> +
> +/* d is 16 bit, clk_divs 12 bit -> no 32 bit overflow */
> +#define DIV_BY_CLKS_PER_SEC(p, d) ((d) * clk_divs[(p)->cks] / (p)->clk_rate)
> +
> +static const unsigned int clk_divs[] = { 1, 4, 16, 32, 64, 128, 1024, 4096 };
> +
> +struct rwdt_priv {
> +	struct rwdt __iomem *wdt;
> +	unsigned long clk_rate;
> +	u8 cks;
> +	struct clk clk;
> +};
> +
> +static void rwdt_wait_cycles(struct rwdt_priv *priv, unsigned int cycles)
> +{
> +	unsigned int delay;
> +
> +	delay = DIV_ROUND_UP(cycles * 1000000, priv->clk_rate);
> +
> +	usleep_range(delay, 2 * delay);
> +}
> +
> +static int rwdt_start(struct udevice *dev, u64 timeout, ulong flags)
> +{
> +	struct rwdt_priv *priv = dev_get_priv(dev);
> +	u64 max_timeout;
> +	u8 val;
> +
> +	max_timeout = DIV_BY_CLKS_PER_SEC(priv, 65536);

Why 65536. This number is used here ...

> +	timeout = min(max_timeout, timeout / 1000);
> +
> +	/* Stop the timer before we modify any register */
> +	val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
> +	writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
> +	/* Delay 2 cycles before setting watchdog counter */
> +	rwdt_wait_cycles(priv, 2);
> +
> +	while (readb_relaxed(&priv->wdt->csra) & RWTCSRA_WRFLG)
> +		cpu_relax();
> +
> +	writel_relaxed((65536 - MUL_BY_CLKS_PER_SEC(priv, timeout)) | CNT_MASK,
> +		       &priv->wdt->cnt);

... and here. Can we move it to a define to document its meaning?

> +
> +	writel_relaxed(priv->cks | RWTCSRA_TME | CSR_MASK, &priv->wdt->csra);

I still see some subtle differences between linux driver (where a
rwdt_write() helper function is present) and this U-Boot driver but with
the rename, it's gotten much easier to review.

Thanks for the improvements !

> +
> +	return 0;
> +}
> +
> +static int rwdt_stop(struct udevice *dev)
> +{
> +	struct rwdt_priv *priv = dev_get_priv(dev);
> +
> +	writel_relaxed(priv->cks | CSR_MASK, &priv->wdt->csra);
> +
> +	return 0;
> +}
> +
> +static int rwdt_reset(struct udevice *dev)
> +{
> +	struct rwdt_priv *priv = dev_get_priv(dev);
> +	u8 val;
> +
> +	/* Stop the timer before we modify any register */
> +	val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
> +	writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
> +	/* Delay 2 cycles before setting watchdog counter */
> +	rwdt_wait_cycles(priv, 2);
> +
> +	writel_relaxed(0xffff | CNT_MASK, &priv->wdt->cnt);
> +	/* smallest divider to reboot soon */
> +	writel_relaxed(0 | CSR_MASK, &priv->wdt->csra);
> +
> +	readb_poll_timeout(&priv->wdt->csra, val, !(val & RWTCSRA_WRFLG), 100);
> +
> +	writel_relaxed(RWTCSRA_TME | CSR_MASK, &priv->wdt->csra);
> +
> +	/* wait 2 cycles, so watchdog will trigger */
> +	rwdt_wait_cycles(priv, 2);
> +
> +	return 0;
> +}
> +
> +static int rwdt_probe(struct udevice *dev)
> +{
> +	struct rwdt_priv *priv = dev_get_priv(dev);
> +	unsigned long clks_per_sec;
> +	int ret, i;
> +
> +	priv->wdt = dev_remap_addr(dev);
> +	if (!priv->wdt)
> +		return -EINVAL;
> +
> +	ret = clk_get_by_index(dev, 0, &priv->clk);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = clk_enable(&priv->clk);
> +	if (ret)
> +		return ret;
> +
> +	priv->clk_rate = clk_get_rate(&priv->clk);
> +	if (!priv->clk_rate) {
> +		ret = -ENOENT;
> +		goto err_clk_disable;
> +	}
> +
> +	for (i = ARRAY_SIZE(clk_divs) - 1; i >= 0; i--) {
> +		clks_per_sec = priv->clk_rate / clk_divs[i];
> +		if (clks_per_sec && clks_per_sec < 65536) {
> +			priv->cks = i;
> +			break;
> +		}
> +	}
> +
> +	/* can't find a suitable clock divider */
> +	if (i < 0) {
> +		ret = -ERANGE;
> +		goto err_clk_disable;
> +	}
> +
> +	return 0;
> +
> +err_clk_disable:
> +	clk_disable(&priv->clk);
> +
> +	return ret;
> +}
> +
> +static const struct wdt_ops rwdt_ops = {
> +	.start = rwdt_start,
> +	.reset = rwdt_reset,
> +	.stop = rwdt_stop,
> +};
> +
> +static const struct udevice_id rwdt_ids[] = {
> +	{ .compatible = "renesas,rcar-gen2-wdt" },
> +	{ .compatible = "renesas,rcar-gen3-wdt" },
> +	{ .compatible = "renesas,rcar-gen4-wdt" },
> +	{}
> +};
> +
> +U_BOOT_DRIVER(wdt_renesas) = {
> +	.name = "wdt_renesas",
> +	.id = UCLASS_WDT,
> +	.of_match = rwdt_ids,
> +	.ops = &rwdt_ops,
> +	.probe	= rwdt_probe,
> +	.priv_auto = sizeof(struct rwdt_priv),
> +};
>
> -- 
> 2.49.0

Re: [PATCH v3 2/2] renesas: Renesas R-Car Gen4 watchdog driver

[Shmuel Melamud]

On Wed, Jun 4, 2025 at 12:02 PM Mattijs Korpershoek
<mkorpershoek@kernel.org> wrote:
>
> Hi Shmuel,
>
> Thank you for the patch.
>
> On Tue, Jun 03, 2025 at 06:06, Shmuel Leib Melamud via B4 Relay <devnull+smelamud.redhat.com@kernel.org> wrote:
>
> > From: Shmuel Leib Melamud <smelamud@redhat.com>
> >
> > Add support of Renesas R-Car Gen4 watchdog timer. Timeouts up to
> > 8184.0s are supported (CKS1 register is not involved). The watchdog
> > uses the clock of type CLK_TYPE_GEN4_MDSEL.
> >
> > The timeout is set in
> > dts/upstream/src/arm64/renesas/r8a779f0-spider-cpu.dtsi section &rwdt.
> >
> > This driver is based on upstream linux commit:
> > e70140ba0d2b("Get rid of 'remove_new' relic from platform driver struct")
> >
> > Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>
> > ---
> >  drivers/watchdog/Kconfig       |   8 ++
> >  drivers/watchdog/Makefile      |   1 +
> >  drivers/watchdog/renesas_wdt.c | 182 +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 191 insertions(+)
> >
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 1bb67f5035231df9f6ce01adb08d074855393143..a0f2948335f9c3b74b7823f039e578ff4030e97c 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -335,6 +335,14 @@ config WDT_K3_RTI_FW_FILE
> >
> >  endif
> >
> > +config WDT_RENESAS
> > +     bool "Renesas watchdog timer support"
> > +     depends on WDT && R8A779F0
> > +     select CLK
> > +     select CLK_RENESAS
> > +     help
> > +       Enables Renesas SoC R8A779F0 watchdog timer support.
> > +
> >  config WDT_SANDBOX
> >       bool "Enable Watchdog Timer support for Sandbox"
> >       depends on SANDBOX && WDT
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index e6bd4c587af6133c405dde6dbada8050debc781c..c4467d6e126d0c3e119bf8014b302e5b1432f541 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -43,6 +43,7 @@ obj-$(CONFIG_WDT_MTK) += mtk_wdt.o
> >  obj-$(CONFIG_WDT_NPCM) += npcm_wdt.o
> >  obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o
> >  obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
> > +obj-$(CONFIG_WDT_RENESAS) += renesas_wdt.o
> >  obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.o
> >  obj-$(CONFIG_WDT_K3_RTI) += rti_wdt.o
> >  obj-$(CONFIG_WDT_SIEMENS_PMIC) += siemens_pmic_wdt.o
> > diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..5dc5b0b51e0d19589ea96e7a66d4b790d377e7f8
> > --- /dev/null
> > +++ b/drivers/watchdog/renesas_wdt.c
> > @@ -0,0 +1,182 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +// Copyright 2025 Red Hat, Inc., Shmuel Leib Melamud <smelamud@redhat.com>
> > +
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <wdt.h>
> > +#include <asm/io.h>
> > +#include <dm/device_compat.h>
> > +#include <linux/delay.h>
> > +#include <linux/iopoll.h>
> > +
> > +#define usleep_range(a, b) udelay((b))
> > +
> > +struct rwdt {
> > +     u32 cnt;
> > +     u32 csra;
> > +     u32 csrb;
> > +};
> > +
> > +#define RWTCSRA_WOVF BIT(4)
> > +#define RWTCSRA_WRFLG        BIT(5)
> > +#define RWTCSRA_TME          BIT(7)
> > +
> > +#define CSR_MASK     0xA5A5A500
> > +#define CNT_MASK     0x5A5A0000
> > +
> > +/*
> > + * In probe, clk_rate is checked to be not more than 16 bit * biggest clock
> > + * divider (12 bits). d is only a factor to fully utilize the WDT counter and
> > + * will not exceed its 16 bits. Thus, no overflow, we stay below 32 bits.
> > + */
> > +#define MUL_BY_CLKS_PER_SEC(p, d) \
> > +     DIV_ROUND_UP((d) * (p)->clk_rate, clk_divs[(p)->cks])
> > +
> > +/* d is 16 bit, clk_divs 12 bit -> no 32 bit overflow */
> > +#define DIV_BY_CLKS_PER_SEC(p, d) ((d) * clk_divs[(p)->cks] / (p)->clk_rate)
> > +
> > +static const unsigned int clk_divs[] = { 1, 4, 16, 32, 64, 128, 1024, 4096 };
> > +
> > +struct rwdt_priv {
> > +     struct rwdt __iomem *wdt;
> > +     unsigned long clk_rate;
> > +     u8 cks;
> > +     struct clk clk;
> > +};
> > +
> > +static void rwdt_wait_cycles(struct rwdt_priv *priv, unsigned int cycles)
> > +{
> > +     unsigned int delay;
> > +
> > +     delay = DIV_ROUND_UP(cycles * 1000000, priv->clk_rate);
> > +
> > +     usleep_range(delay, 2 * delay);
> > +}
> > +
> > +static int rwdt_start(struct udevice *dev, u64 timeout, ulong flags)
> > +{
> > +     struct rwdt_priv *priv = dev_get_priv(dev);
> > +     u64 max_timeout;
> > +     u8 val;
> > +
> > +     max_timeout = DIV_BY_CLKS_PER_SEC(priv, 65536);
>
> Why 65536. This number is used here ...

65536 here is the maximal value of the counter plus 1. If we divide it
by the number of impulses per second, we get the maximal timeout in
seconds.

> > +     timeout = min(max_timeout, timeout / 1000);
> > +
> > +     /* Stop the timer before we modify any register */
> > +     val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
> > +     writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
> > +     /* Delay 2 cycles before setting watchdog counter */
> > +     rwdt_wait_cycles(priv, 2);
> > +
> > +     while (readb_relaxed(&priv->wdt->csra) & RWTCSRA_WRFLG)
> > +             cpu_relax();
> > +
> > +     writel_relaxed((65536 - MUL_BY_CLKS_PER_SEC(priv, timeout)) | CNT_MASK,
> > +                    &priv->wdt->cnt);
>
> ... and here. Can we move it to a define to document its meaning?

65536 here is for negation. Since the counter is incremented on every
impulse, the timeout is the number of impulses that are left till
overflow of the counter.

In both cases it is the maximal value of the counter, so maybe it
makes sense to add a define for it.

> > +
> > +     writel_relaxed(priv->cks | RWTCSRA_TME | CSR_MASK, &priv->wdt->csra);
>
> I still see some subtle differences between linux driver (where a
> rwdt_write() helper function is present) and this U-Boot driver but with
> the rename, it's gotten much easier to review.
>
> Thanks for the improvements !
>
> > +
> > +     return 0;
> > +}
> > +
> > +static int rwdt_stop(struct udevice *dev)
> > +{
> > +     struct rwdt_priv *priv = dev_get_priv(dev);
> > +
> > +     writel_relaxed(priv->cks | CSR_MASK, &priv->wdt->csra);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rwdt_reset(struct udevice *dev)
> > +{
> > +     struct rwdt_priv *priv = dev_get_priv(dev);
> > +     u8 val;
> > +
> > +     /* Stop the timer before we modify any register */
> > +     val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
> > +     writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
> > +     /* Delay 2 cycles before setting watchdog counter */
> > +     rwdt_wait_cycles(priv, 2);
> > +
> > +     writel_relaxed(0xffff | CNT_MASK, &priv->wdt->cnt);
> > +     /* smallest divider to reboot soon */
> > +     writel_relaxed(0 | CSR_MASK, &priv->wdt->csra);
> > +
> > +     readb_poll_timeout(&priv->wdt->csra, val, !(val & RWTCSRA_WRFLG), 100);
> > +
> > +     writel_relaxed(RWTCSRA_TME | CSR_MASK, &priv->wdt->csra);
> > +
> > +     /* wait 2 cycles, so watchdog will trigger */
> > +     rwdt_wait_cycles(priv, 2);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rwdt_probe(struct udevice *dev)
> > +{
> > +     struct rwdt_priv *priv = dev_get_priv(dev);
> > +     unsigned long clks_per_sec;
> > +     int ret, i;
> > +
> > +     priv->wdt = dev_remap_addr(dev);
> > +     if (!priv->wdt)
> > +             return -EINVAL;
> > +
> > +     ret = clk_get_by_index(dev, 0, &priv->clk);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = clk_enable(&priv->clk);
> > +     if (ret)
> > +             return ret;
> > +
> > +     priv->clk_rate = clk_get_rate(&priv->clk);
> > +     if (!priv->clk_rate) {
> > +             ret = -ENOENT;
> > +             goto err_clk_disable;
> > +     }
> > +
> > +     for (i = ARRAY_SIZE(clk_divs) - 1; i >= 0; i--) {
> > +             clks_per_sec = priv->clk_rate / clk_divs[i];
> > +             if (clks_per_sec && clks_per_sec < 65536) {
> > +                     priv->cks = i;
> > +                     break;
> > +             }
> > +     }
> > +
> > +     /* can't find a suitable clock divider */
> > +     if (i < 0) {
> > +             ret = -ERANGE;
> > +             goto err_clk_disable;
> > +     }
> > +
> > +     return 0;
> > +
> > +err_clk_disable:
> > +     clk_disable(&priv->clk);
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct wdt_ops rwdt_ops = {
> > +     .start = rwdt_start,
> > +     .reset = rwdt_reset,
> > +     .stop = rwdt_stop,
> > +};
> > +
> > +static const struct udevice_id rwdt_ids[] = {
> > +     { .compatible = "renesas,rcar-gen2-wdt" },
> > +     { .compatible = "renesas,rcar-gen3-wdt" },
> > +     { .compatible = "renesas,rcar-gen4-wdt" },
> > +     {}
> > +};
> > +
> > +U_BOOT_DRIVER(wdt_renesas) = {
> > +     .name = "wdt_renesas",
> > +     .id = UCLASS_WDT,
> > +     .of_match = rwdt_ids,
> > +     .ops = &rwdt_ops,
> > +     .probe  = rwdt_probe,
> > +     .priv_auto = sizeof(struct rwdt_priv),
> > +};
> >
> > --
> > 2.49.0
>

Re: [PATCH v3 2/2] renesas: Renesas R-Car Gen4 watchdog driver

[Mattijs Korpershoek]

Hi Shmuel,

On jeu., juin 05, 2025 at 06:33, Shmuel Melamud <smelamud@redhat.com> wrote:

> On Wed, Jun 4, 2025 at 12:02 PM Mattijs Korpershoek
> <mkorpershoek@kernel.org> wrote:
>>
>> Hi Shmuel,
>>
>> Thank you for the patch.
>>
>> On Tue, Jun 03, 2025 at 06:06, Shmuel Leib Melamud via B4 Relay <devnull+smelamud.redhat.com@kernel.org> wrote:
>>
>> > From: Shmuel Leib Melamud <smelamud@redhat.com>

[...]

>> >
>> > +
>> > +static int rwdt_start(struct udevice *dev, u64 timeout, ulong flags)
>> > +{
>> > +     struct rwdt_priv *priv = dev_get_priv(dev);
>> > +     u64 max_timeout;
>> > +     u8 val;
>> > +
>> > +     max_timeout = DIV_BY_CLKS_PER_SEC(priv, 65536);
>>
>> Why 65536. This number is used here ...
>
> 65536 here is the maximal value of the counter plus 1. If we divide it
> by the number of impulses per second, we get the maximal timeout in
> seconds.
>
>> > +     timeout = min(max_timeout, timeout / 1000);
>> > +
>> > +     /* Stop the timer before we modify any register */
>> > +     val = readb_relaxed(&priv->wdt->csra) & ~RWTCSRA_TME;
>> > +     writel_relaxed(val | CSR_MASK, &priv->wdt->csra);
>> > +     /* Delay 2 cycles before setting watchdog counter */
>> > +     rwdt_wait_cycles(priv, 2);
>> > +
>> > +     while (readb_relaxed(&priv->wdt->csra) & RWTCSRA_WRFLG)
>> > +             cpu_relax();
>> > +
>> > +     writel_relaxed((65536 - MUL_BY_CLKS_PER_SEC(priv, timeout)) | CNT_MASK,
>> > +                    &priv->wdt->cnt);
>>
>> ... and here. Can we move it to a define to document its meaning?
>
> 65536 here is for negation. Since the counter is incremented on every
> impulse, the timeout is the number of impulses that are left till
> overflow of the counter.
>
> In both cases it is the maximal value of the counter, so maybe it
> makes sense to add a define for it.

Yes, please move this to a #define. With this constant moved to a
#define, you can add the following to v4 patch:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

Thanks,
Mattijs

>
>> > +

Re: [PATCH v3 1/2] renesas: Handle CLK_TYPE_GEN4_MDSEL in gen3_clk_get_rate64()

[Marek Vasut]

On 6/3/25 5:06 AM, Shmuel Leib Melamud via B4 Relay wrote:
> From: Shmuel Leib Melamud <smelamud@redhat.com>
> 
> Add support of CLK_TYPE_GEN4_MDSEL clock type to gen3_clk_get_rate64()
> function. In particular, this type of clock is used by Renesas R-Car
> Gen4 watchdog. It operates similarly to CLK_TYPE_GEN3_MDSEL clock.
> 
> Signed-off-by: Shmuel Leib Melamud <smelamud@redhat.com>

Reviewed-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Thank you

Re: [PATCH v3 2/2] renesas: Renesas R-Car Gen4 watchdog driver

[Marek Vasut]

On 6/3/25 5:06 AM, Shmuel Leib Melamud via B4 Relay wrote:

[...]

> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2025 Red Hat, Inc., Shmuel Leib Melamud <smelamud@redhat.com>
> +
> +#include <clk.h>
> +#include <dm.h>
> +#include <wdt.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>

Nitpick, keep the list sorted.

> +#define usleep_range(a, b) udelay((b))

Shouldn't this be udelay((a)) , the shorter (a) delay should be 
sufficient in all cases, shouldn't it ?

[...]

> +static int rwdt_probe(struct udevice *dev)
> +{
> +	struct rwdt_priv *priv = dev_get_priv(dev);
> +	unsigned long clks_per_sec;
> +	int ret, i;
> +
> +	priv->wdt = dev_remap_addr(dev);
> +	if (!priv->wdt)
> +		return -EINVAL;
> +
> +	ret = clk_get_by_index(dev, 0, &priv->clk);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = clk_enable(&priv->clk);
> +	if (ret)
> +		return ret;
> +
> +	priv->clk_rate = clk_get_rate(&priv->clk);
> +	if (!priv->clk_rate) {
> +		ret = -ENOENT;
> +		goto err_clk_disable;
> +	}

This loop below could use a code comment clarifying what this does.

> +	for (i = ARRAY_SIZE(clk_divs) - 1; i >= 0; i--) {
> +		clks_per_sec = priv->clk_rate / clk_divs[i];
> +		if (clks_per_sec && clks_per_sec < 65536) {
> +			priv->cks = i;
> +			break;
> +		}
> +	}

A few basic nitpicks, very nice otherwise, thanks !