Re: [PATCH 46/58] clocksource/drivers: Add a new driver for the Atmel ARM TC blocks

[Daniel Lezcano <daniel.lezcano@linaro.org>]

+Mark Rutland, +Rob Herring


Alexandre, Boris, have a look at https://www.spinics.net/lists/arm-kernel/msg572652.html

That will tell you the story.

On Thu, Jun 08, 2017 at 07:42:36AM +0200, Boris Brezillon wrote:
> 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>;
> 		};
> 	};

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog