linux-kernel - Re: [PATCH v2 3/4] clocksource/drivers/timer-vt8500: Add watchdog functionality

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABjd4YyX1nrM1qREm-dtzQUCM=TH1L8w7KeZ4ZUxg7tNH84TJg@mail.gmail.com>
Date: Tue, 13 May 2025 20:02:42 +0400
From: Alexey Charkov <alchark@...il.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Rob Herring <robh@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH v2 3/4] clocksource/drivers/timer-vt8500: Add watchdog functionality

On Tue, May 13, 2025 at 5:40 PM Daniel Lezcano
<daniel.lezcano@...aro.org> wrote:
>
> On Wed, May 07, 2025 at 10:58:32AM +0400, Alexey Charkov wrote:
> > VIA/WonderMedia system timer IP 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 match register 0 for watchdog
> > respectively.
>
> This code falls under the watchdog umbrella not in the clocksource. It
> is better to find a way to make the timer and the watchdog separated.
>
> The timer-gxp.c is dynamically allocating a watchdog platform device
> with the shared pointer to the timer counter. IMO, it is a good
> example to split this up.

Thanks for the pointer Daniel!

I guess in this case I'll need to pass the pointer to a full
clocksource read function as platform data for the watchdog, as it's a
bit more messy here than timer-gxp.c in that this hardware cannot do
an atomic MMIO read of the counter and needs a synchronization request
to be issued and cleared before reading the value.

The clocksource code will then only instantiate a platform device for
the watchdog when it can ensure that nothing uses the first match
register.

> > Signed-off-by: Alexey Charkov <alchark@...il.com>
> > ---
> >  drivers/clocksource/Kconfig        | 11 +++++++
> >  drivers/clocksource/timer-vt8500.c | 61 ++++++++++++++++++++++++++++++++++----
> >  2 files changed, 67 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 487c8525996724fbf9c6e9726dabb478d86513b9..8f5e41ff23386d9ecb46b38603dae485db71cfc7 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -181,6 +181,17 @@ config VT8500_TIMER
> >       help
> >         Enables support for the VT8500 driver.
> >
> > +config VT8500_TIMER_WATCHDOG
> > +     bool "Enable VT8500 watchdog functionality"
> > +     depends on VT8500_TIMER
> > +     depends on WATCHDOG && WATCHDOG_CORE=y
>
> if WATCHDOG_CORE=y then WATCHDOG=y because the first one can be
> enabled only if the second one is enabled.

Noted, thanks. Will replace it with just WATCHDOG_CORE=y in the next
version. In fact, if the watchdog functionality becomes a separate
platform driver then it could also potentially be built as a module,
so "=y" might also go.

Best regards,
Alexey