| Message ID | 1397549245-3338-3-git-send-email-antoine.tenart@free-electrons.com |
|---|---|
| State | New |
| Headers | show |
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 <antoine.tenart@free-electrons.com> > --- > 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 <sebastian.hesselbarth@gmail.com> > + 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 { >
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 <sebastian.hesselbarth@gmail.com> > > >+ 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 { > > >
Hi Sebastian, On Tue, 15 Apr 2014 02:23:03 -0700 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> 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 <antoine.tenart@free-electrons.com> > > --- > > 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
On 04/15/2014 12:00 PM, Jisheng Zhang wrote: > On Tue, 15 Apr 2014 02:23:03 -0700 > Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> 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 <antoine.tenart@free-electrons.com> >>> --- >>> 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
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 <sebastian.hesselbarth@gmail.com> 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 <antoine.tenart@free-electrons.com> > >>>--- > >>> 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 ?
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 <sebastian.hesselbarth@gmail.com> 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
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 {
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 <antoine.tenart@free-electrons.com> --- arch/arm/boot/dts/berlin2q.dtsi | 102 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)