linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function
@ 2025-05-20 20:01 Alexey Charkov
  2025-05-20 20:01 ` [PATCH v4 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML Alexey Charkov
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Alexey Charkov @ 2025-05-20 20:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	Guenter Roeck
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-watchdog,
	Alexey Charkov

Add named defines for all registers and bits in timer-vt8500.
Move the system events timer from channel 0 to channel 1 when enough
information is provided by the device tree (i.e. more than one IRQ).
Use channel 0 for the system watchdog

Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
Changes in v4:
- Changed the watchdog driver to use auxiliary bus instead of platform
  (thanks Guenter)
- Split out the register definitions into a header file shared between
  the clocksource driver and the watchdog driver
- Replaced -1UL with U32_MAX in watchdog max_heartbeat_ms to make builds
  on non-32bit platforms happier
- Link to v3: https://lore.kernel.org/r/20250515-vt8500-timer-updates-v3-0-2197a1b062bd@gmail.com

Changes in v3:
- Dropped the DTS patch already applied by Krzysztof
- Rebased onto v6.15-rc5 as requested by Daniel
- Split out the watchdog code into a dedicated platform driver, like
  timer-gxp does (thanks Daniel)
- Link to v2: https://lore.kernel.org/r/20250507-vt8500-timer-updates-v2-0-65e5d1b0855e@gmail.com

Changes in v2:
- Included the previously reviewed binding change that is directly related
  to this series as the first patch here (thanks Krzysztof)
- Created a separate config symbol for the watchdog function to let users
  build a kernel without forcing watchdog functionality upon them
  (thanks Krzysztof)
- Link to the previous binding submission: https://lore.kernel.org/all/20250506-via_vt8500_timer_binding-v3-1-88450907503f@gmail.com/
- Link to v1: https://lore.kernel.org/r/20250507-vt8500-timer-updates-v1-0-6b76f7f340a6@gmail.com

---
Alexey Charkov (4):
      dt-bindings: timer: via,vt8500-timer: Convert to YAML
      clocksource/drivers/timer-vt8500: Add defines for magic constants
      clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
      watchdog: Add support for VIA/WonderMedia SoC watchdog functionality

 .../devicetree/bindings/timer/via,vt8500-timer.txt |  15 ---
 .../bindings/timer/via,vt8500-timer.yaml           |  51 +++++++
 MAINTAINERS                                        |   3 +
 drivers/clocksource/Kconfig                        |   1 +
 drivers/clocksource/timer-vt8500.c                 | 148 ++++++++++++++++-----
 drivers/watchdog/Kconfig                           |  15 +++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/vt8500-wdt.c                      |  80 +++++++++++
 include/linux/vt8500-timer.h                       |  48 +++++++
 9 files changed, 314 insertions(+), 48 deletions(-)
---
base-commit: 088d13246a4672bc03aec664675138e3f5bff68c
change-id: 20250506-vt8500-timer-updates-44a0d22cd720

Best regards,
-- 
Alexey Charkov <alchark@gmail.com>



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

* [PATCH v4 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML
  2025-05-20 20:01 [PATCH v4 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
@ 2025-05-20 20:01 ` Alexey Charkov
  2025-05-20 20:01 ` [PATCH v4 2/4] clocksource/drivers/timer-vt8500: Add defines for magic constants Alexey Charkov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Alexey Charkov @ 2025-05-20 20:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	Guenter Roeck
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-watchdog,
	Alexey Charkov

Rewrite the textual description for the VIA/WonderMedia timer
as YAML schema.

The IP can generate up to four interrupts from four respective match
registers, so reflect that in the schema.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
 .../devicetree/bindings/timer/via,vt8500-timer.txt | 15 -------
 .../bindings/timer/via,vt8500-timer.yaml           | 51 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 3 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/via,vt8500-timer.txt b/Documentation/devicetree/bindings/timer/via,vt8500-timer.txt
deleted file mode 100644
index 901c73f0d8ef05fb54d517b807d04f80eef2e736..0000000000000000000000000000000000000000
--- a/Documentation/devicetree/bindings/timer/via,vt8500-timer.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-VIA/Wondermedia VT8500 Timer
------------------------------------------------------
-
-Required properties:
-- compatible : "via,vt8500-timer"
-- reg : Should contain 1 register ranges(address and length)
-- interrupts : interrupt for the timer
-
-Example:
-
-	timer@d8130100 {
-		compatible = "via,vt8500-timer";
-		reg = <0xd8130100 0x28>;
-		interrupts = <36>;
-	};
diff --git a/Documentation/devicetree/bindings/timer/via,vt8500-timer.yaml b/Documentation/devicetree/bindings/timer/via,vt8500-timer.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..e748149948f3140d4a158f800b91e70bf9c4f042
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/via,vt8500-timer.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/via,vt8500-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: VIA/Wondermedia VT8500 Timer
+
+description:
+  This is the timer block that is a standalone part of the system power
+  management controller on VIA/WonderMedia SoCs (VIA VT8500 and alike).
+  The hardware has a single 32-bit counter running at 3 MHz and four match
+  registers, each of which is associated with a dedicated match interrupt,
+  and the first of which can also serve as the system watchdog (if the
+  watchdog function is enabled, it will reset the system upon match instead
+  of triggering its respective interrupt)
+
+maintainers:
+  - Alexey Charkov <alchark@gmail.com>
+
+properties:
+  compatible:
+    const: via,vt8500-timer
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    items:
+      - description: Channel 0 match. Note that if the watchdog function
+          is enabled, this interrupt will not fire and the system will
+          reboot instead once the counter reaches match register 0 value
+      - description: Channel 1 match
+      - description: Channel 2 match
+      - description: Channel 3 match
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    timer@d8130100 {
+        compatible = "via,vt8500-timer";
+        reg = <0xd8130100 0x28>;
+        interrupts = <36>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3563492e4eba48b9e9389687fc9ac2a881c47ddf..783e5ee6854b69cca87b6f0763844d28b4b2213f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3435,6 +3435,7 @@ M:	Krzysztof Kozlowski <krzk@kernel.org>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Odd Fixes
 F:	Documentation/devicetree/bindings/i2c/i2c-wmt.txt
+F:	Documentation/devicetree/bindings/timer/via,vt8500-timer.yaml
 F:	arch/arm/boot/dts/vt8500/
 F:	arch/arm/mach-vt8500/
 F:	drivers/clocksource/timer-vt8500.c

-- 
2.49.0



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

* [PATCH v4 2/4] clocksource/drivers/timer-vt8500: Add defines for magic constants
  2025-05-20 20:01 [PATCH v4 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
  2025-05-20 20:01 ` [PATCH v4 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML Alexey Charkov
@ 2025-05-20 20:01 ` Alexey Charkov
  2025-05-20 20:01 ` [PATCH v4 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality Alexey Charkov
  2025-05-20 20:01 ` [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC " Alexey Charkov
  3 siblings, 0 replies; 10+ messages in thread
From: Alexey Charkov @ 2025-05-20 20:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	Guenter Roeck
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-watchdog,
	Alexey Charkov

Add defines for all known registers and their bits to make the code more
self-explanatory. While at that, replace _VAL suffixes with more
intuitive _REG suffixes on register offsets.

No functional changes.

Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
 drivers/clocksource/timer-vt8500.c | 65 ++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 23 deletions(-)

diff --git a/drivers/clocksource/timer-vt8500.c b/drivers/clocksource/timer-vt8500.c
index a469b1b5f97233202bf01298b9f612e07026c20c..9f28f30dcaf83ab4e9c89952175b0d4c75bd6b40 100644
--- a/drivers/clocksource/timer-vt8500.c
+++ b/drivers/clocksource/timer-vt8500.c
@@ -24,15 +24,31 @@
 
 #define VT8500_TIMER_OFFSET	0x0100
 #define VT8500_TIMER_HZ		3000000
-#define TIMER_MATCH_VAL		0x0000
-#define TIMER_COUNT_VAL		0x0010
-#define TIMER_STATUS_VAL	0x0014
-#define TIMER_IER_VAL		0x001c		/* interrupt enable */
-#define TIMER_CTRL_VAL		0x0020
-#define TIMER_AS_VAL		0x0024		/* access status */
-#define TIMER_COUNT_R_ACTIVE	(1 << 5)	/* not ready for read */
-#define TIMER_COUNT_W_ACTIVE	(1 << 4)	/* not ready for write */
-#define TIMER_MATCH_W_ACTIVE	(1 << 0)	/* not ready for write */
+
+#define TIMER_MATCH_REG(x)	(4 * (x))
+#define TIMER_COUNT_REG		0x0010	 /* clocksource counter */
+
+#define TIMER_STATUS_REG	0x0014
+#define TIMER_STATUS_MATCH(x)	BIT((x))
+#define TIMER_STATUS_CLEARALL	(TIMER_STATUS_MATCH(0) | \
+				 TIMER_STATUS_MATCH(1) | \
+				 TIMER_STATUS_MATCH(2) | \
+				 TIMER_STATUS_MATCH(3))
+
+#define TIMER_WATCHDOG_EN_REG	0x0018
+#define TIMER_WD_EN		BIT(0)
+
+#define TIMER_INT_EN_REG	0x001c	 /* interrupt enable */
+#define TIMER_INT_EN_MATCH(x)	BIT((x))
+
+#define TIMER_CTRL_REG		0x0020
+#define TIMER_CTRL_ENABLE	BIT(0)	 /* enable clocksource counter */
+#define TIMER_CTRL_RD_REQ	BIT(1)	 /* request counter read */
+
+#define TIMER_ACC_STS_REG	0x0024	 /* access status */
+#define TIMER_ACC_WR_MATCH(x)	BIT((x)) /* writing Match (x) value */
+#define TIMER_ACC_WR_COUNTER	BIT(4)	 /* writing clocksource counter */
+#define TIMER_ACC_RD_COUNTER	BIT(5)	 /* reading clocksource counter */
 
 #define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
 
@@ -43,11 +59,12 @@ static void __iomem *regbase;
 static u64 vt8500_timer_read(struct clocksource *cs)
 {
 	int loops = msecs_to_loops(10);
-	writel(3, regbase + TIMER_CTRL_VAL);
-	while ((readl((regbase + TIMER_AS_VAL)) & TIMER_COUNT_R_ACTIVE)
-						&& --loops)
+
+	writel(TIMER_CTRL_ENABLE | TIMER_CTRL_RD_REQ, regbase + TIMER_CTRL_REG);
+	while (readl(regbase + TIMER_ACC_STS_REG) & TIMER_ACC_RD_COUNTER
+	     && --loops)
 		cpu_relax();
-	return readl(regbase + TIMER_COUNT_VAL);
+	return readl(regbase + TIMER_COUNT_REG);
 }
 
 static struct clocksource clocksource = {
@@ -63,23 +80,25 @@ static int vt8500_timer_set_next_event(unsigned long cycles,
 {
 	int loops = msecs_to_loops(10);
 	u64 alarm = clocksource.read(&clocksource) + cycles;
-	while ((readl(regbase + TIMER_AS_VAL) & TIMER_MATCH_W_ACTIVE)
-						&& --loops)
+
+	while (readl(regbase + TIMER_ACC_STS_REG) & TIMER_ACC_WR_MATCH(0)
+	       && --loops)
 		cpu_relax();
-	writel((unsigned long)alarm, regbase + TIMER_MATCH_VAL);
+	writel((unsigned long)alarm, regbase + TIMER_MATCH_REG(0));
 
 	if ((signed)(alarm - clocksource.read(&clocksource)) <= MIN_OSCR_DELTA)
 		return -ETIME;
 
-	writel(1, regbase + TIMER_IER_VAL);
+	writel(TIMER_INT_EN_MATCH(0), regbase + TIMER_INT_EN_REG);
 
 	return 0;
 }
 
 static int vt8500_shutdown(struct clock_event_device *evt)
 {
-	writel(readl(regbase + TIMER_CTRL_VAL) | 1, regbase + TIMER_CTRL_VAL);
-	writel(0, regbase + TIMER_IER_VAL);
+	writel(readl(regbase + TIMER_CTRL_REG) | TIMER_CTRL_ENABLE,
+	       regbase + TIMER_CTRL_REG);
+	writel(0, regbase + TIMER_INT_EN_REG);
 	return 0;
 }
 
@@ -95,7 +114,7 @@ static struct clock_event_device clockevent = {
 static irqreturn_t vt8500_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
-	writel(0xf, regbase + TIMER_STATUS_VAL);
+	writel(TIMER_STATUS_CLEARALL, regbase + TIMER_STATUS_REG);
 	evt->event_handler(evt);
 
 	return IRQ_HANDLED;
@@ -119,9 +138,9 @@ static int __init vt8500_timer_init(struct device_node *np)
 		return -EINVAL;
 	}
 
-	writel(1, regbase + TIMER_CTRL_VAL);
-	writel(0xf, regbase + TIMER_STATUS_VAL);
-	writel(~0, regbase + TIMER_MATCH_VAL);
+	writel(TIMER_CTRL_ENABLE, regbase + TIMER_CTRL_REG);
+	writel(TIMER_STATUS_CLEARALL, regbase + TIMER_STATUS_REG);
+	writel(~0, regbase + TIMER_MATCH_REG(0));
 
 	ret = clocksource_register_hz(&clocksource, VT8500_TIMER_HZ);
 	if (ret) {

-- 
2.49.0



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

* [PATCH v4 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
  2025-05-20 20:01 [PATCH v4 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
  2025-05-20 20:01 ` [PATCH v4 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML Alexey Charkov
  2025-05-20 20:01 ` [PATCH v4 2/4] clocksource/drivers/timer-vt8500: Add defines for magic constants Alexey Charkov
@ 2025-05-20 20:01 ` Alexey Charkov
  2025-05-20 20:01 ` [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC " Alexey Charkov
  3 siblings, 0 replies; 10+ messages in thread
From: Alexey Charkov @ 2025-05-20 20:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	Guenter Roeck
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-watchdog,
	Alexey Charkov

VIA/WonderMedia system timer can generate a watchdog reset when its
clocksource counter matches the value in the match register 0 and
watchdog function is enabled. For this to work, obvously the clock event
device must use a different match register (1~3) and respective interrupt.

Check if at least two interrupts are provided by the device tree, then use
match register 1 for system clock events and reserve match register 0 for
the watchdog. Instantiate an auxiliary device for the watchdog

Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
 MAINTAINERS                        |   1 +
 drivers/clocksource/Kconfig        |   1 +
 drivers/clocksource/timer-vt8500.c | 141 +++++++++++++++++++++++++++----------
 include/linux/vt8500-timer.h       |  48 +++++++++++++
 4 files changed, 152 insertions(+), 39 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 783e5ee6854b69cca87b6f0763844d28b4b2213f..5362095240627f613638197fda275db6edc16cf7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3447,6 +3447,7 @@ F:	drivers/tty/serial/vt8500_serial.c
 F:	drivers/video/fbdev/vt8500lcdfb.*
 F:	drivers/video/fbdev/wm8505fb*
 F:	drivers/video/fbdev/wmt_ge_rops.*
+F:	include/linux/vt8500-timer.h
 
 ARM/ZYNQ ARCHITECTURE
 M:	Michal Simek <michal.simek@amd.com>
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 487c8525996724fbf9c6e9726dabb478d86513b9..92f071aade10b7c0f0bba4b47dc6228a5e50360f 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -178,6 +178,7 @@ config TEGRA186_TIMER
 config VT8500_TIMER
 	bool "VT8500 timer driver" if COMPILE_TEST
 	depends on HAS_IOMEM
+	select AUXILIARY_BUS
 	help
 	  Enables support for the VT8500 driver.
 
diff --git a/drivers/clocksource/timer-vt8500.c b/drivers/clocksource/timer-vt8500.c
index 9f28f30dcaf83ab4e9c89952175b0d4c75bd6b40..91ebb63e14a85ba34c09f89855b4d5f3f9585d03 100644
--- a/drivers/clocksource/timer-vt8500.c
+++ b/drivers/clocksource/timer-vt8500.c
@@ -11,50 +11,24 @@
  * Alexey Charkov. Minor changes have been made for Device Tree Support.
  */
 
+#include <linux/auxiliary_bus.h>
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
 #include <linux/clocksource.h>
 #include <linux/clockchips.h>
-#include <linux/delay.h>
 
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
-
-#define VT8500_TIMER_OFFSET	0x0100
-#define VT8500_TIMER_HZ		3000000
-
-#define TIMER_MATCH_REG(x)	(4 * (x))
-#define TIMER_COUNT_REG		0x0010	 /* clocksource counter */
-
-#define TIMER_STATUS_REG	0x0014
-#define TIMER_STATUS_MATCH(x)	BIT((x))
-#define TIMER_STATUS_CLEARALL	(TIMER_STATUS_MATCH(0) | \
-				 TIMER_STATUS_MATCH(1) | \
-				 TIMER_STATUS_MATCH(2) | \
-				 TIMER_STATUS_MATCH(3))
-
-#define TIMER_WATCHDOG_EN_REG	0x0018
-#define TIMER_WD_EN		BIT(0)
-
-#define TIMER_INT_EN_REG	0x001c	 /* interrupt enable */
-#define TIMER_INT_EN_MATCH(x)	BIT((x))
-
-#define TIMER_CTRL_REG		0x0020
-#define TIMER_CTRL_ENABLE	BIT(0)	 /* enable clocksource counter */
-#define TIMER_CTRL_RD_REQ	BIT(1)	 /* request counter read */
-
-#define TIMER_ACC_STS_REG	0x0024	 /* access status */
-#define TIMER_ACC_WR_MATCH(x)	BIT((x)) /* writing Match (x) value */
-#define TIMER_ACC_WR_COUNTER	BIT(4)	 /* writing clocksource counter */
-#define TIMER_ACC_RD_COUNTER	BIT(5)	 /* reading clocksource counter */
-
-#define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
-
-#define MIN_OSCR_DELTA		16
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/vt8500-timer.h>
 
 static void __iomem *regbase;
+static unsigned int sys_timer_ch;	 /* which match register to use
+					  * for the system timer
+					  */
 
 static u64 vt8500_timer_read(struct clocksource *cs)
 {
@@ -75,21 +49,26 @@ static struct clocksource clocksource = {
 	.flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
+static u64 vt8500_timer_next(u64 cycles)
+{
+	return clocksource.read(&clocksource) + cycles;
+}
+
 static int vt8500_timer_set_next_event(unsigned long cycles,
 				    struct clock_event_device *evt)
 {
 	int loops = msecs_to_loops(10);
-	u64 alarm = clocksource.read(&clocksource) + cycles;
+	u64 alarm = vt8500_timer_next(cycles);
 
-	while (readl(regbase + TIMER_ACC_STS_REG) & TIMER_ACC_WR_MATCH(0)
+	while (readl(regbase + TIMER_ACC_STS_REG) & TIMER_ACC_WR_MATCH(sys_timer_ch)
 	       && --loops)
 		cpu_relax();
-	writel((unsigned long)alarm, regbase + TIMER_MATCH_REG(0));
+	writel((unsigned long)alarm, regbase + TIMER_MATCH_REG(sys_timer_ch));
 
 	if ((signed)(alarm - clocksource.read(&clocksource)) <= MIN_OSCR_DELTA)
 		return -ETIME;
 
-	writel(TIMER_INT_EN_MATCH(0), regbase + TIMER_INT_EN_REG);
+	writel(TIMER_INT_EN_MATCH(sys_timer_ch), regbase + TIMER_INT_EN_REG);
 
 	return 0;
 }
@@ -131,7 +110,9 @@ static int __init vt8500_timer_init(struct device_node *np)
 		return -ENXIO;
 	}
 
-	timer_irq = irq_of_parse_and_map(np, 0);
+	sys_timer_ch = of_irq_count(np) > 1 ? 1 : 0;
+
+	timer_irq = irq_of_parse_and_map(np, sys_timer_ch);
 	if (!timer_irq) {
 		pr_err("%s: Missing irq description in Device Tree\n",
 								__func__);
@@ -140,7 +121,7 @@ static int __init vt8500_timer_init(struct device_node *np)
 
 	writel(TIMER_CTRL_ENABLE, regbase + TIMER_CTRL_REG);
 	writel(TIMER_STATUS_CLEARALL, regbase + TIMER_STATUS_REG);
-	writel(~0, regbase + TIMER_MATCH_REG(0));
+	writel(~0, regbase + TIMER_MATCH_REG(sys_timer_ch));
 
 	ret = clocksource_register_hz(&clocksource, VT8500_TIMER_HZ);
 	if (ret) {
@@ -166,4 +147,86 @@ static int __init vt8500_timer_init(struct device_node *np)
 	return 0;
 }
 
+static void vt8500_timer_aux_uninit(void *data)
+{
+	auxiliary_device_uninit(data);
+}
+
+static void vt8500_timer_aux_delete(void *data)
+{
+	auxiliary_device_delete(data);
+}
+
+static void vt8500_timer_aux_release(struct device *dev)
+{
+	struct auxiliary_device *aux;
+
+	aux = container_of(dev, struct auxiliary_device, dev);
+	kfree(aux);
+}
+
+/*
+ * This probe gets called after the timer is already up and running. This will
+ * create the watchdog device as a child since the registers are shared.
+ */
+static int vt8500_timer_probe(struct platform_device *pdev)
+{
+	struct vt8500_wdt_info *wdt_info;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	if (!sys_timer_ch) {
+		dev_info(dev, "Not enabling watchdog: only one irq was given");
+		return 0;
+	}
+
+	if (!regbase)
+		return dev_err_probe(dev, -ENOMEM,
+			"Timer not initialized, cannot create watchdog");
+
+	wdt_info = kzalloc(sizeof(*wdt_info), GFP_KERNEL);
+	if (!wdt_info)
+		return dev_err_probe(dev, -ENOMEM,
+			"Failed to allocate vt8500-wdt info");
+
+	wdt_info->timer_next = &vt8500_timer_next;
+	wdt_info->wdt_en = regbase + TIMER_WATCHDOG_EN_REG;
+	wdt_info->wdt_match = regbase + TIMER_MATCH_REG(0);
+	wdt_info->auxdev.name = "vt8500-wdt";
+	wdt_info->auxdev.dev.parent = dev;
+	wdt_info->auxdev.dev.release = &vt8500_timer_aux_release;
+
+	ret = auxiliary_device_init(&wdt_info->auxdev);
+	if (ret) {
+		kfree(wdt_info);
+		return ret;
+	}
+	ret = devm_add_action_or_reset(dev, vt8500_timer_aux_uninit,
+				       &wdt_info->auxdev);
+	if (ret)
+		return ret;
+
+	ret = auxiliary_device_add(&wdt_info->auxdev);
+	if (ret)
+		return ret;
+	return devm_add_action_or_reset(dev, vt8500_timer_aux_delete,
+					&wdt_info->auxdev);
+}
+
+static const struct of_device_id vt8500_timer_of_match[] = {
+	{ .compatible = "via,vt8500-timer", },
+	{},
+};
+
+static struct platform_driver vt8500_timer_driver = {
+	.probe  = vt8500_timer_probe,
+	.driver = {
+		.name = "vt8500-timer",
+		.of_match_table = vt8500_timer_of_match,
+		.suppress_bind_attrs = true,
+	},
+};
+
+builtin_platform_driver(vt8500_timer_driver);
+
 TIMER_OF_DECLARE(vt8500, "via,vt8500-timer", vt8500_timer_init);
diff --git a/include/linux/vt8500-timer.h b/include/linux/vt8500-timer.h
new file mode 100644
index 0000000000000000000000000000000000000000..97ee0716cd84c447b38bbc05535f63fc43e73ef3
--- /dev/null
+++ b/include/linux/vt8500-timer.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef LINUX_VT8500_TIMER_H_
+#define LINUX_VT8500_TIMER_H_
+
+#include <linux/auxiliary_bus.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/types.h>
+
+#define VT8500_TIMER_HZ		3000000
+
+#define TIMER_MATCH_REG(x)	(4 * (x))
+#define TIMER_COUNT_REG		0x0010	 /* clocksource counter */
+
+#define TIMER_STATUS_REG	0x0014
+#define TIMER_STATUS_MATCH(x)	BIT((x))
+#define TIMER_STATUS_CLEARALL	(TIMER_STATUS_MATCH(0) | \
+				 TIMER_STATUS_MATCH(1) | \
+				 TIMER_STATUS_MATCH(2) | \
+				 TIMER_STATUS_MATCH(3))
+
+#define TIMER_WATCHDOG_EN_REG	0x0018
+#define TIMER_WD_EN		BIT(0)
+
+#define TIMER_INT_EN_REG	0x001c	 /* interrupt enable */
+#define TIMER_INT_EN_MATCH(x)	BIT((x))
+
+#define TIMER_CTRL_REG		0x0020
+#define TIMER_CTRL_ENABLE	BIT(0)	 /* enable clocksource counter */
+#define TIMER_CTRL_RD_REQ	BIT(1)	 /* request counter read */
+
+#define TIMER_ACC_STS_REG	0x0024	 /* access status */
+#define TIMER_ACC_WR_MATCH(x)	BIT((x)) /* writing Match (x) value */
+#define TIMER_ACC_WR_COUNTER	BIT(4)	 /* writing clocksource counter */
+#define TIMER_ACC_RD_COUNTER	BIT(5)	 /* reading clocksource counter */
+
+#define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
+
+#define MIN_OSCR_DELTA		16
+
+struct vt8500_wdt_info {
+	struct auxiliary_device auxdev;
+	u64 (*timer_next)(u64 cycles);
+	void __iomem *wdt_en;
+	void __iomem *wdt_match;
+};
+
+#endif /* LINUX_VT8500_TIMER_H_ */

-- 
2.49.0



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

* [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
  2025-05-20 20:01 [PATCH v4 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
                   ` (2 preceding siblings ...)
  2025-05-20 20:01 ` [PATCH v4 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality Alexey Charkov
@ 2025-05-20 20:01 ` Alexey Charkov
  2025-05-20 23:15   ` Guenter Roeck
  3 siblings, 1 reply; 10+ messages in thread
From: Alexey Charkov @ 2025-05-20 20:01 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	Guenter Roeck
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-watchdog,
	Alexey Charkov

VIA/WonderMedia SoCs can use their system timer's first channel as a
watchdog device which will reset the system if the clocksource counter
matches the value given in its match register 0 and if the watchdog
function is enabled.

Since the watchdog function is tightly coupled to the timer itself, it
is implemented as an auxiliary device of the timer device

Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
 MAINTAINERS                   |  1 +
 drivers/watchdog/Kconfig      | 15 ++++++++
 drivers/watchdog/Makefile     |  1 +
 drivers/watchdog/vt8500-wdt.c | 80 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5362095240627f613638197fda275db6edc16cf7..97d1842625dbdf7fdca3556260662dab469ed091 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3447,6 +3447,7 @@ F:	drivers/tty/serial/vt8500_serial.c
 F:	drivers/video/fbdev/vt8500lcdfb.*
 F:	drivers/video/fbdev/wm8505fb*
 F:	drivers/video/fbdev/wmt_ge_rops.*
+F:	drivers/watchdog/vt8500-wdt.c
 F:	include/linux/vt8500-timer.h
 
 ARM/ZYNQ ARCHITECTURE
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0d8d37f712e8cfb4bf8156853baa13c23a57d6d9..8049ae630301a98123f97f6e3ee868bd3bf1534a 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -2003,6 +2003,21 @@ config PIC32_DMT
 	  To compile this driver as a loadable module, choose M here.
 	  The module will be called pic32-dmt.
 
+config VT8500_WATCHDOG
+	tristate "VIA/WonderMedia VT8500 watchdog support"
+	depends on ARCH_VT8500 || COMPILE_TEST
+	select WATCHDOG_CORE
+	select AUXILIARY_BUS
+	help
+	  VIA/WonderMedia SoCs can use their system timer as a hardware
+	  watchdog, as long as the first timer channel is free from other
+	  uses and respective function is enabled in its registers. To
+	  make use of it, say Y here and ensure that the device tree
+	  lists at least two interrupts for the VT8500 timer device.
+
+	  To compile this driver as a module, choose M here.
+	  The module will be called vt8500-wdt.
+
 # PARISC Architecture
 
 # POWERPC Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index c9482904bf870a085c7fce2a439ac5089b6e6fee..3072786bf226c357102be3734fe6e701f753d45b 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o
 obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o
 obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o
 obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o
+obj-$(CONFIG_VT8500_WATCHDOG) += vt8500-wdt.o
 
 # X86 (i386 + ia64 + x86_64) Architecture
 obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
diff --git a/drivers/watchdog/vt8500-wdt.c b/drivers/watchdog/vt8500-wdt.c
new file mode 100644
index 0000000000000000000000000000000000000000..54fe5ad7695de36f6b3c1d28e955f00bef00e9db
--- /dev/null
+++ b/drivers/watchdog/vt8500-wdt.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2025 Alexey Charkov <alchark@gmail.com */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/container_of.h>
+#include <linux/io.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+#include <linux/vt8500-timer.h>
+
+static int vt8500_watchdog_start(struct watchdog_device *wdd)
+{
+	struct vt8500_wdt_info *info = watchdog_get_drvdata(wdd);
+	u64 deadline = info->timer_next(wdd->timeout * VT8500_TIMER_HZ);
+
+	writel((u32)deadline, info->wdt_match);
+	writel(TIMER_WD_EN, info->wdt_en);
+	return 0;
+}
+
+static int vt8500_watchdog_stop(struct watchdog_device *wdd)
+{
+	struct vt8500_wdt_info *info = watchdog_get_drvdata(wdd);
+
+	writel(0, info->wdt_en);
+	return 0;
+}
+
+static const struct watchdog_ops vt8500_watchdog_ops = {
+	.start			= vt8500_watchdog_start,
+	.stop			= vt8500_watchdog_stop,
+};
+
+static const struct watchdog_info vt8500_watchdog_info = {
+	.identity		= "VIA VT8500 watchdog",
+	.options		= WDIOF_MAGICCLOSE |
+				  WDIOF_KEEPALIVEPING |
+				  WDIOF_SETTIMEOUT,
+};
+
+static int vt8500_wdt_probe(struct auxiliary_device *auxdev,
+			    const struct auxiliary_device_id *id)
+{
+	struct vt8500_wdt_info *info;
+	struct watchdog_device *wdd;
+
+	wdd = devm_kzalloc(&auxdev->dev, sizeof(*wdd), GFP_KERNEL);
+	if (!wdd)
+		return -ENOMEM;
+
+	wdd->info = &vt8500_watchdog_info;
+	wdd->ops = &vt8500_watchdog_ops;
+	wdd->max_hw_heartbeat_ms = U32_MAX / (VT8500_TIMER_HZ / 1000);
+	wdd->parent = &auxdev->dev;
+
+	info = container_of(auxdev, struct vt8500_wdt_info, auxdev);
+	watchdog_set_drvdata(wdd, info);
+
+	return devm_watchdog_register_device(&auxdev->dev, wdd);
+}
+
+static const struct auxiliary_device_id vt8500_wdt_ids[] = {
+	{ .name = "timer_vt8500.vt8500-wdt" },
+	{},
+};
+
+MODULE_DEVICE_TABLE(auxiliary, my_auxiliary_id_table);
+
+static struct auxiliary_driver vt8500_wdt_driver = {
+	.name =	"vt8500-wdt",
+	.probe = vt8500_wdt_probe,
+	.id_table = vt8500_wdt_ids,
+};
+module_auxiliary_driver(vt8500_wdt_driver);
+
+MODULE_AUTHOR("Alexey Charkov <alchark@gmail.com>");
+MODULE_DESCRIPTION("Driver for the VIA VT8500 watchdog timer");
+MODULE_LICENSE("GPL");

-- 
2.49.0



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

* Re: [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
  2025-05-20 20:01 ` [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC " Alexey Charkov
@ 2025-05-20 23:15   ` Guenter Roeck
  2025-05-21  6:20     ` Alexey Charkov
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2025-05-20 23:15 UTC (permalink / raw)
  To: Alexey Charkov, Krzysztof Kozlowski, Daniel Lezcano,
	Thomas Gleixner, Rob Herring, Conor Dooley, Krzysztof Kozlowski,
	Wim Van Sebroeck
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-watchdog

On 5/20/25 13:01, Alexey Charkov wrote:
> VIA/WonderMedia SoCs can use their system timer's first channel as a
> watchdog device which will reset the system if the clocksource counter
> matches the value given in its match register 0 and if the watchdog
> function is enabled.
> 
> Since the watchdog function is tightly coupled to the timer itself, it
> is implemented as an auxiliary device of the timer device
> 
> Signed-off-by: Alexey Charkov <alchark@gmail.com>
> ---
>   MAINTAINERS                   |  1 +
>   drivers/watchdog/Kconfig      | 15 ++++++++
>   drivers/watchdog/Makefile     |  1 +
>   drivers/watchdog/vt8500-wdt.c | 80 +++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 97 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5362095240627f613638197fda275db6edc16cf7..97d1842625dbdf7fdca3556260662dab469ed091 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3447,6 +3447,7 @@ F:	drivers/tty/serial/vt8500_serial.c
>   F:	drivers/video/fbdev/vt8500lcdfb.*
>   F:	drivers/video/fbdev/wm8505fb*
>   F:	drivers/video/fbdev/wmt_ge_rops.*
> +F:	drivers/watchdog/vt8500-wdt.c
>   F:	include/linux/vt8500-timer.h
>   
>   ARM/ZYNQ ARCHITECTURE
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0d8d37f712e8cfb4bf8156853baa13c23a57d6d9..8049ae630301a98123f97f6e3ee868bd3bf1534a 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -2003,6 +2003,21 @@ config PIC32_DMT
>   	  To compile this driver as a loadable module, choose M here.
>   	  The module will be called pic32-dmt.
>   
> +config VT8500_WATCHDOG
> +	tristate "VIA/WonderMedia VT8500 watchdog support"
> +	depends on ARCH_VT8500 || COMPILE_TEST
> +	select WATCHDOG_CORE
> +	select AUXILIARY_BUS
> +	help
> +	  VIA/WonderMedia SoCs can use their system timer as a hardware
> +	  watchdog, as long as the first timer channel is free from other
> +	  uses and respective function is enabled in its registers. To
> +	  make use of it, say Y here and ensure that the device tree
> +	  lists at least two interrupts for the VT8500 timer device.
> +
> +	  To compile this driver as a module, choose M here.
> +	  The module will be called vt8500-wdt.
> +
>   # PARISC Architecture
>   
>   # POWERPC Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index c9482904bf870a085c7fce2a439ac5089b6e6fee..3072786bf226c357102be3734fe6e701f753d45b 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o
>   obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o
>   obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o
>   obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o
> +obj-$(CONFIG_VT8500_WATCHDOG) += vt8500-wdt.o
>   
>   # X86 (i386 + ia64 + x86_64) Architecture
>   obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
> diff --git a/drivers/watchdog/vt8500-wdt.c b/drivers/watchdog/vt8500-wdt.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..54fe5ad7695de36f6b3c1d28e955f00bef00e9db
> --- /dev/null
> +++ b/drivers/watchdog/vt8500-wdt.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2025 Alexey Charkov <alchark@gmail.com */
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/container_of.h>
> +#include <linux/io.h>
> +#include <linux/limits.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/watchdog.h>
> +#include <linux/vt8500-timer.h>
> +
> +static int vt8500_watchdog_start(struct watchdog_device *wdd)
> +{
> +	struct vt8500_wdt_info *info = watchdog_get_drvdata(wdd);
> +	u64 deadline = info->timer_next(wdd->timeout * VT8500_TIMER_HZ);
> +

wdd->timeout is the soft timeout, which may be larger
than the maximum hardware timeout. That means you'll need
to use something like "min(wdd->timeout, U32_MAX / VT8500_TIMER_HZ)
* VT8500_TIMER_HZ" as parameter to the timer_next call.

> +	writel((u32)deadline, info->wdt_match);

Can deadline overflow ?

> +	writel(TIMER_WD_EN, info->wdt_en);
> +	return 0;
> +}
> +
> +static int vt8500_watchdog_stop(struct watchdog_device *wdd)
> +{
> +	struct vt8500_wdt_info *info = watchdog_get_drvdata(wdd);
> +
> +	writel(0, info->wdt_en);
> +	return 0;
> +}
> +
> +static const struct watchdog_ops vt8500_watchdog_ops = {
> +	.start			= vt8500_watchdog_start,
> +	.stop			= vt8500_watchdog_stop,
> +};
> +
> +static const struct watchdog_info vt8500_watchdog_info = {
> +	.identity		= "VIA VT8500 watchdog",
> +	.options		= WDIOF_MAGICCLOSE |
> +				  WDIOF_KEEPALIVEPING |
> +				  WDIOF_SETTIMEOUT,
> +};
> +
> +static int vt8500_wdt_probe(struct auxiliary_device *auxdev,
> +			    const struct auxiliary_device_id *id)
> +{
> +	struct vt8500_wdt_info *info;
> +	struct watchdog_device *wdd;
> +
> +	wdd = devm_kzalloc(&auxdev->dev, sizeof(*wdd), GFP_KERNEL);
> +	if (!wdd)
> +		return -ENOMEM;
> +
> +	wdd->info = &vt8500_watchdog_info;
> +	wdd->ops = &vt8500_watchdog_ops;
> +	wdd->max_hw_heartbeat_ms = U32_MAX / (VT8500_TIMER_HZ / 1000);
> +	wdd->parent = &auxdev->dev;
> +
> +	info = container_of(auxdev, struct vt8500_wdt_info, auxdev);
> +	watchdog_set_drvdata(wdd, info);
> +
> +	return devm_watchdog_register_device(&auxdev->dev, wdd);
> +}
> +
> +static const struct auxiliary_device_id vt8500_wdt_ids[] = {
> +	{ .name = "timer_vt8500.vt8500-wdt" },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(auxiliary, my_auxiliary_id_table);
> +
> +static struct auxiliary_driver vt8500_wdt_driver = {
> +	.name =	"vt8500-wdt",
> +	.probe = vt8500_wdt_probe,
> +	.id_table = vt8500_wdt_ids,
> +};
> +module_auxiliary_driver(vt8500_wdt_driver);
> +
> +MODULE_AUTHOR("Alexey Charkov <alchark@gmail.com>");
> +MODULE_DESCRIPTION("Driver for the VIA VT8500 watchdog timer");
> +MODULE_LICENSE("GPL");
> 



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

* Re: [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
  2025-05-20 23:15   ` Guenter Roeck
@ 2025-05-21  6:20     ` Alexey Charkov
  2025-05-21 13:33       ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Alexey Charkov @ 2025-05-21  6:20 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	linux-arm-kernel, linux-kernel, devicetree, linux-watchdog

On Wed, May 21, 2025 at 3:15 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 5/20/25 13:01, Alexey Charkov wrote:
> > VIA/WonderMedia SoCs can use their system timer's first channel as a
> > watchdog device which will reset the system if the clocksource counter
> > matches the value given in its match register 0 and if the watchdog
> > function is enabled.
> >
> > Since the watchdog function is tightly coupled to the timer itself, it
> > is implemented as an auxiliary device of the timer device
> >
> > Signed-off-by: Alexey Charkov <alchark@gmail.com>
> > ---
> >   MAINTAINERS                   |  1 +
> >   drivers/watchdog/Kconfig      | 15 ++++++++
> >   drivers/watchdog/Makefile     |  1 +
> >   drivers/watchdog/vt8500-wdt.c | 80 +++++++++++++++++++++++++++++++++++++++++++
> >   4 files changed, 97 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5362095240627f613638197fda275db6edc16cf7..97d1842625dbdf7fdca3556260662dab469ed091 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3447,6 +3447,7 @@ F:      drivers/tty/serial/vt8500_serial.c
> >   F:  drivers/video/fbdev/vt8500lcdfb.*
> >   F:  drivers/video/fbdev/wm8505fb*
> >   F:  drivers/video/fbdev/wmt_ge_rops.*
> > +F:   drivers/watchdog/vt8500-wdt.c
> >   F:  include/linux/vt8500-timer.h
> >
> >   ARM/ZYNQ ARCHITECTURE
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 0d8d37f712e8cfb4bf8156853baa13c23a57d6d9..8049ae630301a98123f97f6e3ee868bd3bf1534a 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -2003,6 +2003,21 @@ config PIC32_DMT
> >         To compile this driver as a loadable module, choose M here.
> >         The module will be called pic32-dmt.
> >
> > +config VT8500_WATCHDOG
> > +     tristate "VIA/WonderMedia VT8500 watchdog support"
> > +     depends on ARCH_VT8500 || COMPILE_TEST
> > +     select WATCHDOG_CORE
> > +     select AUXILIARY_BUS
> > +     help
> > +       VIA/WonderMedia SoCs can use their system timer as a hardware
> > +       watchdog, as long as the first timer channel is free from other
> > +       uses and respective function is enabled in its registers. To
> > +       make use of it, say Y here and ensure that the device tree
> > +       lists at least two interrupts for the VT8500 timer device.
> > +
> > +       To compile this driver as a module, choose M here.
> > +       The module will be called vt8500-wdt.
> > +
> >   # PARISC Architecture
> >
> >   # POWERPC Architecture
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index c9482904bf870a085c7fce2a439ac5089b6e6fee..3072786bf226c357102be3734fe6e701f753d45b 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -101,6 +101,7 @@ obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o
> >   obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o
> >   obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o
> >   obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o
> > +obj-$(CONFIG_VT8500_WATCHDOG) += vt8500-wdt.o
> >
> >   # X86 (i386 + ia64 + x86_64) Architecture
> >   obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
> > diff --git a/drivers/watchdog/vt8500-wdt.c b/drivers/watchdog/vt8500-wdt.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..54fe5ad7695de36f6b3c1d28e955f00bef00e9db
> > --- /dev/null
> > +++ b/drivers/watchdog/vt8500-wdt.c
> > @@ -0,0 +1,80 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (C) 2025 Alexey Charkov <alchark@gmail.com */
> > +
> > +#include <linux/auxiliary_bus.h>
> > +#include <linux/container_of.h>
> > +#include <linux/io.h>
> > +#include <linux/limits.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/vt8500-timer.h>
> > +
> > +static int vt8500_watchdog_start(struct watchdog_device *wdd)
> > +{
> > +     struct vt8500_wdt_info *info = watchdog_get_drvdata(wdd);
> > +     u64 deadline = info->timer_next(wdd->timeout * VT8500_TIMER_HZ);
> > +
>
> wdd->timeout is the soft timeout, which may be larger
> than the maximum hardware timeout. That means you'll need
> to use something like "min(wdd->timeout, U32_MAX / VT8500_TIMER_HZ)
> * VT8500_TIMER_HZ" as parameter to the timer_next call.

Indeed. For some reason I thought the core splits up large user
requested timeout values into at most max_hw_heartbeat_ms long chunks
and feeds those to the driver via the timeout field, but now I see it
doesn't.

Do I get it right that the core worker will try and do its last ping
of the hardware at exactly max_hw_heartbeat_ms before the user
specified deadline?

> > +     writel((u32)deadline, info->wdt_match);
>
> Can deadline overflow ?

Yes. The underlying hardware counter is a u32 value incrementing at
VT8500_TIMER_HZ and continuing past wraparound. This register sets the
next match value: once the hardware counter reaches the value of
(u32)deadline (which might be after the counter wrapping around iff
deadline > U32_MAX) the system resets. Perhaps it warrants a comment
in code.

Thanks for your review, Guenter!

Best regards,
Alexey


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

* Re: [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
  2025-05-21  6:20     ` Alexey Charkov
@ 2025-05-21 13:33       ` Guenter Roeck
  2025-05-21 14:15         ` Alexey Charkov
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2025-05-21 13:33 UTC (permalink / raw)
  To: Alexey Charkov
  Cc: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	linux-arm-kernel, linux-kernel, devicetree, linux-watchdog

On 5/20/25 23:20, Alexey Charkov wrote:
> On Wed, May 21, 2025 at 3:15 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 5/20/25 13:01, Alexey Charkov wrote:
>>> VIA/WonderMedia SoCs can use their system timer's first channel as a
>>> watchdog device which will reset the system if the clocksource counter
>>> matches the value given in its match register 0 and if the watchdog
>>> function is enabled.
>>>
>>> Since the watchdog function is tightly coupled to the timer itself, it
>>> is implemented as an auxiliary device of the timer device
>>>
>>> Signed-off-by: Alexey Charkov <alchark@gmail.com>
>>> ---
>>>    MAINTAINERS                   |  1 +
>>>    drivers/watchdog/Kconfig      | 15 ++++++++
>>>    drivers/watchdog/Makefile     |  1 +
>>>    drivers/watchdog/vt8500-wdt.c | 80 +++++++++++++++++++++++++++++++++++++++++++
>>>    4 files changed, 97 insertions(+)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 5362095240627f613638197fda275db6edc16cf7..97d1842625dbdf7fdca3556260662dab469ed091 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -3447,6 +3447,7 @@ F:      drivers/tty/serial/vt8500_serial.c
>>>    F:  drivers/video/fbdev/vt8500lcdfb.*
>>>    F:  drivers/video/fbdev/wm8505fb*
>>>    F:  drivers/video/fbdev/wmt_ge_rops.*
>>> +F:   drivers/watchdog/vt8500-wdt.c
>>>    F:  include/linux/vt8500-timer.h
>>>
>>>    ARM/ZYNQ ARCHITECTURE
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 0d8d37f712e8cfb4bf8156853baa13c23a57d6d9..8049ae630301a98123f97f6e3ee868bd3bf1534a 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -2003,6 +2003,21 @@ config PIC32_DMT
>>>          To compile this driver as a loadable module, choose M here.
>>>          The module will be called pic32-dmt.
>>>
>>> +config VT8500_WATCHDOG
>>> +     tristate "VIA/WonderMedia VT8500 watchdog support"
>>> +     depends on ARCH_VT8500 || COMPILE_TEST
>>> +     select WATCHDOG_CORE
>>> +     select AUXILIARY_BUS
>>> +     help
>>> +       VIA/WonderMedia SoCs can use their system timer as a hardware
>>> +       watchdog, as long as the first timer channel is free from other
>>> +       uses and respective function is enabled in its registers. To
>>> +       make use of it, say Y here and ensure that the device tree
>>> +       lists at least two interrupts for the VT8500 timer device.
>>> +
>>> +       To compile this driver as a module, choose M here.
>>> +       The module will be called vt8500-wdt.
>>> +
>>>    # PARISC Architecture
>>>
>>>    # POWERPC Architecture
>>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>>> index c9482904bf870a085c7fce2a439ac5089b6e6fee..3072786bf226c357102be3734fe6e701f753d45b 100644
>>> --- a/drivers/watchdog/Makefile
>>> +++ b/drivers/watchdog/Makefile
>>> @@ -101,6 +101,7 @@ obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o
>>>    obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o
>>>    obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o
>>>    obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o
>>> +obj-$(CONFIG_VT8500_WATCHDOG) += vt8500-wdt.o
>>>
>>>    # X86 (i386 + ia64 + x86_64) Architecture
>>>    obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
>>> diff --git a/drivers/watchdog/vt8500-wdt.c b/drivers/watchdog/vt8500-wdt.c
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..54fe5ad7695de36f6b3c1d28e955f00bef00e9db
>>> --- /dev/null
>>> +++ b/drivers/watchdog/vt8500-wdt.c
>>> @@ -0,0 +1,80 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/* Copyright (C) 2025 Alexey Charkov <alchark@gmail.com */
>>> +
>>> +#include <linux/auxiliary_bus.h>
>>> +#include <linux/container_of.h>
>>> +#include <linux/io.h>
>>> +#include <linux/limits.h>
>>> +#include <linux/module.h>
>>> +#include <linux/types.h>
>>> +#include <linux/watchdog.h>
>>> +#include <linux/vt8500-timer.h>
>>> +
>>> +static int vt8500_watchdog_start(struct watchdog_device *wdd)
>>> +{
>>> +     struct vt8500_wdt_info *info = watchdog_get_drvdata(wdd);
>>> +     u64 deadline = info->timer_next(wdd->timeout * VT8500_TIMER_HZ);
>>> +
>>
>> wdd->timeout is the soft timeout, which may be larger
>> than the maximum hardware timeout. That means you'll need
>> to use something like "min(wdd->timeout, U32_MAX / VT8500_TIMER_HZ)
>> * VT8500_TIMER_HZ" as parameter to the timer_next call.
> 
> Indeed. For some reason I thought the core splits up large user
> requested timeout values into at most max_hw_heartbeat_ms long chunks
> and feeds those to the driver via the timeout field, but now I see it
> doesn't.
> 
> Do I get it right that the core worker will try and do its last ping
> of the hardware at exactly max_hw_heartbeat_ms before the user
> specified deadline?
> 

Where do you see that ? In the watchdog core:

         hw_heartbeat_ms = min_not_zero(timeout_ms, wdd->max_hw_heartbeat_ms);
         keepalive_interval = ms_to_ktime(hw_heartbeat_ms / 2);

>>> +     writel((u32)deadline, info->wdt_match);
>>
>> Can deadline overflow ?
> 
> Yes. The underlying hardware counter is a u32 value incrementing at
> VT8500_TIMER_HZ and continuing past wraparound. This register sets the
> next match value: once the hardware counter reaches the value of
> (u32)deadline (which might be after the counter wrapping around iff
> deadline > U32_MAX) the system resets. Perhaps it warrants a comment
> in code.
> 

Ok. Yes, a comment would help.

Thanks,
Guenter

> Thanks for your review, Guenter!
> 
> Best regards,
> Alexey



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

* Re: [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
  2025-05-21 13:33       ` Guenter Roeck
@ 2025-05-21 14:15         ` Alexey Charkov
  2025-05-21 20:03           ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Alexey Charkov @ 2025-05-21 14:15 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	linux-arm-kernel, linux-kernel, devicetree, linux-watchdog

On Wed, May 21, 2025 at 5:33 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 5/20/25 23:20, Alexey Charkov wrote:
> > On Wed, May 21, 2025 at 3:15 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On 5/20/25 13:01, Alexey Charkov wrote:
> >>> VIA/WonderMedia SoCs can use their system timer's first channel as a
> >>> watchdog device which will reset the system if the clocksource counter
> >>> matches the value given in its match register 0 and if the watchdog
> >>> function is enabled.
> >>>
> >>> Since the watchdog function is tightly coupled to the timer itself, it
> >>> is implemented as an auxiliary device of the timer device
> >>>
> >>> Signed-off-by: Alexey Charkov <alchark@gmail.com>
> >>> ---
> >>>    MAINTAINERS                   |  1 +
> >>>    drivers/watchdog/Kconfig      | 15 ++++++++
> >>>    drivers/watchdog/Makefile     |  1 +
> >>>    drivers/watchdog/vt8500-wdt.c | 80 +++++++++++++++++++++++++++++++++++++++++++
> >>>    4 files changed, 97 insertions(+)
> >>>
> >>> diff --git a/MAINTAINERS b/MAINTAINERS
> >>> index 5362095240627f613638197fda275db6edc16cf7..97d1842625dbdf7fdca3556260662dab469ed091 100644
> >>> --- a/MAINTAINERS
> >>> +++ b/MAINTAINERS
> >>> @@ -3447,6 +3447,7 @@ F:      drivers/tty/serial/vt8500_serial.c
> >>>    F:  drivers/video/fbdev/vt8500lcdfb.*
> >>>    F:  drivers/video/fbdev/wm8505fb*
> >>>    F:  drivers/video/fbdev/wmt_ge_rops.*
> >>> +F:   drivers/watchdog/vt8500-wdt.c
> >>>    F:  include/linux/vt8500-timer.h
> >>>
> >>>    ARM/ZYNQ ARCHITECTURE
> >>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>> index 0d8d37f712e8cfb4bf8156853baa13c23a57d6d9..8049ae630301a98123f97f6e3ee868bd3bf1534a 100644
> >>> --- a/drivers/watchdog/Kconfig
> >>> +++ b/drivers/watchdog/Kconfig
> >>> @@ -2003,6 +2003,21 @@ config PIC32_DMT
> >>>          To compile this driver as a loadable module, choose M here.
> >>>          The module will be called pic32-dmt.
> >>>
> >>> +config VT8500_WATCHDOG
> >>> +     tristate "VIA/WonderMedia VT8500 watchdog support"
> >>> +     depends on ARCH_VT8500 || COMPILE_TEST
> >>> +     select WATCHDOG_CORE
> >>> +     select AUXILIARY_BUS
> >>> +     help
> >>> +       VIA/WonderMedia SoCs can use their system timer as a hardware
> >>> +       watchdog, as long as the first timer channel is free from other
> >>> +       uses and respective function is enabled in its registers. To
> >>> +       make use of it, say Y here and ensure that the device tree
> >>> +       lists at least two interrupts for the VT8500 timer device.
> >>> +
> >>> +       To compile this driver as a module, choose M here.
> >>> +       The module will be called vt8500-wdt.
> >>> +
> >>>    # PARISC Architecture
> >>>
> >>>    # POWERPC Architecture
> >>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> >>> index c9482904bf870a085c7fce2a439ac5089b6e6fee..3072786bf226c357102be3734fe6e701f753d45b 100644
> >>> --- a/drivers/watchdog/Makefile
> >>> +++ b/drivers/watchdog/Makefile
> >>> @@ -101,6 +101,7 @@ obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o
> >>>    obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o
> >>>    obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o
> >>>    obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o
> >>> +obj-$(CONFIG_VT8500_WATCHDOG) += vt8500-wdt.o
> >>>
> >>>    # X86 (i386 + ia64 + x86_64) Architecture
> >>>    obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
> >>> diff --git a/drivers/watchdog/vt8500-wdt.c b/drivers/watchdog/vt8500-wdt.c
> >>> new file mode 100644
> >>> index 0000000000000000000000000000000000000000..54fe5ad7695de36f6b3c1d28e955f00bef00e9db
> >>> --- /dev/null
> >>> +++ b/drivers/watchdog/vt8500-wdt.c
> >>> @@ -0,0 +1,80 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/* Copyright (C) 2025 Alexey Charkov <alchark@gmail.com */
> >>> +
> >>> +#include <linux/auxiliary_bus.h>
> >>> +#include <linux/container_of.h>
> >>> +#include <linux/io.h>
> >>> +#include <linux/limits.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/types.h>
> >>> +#include <linux/watchdog.h>
> >>> +#include <linux/vt8500-timer.h>
> >>> +
> >>> +static int vt8500_watchdog_start(struct watchdog_device *wdd)
> >>> +{
> >>> +     struct vt8500_wdt_info *info = watchdog_get_drvdata(wdd);
> >>> +     u64 deadline = info->timer_next(wdd->timeout * VT8500_TIMER_HZ);
> >>> +
> >>
> >> wdd->timeout is the soft timeout, which may be larger
> >> than the maximum hardware timeout. That means you'll need
> >> to use something like "min(wdd->timeout, U32_MAX / VT8500_TIMER_HZ)
> >> * VT8500_TIMER_HZ" as parameter to the timer_next call.
> >
> > Indeed. For some reason I thought the core splits up large user
> > requested timeout values into at most max_hw_heartbeat_ms long chunks
> > and feeds those to the driver via the timeout field, but now I see it
> > doesn't.
> >
> > Do I get it right that the core worker will try and do its last ping
> > of the hardware at exactly max_hw_heartbeat_ms before the user
> > specified deadline?
> >
>
> Where do you see that ? In the watchdog core:
>
>          hw_heartbeat_ms = min_not_zero(timeout_ms, wdd->max_hw_heartbeat_ms);
>          keepalive_interval = ms_to_ktime(hw_heartbeat_ms / 2);

This comment [1] which follows the lines you've pasted: "To ensure
that the watchdog times out wdd->timeout seconds after the most recent
ping from userspace, the last worker ping has to come in
hw_heartbeat_ms before this timeout."

I'm not exactly sure I'm following the code path there 100%, so wanted
to double check. Meanwhile, I've just sent a new version [2] adding
the min function as you pointed out.

[1] https://elixir.bootlin.com/linux/v6.14.6/source/drivers/watchdog/watchdog_dev.c#L120
[2] https://lore.kernel.org/all/20250521-vt8500-timer-updates-v5-0-7e4bd11df72e@gmail.com/

Best regards,
Alexey


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

* Re: [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
  2025-05-21 14:15         ` Alexey Charkov
@ 2025-05-21 20:03           ` Guenter Roeck
  0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2025-05-21 20:03 UTC (permalink / raw)
  To: Alexey Charkov
  Cc: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
	Conor Dooley, Krzysztof Kozlowski, Wim Van Sebroeck,
	linux-arm-kernel, linux-kernel, devicetree, linux-watchdog

On 5/21/25 07:15, Alexey Charkov wrote:
...
>>> Do I get it right that the core worker will try and do its last ping
>>> of the hardware at exactly max_hw_heartbeat_ms before the user
>>> specified deadline?
>>>
>>
>> Where do you see that ? In the watchdog core:
>>
>>           hw_heartbeat_ms = min_not_zero(timeout_ms, wdd->max_hw_heartbeat_ms);
>>           keepalive_interval = ms_to_ktime(hw_heartbeat_ms / 2);
> 
> This comment [1] which follows the lines you've pasted: "To ensure
> that the watchdog times out wdd->timeout seconds after the most recent
> ping from userspace, the last worker ping has to come in
> hw_heartbeat_ms before this timeout."
> 

Ah, yes. Sorry, I misunderstood your question. This is absolutely correct.
If timeout is, say, 10 seconds, and the maximum hardware timeout is 8 seconds,
the last heartbeat must be triggered by the kernel 2 seconds after the last
userspace heartbeat request to ensure that the actual timeout happens 10
seconds after the most recent heartbeat request from userspace.

Guenter



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

end of thread, other threads:[~2025-05-21 20:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-20 20:01 [PATCH v4 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
2025-05-20 20:01 ` [PATCH v4 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML Alexey Charkov
2025-05-20 20:01 ` [PATCH v4 2/4] clocksource/drivers/timer-vt8500: Add defines for magic constants Alexey Charkov
2025-05-20 20:01 ` [PATCH v4 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality Alexey Charkov
2025-05-20 20:01 ` [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC " Alexey Charkov
2025-05-20 23:15   ` Guenter Roeck
2025-05-21  6:20     ` Alexey Charkov
2025-05-21 13:33       ` Guenter Roeck
2025-05-21 14:15         ` Alexey Charkov
2025-05-21 20:03           ` Guenter Roeck

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