RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

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

 



Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
> 
> Hi Biju,
> 
> On Tue, Dec 6, 2022 at 9:13 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > > Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM)
> > > support On Mon, Dec 05, 2022 at 02:59:49PM +0000, Biju Das wrote:
> > > > This patch series aims to add support for Compare-Match Timer
> > > > (TIM) module found on RZ/V2M SoC.
> > > >
> > > > it is composed of 32 channels and channels 0-7 and 24-32 are
> > > > reserved for ISP usage.
> > > >
> > > > Channel 22 is modelled as clock source and Channel 23 is modelled
> > > > as clock event driver and the rest of the channels are modelled as
> > > > counter driver as it provides
> > >
> > > Why did you pick those 2 counters for those functions?
> >
> > Currently it uses architecture timer for broadcast timer, so I thought
> > Since TIM has 24 channels, use 1 channel for broadcast timer and 1
> > Channel for clock source. But having said that SoC has an aarch64
> > architecture clock source strictly speaking we don't need this.
> >
> > > Unless the h/w blocks are different, this is an abuse of compatible
> > > strings. What's the h/w difference that makes you care which counter
> > > the OS picks? That's what the DT should describe. If any timer will
> > > do, just let the OS pick.
> >
> > There is no HW difference. Same HW block can be used for mutually
> > exclusive functionality.
> >
> > One is for Linux Clock source/event functionality((scheduler
> > tick/broadcast tick etc) and
> >
> > the other purpose is to expose count and event ticks from this module
> > to user space, so that wide range of applications can make use of it.
> >
> > If it is an abuse of compatible strings for mutually exclusive
> > functionality , then I would like to drop clock source and use all the
> > channels as Either clock events(for broadcast ticks and real time
> usage??) or as counters.
> >
> > If this is not OK, then I need to pick one. I will go with counters.
> >
> > Please share your thoughts.
> 
> Can't you handle this like sh_cmt.c does:
> 
>         /*
>          * Use the first channel as a clock event device and the second
> channel
>          * as a clock source. If only one channel is available use it for
> both.
>          */

Currently it is handled like above except for the case " If only one channel is available use it for
Both", see patch#3 [1] probe function.
The only difference is here we have separate address space, clocks, and interrupts for each channel.
[1]
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20221205145955.391526-4-biju.das.jz@xxxxxxxxxxxxxx/

Our customer BSP, uses this hw module for exposing timer interrupt event to user space
by using custom driver. The same functionality can be achieved through counter driver.
That is the reason, I have added counter driver to expose this functionality as well.

> 
> > > > 1) counter for counting
> > > > 2) configurable counter value for generating timer interrupt
> > > > 3) userspace event for each interrupt.
> > > >
> > > > logs:-
> > > > Counter driver:
> > > > Counter driver is tested by reading counts and interrupts tested
> > > > by counter-example in tools/counter/counter_example.c
> > > >
> > > > Count snapshot value:
> > > > 3114
> > > > Output from counter_example when it triggers interrupts:
> > > > Timestamp 0: 24142152969        Count 0: 5
> > > > Error Message 0: Success
> > > >
> > > > Clock source:
> > > > Clock source driver is tested by clock-source-switch app.
> > > > [ 1275.703567] clocksource: Switched to clocksource
> > > > arch_sys_counter [ 1275.710189] clocksource: Switched to
> > > > clocksource a4000b00.timer
> > >
> > > Do you have any use case to really switch. Doing so disables the
> > > vDSO access to the clocksource.
> >
> > Not really. Architecture timer should be sufficient for clocksource.
> 
> When multiple clocksources are registered, the clocksource subsystems
> picks the best one anyway, right?

Yes, it picks based on the rating.

Cheers,
Biju




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux