On Tue, 2014-05-13 at 04:08PM +0200, Matthias Brugger wrote:

2014-05-13 15:08 GMT+02:00 S?ren Brinkmann [off-list ref]:

quoted

Hi Matthias,

On Tue, 2014-05-13 at 01:49AM +0200, Matthias Brugger wrote:

quoted

Add binding documentation for the General Porpose Timer driver of
the Mediatek SoCs.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
 .../devicetree/bindings/timer/mediatek,mtk-timer.txt   | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
new file mode 100644
index 0000000..d0f2df3
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt

@@ -0,0 +1,18 @@
+Mediatek MT6589, MT6577 and MT6572 Timers
+---------------------------------------
+
+Required properties:
+- compatible: Should be "mediatek,mtk6589-timer"
+- reg: Should contain location and length for timers register.
+- clocks: phandle to the clock source; the first refers to a 13 MHz fixed
+             system clock and the second handle to a 32 KHz fixed RTC
+             clock.

Are these frequencies mandatory to the timer or an implementation
detail of the SOC you're working with? I suspect, it might be possible
to see the same timer in a different SOC implementation with different
frequencies? In that case - or probably in general - the frequencies
should not be part of the binding, IMHO.

I had a look at the implementation of several Mediatek SoCs and all of
them have the timer with the same clock connections.
I don't know if the IP can be or is actually used with other clocksources.
I can try to reformulate the description to make the clock constraints
sound less mandatory.
Although I would prefer to leave a mention about the clock frequencies
(like e.g. in the allwinner,sun4i-timer).

Since the bindings should be generic, I think the frequencies shouldn't
be here. From DT/HW perspective their frequencies is irrelevant and a
pure implementation detail, IMHO.

quoted

quoted

+
+Examples:
+
+     timer {
+             compatible = "mediatek,mtk6589-timer";
+             reg = <0x10008000 0x80>;
+             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
+             clocks = <&system_clk>, <&rtc_clk>;

Might be just my personal preference, but you could also add the clock-names
property which would relax the ordering requirement a bit and would
clearly  identify the IP's clocks.

In the v1 of the patch series I had the clock names included [1] but
because of comments of others I removed them [1].
Actually the driver just use the first clock, but from my
understanding the DT reflects as well the HW configuration and for
this reason I put the two clocks in the DT.

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/69155/

I had cases where the same IP was implemented by different SOC vendors
differently. In that cases having the clock-names in the bindings is
extremely helpful when you try to match the binding with the IP docs.
Hence, I'd recommend being explicit and list the names.

	S?ren