* Re: [PATCH v3 14/19] ARM: shmobile: r8a7779: Add TMU devices to DT [not found] ` <20140616084707.GF11582@verge.net.au> @ 2014-06-16 14:22 ` Laurent Pinchart 2014-06-16 16:19 ` Geert Uytterhoeven 2014-06-16 23:34 ` Simon Horman 0 siblings, 2 replies; 4+ messages in thread From: Laurent Pinchart @ 2014-06-16 14:22 UTC (permalink / raw) To: Simon Horman Cc: Laurent Pinchart, linux-sh, linux-arm-kernel, Daniel Lezcano, Thomas Gleixner, devicetree Hi Simon, On Monday 16 June 2014 17:47:07 Simon Horman wrote: > On Sat, Jun 14, 2014 at 06:23:36PM +0200, Laurent Pinchart wrote: > > Add the TMU0, TMU1 and TMU2 counters to the r8a7779 device tree and make > > them disabled by default. > > > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@ideasonboard.com> > > --- > > > > arch/arm/boot/dts/r8a7779.dtsi | 42 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > > > diff --git a/arch/arm/boot/dts/r8a7779.dtsi > > b/arch/arm/boot/dts/r8a7779.dtsi index 94e2fc8..bf716ce 100644 > > --- a/arch/arm/boot/dts/r8a7779.dtsi > > +++ b/arch/arm/boot/dts/r8a7779.dtsi > > @@ -266,6 +266,48 @@ > > reg = <0xffc48000 0x38>; > > }; > > > > + tmu0: timer@ffd80000 { > > + compatible = "renesas,tmu"; > > + reg = <0xffd80000 0x30>; > > + interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>, > > + <0 41 IRQ_TYPE_LEVEL_HIGH>, > > + <0 42 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&mstp0_clks R8A7779_CLK_TMU0>; > > + clock-names = "fck"; > > + > > + #renesas,channels = <3>; > > + > > + status = "disabled"; > > + }; > > + > > + tmu1: timer@ffd81000 { > > + compatible = "renesas,tmu"; > > + reg = <0xffd81000 0x30>; > > + interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>, > > + <0 45 IRQ_TYPE_LEVEL_HIGH>, > > + <0 46 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&mstp0_clks R8A7779_CLK_TMU1>; > > + clock-names = "fck"; > > + > > + #renesas,channels = <3>; > > + > > + status = "disabled"; > > + }; > > + > > + tmu2: timer@ffd82000 { > > + compatible = "renesas,tmu"; > > + reg = <0xffd82000 0x30>; > > + interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>, > > + <0 49 IRQ_TYPE_LEVEL_HIGH>, > > + <0 50 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&mstp0_clks R8A7779_CLK_TMU2>; > > + clock-names = "fck"; > > + > > + #renesas,channels = <3>; > > + > > + status = "disabled"; > > + }; > > + > > sata: sata@fc600000 { > > compatible = "renesas,rcar-sata"; > > reg = <0xfc600000 0x2000>; > > There are no interrupt-parents in in the nodes above nodes > although the documentation of the binding has one in the example. > Is it just an oversight in this patch? Not really. The interrupt-parent property is required but can be inherited. Furthermore, the interrupts-extended property can be used to replace the interrupts and interrupt-parent properties. This should all be documented in the DT bindings, but not by duplicating the full explanation in dozens of different flavours in all bindings. We need standard wordings for common properties such as interrupts, clocks or reg. > I believe that the IRQ numbers are not correct. > > In setup-r8a7779.c I see gic_iid(0x40). But above I see 40. > I think the correct value is 0x40 - 32 = 32. Likewise > for the other IRQs of tmu0. And looking at the documentaiton > this seems true for tmu1 and tmu2 too. > > I have successfully booted a marzen board with the following incremental > patch to the DT nodes. You're right, sorry about that. I'll fix the interrupt numbers and resubmit. The interrupt-parent is specified at the top level so there's no need to add it to the TMU nodes. > diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi > index bf716ce..81714ce 100644 > --- a/arch/arm/boot/dts/r8a7779.dtsi > +++ b/arch/arm/boot/dts/r8a7779.dtsi > @@ -269,9 +269,10 @@ > tmu0: timer@ffd80000 { > compatible = "renesas,tmu"; > reg = <0xffd80000 0x30>; > - interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>, > - <0 41 IRQ_TYPE_LEVEL_HIGH>, > - <0 42 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-parent = <&gic>; > + interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>, > + <0 33 IRQ_TYPE_LEVEL_HIGH>, > + <0 34 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&mstp0_clks R8A7779_CLK_TMU0>; > clock-names = "fck"; > > @@ -283,9 +284,10 @@ > tmu1: timer@ffd81000 { > compatible = "renesas,tmu"; > reg = <0xffd81000 0x30>; > - interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>, > - <0 45 IRQ_TYPE_LEVEL_HIGH>, > - <0 46 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-parent = <&gic>; > + interrupts = <0 35 IRQ_TYPE_LEVEL_HIGH>, > + <0 36 IRQ_TYPE_LEVEL_HIGH>, > + <0 37 IRQ_TYPE_LEVEL_HIGH>; If I'm not mistaken the interrupts should be 36, 37 and 38. > clocks = <&mstp0_clks R8A7779_CLK_TMU1>; > clock-names = "fck"; > > @@ -296,10 +298,11 @@ > > tmu2: timer@ffd82000 { > compatible = "renesas,tmu"; > + interrupt-parent = <&gic>; > reg = <0xffd82000 0x30>; > - interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>, > - <0 49 IRQ_TYPE_LEVEL_HIGH>, > - <0 50 IRQ_TYPE_LEVEL_HIGH>; > + interrupts = <0 38 IRQ_TYPE_LEVEL_HIGH>, > + <0 39 IRQ_TYPE_LEVEL_HIGH>, > + <0 40 IRQ_TYPE_LEVEL_HIGH>; And 40, 41 and 42 here. > clocks = <&mstp0_clks R8A7779_CLK_TMU2>; > clock-names = "fck"; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 14/19] ARM: shmobile: r8a7779: Add TMU devices to DT 2014-06-16 14:22 ` [PATCH v3 14/19] ARM: shmobile: r8a7779: Add TMU devices to DT Laurent Pinchart @ 2014-06-16 16:19 ` Geert Uytterhoeven 2014-06-16 20:31 ` Laurent Pinchart 2014-06-16 23:34 ` Simon Horman 1 sibling, 1 reply; 4+ messages in thread From: Geert Uytterhoeven @ 2014-06-16 16:19 UTC (permalink / raw) To: Laurent Pinchart Cc: Simon Horman, Laurent Pinchart, Linux-sh list, linux-arm-kernel@lists.infradead.org, Daniel Lezcano, Thomas Gleixner, devicetree@vger.kernel.org Hi Laurent, On Mon, Jun 16, 2014 at 4:22 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: >> diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi >> index bf716ce..81714ce 100644 >> --- a/arch/arm/boot/dts/r8a7779.dtsi >> +++ b/arch/arm/boot/dts/r8a7779.dtsi >> @@ -269,9 +269,10 @@ >> tmu0: timer@ffd80000 { >> compatible = "renesas,tmu"; >> reg = <0xffd80000 0x30>; >> - interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>, >> - <0 41 IRQ_TYPE_LEVEL_HIGH>, >> - <0 42 IRQ_TYPE_LEVEL_HIGH>; >> + interrupt-parent = <&gic>; >> + interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>, >> + <0 33 IRQ_TYPE_LEVEL_HIGH>, >> + <0 34 IRQ_TYPE_LEVEL_HIGH>; >> clocks = <&mstp0_clks R8A7779_CLK_TMU0>; >> clock-names = "fck"; >> >> @@ -283,9 +284,10 @@ >> tmu1: timer@ffd81000 { >> compatible = "renesas,tmu"; >> reg = <0xffd81000 0x30>; >> - interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>, >> - <0 45 IRQ_TYPE_LEVEL_HIGH>, >> - <0 46 IRQ_TYPE_LEVEL_HIGH>; >> + interrupt-parent = <&gic>; >> + interrupts = <0 35 IRQ_TYPE_LEVEL_HIGH>, >> + <0 36 IRQ_TYPE_LEVEL_HIGH>, >> + <0 37 IRQ_TYPE_LEVEL_HIGH>; > > If I'm not mistaken the interrupts should be 36, 37 and 38. (diving into the documentation) Yes, probably 36, 37, and 38. I say "probably", as I find it non-trivial to convert from docs to DT. What happens with 35, which is "TMU21 (input capture)"? As DT describes the hardware, shouldn't it be mentioned here? >> clocks = <&mstp0_clks R8A7779_CLK_TMU1>; >> clock-names = "fck"; >> >> @@ -296,10 +298,11 @@ >> >> tmu2: timer@ffd82000 { >> compatible = "renesas,tmu"; >> + interrupt-parent = <&gic>; >> reg = <0xffd82000 0x30>; >> - interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>, >> - <0 49 IRQ_TYPE_LEVEL_HIGH>, >> - <0 50 IRQ_TYPE_LEVEL_HIGH>; >> + interrupts = <0 38 IRQ_TYPE_LEVEL_HIGH>, >> + <0 39 IRQ_TYPE_LEVEL_HIGH>, >> + <0 40 IRQ_TYPE_LEVEL_HIGH>; > > And 40, 41 and 42 here. Likewise for "TMU51 (input capture)"? >> clocks = <&mstp0_clks R8A7779_CLK_TMU2>; >> clock-names = "fck"; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 14/19] ARM: shmobile: r8a7779: Add TMU devices to DT 2014-06-16 16:19 ` Geert Uytterhoeven @ 2014-06-16 20:31 ` Laurent Pinchart 0 siblings, 0 replies; 4+ messages in thread From: Laurent Pinchart @ 2014-06-16 20:31 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Simon Horman, Laurent Pinchart, Linux-sh list, linux-arm-kernel@lists.infradead.org, Daniel Lezcano, Thomas Gleixner, devicetree@vger.kernel.org Hi Geert, On Monday 16 June 2014 18:19:02 Geert Uytterhoeven wrote: > On Mon, Jun 16, 2014 at 4:22 PM, Laurent Pinchart wrote: > >> diff --git a/arch/arm/boot/dts/r8a7779.dtsi > >> b/arch/arm/boot/dts/r8a7779.dtsi index bf716ce..81714ce 100644 > >> --- a/arch/arm/boot/dts/r8a7779.dtsi > >> +++ b/arch/arm/boot/dts/r8a7779.dtsi > >> @@ -269,9 +269,10 @@ > >> tmu0: timer@ffd80000 { > >> compatible = "renesas,tmu"; > >> reg = <0xffd80000 0x30>; > >> - interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>, > >> - <0 41 IRQ_TYPE_LEVEL_HIGH>, > >> - <0 42 IRQ_TYPE_LEVEL_HIGH>; > >> + interrupt-parent = <&gic>; > >> + interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>, > >> + <0 33 IRQ_TYPE_LEVEL_HIGH>, > >> + <0 34 IRQ_TYPE_LEVEL_HIGH>; > >> > >> clocks = <&mstp0_clks R8A7779_CLK_TMU0>; > >> clock-names = "fck"; > >> > >> @@ -283,9 +284,10 @@ > >> tmu1: timer@ffd81000 { > >> compatible = "renesas,tmu"; > >> reg = <0xffd81000 0x30>; > >> - interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>, > >> - <0 45 IRQ_TYPE_LEVEL_HIGH>, > >> - <0 46 IRQ_TYPE_LEVEL_HIGH>; > >> + interrupt-parent = <&gic>; > >> + interrupts = <0 35 IRQ_TYPE_LEVEL_HIGH>, > >> + <0 36 IRQ_TYPE_LEVEL_HIGH>, > >> + <0 37 IRQ_TYPE_LEVEL_HIGH>; > > > > If I'm not mistaken the interrupts should be 36, 37 and 38. > > (diving into the documentation) > > Yes, probably 36, 37, and 38. I say "probably", as I find it non-trivial > to convert from docs to DT. > > What happens with 35, which is "TMU21 (input capture)"? > As DT describes the hardware, shouldn't it be mentioned here? Input capture isn't supported by the TMU driver yet. I haven't checked how the feature is implemented in the hardware, which is why I've left it out from the DT bindings for now. > >> clocks = <&mstp0_clks R8A7779_CLK_TMU1>; > >> clock-names = "fck"; > >> > >> @@ -296,10 +298,11 @@ > >> > >> tmu2: timer@ffd82000 { > >> compatible = "renesas,tmu"; > >> + interrupt-parent = <&gic>; > >> reg = <0xffd82000 0x30>; > >> - interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>, > >> - <0 49 IRQ_TYPE_LEVEL_HIGH>, > >> - <0 50 IRQ_TYPE_LEVEL_HIGH>; > >> + interrupts = <0 38 IRQ_TYPE_LEVEL_HIGH>, > >> + <0 39 IRQ_TYPE_LEVEL_HIGH>, > >> + <0 40 IRQ_TYPE_LEVEL_HIGH>; > > > > And 40, 41 and 42 here. > > Likewise for "TMU51 (input capture)"? > > >> clocks = <&mstp0_clks R8A7779_CLK_TMU2>; > >> clock-names = "fck"; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 14/19] ARM: shmobile: r8a7779: Add TMU devices to DT 2014-06-16 14:22 ` [PATCH v3 14/19] ARM: shmobile: r8a7779: Add TMU devices to DT Laurent Pinchart 2014-06-16 16:19 ` Geert Uytterhoeven @ 2014-06-16 23:34 ` Simon Horman 1 sibling, 0 replies; 4+ messages in thread From: Simon Horman @ 2014-06-16 23:34 UTC (permalink / raw) To: Laurent Pinchart Cc: Laurent Pinchart, linux-sh, linux-arm-kernel, Daniel Lezcano, Thomas Gleixner, devicetree On Mon, Jun 16, 2014 at 04:22:11PM +0200, Laurent Pinchart wrote: > Hi Simon, > > On Monday 16 June 2014 17:47:07 Simon Horman wrote: > > On Sat, Jun 14, 2014 at 06:23:36PM +0200, Laurent Pinchart wrote: > > > Add the TMU0, TMU1 and TMU2 counters to the r8a7779 device tree and make > > > them disabled by default. > > > > > > Signed-off-by: Laurent Pinchart > > > <laurent.pinchart+renesas@ideasonboard.com> > > > --- > > > > > > arch/arm/boot/dts/r8a7779.dtsi | 42 +++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 42 insertions(+) > > > > > > diff --git a/arch/arm/boot/dts/r8a7779.dtsi > > > b/arch/arm/boot/dts/r8a7779.dtsi index 94e2fc8..bf716ce 100644 > > > --- a/arch/arm/boot/dts/r8a7779.dtsi > > > +++ b/arch/arm/boot/dts/r8a7779.dtsi > > > @@ -266,6 +266,48 @@ > > > reg = <0xffc48000 0x38>; > > > }; > > > > > > + tmu0: timer@ffd80000 { > > > + compatible = "renesas,tmu"; > > > + reg = <0xffd80000 0x30>; > > > + interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>, > > > + <0 41 IRQ_TYPE_LEVEL_HIGH>, > > > + <0 42 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&mstp0_clks R8A7779_CLK_TMU0>; > > > + clock-names = "fck"; > > > + > > > + #renesas,channels = <3>; > > > + > > > + status = "disabled"; > > > + }; > > > + > > > + tmu1: timer@ffd81000 { > > > + compatible = "renesas,tmu"; > > > + reg = <0xffd81000 0x30>; > > > + interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>, > > > + <0 45 IRQ_TYPE_LEVEL_HIGH>, > > > + <0 46 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&mstp0_clks R8A7779_CLK_TMU1>; > > > + clock-names = "fck"; > > > + > > > + #renesas,channels = <3>; > > > + > > > + status = "disabled"; > > > + }; > > > + > > > + tmu2: timer@ffd82000 { > > > + compatible = "renesas,tmu"; > > > + reg = <0xffd82000 0x30>; > > > + interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>, > > > + <0 49 IRQ_TYPE_LEVEL_HIGH>, > > > + <0 50 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&mstp0_clks R8A7779_CLK_TMU2>; > > > + clock-names = "fck"; > > > + > > > + #renesas,channels = <3>; > > > + > > > + status = "disabled"; > > > + }; > > > + > > > sata: sata@fc600000 { > > > compatible = "renesas,rcar-sata"; > > > reg = <0xfc600000 0x2000>; > > > > There are no interrupt-parents in in the nodes above nodes > > although the documentation of the binding has one in the example. > > Is it just an oversight in this patch? > > Not really. > > The interrupt-parent property is required but can be inherited. Furthermore, > the interrupts-extended property can be used to replace the interrupts and > interrupt-parent properties. This should all be documented in the DT bindings, > but not by duplicating the full explanation in dozens of different flavours in > all bindings. We need standard wordings for common properties such as > interrupts, clocks or reg. Thanks for the clarification, I'm fine with that. > > I believe that the IRQ numbers are not correct. > > > > In setup-r8a7779.c I see gic_iid(0x40). But above I see 40. > > I think the correct value is 0x40 - 32 = 32. Likewise > > for the other IRQs of tmu0. And looking at the documentaiton > > this seems true for tmu1 and tmu2 too. > > > > I have successfully booted a marzen board with the following incremental > > patch to the DT nodes. > > You're right, sorry about that. I'll fix the interrupt numbers and resubmit. > The interrupt-parent is specified at the top level so there's no need to add > it to the TMU nodes. > > > diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi > > index bf716ce..81714ce 100644 > > --- a/arch/arm/boot/dts/r8a7779.dtsi > > +++ b/arch/arm/boot/dts/r8a7779.dtsi > > @@ -269,9 +269,10 @@ > > tmu0: timer@ffd80000 { > > compatible = "renesas,tmu"; > > reg = <0xffd80000 0x30>; > > - interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>, > > - <0 41 IRQ_TYPE_LEVEL_HIGH>, > > - <0 42 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>, > > + <0 33 IRQ_TYPE_LEVEL_HIGH>, > > + <0 34 IRQ_TYPE_LEVEL_HIGH>; > > clocks = <&mstp0_clks R8A7779_CLK_TMU0>; > > clock-names = "fck"; > > > > @@ -283,9 +284,10 @@ > > tmu1: timer@ffd81000 { > > compatible = "renesas,tmu"; > > reg = <0xffd81000 0x30>; > > - interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>, > > - <0 45 IRQ_TYPE_LEVEL_HIGH>, > > - <0 46 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 35 IRQ_TYPE_LEVEL_HIGH>, > > + <0 36 IRQ_TYPE_LEVEL_HIGH>, > > + <0 37 IRQ_TYPE_LEVEL_HIGH>; > > If I'm not mistaken the interrupts should be 36, 37 and 38. Yes, sorry about that. > > clocks = <&mstp0_clks R8A7779_CLK_TMU1>; > > clock-names = "fck"; > > > > @@ -296,10 +298,11 @@ > > > > tmu2: timer@ffd82000 { > > compatible = "renesas,tmu"; > > + interrupt-parent = <&gic>; > > reg = <0xffd82000 0x30>; > > - interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>, > > - <0 49 IRQ_TYPE_LEVEL_HIGH>, > > - <0 50 IRQ_TYPE_LEVEL_HIGH>; > > + interrupts = <0 38 IRQ_TYPE_LEVEL_HIGH>, > > + <0 39 IRQ_TYPE_LEVEL_HIGH>, > > + <0 40 IRQ_TYPE_LEVEL_HIGH>; > > And 40, 41 and 42 here. Yes. I wonder how I managed to mess these up. > > clocks = <&mstp0_clks R8A7779_CLK_TMU2>; > > clock-names = "fck"; ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-06-16 23:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1402763021-4067-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
[not found] ` <1402763021-4067-15-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
[not found] ` <20140616084707.GF11582@verge.net.au>
2014-06-16 14:22 ` [PATCH v3 14/19] ARM: shmobile: r8a7779: Add TMU devices to DT Laurent Pinchart
2014-06-16 16:19 ` Geert Uytterhoeven
2014-06-16 20:31 ` Laurent Pinchart
2014-06-16 23:34 ` Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).