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

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Biju,

On Tue, Dec 6, 2022 at 9:13 AM Biju Das <biju.das.jz@bp.renesas.com> 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.
         */

> > > 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?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds