2014-04-15 08:07:42

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 0/2] ARM: berlin: add GPIO support for the BG2Q

This series add the support for the GPIOs of the Berlin BG2Q. We use the
newly integrated dwapb GPIO driver here.

This applies on top of Alexandre's BG2Q symbol introduction[1] and the dwapb
gpio patch fixing IRQ initialization[2].

[1] https://patchwork.kernel.org/patch/3876141/
[2] https://lkml.org/lkml/2014/4/7/96

Antoine Ténart (2):
ARM: berlin: add the LIBGPIO as a dependency for the BG2Q
ARM: dts: berlin: add GPIO nodes for the BG2Q

arch/arm/boot/dts/berlin2q.dtsi | 102 ++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-berlin/Kconfig | 1 +
2 files changed, 103 insertions(+)

--
1.8.3.2


2014-04-15 08:07:46

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 2/2] ARM: dts: berlin: add GPIO nodes for the BG2Q

The Marvell Berlin BG2Q has 6 GPIO ports compatible with the snps,dw-apb-gpio
driver. This patch add the corresponding device tree nodes.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/boot/dts/berlin2q.dtsi | 102 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index e6e556055dfc..b2625f896bc5 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -109,6 +109,78 @@
ranges = <0 0xe80000 0x10000>;
interrupt-parent = <&aic>;

+ gpio0: gpio@0400 {
+ compatible = "snps,dw-apb-gpio";
+ reg = <0x0400 0x400>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ porta: gpio-controller@0 {
+ compatible = "snps,dw-apb-gpio-port";
+ gpio-controller;
+ #gpio-cells = <2>;
+ snps,nr-gpios = <32>;
+ reg = <0>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <0>;
+ };
+ };
+
+ gpio1: gpio@0800 {
+ compatible = "snps,dw-apb-gpio";
+ reg = <0x0800 0x400>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ portb: gpio-controller@1 {
+ compatible = "snps,dw-apb-gpio-port";
+ gpio-controller;
+ #gpio-cells = <2>;
+ snps,nr-gpios = <32>;
+ reg = <0>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <1>;
+ };
+ };
+
+ gpio2: gpio@0c00 {
+ compatible = "snps,dw-apb-gpio";
+ reg = <0x0c00 0x400>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ portc: gpio-controller@2 {
+ compatible = "snps,dw-apb-gpio-port";
+ gpio-controller;
+ #gpio-cells = <2>;
+ snps,nr-gpios = <32>;
+ reg = <0>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <2>;
+ };
+ };
+
+ gpio3: gpio@1000 {
+ compatible = "snps,dw-apb-gpio";
+ reg = <0x1000 0x400>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ portd: gpio-controller@3 {
+ compatible = "snps,dw-apb-gpio-port";
+ gpio-controller;
+ #gpio-cells = <2>;
+ snps,nr-gpios = <32>;
+ reg = <0>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <3>;
+ };
+ };
+
timer0: timer@2c00 {
compatible = "snps,dw-apb-timer";
reg = <0x2c00 0x14>;
@@ -181,6 +253,36 @@
interrupt-parent = <&gic>;
interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
};
+
+ gpio4: gpio@5000 {
+ compatible = "snps,dw-apb-gpio";
+ reg = <0x5000 0x400>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ porte: gpio-controller@4 {
+ compatible = "snps,dw-apb-gpio-port";
+ gpio-controller;
+ #gpio-cells = <2>;
+ snps,nr-gpios = <32>;
+ reg = <0>;
+ };
+ };
+
+ gpio5: gpio@c000 {
+ compatible = "snps,dw-apb-gpio";
+ reg = <0xc000 0x400>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ portf: gpio-controller@5 {
+ compatible = "snps,dw-apb-gpio-port";
+ gpio-controller;
+ #gpio-cells = <2>;
+ snps,nr-gpios = <32>;
+ reg = <0>;
+ };
+ };
};

pinctrl: pinctrl@0 {
--
1.8.3.2

2014-04-15 08:07:37

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 1/2] ARM: berlin: add the LIBGPIO as a dependency for the BG2Q

The BG2Q has GPIOs driven by the dwapb GPIO driver. Add the LIBGPIO as a
dependency to be able to support them.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/mach-berlin/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
index 291f1cac6c3d..c31968712095 100644
--- a/arch/arm/mach-berlin/Kconfig
+++ b/arch/arm/mach-berlin/Kconfig
@@ -28,6 +28,7 @@ config MACH_BERLIN_BG2Q
select CPU_V7
select HAVE_ARM_TWD if SMP
select HAVE_SMP
+ select ARCH_REQUIRE_GPIOLIB

endmenu

--
1.8.3.2

2014-04-15 09:10:21

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: berlin: add the LIBGPIO as a dependency for the BG2Q

Hi Antoine,

On Tue, 15 Apr 2014 01:07:24 -0700
Antoine Ténart <[email protected]> wrote:

> The BG2Q has GPIOs driven by the dwapb GPIO driver. Add the LIBGPIO as a
> dependency to be able to support them.
>
> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> arch/arm/mach-berlin/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
> index 291f1cac6c3d..c31968712095 100644
> --- a/arch/arm/mach-berlin/Kconfig
> +++ b/arch/arm/mach-berlin/Kconfig
> @@ -28,6 +28,7 @@ config MACH_BERLIN_BG2Q
> select CPU_V7
> select HAVE_ARM_TWD if SMP
> select HAVE_SMP
> + select ARCH_REQUIRE_GPIOLIB

alphabetically sort.

Others looks good ;)

Thanks,
Jisheng

2014-04-15 09:16:55

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: berlin: add the LIBGPIO as a dependency for the BG2Q

On 04/15/2014 10:07 AM, Antoine Ténart wrote:
> The BG2Q has GPIOs driven by the dwapb GPIO driver. Add the LIBGPIO as a
> dependency to be able to support them.
>
> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> arch/arm/mach-berlin/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
> index 291f1cac6c3d..c31968712095 100644
> --- a/arch/arm/mach-berlin/Kconfig
> +++ b/arch/arm/mach-berlin/Kconfig
> @@ -28,6 +28,7 @@ config MACH_BERLIN_BG2Q
> select CPU_V7
> select HAVE_ARM_TWD if SMP
> select HAVE_SMP
> + select ARCH_REQUIRE_GPIOLIB

Apart from Jisheng's comment, I think we can directly move it
into arch/arm/Kconfig's MACH_BERLIN directly?

Sebastian

> endmenu
>
>

2014-04-15 09:23:12

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: berlin: add GPIO nodes for the BG2Q

On 04/15/2014 10:07 AM, Antoine Ténart wrote:
> The Marvell Berlin BG2Q has 6 GPIO ports compatible with the snps,dw-apb-gpio
> driver. This patch add the corresponding device tree nodes.
>
> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> arch/arm/boot/dts/berlin2q.dtsi | 102 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 102 insertions(+)
>
> diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
> index e6e556055dfc..b2625f896bc5 100644
> --- a/arch/arm/boot/dts/berlin2q.dtsi
> +++ b/arch/arm/boot/dts/berlin2q.dtsi
> @@ -109,6 +109,78 @@
> ranges = <0 0xe80000 0x10000>;
> interrupt-parent = <&aic>;
>
> + gpio0: gpio@0400 {
> + compatible = "snps,dw-apb-gpio";
> + reg = <0x0400 0x400>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + porta: gpio-controller@0 {

ePAPR recommended name is even more generic, i.e. "gpio". If
that clashed in any way with other numbered names, I suggest
to rename to "gpio-port" as actually the controller is the
parent node and this represents one port (in the nomenclature
of DW-APB-GPIO).

> + compatible = "snps,dw-apb-gpio-port";
> + gpio-controller;
> + #gpio-cells = <2>;
> + snps,nr-gpios = <32>;

32 gpio pins for each of the 6 GPIO controllers? Either BG2Q is a GPIO
beast or it is a mistake :P

Can you please double-check?

I am fine with using nr-gpios property now, but I guess BG2Q also
has that CONFIG[1,2] registers to actually read out the features
synthesized in? If I find some time, I'll prepare a patch for
dw-apb-gpio to exploit that (optional) information instead of
using nr-gpios.

Otherwise,

Acked-by: Sebastian Hesselbarth <[email protected]>

> + reg = <0>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = <0>;
> + };
> + };
> +
> + gpio1: gpio@0800 {
> + compatible = "snps,dw-apb-gpio";
> + reg = <0x0800 0x400>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + portb: gpio-controller@1 {
> + compatible = "snps,dw-apb-gpio-port";
> + gpio-controller;
> + #gpio-cells = <2>;
> + snps,nr-gpios = <32>;
> + reg = <0>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = <1>;
> + };
> + };
> +
> + gpio2: gpio@0c00 {
> + compatible = "snps,dw-apb-gpio";
> + reg = <0x0c00 0x400>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + portc: gpio-controller@2 {
> + compatible = "snps,dw-apb-gpio-port";
> + gpio-controller;
> + #gpio-cells = <2>;
> + snps,nr-gpios = <32>;
> + reg = <0>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = <2>;
> + };
> + };
> +
> + gpio3: gpio@1000 {
> + compatible = "snps,dw-apb-gpio";
> + reg = <0x1000 0x400>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + portd: gpio-controller@3 {
> + compatible = "snps,dw-apb-gpio-port";
> + gpio-controller;
> + #gpio-cells = <2>;
> + snps,nr-gpios = <32>;
> + reg = <0>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = <3>;
> + };
> + };
> +
> timer0: timer@2c00 {
> compatible = "snps,dw-apb-timer";
> reg = <0x2c00 0x14>;
> @@ -181,6 +253,36 @@
> interrupt-parent = <&gic>;
> interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> };
> +
> + gpio4: gpio@5000 {
> + compatible = "snps,dw-apb-gpio";
> + reg = <0x5000 0x400>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + porte: gpio-controller@4 {
> + compatible = "snps,dw-apb-gpio-port";
> + gpio-controller;
> + #gpio-cells = <2>;
> + snps,nr-gpios = <32>;
> + reg = <0>;
> + };
> + };
> +
> + gpio5: gpio@c000 {
> + compatible = "snps,dw-apb-gpio";
> + reg = <0xc000 0x400>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + portf: gpio-controller@5 {
> + compatible = "snps,dw-apb-gpio-port";
> + gpio-controller;
> + #gpio-cells = <2>;
> + snps,nr-gpios = <32>;
> + reg = <0>;
> + };
> + };
> };
>
> pinctrl: pinctrl@0 {
>

2014-04-15 09:26:55

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: berlin: add the LIBGPIO as a dependency for the BG2Q

Hi Jisheng,

On Tue, Apr 15, 2014 at 05:07:50PM +0800, Jisheng Zhang wrote:
> On Tue, 15 Apr 2014 01:07:24 -0700
> Antoine Ténart <[email protected]> wrote:
[…]
> > diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
> > index 291f1cac6c3d..c31968712095 100644
> > --- a/arch/arm/mach-berlin/Kconfig
> > +++ b/arch/arm/mach-berlin/Kconfig
> > @@ -28,6 +28,7 @@ config MACH_BERLIN_BG2Q
> > select CPU_V7
> > select HAVE_ARM_TWD if SMP
> > select HAVE_SMP
> > + select ARCH_REQUIRE_GPIOLIB
>
> alphabetically sort.

You're right, will do.

Antoine

--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-04-15 09:28:25

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: berlin: add the LIBGPIO as a dependency for the BG2Q

Sebastian,

On Tue, Apr 15, 2014 at 11:16:49AM +0200, Sebastian Hesselbarth wrote:
> On 04/15/2014 10:07 AM, Antoine Ténart wrote:

[…]

> >diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
> >index 291f1cac6c3d..c31968712095 100644
> >--- a/arch/arm/mach-berlin/Kconfig
> >+++ b/arch/arm/mach-berlin/Kconfig
> >@@ -28,6 +28,7 @@ config MACH_BERLIN_BG2Q
> > select CPU_V7
> > select HAVE_ARM_TWD if SMP
> > select HAVE_SMP
> >+ select ARCH_REQUIRE_GPIOLIB
>
> Apart from Jisheng's comment, I think we can directly move it
> into arch/arm/Kconfig's MACH_BERLIN directly?

I think so.

Antoine

--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-04-15 09:41:58

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: berlin: add GPIO nodes for the BG2Q

Sebastian,

On Tue, Apr 15, 2014 at 11:23:03AM +0200, Sebastian Hesselbarth wrote:
> On 04/15/2014 10:07 AM, Antoine Ténart wrote:

[…]

> >diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
> >index e6e556055dfc..b2625f896bc5 100644
> >--- a/arch/arm/boot/dts/berlin2q.dtsi
> >+++ b/arch/arm/boot/dts/berlin2q.dtsi
> >@@ -109,6 +109,78 @@
> > ranges = <0 0xe80000 0x10000>;
> > interrupt-parent = <&aic>;
> >
> >+ gpio0: gpio@0400 {
> >+ compatible = "snps,dw-apb-gpio";
> >+ reg = <0x0400 0x400>;
> >+ #address-cells = <1>;
> >+ #size-cells = <0>;
> >+
> >+ porta: gpio-controller@0 {
>
> ePAPR recommended name is even more generic, i.e. "gpio". If
> that clashed in any way with other numbered names, I suggest
> to rename to "gpio-port" as actually the controller is the
> parent node and this represents one port (in the nomenclature
> of DW-APB-GPIO).

I followed the dwapb GPIO binding documentation, but I think this is a good
idea. I'll update.

> >+ compatible = "snps,dw-apb-gpio-port";
> >+ gpio-controller;
> >+ #gpio-cells = <2>;
> >+ snps,nr-gpios = <32>;
>
> 32 gpio pins for each of the 6 GPIO controllers? Either BG2Q is a GPIO
> beast or it is a mistake :P
>
> Can you please double-check?

Maybe Jisheng can confirm this.

> I am fine with using nr-gpios property now, but I guess BG2Q also
> has that CONFIG[1,2] registers to actually read out the features
> synthesized in? If I find some time, I'll prepare a patch for
> dw-apb-gpio to exploit that (optional) information instead of
> using nr-gpios.

Maybe, that would be nice.


Antoine

> Otherwise,
>
> Acked-by: Sebastian Hesselbarth <[email protected]>
>
> >+ reg = <0>;
> >+ interrupt-controller;
> >+ #interrupt-cells = <2>;
> >+ interrupts = <0>;
> >+ };
> >+ };
> >+
> >+ gpio1: gpio@0800 {
> >+ compatible = "snps,dw-apb-gpio";
> >+ reg = <0x0800 0x400>;
> >+ #address-cells = <1>;
> >+ #size-cells = <0>;
> >+
> >+ portb: gpio-controller@1 {
> >+ compatible = "snps,dw-apb-gpio-port";
> >+ gpio-controller;
> >+ #gpio-cells = <2>;
> >+ snps,nr-gpios = <32>;
> >+ reg = <0>;
> >+ interrupt-controller;
> >+ #interrupt-cells = <2>;
> >+ interrupts = <1>;
> >+ };
> >+ };
> >+
> >+ gpio2: gpio@0c00 {
> >+ compatible = "snps,dw-apb-gpio";
> >+ reg = <0x0c00 0x400>;
> >+ #address-cells = <1>;
> >+ #size-cells = <0>;
> >+
> >+ portc: gpio-controller@2 {
> >+ compatible = "snps,dw-apb-gpio-port";
> >+ gpio-controller;
> >+ #gpio-cells = <2>;
> >+ snps,nr-gpios = <32>;
> >+ reg = <0>;
> >+ interrupt-controller;
> >+ #interrupt-cells = <2>;
> >+ interrupts = <2>;
> >+ };
> >+ };
> >+
> >+ gpio3: gpio@1000 {
> >+ compatible = "snps,dw-apb-gpio";
> >+ reg = <0x1000 0x400>;
> >+ #address-cells = <1>;
> >+ #size-cells = <0>;
> >+
> >+ portd: gpio-controller@3 {
> >+ compatible = "snps,dw-apb-gpio-port";
> >+ gpio-controller;
> >+ #gpio-cells = <2>;
> >+ snps,nr-gpios = <32>;
> >+ reg = <0>;
> >+ interrupt-controller;
> >+ #interrupt-cells = <2>;
> >+ interrupts = <3>;
> >+ };
> >+ };
> >+
> > timer0: timer@2c00 {
> > compatible = "snps,dw-apb-timer";
> > reg = <0x2c00 0x14>;
> >@@ -181,6 +253,36 @@
> > interrupt-parent = <&gic>;
> > interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> > };
> >+
> >+ gpio4: gpio@5000 {
> >+ compatible = "snps,dw-apb-gpio";
> >+ reg = <0x5000 0x400>;
> >+ #address-cells = <1>;
> >+ #size-cells = <0>;
> >+
> >+ porte: gpio-controller@4 {
> >+ compatible = "snps,dw-apb-gpio-port";
> >+ gpio-controller;
> >+ #gpio-cells = <2>;
> >+ snps,nr-gpios = <32>;
> >+ reg = <0>;
> >+ };
> >+ };
> >+
> >+ gpio5: gpio@c000 {
> >+ compatible = "snps,dw-apb-gpio";
> >+ reg = <0xc000 0x400>;
> >+ #address-cells = <1>;
> >+ #size-cells = <0>;
> >+
> >+ portf: gpio-controller@5 {
> >+ compatible = "snps,dw-apb-gpio-port";
> >+ gpio-controller;
> >+ #gpio-cells = <2>;
> >+ snps,nr-gpios = <32>;
> >+ reg = <0>;
> >+ };
> >+ };
> > };
> >
> > pinctrl: pinctrl@0 {
> >
>

--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-04-15 10:03:26

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: berlin: add GPIO nodes for the BG2Q

Hi Sebastian,

On Tue, 15 Apr 2014 02:23:03 -0700
Sebastian Hesselbarth <[email protected]> wrote:

> On 04/15/2014 10:07 AM, Antoine Ténart wrote:
> > The Marvell Berlin BG2Q has 6 GPIO ports compatible with the
> > snps,dw-apb-gpio driver. This patch add the corresponding device tree
> > nodes.
> >
> > Signed-off-by: Antoine Ténart <[email protected]>
> > ---
> > arch/arm/boot/dts/berlin2q.dtsi | 102
> > ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/berlin2q.dtsi
> > b/arch/arm/boot/dts/berlin2q.dtsi index e6e556055dfc..b2625f896bc5 100644
> > --- a/arch/arm/boot/dts/berlin2q.dtsi
> > +++ b/arch/arm/boot/dts/berlin2q.dtsi
> > @@ -109,6 +109,78 @@
> > ranges = <0 0xe80000 0x10000>;
> > interrupt-parent = <&aic>;
> >
> > + gpio0: gpio@0400 {
> > + compatible = "snps,dw-apb-gpio";
> > + reg = <0x0400 0x400>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + porta: gpio-controller@0 {
>
> ePAPR recommended name is even more generic, i.e. "gpio". If
> that clashed in any way with other numbered names, I suggest
> to rename to "gpio-port" as actually the controller is the
> parent node and this represents one port (in the nomenclature
> of DW-APB-GPIO).
>
> > + compatible =
> > "snps,dw-apb-gpio-port";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + snps,nr-gpios = <32>;
>
> 32 gpio pins for each of the 6 GPIO controllers? Either BG2Q is a GPIO

Yes, BG2Q support 32 pins every port

> beast or it is a mistake :P
>
> Can you please double-check?
>
> I am fine with using nr-gpios property now, but I guess BG2Q also
> has that CONFIG[1,2] registers to actually read out the features
> synthesized in? If I find some time, I'll prepare a patch for
> dw-apb-gpio to exploit that (optional) information instead of
> using nr-gpios.

The problem is CONFIG1/2 registers don't exist on some versions.
For example, the version used in BG2/BG2CD. So nr-gpio is necessary
if we want to support these versions.

Thanks,
Jisheng

2014-04-15 10:26:19

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: berlin: add GPIO nodes for the BG2Q

On 04/15/2014 12:00 PM, Jisheng Zhang wrote:
> On Tue, 15 Apr 2014 02:23:03 -0700
> Sebastian Hesselbarth <[email protected]> wrote:
>> On 04/15/2014 10:07 AM, Antoine Ténart wrote:
>>> The Marvell Berlin BG2Q has 6 GPIO ports compatible with the
>>> snps,dw-apb-gpio driver. This patch add the corresponding device tree
>>> nodes.
>>>
>>> Signed-off-by: Antoine Ténart <[email protected]>
>>> ---
>>> arch/arm/boot/dts/berlin2q.dtsi | 102
>>> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/berlin2q.dtsi
>>> b/arch/arm/boot/dts/berlin2q.dtsi index e6e556055dfc..b2625f896bc5 100644
>>> --- a/arch/arm/boot/dts/berlin2q.dtsi
>>> +++ b/arch/arm/boot/dts/berlin2q.dtsi
[...]
>>> + compatible =
>>> "snps,dw-apb-gpio-port";
>>> + gpio-controller;
>>> + #gpio-cells = <2>;
>>> + snps,nr-gpios = <32>;
>>
>> 32 gpio pins for each of the 6 GPIO controllers? Either BG2Q is a GPIO
>
> Yes, BG2Q support 32 pins every port

Wow. Thanks for confirming this!

>> I am fine with using nr-gpios property now, but I guess BG2Q also
>> has that CONFIG[1,2] registers to actually read out the features
>> synthesized in? If I find some time, I'll prepare a patch for
>> dw-apb-gpio to exploit that (optional) information instead of
>> using nr-gpios.
>
> The problem is CONFIG1/2 registers don't exist on some versions.
> For example, the version used in BG2/BG2CD. So nr-gpio is necessary
> if we want to support these versions.

Hmm, are you sure about BG2? I remember reading it and it contains
sane values. Anyway, a proper patch for dw-apb-gpio would include
checking for sane (e.g. non-zero) values. And nr-gpios will always
stay as fall-back just because e.g. sunxi does not have the CONFIG
registers.

Sebastian

2014-04-15 11:48:53

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: berlin: add GPIO nodes for the BG2Q

On 15/04/2014 at 12:26:11 +0200, Sebastian Hesselbarth wrote :
> On 04/15/2014 12:00 PM, Jisheng Zhang wrote:
> >On Tue, 15 Apr 2014 02:23:03 -0700
> >Sebastian Hesselbarth <[email protected]> wrote:
> >>On 04/15/2014 10:07 AM, Antoine T?nart wrote:
> >>>The Marvell Berlin BG2Q has 6 GPIO ports compatible with the
> >>>snps,dw-apb-gpio driver. This patch add the corresponding device tree
> >>>nodes.
> >>>
> >>>Signed-off-by: Antoine T?nart <[email protected]>
> >>>---
> >>> arch/arm/boot/dts/berlin2q.dtsi | 102
> >>>++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
> >>>
> >>>diff --git a/arch/arm/boot/dts/berlin2q.dtsi
> >>>b/arch/arm/boot/dts/berlin2q.dtsi index e6e556055dfc..b2625f896bc5 100644
> >>>--- a/arch/arm/boot/dts/berlin2q.dtsi
> >>>+++ b/arch/arm/boot/dts/berlin2q.dtsi
> [...]
> >>>+ compatible =
> >>>"snps,dw-apb-gpio-port";
> >>>+ gpio-controller;
> >>>+ #gpio-cells = <2>;
> >>>+ snps,nr-gpios = <32>;
> >>
> >>32 gpio pins for each of the 6 GPIO controllers? Either BG2Q is a GPIO
> >
> >Yes, BG2Q support 32 pins every port
>
> Wow. Thanks for confirming this!
>
> >>I am fine with using nr-gpios property now, but I guess BG2Q also
> >>has that CONFIG[1,2] registers to actually read out the features
> >>synthesized in? If I find some time, I'll prepare a patch for
> >>dw-apb-gpio to exploit that (optional) information instead of
> >>using nr-gpios.
> >
> >The problem is CONFIG1/2 registers don't exist on some versions.
> >For example, the version used in BG2/BG2CD. So nr-gpio is necessary
> >if we want to support these versions.
>
> Hmm, are you sure about BG2? I remember reading it and it contains
> sane values. Anyway, a proper patch for dw-apb-gpio would include
> checking for sane (e.g. non-zero) values. And nr-gpios will always
> stay as fall-back just because e.g. sunxi does not have the CONFIG
> registers.
>

Shouldn't that be use nr-gpios and if not available, read the CONFIG
registers? Else, what about bogus registers ?

--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-04-15 12:51:07

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: berlin: add GPIO nodes for the BG2Q

On 04/15/2014 01:48 PM, Alexandre Belloni wrote:
> On 15/04/2014 at 12:26:11 +0200, Sebastian Hesselbarth wrote :
>> On 04/15/2014 12:00 PM, Jisheng Zhang wrote:
>>> On Tue, 15 Apr 2014 02:23:03 -0700
>>> Sebastian Hesselbarth <[email protected]> wrote:
>>>> I am fine with using nr-gpios property now, but I guess BG2Q also
>>>> has that CONFIG[1,2] registers to actually read out the features
>>>> synthesized in? If I find some time, I'll prepare a patch for
>>>> dw-apb-gpio to exploit that (optional) information instead of
>>>> using nr-gpios.
>>>
>>> The problem is CONFIG1/2 registers don't exist on some versions.
>>> For example, the version used in BG2/BG2CD. So nr-gpio is necessary
>>> if we want to support these versions.
>>
>> Hmm, are you sure about BG2? I remember reading it and it contains
>> sane values. Anyway, a proper patch for dw-apb-gpio would include
>> checking for sane (e.g. non-zero) values. And nr-gpios will always
>> stay as fall-back just because e.g. sunxi does not have the CONFIG
>> registers.
>
> Shouldn't that be use nr-gpios and if not available, read the CONFIG
> registers? Else, what about bogus registers ?

Probably, it is a better idea to only look for CONFIG registers if
nr-gpios is not set. Anyway, I'll really have to find more time first ;)

Sebastian