diff mbox

[v5,2/6] dt-bindings: add mtk-timer bindings

Message ID 1400689583-21119-3-git-send-email-matthias.bgg@gmail.com
State Superseded, archived
Headers show

Commit Message

Matthias Brugger May 21, 2014, 4:26 p.m. UTC
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

Comments

Soren Brinkmann May 21, 2014, 4:34 p.m. UTC | #1
Hi Matthias,

On Wed, 2014-05-21 at 06:26PM +0200, Matthias Brugger wrote:
> Add binding documentation for the General Porpose Timer driver of

Typo: Purpose

> 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..7d909f7
> --- /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,mtk6577-timer"
> +- reg: Should contain location and length for timers register.
> +- clocks: Clocks driving the timer hardware. This list shoud include two
> +	clocks. The order is system clock and as second clock the RTC clock.
> +
> +Examples:
> +
> +	timer {

The node name should be timer@0x10008000

> +		compatible = "mediatek,mtk6577-timer";
> +		reg = <0x10008000 0x80>;
> +		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&system_clk>, <&rtc_clk>;
> +		clock-names = "system-clk", "rtc-clk";

I'm still not convinced that the timer IP calls its clock inputs this
way, but well.

Other than that ACK.

	Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Soren Brinkmann May 21, 2014, 4:53 p.m. UTC | #2
On Wed, 2014-05-21 at 06:54PM +0200, Heiko Stübner wrote:
> Am Mittwoch, 21. Mai 2014, 09:34:10 schrieb Sören Brinkmann:
> > Hi Matthias,
> > 
> > On Wed, 2014-05-21 at 06:26PM +0200, Matthias Brugger wrote:
> > > Add binding documentation for the General Porpose Timer driver of
> > 
> > Typo: Purpose
> > 
> > > 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..7d909f7
> > > --- /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,mtk6577-timer"
> > > +- reg: Should contain location and length for timers register.
> > > +- clocks: Clocks driving the timer hardware. This list shoud include two
> > > +	clocks. The order is system clock and as second clock the RTC clock.
> > > +
> > > +Examples:
> > > +
> > > +	timer {
> > 
> > The node name should be timer@0x10008000
> 
> I think the more commonly used variant is
> 
> 	timer@10008000
> 
> without the 0x. While some dts files really use @0x they are in the minority.

You're right.

> 
> 
> > 
> > > +		compatible = "mediatek,mtk6577-timer";
> > > +		reg = <0x10008000 0x80>;
> > > +		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> > > +		clocks = <&system_clk>, <&rtc_clk>;
> > > +		clock-names = "system-clk", "rtc-clk";
> > 
> > I'm still not convinced that the timer IP calls its clock inputs this
> > way, but well.
> 
> Maybe this might convince you ;-)
> 
> "The GPT includes 5 32-bit timers and one 64-bit timer. Each timer has 4 
> operation modes, which are ONE-SHOT,  REPEAT,  KEEP-GO and  FREERUN, and can 
> operate on one of the 2 clock sources, RTC clock (32.768kHz) and system clock 
> (13MHz)."

Is this copied from the timer data sheet or the SOC documentation?

	Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Heiko Stübner May 21, 2014, 4:54 p.m. UTC | #3
Am Mittwoch, 21. Mai 2014, 09:34:10 schrieb Sören Brinkmann:
> Hi Matthias,
> 
> On Wed, 2014-05-21 at 06:26PM +0200, Matthias Brugger wrote:
> > Add binding documentation for the General Porpose Timer driver of
> 
> Typo: Purpose
> 
> > 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..7d909f7
> > --- /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,mtk6577-timer"
> > +- reg: Should contain location and length for timers register.
> > +- clocks: Clocks driving the timer hardware. This list shoud include two
> > +	clocks. The order is system clock and as second clock the RTC clock.
> > +
> > +Examples:
> > +
> > +	timer {
> 
> The node name should be timer@0x10008000

I think the more commonly used variant is

	timer@10008000

without the 0x. While some dts files really use @0x they are in the minority.


> 
> > +		compatible = "mediatek,mtk6577-timer";
> > +		reg = <0x10008000 0x80>;
> > +		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> > +		clocks = <&system_clk>, <&rtc_clk>;
> > +		clock-names = "system-clk", "rtc-clk";
> 
> I'm still not convinced that the timer IP calls its clock inputs this
> way, but well.

Maybe this might convince you ;-)

"The GPT includes 5 32-bit timers and one 64-bit timer. Each timer has 4 
operation modes, which are ONE-SHOT,  REPEAT,  KEEP-GO and  FREERUN, and can 
operate on one of the 2 clock sources, RTC clock (32.768kHz) and system clock 
(13MHz)."


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Heiko Stübner May 21, 2014, 5:18 p.m. UTC | #4
Am Mittwoch, 21. Mai 2014, 09:53:57 schrieb Sören Brinkmann:
> On Wed, 2014-05-21 at 06:54PM +0200, Heiko Stübner wrote:
> > Am Mittwoch, 21. Mai 2014, 09:34:10 schrieb Sören Brinkmann:
> > > Hi Matthias,
> > > 
> > > On Wed, 2014-05-21 at 06:26PM +0200, Matthias Brugger wrote:
> > > > +		compatible = "mediatek,mtk6577-timer";
> > > > +		reg = <0x10008000 0x80>;
> > > > +		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> > > > +		clocks = <&system_clk>, <&rtc_clk>;
> > > > +		clock-names = "system-clk", "rtc-clk";
> > > 
> > > I'm still not convinced that the timer IP calls its clock inputs this
> > > way, but well.
> > 
> > Maybe this might convince you ;-)
> > 
> > "The GPT includes 5 32-bit timers and one 64-bit timer. Each timer has 4
> > operation modes, which are ONE-SHOT,  REPEAT,  KEEP-GO and  FREERUN, and
> > can operate on one of the 2 clock sources, RTC clock (32.768kHz) and
> > system clock (13MHz)."
> 
> Is this copied from the timer data sheet or the SOC documentation?

That is from the processor datasheet containing the timer documentation.
I don't think it's customary for soc vendors to provide additional individual 
documentation for self-developed IPs contained in a SoC.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Soren Brinkmann May 21, 2014, 5:21 p.m. UTC | #5
On Wed, 2014-05-21 at 07:18PM +0200, Heiko Stübner wrote:
> Am Mittwoch, 21. Mai 2014, 09:53:57 schrieb Sören Brinkmann:
> > On Wed, 2014-05-21 at 06:54PM +0200, Heiko Stübner wrote:
> > > Am Mittwoch, 21. Mai 2014, 09:34:10 schrieb Sören Brinkmann:
> > > > Hi Matthias,
> > > > 
> > > > On Wed, 2014-05-21 at 06:26PM +0200, Matthias Brugger wrote:
> > > > > +		compatible = "mediatek,mtk6577-timer";
> > > > > +		reg = <0x10008000 0x80>;
> > > > > +		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
> > > > > +		clocks = <&system_clk>, <&rtc_clk>;
> > > > > +		clock-names = "system-clk", "rtc-clk";
> > > > 
> > > > I'm still not convinced that the timer IP calls its clock inputs this
> > > > way, but well.
> > > 
> > > Maybe this might convince you ;-)
> > > 
> > > "The GPT includes 5 32-bit timers and one 64-bit timer. Each timer has 4
> > > operation modes, which are ONE-SHOT,  REPEAT,  KEEP-GO and  FREERUN, and
> > > can operate on one of the 2 clock sources, RTC clock (32.768kHz) and
> > > system clock (13MHz)."
> > 
> > Is this copied from the timer data sheet or the SOC documentation?
> 
> That is from the processor datasheet containing the timer documentation.
> I don't think it's customary for soc vendors to provide additional individual 
> documentation for self-developed IPs contained in a SoC.

May be, but that is what might create the problem I'm talking about. The
next generation SOC using the same IP may use different clocks to
drive it. Now you're stuck with RTC and system clock as names, but they
don't apply to the latest integration anymore. That's why I advocate
using the IP's naming for its clock signals as opposed to the SOC's
clock names.
So, my concern is still standing. But I see, that this might not be
resolvable, in case such information is not available. Nevertheless,
I see the currently used names as implementation details of the SOC
and not a property of the here described timer IP. But might be that
this is as good as it gets.

	Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
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..7d909f7
--- /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,mtk6577-timer"
+- reg: Should contain location and length for timers register.
+- clocks: Clocks driving the timer hardware. This list shoud include two
+	clocks. The order is system clock and as second clock the RTC clock.
+
+Examples:
+
+	timer {
+		compatible = "mediatek,mtk6577-timer";
+		reg = <0x10008000 0x80>;
+		interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&system_clk>, <&rtc_clk>;
+		clock-names = "system-clk", "rtc-clk";
+	};