Re: [PATCH v3] ARM: at91: Document new TCB bindings

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Mon, 4 Jul 2016 14:11:27 +0200
Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> On 04/07/2016 at 14:03:58 +0200, Boris Brezillon wrote :
> > On Mon, 4 Jul 2016 12:36:31 +0200
> > Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> >   
> > > On 04/07/2016 at 12:24:52 +0200, Boris Brezillon wrote :  
> > > > On Fri,  1 Jul 2016 23:52:05 +0200
> > > > Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:    
> > > > > +One interrupt per TC block:
> > > > > +	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>;
> > > > > +		};
> > > > > +
> > > > > +		timer@2 {
> > > > > +			compatible = "atmel,tcb-timer";
> > > > > +			reg = <2>;
> > > > > +		};    
> > > > 
> > > > And how can you differentiate the clocksource from the clkevent?
> > > >     
> > > 
> > > It doesn't really matter actually, I'll do the selection in the driver,
> > > as suggested by Rob.
> > >   
> > 
> > Yes, I've read Rob's review, but then what's the point of defining 2
> > timer nodes, just do the detection based on the number of channels
> > you've reserved for the timer and define a single node.  
> 
> I agree this is a really hypothetical use case but one may want to have
> the clocksource on one TCB and the clockevent on another. This would
> allow to have for example a quadrature decoder and the clocksource on
> one TCB and another quadrature decoder and the clockevent device on
> another TCB.
> 
> 

Hm, that sounds like a valid but unlikely use case, but it makes the
DT bindinds even more obscure. When should one define a single timer
node, when to define two, how many channels per timer depending on the
type of timer you want to instantiate, etc. That should probably be
documented.

ITOH, by choosing the single timer-node approach, you hide everything in
the driver, and let the implementation choose the correct channels for
the clkevent and clksource devices.