Messages in this thread | | | Date | Thu, 8 Jun 2017 07:42:36 +0200 | From | Boris Brezillon <> | Subject | Re: [PATCH 46/58] clocksource/drivers: Add a new driver for the Atmel ARM TC blocks |
| |
Le Thu, 8 Jun 2017 01:17:15 +0200, Alexandre Belloni <alexandre.belloni@free-electrons.com> a écrit :
> On 07/06/2017 at 23:08:48 +0200, Daniel Lezcano wrote: > > > I was going to agree but this is not flexible enough because the > > > quadrature decoder always uses the first two channels. So on some > > > products, we may have: > > > - TCB0: > > > o channels 0,1: qdec > > > o channel 2: clocksource > > > > > > - TCB1: > > > o channels 0,1: qdec > > > o channel 2: clockevent > > > > > > This avoids wasting TCB channels. > > > > Ok. In this case you can check if the interrupt is specified for the node, if > > yes, then it is a clockevent. > > > > But currently it is always specified in the SoC's dtsi. I don't find > that too practical to push that to the board's dts. Also, lying by > omission (the IRQ is always wired) in the DT is not different from > having a property selecting which timer is the clocksource and which is > the clockevent. >
I agree with Alexandre here. Really, there's not much we can do to detect which timer should be used as a clockevent and which one should be used as a clocksource except explicitly specifying it in the DT. Having an interrupt defined in one case (clockevent) and undefined in the other case (clocksource), is just as hack-ish as the detection logic Alexandre developed to avoid explicitly specifying the function assigned to a specific timer.
Can we please find a solution that makes everyone happy (DT, clocksoure/clockevent and at91 maintainers)?
How about adding a linux,timer-function property to specify which function this timer is providing?
Something like that for example:
tcb0: timer@fff7c000 { compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon"; #address-cells = <1>; #size-cells = <0>; reg = <0xfff7c000 0x100>; interrupts = <18 4>; clocks = <&tcb0_clk>, <&clk32k>; clock-names = "t0_clk", "slow_clk";
timer@0 { compatible = "atmel,tcb-timer"; reg = <0>, <1>; linux,timer-function = "clocksource"; };
timer@2 { compatible = "atmel,tcb-timer"; reg = <2>; linux,timer-function = "clockevent"; }; };
Alternatively, we could have a property or a node in chosen describing which timer should be used:
chosen { clockevent { timer = <&timer2>; };
clocksource { timer = <&timer0>; };
/* * or * * clockevent = <&timer2>; * clocksource = <&timer0>; * * but I think the clocksource/clockevent node approach * is more future proof in case we need to add extra * information like the expected resolution/precision or * anything that could be tweakable. */ };
tcb0: timer@fff7c000 { compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon"; #address-cells = <1>; #size-cells = <0>; reg = <0xfff7c000 0x100>; interrupts = <18 4>; clocks = <&tcb0_clk>, <&clk32k>; clock-names = "t0_clk", "slow_clk";
timer0: timer@0 { compatible = "atmel,tcb-timer"; reg = <0>, <1>; };
timer2: timer@2 { compatible = "atmel,tcb-timer"; reg = <2>; }; };
| |