[PATCH 1/7] ARM: dts: socfpga: add gpio pieces

[PATCH 1/7] ARM: dts: socfpga: add gpio pieces

[Sebastian Andrzej Siewior]

The cycloneV has three gpio controllers, the first two with 29 gpios, the last
one with 27. This patch adds the three controller with the gpio driver which is
now sitting the gpio tree.

Cc: devicetree@vger.kernel.org
Acked-by: Alan Tull <atull@altera.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2:
	- #gpio-cells = <2>
	- third gpio block has now only 27 gpios

 arch/arm/boot/dts/socfpga.dtsi | 64 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 537f1a5..2a84e67 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -463,6 +463,70 @@
 			status = "disabled";
 		};
 
+		gpio@ff708000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0xff708000 0x1000>;
+			clocks = <&per_base_clk>;
+			status = "disabled";
+
+			gpio0: gpio-controller@0 {
+				compatible = "snps,dw-apb-gpio-port";
+				gpio-controller;
+				#gpio-cells = <2>;
+				snps,nr-gpios = <29>;
+				reg = <0>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 164 4>;
+			};
+		};
+
+		gpio@ff709000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0xff709000 0x1000>;
+			clocks = <&per_base_clk>;
+			status = "disabled";
+
+			gpio1: gpio-controller@0 {
+				compatible = "snps,dw-apb-gpio-port";
+				gpio-controller;
+				#gpio-cells = <2>;
+				snps,nr-gpios = <29>;
+				reg = <0>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 165 4>;
+			};
+		};
+
+		gpio@ff70a000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0xff70a000 0x1000>;
+			clocks = <&per_base_clk>;
+			status = "disabled";
+
+			gpio2: gpio-controller@0 {
+				compatible = "snps,dw-apb-gpio-port";
+				gpio-controller;
+				#gpio-cells = <2>;
+				/*
+				 * Despite what the documentation says here, the
+				 * third gpio block has only 27 gpios available
+				 */
+				snps,nr-gpios = <27>;
+				reg = <0>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 166 4>;
+			};
+		};
+
 		L2: l2-cache@fffef000 {
 			compatible = "arm,pl310-cache";
 			reg = <0xfffef000 0x1000>;
-- 
1.9.1

Re: [PATCH 1/7] ARM: dts: socfpga: add gpio pieces

[Olof Johansson]

Hi,

I saw this patch as it came in through Dinh's pull request, see below:


On Sat, Mar 22, 2014 at 9:16 AM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> The cycloneV has three gpio controllers, the first two with 29 gpios, the last
> one with 27. This patch adds the three controller with the gpio driver which is
> now sitting the gpio tree.
>
> Cc: devicetree@vger.kernel.org
> Acked-by: Alan Tull <atull@altera.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> v1…v2:
>         - #gpio-cells = <2>
>         - third gpio block has now only 27 gpios
>
>  arch/arm/boot/dts/socfpga.dtsi | 64 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index 537f1a5..2a84e67 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -463,6 +463,70 @@
>                         status = "disabled";
>                 };
>
> +               gpio@ff708000 {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       compatible = "snps,dw-apb-gpio";
> +                       reg = <0xff708000 0x1000>;
> +                       clocks = <&per_base_clk>;
> +                       status = "disabled";
> +
> +                       gpio0: gpio-controller@0 {
> +                               compatible = "snps,dw-apb-gpio-port";
> +                               gpio-controller;
> +                               #gpio-cells = <2>;
> +                               snps,nr-gpios = <29>;
> +                               reg = <0>;
> +                               interrupt-controller;
> +                               #interrupt-cells = <2>;
> +                               interrupts = <0 164 4>;
> +                       };
> +               };

This is an odd setup. We usually would have it all in one node, since
the @ff708000 is the GPIO controller, instead of adding a subnode with
the actual GPIO info.

So I would have expected something more like:


gpio0: gpio-controller@ff708000 {
                       #address-cells = <1>;
                       #size-cells = <0>;
                       compatible = "snps,dw-apb-gpio";
                       reg = <0xff708000 0x1000>;
                       interrupts = <0 164 4>;
                      clocks = <&per_base_clk>;
                       status = "disabled";
                       gpio-controller;
                       #gpio-cells = <2>;
                       snps,nr-gpios = <29>;
                       interrupt-controller;
                       #interrupt-cells = <2>;
               };


... or is there some underlying reason for having the two-layer
approach that isn't obvious from this device tree?


-Olof

Re: [PATCH 1/7] ARM: dts: socfpga: add gpio pieces

[Alan Tull]

On Mon, May 5, 2014 at 5:02 PM, Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org> wrote:
> Hi,
>
> I saw this patch as it came in through Dinh's pull request, see below:
>
>
> On Sat, Mar 22, 2014 at 9:16 AM, Sebastian Andrzej Siewior
> <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
>> The cycloneV has three gpio controllers, the first two with 29 gpios, the last
>> one with 27. This patch adds the three controller with the gpio driver which is
>> now sitting the gpio tree.
>>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Acked-by: Alan Tull <atull-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>> ---
>> v1…v2:
>>         - #gpio-cells = <2>
>>         - third gpio block has now only 27 gpios
>>
>>  arch/arm/boot/dts/socfpga.dtsi | 64 ++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 64 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
>> index 537f1a5..2a84e67 100644
>> --- a/arch/arm/boot/dts/socfpga.dtsi
>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>> @@ -463,6 +463,70 @@
>>                         status = "disabled";
>>                 };
>>
>> +               gpio@ff708000 {
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +                       compatible = "snps,dw-apb-gpio";
>> +                       reg = <0xff708000 0x1000>;
>> +                       clocks = <&per_base_clk>;
>> +                       status = "disabled";
>> +
>> +                       gpio0: gpio-controller@0 {
>> +                               compatible = "snps,dw-apb-gpio-port";
>> +                               gpio-controller;
>> +                               #gpio-cells = <2>;
>> +                               snps,nr-gpios = <29>;
>> +                               reg = <0>;
>> +                               interrupt-controller;
>> +                               #interrupt-cells = <2>;
>> +                               interrupts = <0 164 4>;
>> +                       };
>> +               };
>
> This is an odd setup. We usually would have it all in one node, since
> the @ff708000 is the GPIO controller, instead of adding a subnode with
> the actual GPIO info.
>
> So I would have expected something more like:
>
>
> gpio0: gpio-controller@ff708000 {
>                        #address-cells = <1>;
>                        #size-cells = <0>;
>                        compatible = "snps,dw-apb-gpio";
>                        reg = <0xff708000 0x1000>;
>                        interrupts = <0 164 4>;
>                       clocks = <&per_base_clk>;
>                        status = "disabled";
>                        gpio-controller;
>                        #gpio-cells = <2>;
>                        snps,nr-gpios = <29>;
>                        interrupt-controller;
>                        #interrupt-cells = <2>;
>                };
>
>
> ... or is there some underlying reason for having the two-layer
> approach that isn't obvious from this device tree?
>
>
> -Olof

The synopsys gpio ip can have 1, 2, or 3 gpio ports of varying widths
per IP block.  The simpler bindings assume one gpio port per IP block.

Alan Tull
aka
delicious quinoa
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] ARM: dts: socfpga: add gpio pieces

[Linus Walleij]

On Tue, May 6, 2014 at 12:02 AM, Olof Johansson <olof@lixom.net> wrote:

> ... or is there some underlying reason for having the two-layer
> approach that isn't obvious from this device tree?

There is, as explained by Alan. One coherent memory range with
several individual ports.

Yours,
Linus Walleij

[PATCH 2/7] gpio: dwapb: correct gpio-cells in binding document

[Sebastian Andrzej Siewior]

The example uses gpio-cells = 1 while it should be two (it is even
mentioned in the text above).

Cc: devicetree@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
index 91cc90c..dd5d2c0 100644
--- a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
@@ -41,7 +41,7 @@ gpio: gpio@20000 {
 	porta: gpio-controller@0 {
 		compatible = "snps,dw-apb-gpio-port";
 		gpio-controller;
-		#gpio-cells = <1>;
+		#gpio-cells = <2>;
 		snps,nr-gpios = <8>;
 		reg = <0>;
 		interrupt-controller;
@@ -53,7 +53,7 @@ gpio: gpio@20000 {
 	portb: gpio-controller@1 {
 		compatible = "snps,dw-apb-gpio-port";
 		gpio-controller;
-		#gpio-cells = <1>;
+		#gpio-cells = <2>;
 		snps,nr-gpios = <8>;
 		reg = <1>;
 	};
-- 
1.9.1

Re: [PATCH 2/7] gpio: dwapb: correct gpio-cells in binding document

[Linus Walleij]

On Sat, Mar 22, 2014 at 5:16 PM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:

> The example uses gpio-cells = 1 while it should be two (it is even
> mentioned in the text above).
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

This is obviously correct so patch applied.

How come I cannot see patch 1/7 in my mailbox :-(

I see patches 2-7/7...

Something on my end?

Yours,
Linus Walleij

Re: [PATCH 2/7] gpio: dwapb: correct gpio-cells in binding document

[Sebastian Andrzej Siewior]

On 03/25/2014 09:49 PM, Linus Walleij wrote:
> This is obviously correct so patch applied.

Thanks.

> How come I cannot see patch 1/7 in my mailbox :-(
> 
> I see patches 2-7/7...
> 
> Something on my end?

Maybe but you can still blame google for loosing your emails.
It make it to lkml:
	https://lkml.org/lkml/2014/3/22/112
> 
> Yours,
> Linus Walleij

Sebastian

Re: [PATCH 2/7] gpio: dwapb: correct gpio-cells in binding document

[Linus Walleij]

On Tue, Mar 25, 2014 at 9:57 PM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> On 03/25/2014 09:49 PM, Linus Walleij wrote:

> Maybe but you can still blame google for loosing your emails.
> It make it to lkml:
>         https://lkml.org/lkml/2014/3/22/112

OK that one should be applied to the ARM SoC tree anyway,
so I need not worry so much...

Yours,
Linus Walleij