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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>




[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux