On Thu, Apr 30, 2020 at 10:31 AM Tony Lindgren <tony@xxxxxxxxxxx> wrote: > > * Rob Herring <robh@xxxxxxxxxx> [200430 14:01]: > > On Mon, Apr 27, 2020 at 08:23:29AM -0700, Tony Lindgren wrote: > > > * Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> [200427 15:03]: > > > > On 27/04/2020 16:31, Tony Lindgren wrote: > > > > > Hi, > > > > > > > > > > * Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> [200427 09:19]: > > > > >> On 17/04/2020 18:55, Tony Lindgren wrote: > > > > >>> --- a/Documentation/devicetree/bindings/timer/ti,timer.txt > > > > >>> +++ b/Documentation/devicetree/bindings/timer/ti,timer.txt > > > > >>> @@ -14,6 +14,8 @@ Required properties: > > > > >>> ti,omap5430-timer (applicable to OMAP543x devices) > > > > >>> ti,am335x-timer (applicable to AM335x devices) > > > > >>> ti,am335x-timer-1ms (applicable to AM335x devices) > > > > >>> + ti,dmtimer-clockevent (when used as for clockevent) > > > > >>> + ti,dmtimer-clocksource (when used as for clocksource) > > > > >> > > > > >> Please, submit a separate patch for this. > > > > >> > > > > >> Before you resend as is, this will be nacked as clocksource / clockevent > > > > >> is not a hardware description but a Linux thing. > > > > >> > > > > >> Finding a way to characterize that from the DT is an endless discussion > > > > >> since years, so I suggest to use a single property for the timer eg > > > > >> <ti,dmtimer> and initialize the clocksource and the clockevent in the > > > > >> driver. > > > > > > > > > > Hmm good point. We still need to specify which timer is a clocksource > > > > > and which one a clockevent somehow. > > > > > > > > > > Maybe we could have a generic properties like the clock framework such as: > > > > > > > > > > assigned-system-clocksource > > > > > assigned-system-clockevent > > > > > > > > I think that will be the same problem :/ > > > > > > Seems like other SoCs have the same issue too with multiple timers > > > to configure. > > > > > > > Is it possible to check the interrupt for the clockevent ? A timer node > > > > with the interrrupt is the clockevent, without it is a clocksource. > > > > > > OK let's try that. So the configuration would become then: > > > > > > compatible = "ti,dmtimer; /* reserved for system timers */ > > > /delete-property/interrupts; /* ok so it's a clocksource */ > > > /delete-property/interrupts-extended; > > > > That's not really what was meant. > > OK, so let's figure out something better then. > > > Let's say you have N timers. Either every timer is exactly the same and > > the OS can just assign them however it wants or there is some difference > > in the h/w making certain timer better for certain functions. Describe > > that difference. It could be clock rate, number of counter bits, always > > on, secure mode access only, has or doesn't have output compare or input > > capture, etc. > > Hmm. Trying to detect this automatically will get messy. For example, > we have few omap3 boards with the following options that also need to > consider if the separate 32KiHz counter is available: > > 1. The best case scenario > > ti,omap-counter32k clocksource > ti,sysc-omap2-timer ti,timer-alwon clockevent (timer1) > > 2. Boards relying on internal clock with unusable 32k counter > > ti,sysc-omap2-timer ti,timer-alwon clocksource (timer12) > ti,sysc-omap2-timer clockevent (typically gpt2) > > In the second case, the 32k counter is unusable, and timer1 > is unusable with the external 32k always on clock. But timer1 > can be used with the system clock just fine for other purposes. > So ideally we would not tag timer1 as disabled :) I'm perfectly fine with a 'broken 32k clk' type property. Though I think the compatibility story is not good changing DT for stuff needed to boot and needed early in boot. It's one thing to break something not required to get a system booted. > For the second case, we could remove ti,timer-alwon property > for timer1, and tag the 32k counter as disabled as the source > clock is unreliable. Then somewhere in the code we would need > to check if ti,omap-counter32k is availabe, then check if > timer1 is always-on, then use timer12 if not a secure device > like n900. IIRC, there's some OMAP timer properties for secure vs. non-secure. (It's not the first time we've had this discussion on TI timers.) > If the board wants to use the system clock as the source for > a higher resolution with assigned-clock-parents, then we'd need > to ignore the always-on property and not use the 32k counter as > the clocksource. Basically to somehow figure out that a higher > resolution configuration is preferred over a > low-power configuration. That could be something you want to pick at run-time. > So what's your take on just adding the generic properties for > assigned-system-clocksource and clockevent? I'm tired of discussing this for 10 years... Rob
