linux-kernel - Re: [PATCH v5 1/3] clocksource/drivers/timer-tegra186: add WDIOC

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: <4ks74upuufmt2ibh5ur5zpazvfj66ak4gyq7v4rtz2zi2u5wsi@rls64ws3rukp>
Date: Tue, 29 Apr 2025 16:23:22 +0200
From: Thierry Reding <thierry.reding@...il.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: Jon Hunter <jonathanh@...dia.com>, Robert Lin <robelin@...dia.com>, 
	tglx@...utronix.de, pohsuns@...dia.com, linux-kernel@...r.kernel.org, 
	linux-tegra@...r.kernel.org, sumitg@...dia.com
Subject: Re: [PATCH v5 1/3] clocksource/drivers/timer-tegra186: add
 WDIOC_GETTIMELEFT support

On Tue, Apr 29, 2025 at 03:19:25PM +0200, Daniel Lezcano wrote:
> On 29/04/2025 11:15, Jon Hunter wrote:
> > Hi Daniel,
> > 
> > On 29/04/2025 09:59, Daniel Lezcano wrote:
> > > On Mon, Apr 21, 2025 at 06:08:19PM +0800, Robert Lin wrote:
> > > > From: Pohsun Su <pohsuns@...dia.com>
> > > > 
> > > > This change adds support for WDIOC_GETTIMELEFT so userspace
> > > > programs can get the number of seconds before system reset by
> > > > the watchdog timer via ioctl.
> > > > 
> > > > Signed-off-by: Pohsun Su <pohsuns@...dia.com>
> > > > Signed-off-by: Robert Lin <robelin@...dia.com>
> > > > ---
> > > 
> > > Hi Robert,
> > > 
> > > I realize that this driver should be split in two and the watchdog
> > > part go
> > > under drivers/watchdog.
> > 
> > Are there any other examples you know of where the timer is split in
> > this way? It is not clear to me how you propose we do this?
> 
> Just keep the clocksource and move the watchdog code (everything related to
> the watchdog_ops) to a new driver under drivers/watchdog

That's a bad idea. This is all a single register space, so we can't have
"proper" drivers (i.e. ones that exclusively request I/O memory regions)
if we split them up.

I understand that it's nice and easy to have things split up along
subsystem boundaries, but sometimes hardware designs just aren't that
cleanly separated.

> BTW, there are three clocksources with the same rating, what is the point of
> having them supported ?
> 
> Is it not the architected clocksource enough ?

The TSC clock source that this driver exposes is different from the
architected timer. It's a SoC-wide clock that is routed to various IP
blocks and used for timestamping events. This clocksource allows these
events to be properly compared, etc.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)