[PATCH v3 03/11] clocksource: sp804: add device tree support

From: haojian.zhuang@linaro.org (Haojian Zhuang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 03/11] clocksource: sp804: add device tree support
Date: Wed, 13 Mar 2013 19:37:52 +0800	[thread overview]
Message-ID: <CAD6h2NR0=yXehExi-=gamsxmX7M2PACiY91F=O2woDYQeNFGmA@mail.gmail.com> (raw)
In-Reply-To: <1363172730.3100.17.camel@hornet>

On 13 March 2013 19:05, Pawel Moll <pawel.moll@arm.com> wrote:
> On Wed, 2013-03-13 at 05:05 +0000, Haojian Zhuang wrote:
>> +++ b/Documentation/devicetree/bindings/timer/arm,sp804.txt
>> @@ -0,0 +1,27 @@
>> +ARM sp804 Dual Timers
>> +---------------------------------------
>> +
>> +Required properties:
>> +- compatible: Should be "arm,sp804" & "arm,primecell"
>> +- interrupts: Should contain the list of Dual Timer interrupts
>> +     interrupts = <0 0 4>, <0 1 4>;
>
> SP804 has three interrupt outputs: TIMINT1 (interrupt generated by the
> first timer), TIMINT2 (the second timer) and TIMINTC (combined - logical
> sum of the two former ones). You may want to describe this somehow to
> make the choice possible later (eg. VE has the combined interrupt wired
> while - if I understand what Rob is saying correctly - Highbank uses
> only the TIMINT1).
>
I prefer to ignore the TIMINTC. It seems that nobody is using this interrupt.

>> +- reg: Should contain location and length for dual timer register.
>> +- clocks: clock driving the dual timer hardware
>> +     clocks = <&timclk0 &timclk1>;
>
> Again, there are three "clock" inputs (even ignoring the APBCLK):
> TIMCLK, TIMCLKEN1, TIMCLKEN2. The real clocking rate for the timer
> depends on the TIMCLK and respective TIMCLKENx - see
> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0271d/CJABHCIG.html
>
> The driver than could make educated choice based on this information.
>
> You may choose to ignore this fact (and require only a clock
> representing the effective rate). I did it for the VE DTS, but it still
> doesn't seem completely right
>
Sure. I can append it.

>> +Optional properties:
>> +- arm,sp804-clocksource: Should contain the register offset of TIMER1 or
>> +  TIMER2 in Dual Timer Controller.
>> +     arm,sp804-clocksource = <0x20>;
>
> You seem to be missing the "arm,sp804-clockevent" one here.
>
Yes, I missed. I'll update the README.