* [PATCH v3 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function
@ 2025-05-15 18:55 Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML Alexey Charkov
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Alexey Charkov @ 2025-05-15 18:55 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 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 | 2 +
drivers/clocksource/timer-vt8500.c | 125 +++++++++++++++++----
drivers/watchdog/Kconfig | 14 +++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/vt8500-wdt.c | 116 +++++++++++++++++++
7 files changed, 285 insertions(+), 39 deletions(-)
---
base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb
change-id: 20250506-vt8500-timer-updates-44a0d22cd720
Best regards,
--
Alexey Charkov <alchark@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML
2025-05-15 18:55 [PATCH v3 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
@ 2025-05-15 18:55 ` Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 2/4] clocksource/drivers/timer-vt8500: Add defines for magic constants Alexey Charkov
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Alexey Charkov @ 2025-05-15 18:55 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 69511c3b2b76fb7090a2a550f4c59a7daf188493..b8b2843491c47182a4fc6c76f5c29b6c411830a6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3434,6 +3434,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] 11+ messages in thread
* [PATCH v3 2/4] clocksource/drivers/timer-vt8500: Add defines for magic constants
2025-05-15 18:55 [PATCH v3 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML Alexey Charkov
@ 2025-05-15 18:55 ` Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC " Alexey Charkov
3 siblings, 0 replies; 11+ messages in thread
From: Alexey Charkov @ 2025-05-15 18:55 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] 11+ messages in thread
* [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
2025-05-15 18:55 [PATCH v3 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 2/4] clocksource/drivers/timer-vt8500: Add defines for magic constants Alexey Charkov
@ 2025-05-15 18:55 ` Alexey Charkov
2025-05-18 1:23 ` kernel test robot
2025-05-15 18:55 ` [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC " Alexey Charkov
3 siblings, 1 reply; 11+ messages in thread
From: Alexey Charkov @ 2025-05-15 18:55 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 a platform device for the watchdog and give it
access to the MMIO registers base of the timer through platform data.
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
drivers/clocksource/timer-vt8500.c | 68 +++++++++++++++++++++++++++++++++++---
1 file changed, 63 insertions(+), 5 deletions(-)
diff --git a/drivers/clocksource/timer-vt8500.c b/drivers/clocksource/timer-vt8500.c
index 9f28f30dcaf83ab4e9c89952175b0d4c75bd6b40..122cc046140d5e9edff20ff41a49b4e62064dd40 100644
--- a/drivers/clocksource/timer-vt8500.c
+++ b/drivers/clocksource/timer-vt8500.c
@@ -21,6 +21,8 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
#define VT8500_TIMER_OFFSET 0x0100
#define VT8500_TIMER_HZ 3000000
@@ -55,6 +57,9 @@
#define MIN_OSCR_DELTA 16
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)
{
@@ -81,15 +86,15 @@ 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_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 +136,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 +147,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 +173,55 @@ static int __init vt8500_timer_init(struct device_node *np)
return 0;
}
+/*
+ * 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 platform_device *vt8500_watchdog_device;
+ 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");
+
+ vt8500_watchdog_device = platform_device_alloc("vt8500-wdt", -1);
+ if (!vt8500_watchdog_device)
+ return dev_err_probe(dev, -ENOMEM,
+ "Failed to allocate vt8500-wdt");
+
+ /* Pass the base address as platform data and nothing else */
+ vt8500_watchdog_device->dev.platform_data = regbase;
+ vt8500_watchdog_device->dev.parent = dev;
+
+ ret = platform_device_add(vt8500_watchdog_device);
+ if (ret)
+ platform_device_put(vt8500_watchdog_device);
+
+ return ret;
+}
+
+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);
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
2025-05-15 18:55 [PATCH v3 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
` (2 preceding siblings ...)
2025-05-15 18:55 ` [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality Alexey Charkov
@ 2025-05-15 18:55 ` Alexey Charkov
2025-05-16 6:55 ` kernel test robot
3 siblings, 1 reply; 11+ messages in thread
From: Alexey Charkov @ 2025-05-15 18:55 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 a child platform device of the timer device, and just
reuses the MMIO registers already remapped by the timer driver. This is
similar to the approach taken by gxp-wdt.c
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
MAINTAINERS | 1 +
drivers/watchdog/Kconfig | 14 +++++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/vt8500-wdt.c | 116 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 132 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b8b2843491c47182a4fc6c76f5c29b6c411830a6..d809643b76ad299ca1c08804540b08cc4a7184a6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3446,6 +3446,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
ARM/ZYNQ ARCHITECTURE
M: Michal Simek <michal.simek@amd.com>
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0d8d37f712e8cfb4bf8156853baa13c23a57d6d9..429e4e775f1f458fd457067a3a039a5d544b4818 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -2003,6 +2003,20 @@ 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
+ 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..8e605c850dd9f42c90104f362b74adac63dd9fdb
--- /dev/null
+++ b/drivers/watchdog/vt8500-wdt.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2025 Alexey Charkov <alchark@gmail.com */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#define TIMER_MATCH_REG(x) (4 * (x))
+#define TIMER_COUNT_REG 0x0010 /* clocksource counter */
+
+#define TIMER_WATCHDOG_EN_REG 0x0018
+#define TIMER_WD_EN BIT(0)
+
+#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 VT8500_TIMER_HZ 3000000
+#define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
+
+struct vt8500_wdt {
+ void __iomem *regbase;
+ struct watchdog_device wdd;
+};
+
+static u64 vt8500_timer_read(void __iomem *regbase)
+{
+ int loops = msecs_to_loops(10);
+
+ 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_REG);
+}
+
+static int vt8500_watchdog_start(struct watchdog_device *wdd)
+{
+ struct vt8500_wdt *drvdata = watchdog_get_drvdata(wdd);
+ u64 cycles = wdd->timeout * VT8500_TIMER_HZ;
+ u64 deadline = vt8500_timer_read(drvdata->regbase) + cycles;
+
+ writel((u32)deadline, drvdata->regbase + TIMER_MATCH_REG(0));
+ writel(TIMER_WD_EN, drvdata->regbase + TIMER_WATCHDOG_EN_REG);
+ return 0;
+}
+
+static int vt8500_watchdog_stop(struct watchdog_device *wdd)
+{
+ struct vt8500_wdt *drvdata = watchdog_get_drvdata(wdd);
+
+ writel(0, drvdata->regbase + TIMER_WATCHDOG_EN_REG);
+ 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 platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct vt8500_wdt *drvdata;
+
+ drvdata = devm_kzalloc(dev, sizeof(struct vt8500_wdt), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ /*
+ * The register area where the timer and watchdog reside is disarranged.
+ * Hence mapping individual register blocks for the timer and watchdog
+ * is not recommended as they would have access to each others
+ * registers. The timer driver creates the watchdog as a child device.
+ * During the watchdogs creation, the timer driver passes the base
+ * address to the watchdog over the private interface.
+ */
+
+ drvdata->regbase = (void __iomem *)dev->platform_data;
+
+ drvdata->wdd.info = &vt8500_watchdog_info;
+ drvdata->wdd.ops = &vt8500_watchdog_ops;
+ drvdata->wdd.max_hw_heartbeat_ms = -1UL / (VT8500_TIMER_HZ / 1000);
+ drvdata->wdd.parent = dev;
+
+ watchdog_set_drvdata(&drvdata->wdd, drvdata);
+
+ return devm_watchdog_register_device(dev, &drvdata->wdd);
+}
+
+static struct platform_driver vt8500_wdt_driver = {
+ .probe = vt8500_wdt_probe,
+ .driver = {
+ .name = "vt8500-wdt",
+ },
+};
+module_platform_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] 11+ messages in thread
* Re: [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
2025-05-15 18:55 ` [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC " Alexey Charkov
@ 2025-05-16 6:55 ` kernel test robot
2025-05-16 13:02 ` Alexey Charkov
0 siblings, 1 reply; 11+ messages in thread
From: kernel test robot @ 2025-05-16 6:55 UTC (permalink / raw)
To: Alexey Charkov, Krzysztof Kozlowski, Daniel Lezcano,
Thomas Gleixner, Rob Herring, Conor Dooley, Wim Van Sebroeck,
Guenter Roeck
Cc: oe-kbuild-all, linux-arm-kernel, linux-kernel, devicetree,
linux-watchdog, Alexey Charkov
Hi Alexey,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 92a09c47464d040866cf2b4cd052bc60555185fb]
url: https://github.com/intel-lab-lkp/linux/commits/Alexey-Charkov/dt-bindings-timer-via-vt8500-timer-Convert-to-YAML/20250516-025729
base: 92a09c47464d040866cf2b4cd052bc60555185fb
patch link: https://lore.kernel.org/r/20250515-vt8500-timer-updates-v3-4-2197a1b062bd%40gmail.com
patch subject: [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
config: sparc-allmodconfig (https://download.01.org/0day-ci/archive/20250516/202505161430.wdURc0TG-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250516/202505161430.wdURc0TG-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505161430.wdURc0TG-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/watchdog/vt8500-wdt.c: In function 'vt8500_wdt_probe':
>> drivers/watchdog/vt8500-wdt.c:98:44: warning: conversion from 'long unsigned int' to 'unsigned int' changes value from '6148914691236517' to '3287081637' [-Woverflow]
98 | drvdata->wdd.max_hw_heartbeat_ms = -1UL / (VT8500_TIMER_HZ / 1000);
| ^
vim +98 drivers/watchdog/vt8500-wdt.c
75
76 static int vt8500_wdt_probe(struct platform_device *pdev)
77 {
78 struct device *dev = &pdev->dev;
79 struct vt8500_wdt *drvdata;
80
81 drvdata = devm_kzalloc(dev, sizeof(struct vt8500_wdt), GFP_KERNEL);
82 if (!drvdata)
83 return -ENOMEM;
84
85 /*
86 * The register area where the timer and watchdog reside is disarranged.
87 * Hence mapping individual register blocks for the timer and watchdog
88 * is not recommended as they would have access to each others
89 * registers. The timer driver creates the watchdog as a child device.
90 * During the watchdogs creation, the timer driver passes the base
91 * address to the watchdog over the private interface.
92 */
93
94 drvdata->regbase = (void __iomem *)dev->platform_data;
95
96 drvdata->wdd.info = &vt8500_watchdog_info;
97 drvdata->wdd.ops = &vt8500_watchdog_ops;
> 98 drvdata->wdd.max_hw_heartbeat_ms = -1UL / (VT8500_TIMER_HZ / 1000);
99 drvdata->wdd.parent = dev;
100
101 watchdog_set_drvdata(&drvdata->wdd, drvdata);
102
103 return devm_watchdog_register_device(dev, &drvdata->wdd);
104 }
105
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
2025-05-16 6:55 ` kernel test robot
@ 2025-05-16 13:02 ` Alexey Charkov
0 siblings, 0 replies; 11+ messages in thread
From: Alexey Charkov @ 2025-05-16 13:02 UTC (permalink / raw)
To: kernel test robot
Cc: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
Conor Dooley, Wim Van Sebroeck, Guenter Roeck, oe-kbuild-all,
linux-arm-kernel, linux-kernel, devicetree, linux-watchdog
On Fri, May 16, 2025 at 9:56 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Alexey,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on 92a09c47464d040866cf2b4cd052bc60555185fb]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Alexey-Charkov/dt-bindings-timer-via-vt8500-timer-Convert-to-YAML/20250516-025729
> base: 92a09c47464d040866cf2b4cd052bc60555185fb
> patch link: https://lore.kernel.org/r/20250515-vt8500-timer-updates-v3-4-2197a1b062bd%40gmail.com
> patch subject: [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality
> config: sparc-allmodconfig (https://download.01.org/0day-ci/archive/20250516/202505161430.wdURc0TG-lkp@intel.com/config)
> compiler: sparc64-linux-gcc (GCC) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250516/202505161430.wdURc0TG-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202505161430.wdURc0TG-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> drivers/watchdog/vt8500-wdt.c: In function 'vt8500_wdt_probe':
> >> drivers/watchdog/vt8500-wdt.c:98:44: warning: conversion from 'long unsigned int' to 'unsigned int' changes value from '6148914691236517' to '3287081637' [-Woverflow]
> 98 | drvdata->wdd.max_hw_heartbeat_ms = -1UL / (VT8500_TIMER_HZ / 1000);
Will change to U32_MAX in the next version.
Best regards,
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
2025-05-15 18:55 ` [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality Alexey Charkov
@ 2025-05-18 1:23 ` kernel test robot
2025-05-19 11:34 ` Alexey Charkov
0 siblings, 1 reply; 11+ messages in thread
From: kernel test robot @ 2025-05-18 1:23 UTC (permalink / raw)
To: Alexey Charkov, Krzysztof Kozlowski, Daniel Lezcano,
Thomas Gleixner, Rob Herring, Conor Dooley, Wim Van Sebroeck,
Guenter Roeck
Cc: oe-kbuild-all, linux-arm-kernel, linux-kernel, devicetree,
linux-watchdog, Alexey Charkov
Hi Alexey,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 92a09c47464d040866cf2b4cd052bc60555185fb]
url: https://github.com/intel-lab-lkp/linux/commits/Alexey-Charkov/dt-bindings-timer-via-vt8500-timer-Convert-to-YAML/20250516-025729
base: 92a09c47464d040866cf2b4cd052bc60555185fb
patch link: https://lore.kernel.org/r/20250515-vt8500-timer-updates-v3-3-2197a1b062bd%40gmail.com
patch subject: [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
config: loongarch-randconfig-r123-20250517 (https://download.01.org/0day-ci/archive/20250518/202505180911.hDevFA1N-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250518/202505180911.hDevFA1N-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505180911.hDevFA1N-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/clocksource/timer-vt8500.c:201:51: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *platform_data @@ got void [noderef] __iomem *static [assigned] [toplevel] regbase @@
drivers/clocksource/timer-vt8500.c:201:51: sparse: expected void *platform_data
drivers/clocksource/timer-vt8500.c:201:51: sparse: got void [noderef] __iomem *static [assigned] [toplevel] regbase
vim +201 drivers/clocksource/timer-vt8500.c
175
176 /*
177 * This probe gets called after the timer is already up and running. This will create
178 * the watchdog device as a child since the registers are shared.
179 */
180 static int vt8500_timer_probe(struct platform_device *pdev)
181 {
182 struct platform_device *vt8500_watchdog_device;
183 struct device *dev = &pdev->dev;
184 int ret;
185
186 if (!sys_timer_ch) {
187 dev_info(dev, "Not enabling watchdog: only one irq was given");
188 return 0;
189 }
190
191 if (!regbase)
192 return dev_err_probe(dev, -ENOMEM,
193 "Timer not initialized, cannot create watchdog");
194
195 vt8500_watchdog_device = platform_device_alloc("vt8500-wdt", -1);
196 if (!vt8500_watchdog_device)
197 return dev_err_probe(dev, -ENOMEM,
198 "Failed to allocate vt8500-wdt");
199
200 /* Pass the base address as platform data and nothing else */
> 201 vt8500_watchdog_device->dev.platform_data = regbase;
202 vt8500_watchdog_device->dev.parent = dev;
203
204 ret = platform_device_add(vt8500_watchdog_device);
205 if (ret)
206 platform_device_put(vt8500_watchdog_device);
207
208 return ret;
209 }
210
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
2025-05-18 1:23 ` kernel test robot
@ 2025-05-19 11:34 ` Alexey Charkov
2025-05-19 13:34 ` Guenter Roeck
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Charkov @ 2025-05-19 11:34 UTC (permalink / raw)
To: kernel test robot
Cc: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
Conor Dooley, Wim Van Sebroeck, Guenter Roeck, oe-kbuild-all,
linux-arm-kernel, linux-kernel, devicetree, linux-watchdog
On Sun, May 18, 2025 at 5:24 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Alexey,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on 92a09c47464d040866cf2b4cd052bc60555185fb]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Alexey-Charkov/dt-bindings-timer-via-vt8500-timer-Convert-to-YAML/20250516-025729
> base: 92a09c47464d040866cf2b4cd052bc60555185fb
> patch link: https://lore.kernel.org/r/20250515-vt8500-timer-updates-v3-3-2197a1b062bd%40gmail.com
> patch subject: [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
> config: loongarch-randconfig-r123-20250517 (https://download.01.org/0day-ci/archive/20250518/202505180911.hDevFA1N-lkp@intel.com/config)
> compiler: loongarch64-linux-gcc (GCC) 14.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20250518/202505180911.hDevFA1N-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202505180911.hDevFA1N-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
> >> drivers/clocksource/timer-vt8500.c:201:51: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *platform_data @@ got void [noderef] __iomem *static [assigned] [toplevel] regbase @@
> drivers/clocksource/timer-vt8500.c:201:51: sparse: expected void *platform_data
> drivers/clocksource/timer-vt8500.c:201:51: sparse: got void [noderef] __iomem *static [assigned] [toplevel] regbase
>
> vim +201 drivers/clocksource/timer-vt8500.c
>
> 175
> 176 /*
> 177 * This probe gets called after the timer is already up and running. This will create
> 178 * the watchdog device as a child since the registers are shared.
> 179 */
> 180 static int vt8500_timer_probe(struct platform_device *pdev)
> 181 {
> 182 struct platform_device *vt8500_watchdog_device;
> 183 struct device *dev = &pdev->dev;
> 184 int ret;
> 185
> 186 if (!sys_timer_ch) {
> 187 dev_info(dev, "Not enabling watchdog: only one irq was given");
> 188 return 0;
> 189 }
> 190
> 191 if (!regbase)
> 192 return dev_err_probe(dev, -ENOMEM,
> 193 "Timer not initialized, cannot create watchdog");
> 194
> 195 vt8500_watchdog_device = platform_device_alloc("vt8500-wdt", -1);
> 196 if (!vt8500_watchdog_device)
> 197 return dev_err_probe(dev, -ENOMEM,
> 198 "Failed to allocate vt8500-wdt");
> 199
> 200 /* Pass the base address as platform data and nothing else */
> > 201 vt8500_watchdog_device->dev.platform_data = regbase;
Frankly, given that this driver only applies to VT8500 (which is ARM
based), the warning appears a bit overzealous. After all, on ARM MMIO
addresses are in the same physical address space as normal memory
addresses, and furthermore this platform_data is never dereferenced
directly anyway.
I could silence the warning either by more aggressive casting or by
wrapping the pointer into some struct, but both of those sound a bit
overreaching. Would appreciate guidance from the list on how to best
approach this.
Best regards,
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
2025-05-19 11:34 ` Alexey Charkov
@ 2025-05-19 13:34 ` Guenter Roeck
2025-05-20 17:39 ` Alexey Charkov
0 siblings, 1 reply; 11+ messages in thread
From: Guenter Roeck @ 2025-05-19 13:34 UTC (permalink / raw)
To: Alexey Charkov, kernel test robot
Cc: Krzysztof Kozlowski, Daniel Lezcano, Thomas Gleixner, Rob Herring,
Conor Dooley, Wim Van Sebroeck, oe-kbuild-all, linux-arm-kernel,
linux-kernel, devicetree, linux-watchdog
On 5/19/25 04:34, Alexey Charkov wrote:
> On Sun, May 18, 2025 at 5:24 AM kernel test robot <lkp@intel.com> wrote:
>>
>> Hi Alexey,
>>
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on 92a09c47464d040866cf2b4cd052bc60555185fb]
>>
>> url: https://github.com/intel-lab-lkp/linux/commits/Alexey-Charkov/dt-bindings-timer-via-vt8500-timer-Convert-to-YAML/20250516-025729
>> base: 92a09c47464d040866cf2b4cd052bc60555185fb
>> patch link: https://lore.kernel.org/r/20250515-vt8500-timer-updates-v3-3-2197a1b062bd%40gmail.com
>> patch subject: [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
>> config: loongarch-randconfig-r123-20250517 (https://download.01.org/0day-ci/archive/20250518/202505180911.hDevFA1N-lkp@intel.com/config)
>> compiler: loongarch64-linux-gcc (GCC) 14.2.0
>> reproduce: (https://download.01.org/0day-ci/archive/20250518/202505180911.hDevFA1N-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202505180911.hDevFA1N-lkp@intel.com/
>>
>> sparse warnings: (new ones prefixed by >>)
>>>> drivers/clocksource/timer-vt8500.c:201:51: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *platform_data @@ got void [noderef] __iomem *static [assigned] [toplevel] regbase @@
>> drivers/clocksource/timer-vt8500.c:201:51: sparse: expected void *platform_data
>> drivers/clocksource/timer-vt8500.c:201:51: sparse: got void [noderef] __iomem *static [assigned] [toplevel] regbase
>>
>> vim +201 drivers/clocksource/timer-vt8500.c
>>
>> 175
>> 176 /*
>> 177 * This probe gets called after the timer is already up and running. This will create
>> 178 * the watchdog device as a child since the registers are shared.
>> 179 */
>> 180 static int vt8500_timer_probe(struct platform_device *pdev)
>> 181 {
>> 182 struct platform_device *vt8500_watchdog_device;
>> 183 struct device *dev = &pdev->dev;
>> 184 int ret;
>> 185
>> 186 if (!sys_timer_ch) {
>> 187 dev_info(dev, "Not enabling watchdog: only one irq was given");
>> 188 return 0;
>> 189 }
>> 190
>> 191 if (!regbase)
>> 192 return dev_err_probe(dev, -ENOMEM,
>> 193 "Timer not initialized, cannot create watchdog");
>> 194
>> 195 vt8500_watchdog_device = platform_device_alloc("vt8500-wdt", -1);
>> 196 if (!vt8500_watchdog_device)
>> 197 return dev_err_probe(dev, -ENOMEM,
>> 198 "Failed to allocate vt8500-wdt");
>> 199
>> 200 /* Pass the base address as platform data and nothing else */
>> > 201 vt8500_watchdog_device->dev.platform_data = regbase;
>
> Frankly, given that this driver only applies to VT8500 (which is ARM
> based), the warning appears a bit overzealous. After all, on ARM MMIO
> addresses are in the same physical address space as normal memory
> addresses, and furthermore this platform_data is never dereferenced
> directly anyway.
Guess we'll need AI compilers in the future to help them know that.
I for my part would argue that "this warning can be ignored" is the
source of many problems flying under the radar.
>
> I could silence the warning either by more aggressive casting or by
> wrapping the pointer into some struct, but both of those sound a bit
> overreaching. Would appreciate guidance from the list on how to best
> approach this.
>
First of all, I am quite sure that using platform drivers for this is the
wrong approach to start with. This seems to be a perfect candidate for
an auxiliary driver.
Second, I do consider passing an iomem pointer as platform data to be
inherently unsafe. I would very much prefer either passing a regmap
pointer or, if that doesn't work, a data structure.
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
2025-05-19 13:34 ` Guenter Roeck
@ 2025-05-20 17:39 ` Alexey Charkov
0 siblings, 0 replies; 11+ messages in thread
From: Alexey Charkov @ 2025-05-20 17:39 UTC (permalink / raw)
To: Guenter Roeck
Cc: kernel test robot, Krzysztof Kozlowski, Daniel Lezcano,
Thomas Gleixner, Rob Herring, Conor Dooley, Wim Van Sebroeck,
oe-kbuild-all, linux-arm-kernel, linux-kernel, devicetree,
linux-watchdog
On Mon, May 19, 2025 at 5:34 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 5/19/25 04:34, Alexey Charkov wrote:
> > On Sun, May 18, 2025 at 5:24 AM kernel test robot <lkp@intel.com> wrote:
> >>
> >> Hi Alexey,
> >>
> >> kernel test robot noticed the following build warnings:
> >>
> >> [auto build test WARNING on 92a09c47464d040866cf2b4cd052bc60555185fb]
> >>
> >> url: https://github.com/intel-lab-lkp/linux/commits/Alexey-Charkov/dt-bindings-timer-via-vt8500-timer-Convert-to-YAML/20250516-025729
> >> base: 92a09c47464d040866cf2b4cd052bc60555185fb
> >> patch link: https://lore.kernel.org/r/20250515-vt8500-timer-updates-v3-3-2197a1b062bd%40gmail.com
> >> patch subject: [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality
> >> config: loongarch-randconfig-r123-20250517 (https://download.01.org/0day-ci/archive/20250518/202505180911.hDevFA1N-lkp@intel.com/config)
> >> compiler: loongarch64-linux-gcc (GCC) 14.2.0
> >> reproduce: (https://download.01.org/0day-ci/archive/20250518/202505180911.hDevFA1N-lkp@intel.com/reproduce)
> >>
> >> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> >> the same patch/commit), kindly add following tags
> >> | Reported-by: kernel test robot <lkp@intel.com>
> >> | Closes: https://lore.kernel.org/oe-kbuild-all/202505180911.hDevFA1N-lkp@intel.com/
> >>
> >> sparse warnings: (new ones prefixed by >>)
> >>>> drivers/clocksource/timer-vt8500.c:201:51: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *platform_data @@ got void [noderef] __iomem *static [assigned] [toplevel] regbase @@
> >> drivers/clocksource/timer-vt8500.c:201:51: sparse: expected void *platform_data
> >> drivers/clocksource/timer-vt8500.c:201:51: sparse: got void [noderef] __iomem *static [assigned] [toplevel] regbase
> >>
> >> vim +201 drivers/clocksource/timer-vt8500.c
> >>
> >> 175
> >> 176 /*
> >> 177 * This probe gets called after the timer is already up and running. This will create
> >> 178 * the watchdog device as a child since the registers are shared.
> >> 179 */
> >> 180 static int vt8500_timer_probe(struct platform_device *pdev)
> >> 181 {
> >> 182 struct platform_device *vt8500_watchdog_device;
> >> 183 struct device *dev = &pdev->dev;
> >> 184 int ret;
> >> 185
> >> 186 if (!sys_timer_ch) {
> >> 187 dev_info(dev, "Not enabling watchdog: only one irq was given");
> >> 188 return 0;
> >> 189 }
> >> 190
> >> 191 if (!regbase)
> >> 192 return dev_err_probe(dev, -ENOMEM,
> >> 193 "Timer not initialized, cannot create watchdog");
> >> 194
> >> 195 vt8500_watchdog_device = platform_device_alloc("vt8500-wdt", -1);
> >> 196 if (!vt8500_watchdog_device)
> >> 197 return dev_err_probe(dev, -ENOMEM,
> >> 198 "Failed to allocate vt8500-wdt");
> >> 199
> >> 200 /* Pass the base address as platform data and nothing else */
> >> > 201 vt8500_watchdog_device->dev.platform_data = regbase;
> >
> > Frankly, given that this driver only applies to VT8500 (which is ARM
> > based), the warning appears a bit overzealous. After all, on ARM MMIO
> > addresses are in the same physical address space as normal memory
> > addresses, and furthermore this platform_data is never dereferenced
> > directly anyway.
>
> Guess we'll need AI compilers in the future to help them know that.
> I for my part would argue that "this warning can be ignored" is the
> source of many problems flying under the radar.
>
> >
> > I could silence the warning either by more aggressive casting or by
> > wrapping the pointer into some struct, but both of those sound a bit
> > overreaching. Would appreciate guidance from the list on how to best
> > approach this.
> >
>
> First of all, I am quite sure that using platform drivers for this is the
> wrong approach to start with. This seems to be a perfect candidate for
> an auxiliary driver.
TIL: auxiliary bus :)
Thanks for the pointer Guenter, it does indeed look like a more
appropriate choice. I'll try and port the driver to use that instead,
and resubmit.
> Second, I do consider passing an iomem pointer as platform data to be
> inherently unsafe. I would very much prefer either passing a regmap
> pointer or, if that doesn't work, a data structure.
I guess it resolves itself with the auxiliary driver approach, as I
can then just upcast the auxiliary device pointer to the parent's
enclosing private struct, which can then contain both a timer read
function and the specific pointers to the two registers the watchdog
needs. No need then for the child to do its arbitrary offsets into the
parent's iomem region - just use what's given directly. It's still
going to be iomem pointer access, but on the other hand putting a
layer of indirection (i.e. regmap) into the system timer code sounds a
bit scary to me.
Best regards,
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-05-20 17:44 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-15 18:55 [PATCH v3 0/4] clocksource/drivers/timer-vt8500: clean up and add watchdog function Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 1/4] dt-bindings: timer: via,vt8500-timer: Convert to YAML Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 2/4] clocksource/drivers/timer-vt8500: Add defines for magic constants Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 3/4] clocksource/drivers/timer-vt8500: Prepare for watchdog functionality Alexey Charkov
2025-05-18 1:23 ` kernel test robot
2025-05-19 11:34 ` Alexey Charkov
2025-05-19 13:34 ` Guenter Roeck
2025-05-20 17:39 ` Alexey Charkov
2025-05-15 18:55 ` [PATCH v3 4/4] watchdog: Add support for VIA/WonderMedia SoC " Alexey Charkov
2025-05-16 6:55 ` kernel test robot
2025-05-16 13:02 ` Alexey Charkov
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).