Re: [PATCH v4 4/4] watchdog: Add support for VIA/WonderMedia SoC watchdog functionality

On 5/20/25 23:20, Alexey Charkov wrote:

On Wed, May 21, 2025 at 3:15 AM Guenter Roeck [off-list ref] wrote:

quoted

On 5/20/25 13:01, Alexey Charkov wrote:

quoted

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

quoted

quoted

+     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