| Message ID | 20250521-vt8500-timer-updates-v4-4-2d4306a16ae4@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | clocksource/drivers/timer-vt8500: clean up and add watchdog function | expand |
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"); >
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
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
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");
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(+)