diff mbox

[v9,3/3] dt-bindings: add mtk-timer bindings

Message ID 1403264929-21325-4-git-send-email-matthias.bgg@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthias Brugger June 20, 2014, 11:48 a.m. UTC
Add binding documentation for the General Porpose Timer driver of
the Mediatek SoCs.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/timer/mediatek,mtk-timer.txt          |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt

Comments

Mark Rutland June 20, 2014, 12:36 p.m. UTC | #1
On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
> Add binding documentation for the General Porpose Timer driver of
> the Mediatek SoCs.
> 
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  .../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..10b7e09
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> @@ -0,0 +1,18 @@
> +Mediatek MT6577, MT6572 and MT6589 Timers
> +---------------------------------------
> +
> +Required properties:
> +- compatible: Should be "mediatek,mt6577-timer"
> +- reg: Should contain location and length for timers register.
> +- clocks: Clocks driving the timer hardware. This list should include two
> +	clocks. The order is system clock and as second clock the RTC clock.
> +
> +Examples:
> +
> +	timer@10008000 {
> +		compatible = "mediatek,mt6577-timer";
> +		reg = <0x10008000 0x80>;
> +		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&system_clk>, <&rtc_clk>;
> +		clock-names = "system-clk", "rtc-clk";

These names should be mentioned above, or removed.

This looks fine otherwise.

Mark.
Matthias Brugger July 1, 2014, 5:46 p.m. UTC | #2
2014-06-20 14:36 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
> On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
>> Add binding documentation for the General Porpose Timer driver of
>> the Mediatek SoCs.
>>
>> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>  .../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..10b7e09
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
>> @@ -0,0 +1,18 @@
>> +Mediatek MT6577, MT6572 and MT6589 Timers
>> +---------------------------------------
>> +
>> +Required properties:
>> +- compatible: Should be "mediatek,mt6577-timer"
>> +- reg: Should contain location and length for timers register.
>> +- clocks: Clocks driving the timer hardware. This list should include two
>> +     clocks. The order is system clock and as second clock the RTC clock.
>> +
>> +Examples:
>> +
>> +     timer@10008000 {
>> +             compatible = "mediatek,mt6577-timer";
>> +             reg = <0x10008000 0x80>;
>> +             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
>> +             clocks = <&system_clk>, <&rtc_clk>;
>> +             clock-names = "system-clk", "rtc-clk";
>
> These names should be mentioned above, or removed.

I added the clock-names to make it the clock order clearer, as it is
done in the arm sp,804 [1].
But I think it is clear enough through the phandle names and the
description of the clocks property.
So I will delete the clock-names in the v10 round.

Cheers,
Matthias

[1] http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/timer/arm,sp804.txt

>
> This looks fine otherwise.
>
> Mark.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Soren Brinkmann July 1, 2014, 6 p.m. UTC | #3
On Tue, 2014-07-01 at 07:46PM +0200, Matthias Brugger wrote:
> 2014-06-20 14:36 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
> > On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
> >> Add binding documentation for the General Porpose Timer driver of
> >> the Mediatek SoCs.
> >>
> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> >> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> ---
> >>  .../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..10b7e09
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> >> @@ -0,0 +1,18 @@
> >> +Mediatek MT6577, MT6572 and MT6589 Timers
> >> +---------------------------------------
> >> +
> >> +Required properties:
> >> +- compatible: Should be "mediatek,mt6577-timer"
> >> +- reg: Should contain location and length for timers register.
> >> +- clocks: Clocks driving the timer hardware. This list should include two
> >> +     clocks. The order is system clock and as second clock the RTC clock.
> >> +
> >> +Examples:
> >> +
> >> +     timer@10008000 {
> >> +             compatible = "mediatek,mt6577-timer";
> >> +             reg = <0x10008000 0x80>;
> >> +             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> >> +             clocks = <&system_clk>, <&rtc_clk>;
> >> +             clock-names = "system-clk", "rtc-clk";
> >
> > These names should be mentioned above, or removed.
> 
> I added the clock-names to make it the clock order clearer, as it is
> done in the arm sp,804 [1].
> But I think it is clear enough through the phandle names and the
> description of the clocks property.
> So I will delete the clock-names in the v10 round.

I think I originally advocated for adding those names. And my argument
was that the names in the dts are probably not clearly identifying the
timer IP's clock inputs since they refer to SOC names for the clocks. We
had this argument for a while now and it doesn't seem to get better
than this. But to explain why I try to get IP vs. SOC clock names in
here is, that I had such a case with Zynq's Ethernet core. That IP has
quite a few clock inputs and luckily the mainline driver for that IP
used the proper names from the IP data sheet for the clocks instead
of the SOC names. That made it relatively easy to match things with the
Zynq integration of that IP. Had that driver used SOC clock names,
things would have been more difficult.

	Sören
Matthias Brugger July 5, 2014, 12:28 a.m. UTC | #4
2014-07-01 20:00 GMT+02:00 Sören Brinkmann <soren.brinkmann@xilinx.com>:
> On Tue, 2014-07-01 at 07:46PM +0200, Matthias Brugger wrote:
>> 2014-06-20 14:36 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
>> > On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
>> >> Add binding documentation for the General Porpose Timer driver of
>> >> the Mediatek SoCs.
>> >>
>> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>> >> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
>> >> Acked-by: Rob Herring <robh@kernel.org>
>> >> ---
>> >>  .../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..10b7e09
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
>> >> @@ -0,0 +1,18 @@
>> >> +Mediatek MT6577, MT6572 and MT6589 Timers
>> >> +---------------------------------------
>> >> +
>> >> +Required properties:
>> >> +- compatible: Should be "mediatek,mt6577-timer"
>> >> +- reg: Should contain location and length for timers register.
>> >> +- clocks: Clocks driving the timer hardware. This list should include two
>> >> +     clocks. The order is system clock and as second clock the RTC clock.
>> >> +
>> >> +Examples:
>> >> +
>> >> +     timer@10008000 {
>> >> +             compatible = "mediatek,mt6577-timer";
>> >> +             reg = <0x10008000 0x80>;
>> >> +             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
>> >> +             clocks = <&system_clk>, <&rtc_clk>;
>> >> +             clock-names = "system-clk", "rtc-clk";
>> >
>> > These names should be mentioned above, or removed.
>>
>> I added the clock-names to make it the clock order clearer, as it is
>> done in the arm sp,804 [1].
>> But I think it is clear enough through the phandle names and the
>> description of the clocks property.
>> So I will delete the clock-names in the v10 round.
>
> I think I originally advocated for adding those names. And my argument
> was that the names in the dts are probably not clearly identifying the
> timer IP's clock inputs since they refer to SOC names for the clocks. We
> had this argument for a while now and it doesn't seem to get better
> than this. But to explain why I try to get IP vs. SOC clock names in
> here is, that I had such a case with Zynq's Ethernet core. That IP has
> quite a few clock inputs and luckily the mainline driver for that IP
> used the proper names from the IP data sheet for the clocks instead
> of the SOC names. That made it relatively easy to match things with the
> Zynq integration of that IP. Had that driver used SOC clock names,
> things would have been more difficult.

Sören, I get the point. The problem is, that we don't have any
documentation other then the SoC datasheet for the timer.
Anyway the clock-names property does not help us in this at all. As I
already mentioned, the clock-names property was to illustrate the
clock order, but as I think, with the comment for the clocks property,
the order is clear enough.

Shall I delete your ack, when I resend the patch without the
clock-names property?

Regards,
Matthias

>
>         Sören
Soren Brinkmann July 7, 2014, 3:49 p.m. UTC | #5
On Sat, 2014-07-05 at 02:28AM +0200, Matthias Brugger wrote:
> 2014-07-01 20:00 GMT+02:00 Sören Brinkmann <soren.brinkmann@xilinx.com>:
> > On Tue, 2014-07-01 at 07:46PM +0200, Matthias Brugger wrote:
> >> 2014-06-20 14:36 GMT+02:00 Mark Rutland <mark.rutland@arm.com>:
> >> > On Fri, Jun 20, 2014 at 12:48:49PM +0100, Matthias Brugger wrote:
> >> >> Add binding documentation for the General Porpose Timer driver of
> >> >> the Mediatek SoCs.
> >> >>
> >> >> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> >> >> Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
> >> >> Acked-by: Rob Herring <robh@kernel.org>
> >> >> ---
> >> >>  .../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..10b7e09
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
> >> >> @@ -0,0 +1,18 @@
> >> >> +Mediatek MT6577, MT6572 and MT6589 Timers
> >> >> +---------------------------------------
> >> >> +
> >> >> +Required properties:
> >> >> +- compatible: Should be "mediatek,mt6577-timer"
> >> >> +- reg: Should contain location and length for timers register.
> >> >> +- clocks: Clocks driving the timer hardware. This list should include two
> >> >> +     clocks. The order is system clock and as second clock the RTC clock.
> >> >> +
> >> >> +Examples:
> >> >> +
> >> >> +     timer@10008000 {
> >> >> +             compatible = "mediatek,mt6577-timer";
> >> >> +             reg = <0x10008000 0x80>;
> >> >> +             interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> >> >> +             clocks = <&system_clk>, <&rtc_clk>;
> >> >> +             clock-names = "system-clk", "rtc-clk";
> >> >
> >> > These names should be mentioned above, or removed.
> >>
> >> I added the clock-names to make it the clock order clearer, as it is
> >> done in the arm sp,804 [1].
> >> But I think it is clear enough through the phandle names and the
> >> description of the clocks property.
> >> So I will delete the clock-names in the v10 round.
> >
> > I think I originally advocated for adding those names. And my argument
> > was that the names in the dts are probably not clearly identifying the
> > timer IP's clock inputs since they refer to SOC names for the clocks. We
> > had this argument for a while now and it doesn't seem to get better
> > than this. But to explain why I try to get IP vs. SOC clock names in
> > here is, that I had such a case with Zynq's Ethernet core. That IP has
> > quite a few clock inputs and luckily the mainline driver for that IP
> > used the proper names from the IP data sheet for the clocks instead
> > of the SOC names. That made it relatively easy to match things with the
> > Zynq integration of that IP. Had that driver used SOC clock names,
> > things would have been more difficult.
> 
> Sören, I get the point. The problem is, that we don't have any
> documentation other then the SoC datasheet for the timer.
> Anyway the clock-names property does not help us in this at all. As I
> already mentioned, the clock-names property was to illustrate the
> clock order, but as I think, with the comment for the clocks property,
> the order is clear enough.
> 
> Shall I delete your ack, when I resend the patch without the
> clock-names property?

No, it's fine.

	Sören
diff mbox

Patch

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..10b7e09
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
@@ -0,0 +1,18 @@ 
+Mediatek MT6577, MT6572 and MT6589 Timers
+---------------------------------------
+
+Required properties:
+- compatible: Should be "mediatek,mt6577-timer"
+- reg: Should contain location and length for timers register.
+- clocks: Clocks driving the timer hardware. This list should include two
+	clocks. The order is system clock and as second clock the RTC clock.
+
+Examples:
+
+	timer@10008000 {
+		compatible = "mediatek,mt6577-timer";
+		reg = <0x10008000 0x80>;
+		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&system_clk>, <&rtc_clk>;
+		clock-names = "system-clk", "rtc-clk";
+	};