On 30/06/24 09:03, Sander Vanheule wrote: > 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? Yes realtek_otto_wdt.c. I thought that was only on openwrt but looks like you've already landed that one. I'll reword my message to reflect that. >> 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@xxxxxx> >> Signed-off-by: Sander Vanheule <sander@xxxxxxxxxxxxx> >> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx> >> --- > 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@xxxxxxxxxxxxx/ OK thanks for the link. I'll try and make sure I'm not rehashing things that have already been discussed. >> + >> +/* 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) I'm not exactly sure either and I've only got the single core RTL9302 to test with so if there were some difference I may not notice. Eventually we're planning a RTL931x based board so that may be something I can check then. >> + .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 >
