> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Sent: Tuesday, March 8, 2022 3:44 PM
> To: Rob Herring <robh@xxxxxxxxxx>
> Cc: Sanil, Shruthi <shruthi.sanil@xxxxxxxxx>; Daniel Lezcano
> <daniel.lezcano@xxxxxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Mark Gross
> <mgross@xxxxxxxxxxxxxxx>; Thokala, Srikanth <srikanth.thokala@xxxxxxxxx>;
> Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@xxxxxxxxx>;
> Sangannavar, Mallikarjunappa <mallikarjunappa.sangannavar@xxxxxxxxx>
> Subject: Re: [PATCH v8 1/2] dt-bindings: timer: Add bindings for Intel Keem
> Bay SoC Timer
>
> On Mon, Mar 07, 2022 at 04:33:23PM -0600, Rob Herring wrote:
> > On Wed, Feb 23, 2022 at 5:31 AM Andy Shevchenko
> > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > >
> > > On Tue, Feb 22, 2022 at 05:13:41PM -0600, Rob Herring wrote:
> > > > On Tue, Feb 22, 2022 at 03:26:53PM +0530, shruthi.sanil@xxxxxxxxx
> wrote:
> > > > > From: Shruthi Sanil <shruthi.sanil@xxxxxxxxx>
> > > > >
> > > > > Add Device Tree bindings for the Timer IP, which can be used as
> > > > > clocksource and clockevent device in the Intel Keem Bay SoC.
> > >
> > > ...
> > >
> > > > > + soc {
> > > > > + #address-cells = <0x2>;
> > > > > + #size-cells = <0x2>;
> > > > > +
> > > > > + gpt@20331000 {
> > > > > + compatible = "intel,keembay-gpt-creg",
> > > > > + "simple-mfd";
> > > >
> > > > It looks like you are splitting things based on Linux
> > > > implementation details. Does this h/w block have different
> > > > combinations of timers and counters? If not, then you don't need
> > > > the child nodes at all. There's plenty of h/w blocks that get used as both
> a clocksource and clockevent.
> > > >
> > > > Maybe I already raised this, but assume I don't remember and this
> > > > patch needs to address any questions I already asked.
> > >
> > > I dunno if I mentioned that hardware seems to have 5 or so devices
> > > behind the block, so ideally it should be one device node that
> > > represents the global register spaces and several children nodes.
> >
> > Is it 5 devices or 9 devices?
>
> 5 devices, one of which is a timer block out of 8 timers.
> You may count them as 12 altogether.
>
> > > However, I am not familiar with the established practices in DT
> > > world, but above seems to me the right thing to do since it
> > > describes the hardware as is (without any linuxisms).
> >
> > The Linuxism in these cases defining 1 node per driver because that's
> > what is convenient for automatic probing. That appears to be exactly
> > the case here. The red flag is nodes with a compatible and nothing
> > else. The next question is whether the sub-devices are blocks that
> > will be assembled in varying combinations and quantities. If not, then
> > not much point subdividing the h/w blocks.
>
> AFAIU the hardware architecture the amount of timers is dependent on the
> IP synthesis configuration. On this platform it's 8, but it may be
> 1 or 2, for example.
Yes, the number of timers can vary between platforms.
For eg., Intel Keem Bay SoC has 8 timers where as in Intel Thunder Bay SoC has 6 timers.
>
> > There's also many cases of having multiple 'identical' timers and
> > wanting to encode which timer gets assigned to clocksource vs.
> > clockevent. But those 'identical' timers aren't if you care about
> > which timer gets assigned where. I *think* that's not the case here
> > unless you are trying to pick the timer for the clockevent by not
> > defining the other timers.
> >
> > Without having a complete picture of what's in 'gpt-creg', I can't
> > give better advice.
>
> I guess they need to share TRM, if possible, to show what this block is.
>
I would like to explain briefly about the Timer IP in the Keem Bay Soc.
The Timers block contains 8 general purpose timers, a free running counter. Each general purpose timer can generate an individual interrupt to the interrupt controller.
The timer block consists of secure and non-secure timers. Hence there are secure and non-secure registers in separate address banks.
The secure register bank consists of the common control register where the timers and counters need to be enabled.
