| Message ID | 1487610788-6939-5-git-send-email-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
Hello,
self-review to save you all some time
On 20/02/2017 18:13, Jacopo Mondi wrote:
> Add pin controller node with 12 gpio controller sub-nodes to
> r7s72100 dtsi.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> arch/arm/boot/dts/r7s72100.dtsi | 81 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 81 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index 3dd427d..a77a431b 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -12,6 +12,7 @@
> #include <dt-bindings/clock/r7s72100-clock.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/pinctrl/r7s72100-pinctrl.h>
I should move this to the board specific .dts file.
It's not needed here.
>
> / {
> compatible = "renesas,r7s72100";
> @@ -171,6 +172,86 @@
> };
> };
>
> + pinctrl: pinctrl@fcfe3000 {
> + compatible = "renesas,r7s72100-ports";
> +
> + #pinctrl-cells = <1>;
> +
> + reg = <0xfcfe3000 0x4248>;
> +
> + port0: gpio@0 {
Geert pointed out offline the if the node has a unit address, it needs a
"reg" property.
I'll change all of these to portX: gpio-X {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 0 16>;
Not all ports have 16 pins available.
This is one of the differences between different RZ/A1 SoC versions
(RZ/A1H, RZ/A1L etc)
I'll change the number of pins to the actual available ones on each port
for r7s72100 (RZ/A1H)
Thanks
j
> + };
> +
> + port1: gpio@1 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 16 16>;
> + };
> +
> + port2: gpio@2 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 32 16>;
> + };
> +
> + port3: gpio@3 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 48 16>;
> + };
> +
> + port4: gpio@4 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 64 16>;
> + };
> +
> + port5: gpio@5 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 80 16>;
> + };
> +
> + port6: gpio@6 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 96 16>;
> + };
> +
> + port7: gpio@7 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 112 16>;
> + };
> +
> + port8: gpio@8 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 128 16>;
> + };
> +
> + port9: gpio@9 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 144 16>;
> + };
> +
> + port10: gpio@10 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 160 16>;
> + };
> +
> + port11: gpio@11 {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl 0 176 16>;
> + };
> + };
> +
> scif0: serial@e8007000 {
> compatible = "renesas,scif-r7s72100", "renesas,scif";
> reg = <0xe8007000 64>;
>
Hi Jacopo, On Tuesday, February 21, 2017, jacopo mondi wrote: > > + gpio-controller; > > + #gpio-cells = <2>; > > + gpio-ranges = <&pinctrl 0 0 16>; > > Not all ports have 16 pins available. > This is one of the differences between different RZ/A1 SoC versions > (RZ/A1H, RZ/A1L etc) > > I'll change the number of pins to the actual available ones on each port > for r7s72100 (RZ/A1H) But wait, what if someone wants to use a RZ/A1L??? I don't want to make a separate dtsi file for RZ/A1L. Is it possible to change the number of port pins in the board dts file? For example: RZ/A1H: P5_0 - P5_10 RZ/A1L: P5_0 - P5_16 So, in a rza1l-board.dts file I would put: &port5 { gpio-ranges = <&pinctrl 0 80 16>; } Will this work? Chris
Hi Chris, On Thu, Mar 2, 2017 at 9:17 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Tuesday, February 21, 2017, jacopo mondi wrote: >> > + gpio-controller; >> > + #gpio-cells = <2>; >> > + gpio-ranges = <&pinctrl 0 0 16>; >> >> Not all ports have 16 pins available. >> This is one of the differences between different RZ/A1 SoC versions >> (RZ/A1H, RZ/A1L etc) >> >> I'll change the number of pins to the actual available ones on each port >> for r7s72100 (RZ/A1H) > > But wait, what if someone wants to use a RZ/A1L??? > > I don't want to make a separate dtsi file for RZ/A1L. > > Is it possible to change the number of port pins in the board dts file? > For example: > RZ/A1H: P5_0 - P5_10 > RZ/A1L: P5_0 - P5_16 That's 17 pins, not 16? > > So, in a rza1l-board.dts file I would put: > > &port5 { > gpio-ranges = <&pinctrl 0 80 16>; > } > > Will this work? Yes, overriding should work. But the number of pins is an SoC-property, not a board-property? Are the differences between RZ/A1H and RZ/A1L just the number of pins? If yes, you could use a hierarchical DTS structure: rza1h-<board>.dts: #include "rza1h.dtsi" // board specifics here rza1l-<board>.dts: #include "rza1l.dtsi" // board specifics here rza1h.dtsi: #include "rza.dtsi" // r7s72100.dtsi? // base SoC overrides &port5 { gpio-ranges = <&pinctrl 0 80 11>; } rza1l.dtsi: #include "rza.dtsi" // r7s72100.dtsi? // base SoC overrides &port5 { gpio-ranges = <&pinctrl 0 80 16>; } Actual naming of DTS files TBD. We could also decide to not have rza1h.dtsi, and assume the base dtsi is for RZ/A1H. 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
Hi Geert, On Friday, March 03, 2017, Geert Uytterhoeven wrote: > > Is it possible to change the number of port pins in the board dts file? > > For example: > > RZ/A1H: P5_0 - P5_10 > > RZ/A1L: P5_0 - P5_16 > > That's 17 pins, not 16? Oops, I meant P5_15 Looking at the parts, here are the differences: RZ/A1H RZ/A1L ------------------ P2_0:15 P2_0:9 P4_0:15 P4_0:7 P5_0:10 P5_0:15 P7_0:15 P7_0:11 P9_0:7 P9_0:5 P10_0:15 none P11_0:15 none > > So, in a rza1l-board.dts file I would put: > > > > &port5 { > > gpio-ranges = <&pinctrl 0 80 16>; > > } > > > > Will this work? > > Yes, overriding should work. But the number of pins is an SoC-property, > not > a board-property? True, but I am trying to figure out how to solve this locally and not make it an upstream problem. > Are the differences between RZ/A1H and RZ/A1L just the number of pins? Yes/No. Internally, the IP blocks are the same, and located at the same register addresses, but come out to different port pins due to the RZ/A1L having smaller packages. Also, the L has less channels than the H. For example: QSPI: H=2, L=1 LCD: H=2, L=1 SCIF: H=8, L=5 CAN: H=5, L=2 Of course there is some IP that only comes in the H. This is why I didn't want to associate "names" with the pins in a pfc driver. I just wanted the board DT to assign a pin to a 'function number'. > If yes, you could use a hierarchical DTS structure: > > rza1h-<board>.dts: > > #include "rza1h.dtsi" > > // board specifics here > > rza1l-<board>.dts: > > #include "rza1l.dtsi" > > // board specifics here > > rza1h.dtsi: > > #include "rza.dtsi" // r7s72100.dtsi? > > // base SoC overrides > &port5 { > gpio-ranges = <&pinctrl 0 80 11>; > } > > rza1l.dtsi: > > #include "rza.dtsi" // r7s72100.dtsi? > > // base SoC overrides > &port5 { > gpio-ranges = <&pinctrl 0 80 16>; > } > > Actual naming of DTS files TBD. OK, so just put the pin differences in the two files. That's a good idea. But, then that's 2 more files to add upstream. As the RZ/A series continues, they might keep doing this kind of thing, so I don't want to get into the habit of adding more and more DT files. # I wish there was some type of if-else syntax in Device Tree so in the board file I could just say: / { model = "RSKRZA1"; compatible = "renesas,rskrza1", "renesas,r7s72100"; variant = "rza1l"; > We could also decide to not have rza1h.dtsi, and assume the base dtsi is > for > RZ/A1H. Honestly, I'm fine with RZ/A1H being the flagship upstream SoC since it is a superset of all the peripherals in RZ/A1M,/A1L,/A1LU,/A1LC. Meaning, this small difference can be handled in a simple board file addition. I was thinking I was going to post some DT examples on eLinux.org anyway, so one of them would just be this RZ/A1L fix. Cheers Chris
Hi Chris, Geert, On Fri, Mar 03, 2017 at 01:24:51PM +0000, Chris Brandt wrote: > Hi Geert, > > On Friday, March 03, 2017, Geert Uytterhoeven wrote: > > > Is it possible to change the number of port pins in the board dts file? > > > For example: > > > RZ/A1H: P5_0 - P5_10 > > > RZ/A1L: P5_0 - P5_16 > > > > That's 17 pins, not 16? > > Oops, I meant P5_15 > > Looking at the parts, here are the differences: > > RZ/A1H RZ/A1L > ------------------ > P2_0:15 P2_0:9 > P4_0:15 P4_0:7 > P5_0:10 P5_0:15 > P7_0:15 P7_0:11 > P9_0:7 P9_0:5 > P10_0:15 none > P11_0:15 none > > > > So, in a rza1l-board.dts file I would put: > > > > > > &port5 { > > > gpio-ranges = <&pinctrl 0 80 16>; > > > } > > > > > > Will this work? > > > > Yes, overriding should work. But the number of pins is an SoC-property, > > not > > a board-property? > > True, but I am trying to figure out how to solve this locally and not make > it an upstream problem. > > > > Are the differences between RZ/A1H and RZ/A1L just the number of pins? > > Yes/No. > > Internally, the IP blocks are the same, and located at the same register > addresses, but come out to different port pins due to the RZ/A1L having smaller > packages. > > Also, the L has less channels than the H. > For example: > QSPI: H=2, L=1 > LCD: H=2, L=1 > SCIF: H=8, L=5 > CAN: H=5, L=2 > > Of course there is some IP that only comes in the H. > > This is why I didn't want to associate "names" with the pins in a pfc driver. > I just wanted the board DT to assign a pin to a 'function number'. > > > > > If yes, you could use a hierarchical DTS structure: > > > > rza1h-<board>.dts: > > > > #include "rza1h.dtsi" > > > > // board specifics here > > > > rza1l-<board>.dts: > > > > #include "rza1l.dtsi" > > > > // board specifics here > > > > rza1h.dtsi: > > > > #include "rza.dtsi" // r7s72100.dtsi? > > > > // base SoC overrides > > &port5 { > > gpio-ranges = <&pinctrl 0 80 11>; > > } > > > > rza1l.dtsi: > > > > #include "rza.dtsi" // r7s72100.dtsi? > > > > // base SoC overrides > > &port5 { > > gpio-ranges = <&pinctrl 0 80 16>; > > } > > > > Actual naming of DTS files TBD. > > OK, so just put the pin differences in the two files. That's a good idea. But, > then that's 2 more files to add upstream. > As the RZ/A series continues, they might keep doing this kind of thing, so I > don't want to get into the habit of adding more and more DT files. > > # I wish there was some type of if-else syntax in Device Tree so in the > board file I could just say: > > / { > model = "RSKRZA1"; > compatible = "renesas,rskrza1", "renesas,r7s72100"; > > variant = "rza1l"; > > > > We could also decide to not have rza1h.dtsi, and assume the base dtsi is > > for > > RZ/A1H. > > Honestly, I'm fine with RZ/A1H being the flagship upstream SoC since it is a > superset of all the peripherals in RZ/A1M,/A1L,/A1LU,/A1LC. Meaning, this small > difference can be handled in a simple board file addition. > > I was thinking I was going to post some DT examples on eLinux.org anyway, so one of > them would just be this RZ/A1L fix. > If we do all agree on having RZ/A1H upstream and let differences between SoCs to be handled in the BSP/downstream board files, I'll send v2 targeting that specific SoC only Thanks j > Cheers > > Chris >
Hi Jacopo, Monday, March 20, 2017, jacopo wrote: > > I was thinking I was going to post some DT examples on eLinux.org > > anyway, so one of them would just be this RZ/A1L fix. > > > > If we do all agree on having RZ/A1H upstream and let differences between > SoCs to be handled in the BSP/downstream board files, I'll send > v2 targeting that specific SoC only I'm fine with that. ...of course since it was my suggestion ;) Chris
diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi index 3dd427d..a77a431b 100644 --- a/arch/arm/boot/dts/r7s72100.dtsi +++ b/arch/arm/boot/dts/r7s72100.dtsi @@ -12,6 +12,7 @@ #include <dt-bindings/clock/r7s72100-clock.h> #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/pinctrl/r7s72100-pinctrl.h> / { compatible = "renesas,r7s72100"; @@ -171,6 +172,86 @@ }; }; + pinctrl: pinctrl@fcfe3000 { + compatible = "renesas,r7s72100-ports"; + + #pinctrl-cells = <1>; + + reg = <0xfcfe3000 0x4248>; + + port0: gpio@0 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 0 16>; + }; + + port1: gpio@1 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 16 16>; + }; + + port2: gpio@2 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 32 16>; + }; + + port3: gpio@3 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 48 16>; + }; + + port4: gpio@4 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 64 16>; + }; + + port5: gpio@5 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 80 16>; + }; + + port6: gpio@6 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 96 16>; + }; + + port7: gpio@7 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 112 16>; + }; + + port8: gpio@8 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 128 16>; + }; + + port9: gpio@9 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 144 16>; + }; + + port10: gpio@10 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 160 16>; + }; + + port11: gpio@11 { + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pinctrl 0 176 16>; + }; + }; + scif0: serial@e8007000 { compatible = "renesas,scif-r7s72100", "renesas,scif"; reg = <0xe8007000 64>;
Add pin controller node with 12 gpio controller sub-nodes to r7s72100 dtsi. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- arch/arm/boot/dts/r7s72100.dtsi | 81 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+)