linux-kernel - Re: [PATCH v3 7/9] clocksource: realtek: Add timer driver for rtl-otto platforms

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] [thread-next>] [day] [month] [year] [list]

Date: Sat, 29 Jun 2024 23:03:12 +0200
From: Sander Vanheule <sander@...nheule.net>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>, tglx@...utronix.de, 
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
 tsbogend@...ha.franken.de, daniel.lezcano@...aro.org,
 paulburton@...nel.org,  peterz@...radead.org, mail@...ger-koblitz.de,
 bert@...t.com, john@...ozen.org
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-mips@...r.kernel.org, kabel@...nel.org, ericwouds@...il.com, Markus
 Stockhausen <markus.stockhausen@....de>
Subject: Re: [PATCH v3 7/9] clocksource: realtek: Add timer driver for
 rtl-otto platforms

Hi Chris,

On Thu, 2024-06-27 at 16:33 +1200, Chris Packham wrote:
> The timer/counter block on the Realtek SoCs provides up to 5 timers. It
> also includes a watchdog timer but this isn't being used currently (it
> will be added as a separate wdt driver).

Do you mean the watchdog timer supported by drivers/watchdog/realtek_otto_wdt.c? Or are
you referring to another watchdog timer?

> One timer will be used per CPU as a local clock event generator. An
> additional timer will be used as an overal stable clocksource.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@....de>
> Signed-off-by: Sander Vanheule <sander@...nheule.net>
> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> ---

For reference, I submitted a driver for the same hardware back in 2022, but didn't manage
to follow up to finalize the submission:

https://lore.kernel.org/all/cover.1642369117.git.sander@svanheule.net/


> +
> +/* Module initialization part. */
> +static DEFINE_PER_CPU(struct timer_of, rttm_to) = {
> +	.flags				= TIMER_OF_BASE | TIMER_OF_CLOCK |
> TIMER_OF_IRQ,
> +	.of_irq = {
> +		.flags			= IRQF_PERCPU | IRQF_TIMER,

In the original review of this code, I had some doubts about the use of IRQF_PERCPU. Maybe
the people in Cc can shed some light on this.

If I understand correctly, the SoC interrupts these timers use are not per-cpu interrupts.
(For comparison, AFAICT the MIPS CPU interrupts are)



> +		.handler		= rttm_timer_interrupt,
> +	},
> +	.clkevt = {
> +		.rating			= 400,
> +		.features		= CLOCK_EVT_FEAT_PERIODIC |
> CLOCK_EVT_FEAT_ONESHOT,

If the use of IRQF_PERCPU is appropriate, I wonder if the driver should also use
CLOCK_EVT_FEAT_PERCPU.


Best,
Sander