diff mbox

[v3,09/20] ARM: shmobile: r8a7790: Add GPIO controller devices to device tree

Message ID Pine.LNX.4.64.1305171423270.31481@axis700.grange (mailing list archive)
State New, archived
Headers show

Commit Message

Guennadi Liakhovetski May 17, 2013, 12:26 p.m. UTC
Hi Laurent

On Wed, 15 May 2013, Laurent Pinchart wrote:

> Add GPIO controller nodes to the r8a7790 core device tree.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  arch/arm/boot/dts/r8a7790.dtsi | 54 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)

Also here a couple of things are missing (presumably, for other SoCs you 
need the same):


Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Comments

Laurent Pinchart May 18, 2013, 6:44 a.m. UTC | #1
Hi Guennadi,

On Friday 17 May 2013 14:26:48 Guennadi Liakhovetski wrote:
> Hi Laurent
> 
> On Wed, 15 May 2013, Laurent Pinchart wrote:
> > Add GPIO controller nodes to the r8a7790 core device tree.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  arch/arm/boot/dts/r8a7790.dtsi | 54 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> 
> Also here a couple of things are missing (presumably, for other SoCs you
> need the same):

Thank you for testing the patch set. I'll integrate the modification in the 
next version for r8a7778, r8a7779 and r8a7790.

As the gpio-ranges property specifies the number of GPIOs handled by the 
controller, what's your opinion on dropping the #gpio-lines property from the 
bindings ?

> diff --git a/arch/arm/boot/dts/r8a7790.dtsi
> b/arch/arm/boot/dts/r8a7790.dtsi
> index 674ee39..cd1a04f 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -51,6 +51,7 @@
>  		interrupts = <0 4 0x4>;
>  		#gpio-cells = <2>;
>  		gpio-controller;
> +		gpio-ranges = <&pfc 0 0 32>;
>  	};
> 
>  	gpio1: gpio@ffc41000 {
> @@ -60,6 +61,7 @@
>  		interrupts = <0 5 0x4>;
>  		#gpio-cells = <2>;
>  		gpio-controller;
> +		gpio-ranges = <&pfc 0 32 32>;
>  	};
> 
>  	gpio2: gpio@ffc42000 {
> @@ -69,6 +71,7 @@
>  		interrupts = <0 6 0x4>;
>  		#gpio-cells = <2>;
>  		gpio-controller;
> +		gpio-ranges = <&pfc 0 64 32>;
>  	};
> 
>  	gpio3: gpio@ffc43000 {
> @@ -78,6 +81,7 @@
>  		interrupts = <0 7 0x4>;
>  		#gpio-cells = <2>;
>  		gpio-controller;
> +		gpio-ranges = <&pfc 0 96 32>;
>  	};
> 
>  	gpio4: gpio@ffc44000 {
> @@ -87,6 +91,7 @@
>  		interrupts = <0 8 0x4>;
>  		#gpio-cells = <2>;
>  		gpio-controller;
> +		gpio-ranges = <&pfc 0 128 32>;
>  	};
> 
>  	gpio5: gpio@ffc45000 {
> @@ -96,6 +101,7 @@
>  		interrupts = <0 9 0x4>;
>  		#gpio-cells = <2>;
>  		gpio-controller;
> +		gpio-ranges = <&pfc 0 160 32>;
>  	};
> 
>  	timer {
> @@ -118,6 +124,7 @@
>  	pfc: pfc@e6060000 {
>  		compatible = "renesas,pfc-r8a7790";
>  		reg = <0 0xe6060000 0 0x250>;
> +		#gpio-range-cells = <3>;
>  	};
> 
>  	/* No MMC_CAP_UHS_DDR50 (dual data rate) capability on r8a7790! */
Guennadi Liakhovetski May 18, 2013, 6:57 a.m. UTC | #2
Hi Laurent

On Sat, 18 May 2013, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Friday 17 May 2013 14:26:48 Guennadi Liakhovetski wrote:
> > Hi Laurent
> > 
> > On Wed, 15 May 2013, Laurent Pinchart wrote:
> > > Add GPIO controller nodes to the r8a7790 core device tree.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > > 
> > >  arch/arm/boot/dts/r8a7790.dtsi | 54 +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 54 insertions(+)
> > 
> > Also here a couple of things are missing (presumably, for other SoCs you
> > need the same):
> 
> Thank you for testing the patch set. I'll integrate the modification in the 
> next version for r8a7778, r8a7779 and r8a7790.
> 
> As the gpio-ranges property specifies the number of GPIOs handled by the 
> controller, what's your opinion on dropping the #gpio-lines property from the 
> bindings ?

Well, gpio-ranges can contain several ranges, so, in a generic case you 
would have to sum them up to get a total count. But if you're ok with that 
or if your platforms only have 1 gpio gange per controller, of course, 
that property seems redundant. OTOH it is already used in r8a7779.dtsi, 
and thus already belongs to the ABI... Not sure whether removing it would 
be accepted.

Thanks
Guennadi

> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi
> > b/arch/arm/boot/dts/r8a7790.dtsi
> > index 674ee39..cd1a04f 100644
> > --- a/arch/arm/boot/dts/r8a7790.dtsi
> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> > @@ -51,6 +51,7 @@
> >  		interrupts = <0 4 0x4>;
> >  		#gpio-cells = <2>;
> >  		gpio-controller;
> > +		gpio-ranges = <&pfc 0 0 32>;
> >  	};
> > 
> >  	gpio1: gpio@ffc41000 {

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
Laurent Pinchart May 18, 2013, 7:03 a.m. UTC | #3
Hi Guennadi,

On Saturday 18 May 2013 08:57:41 Guennadi Liakhovetski wrote:
> On Sat, 18 May 2013, Laurent Pinchart wrote:
> > On Friday 17 May 2013 14:26:48 Guennadi Liakhovetski wrote:
> > > On Wed, 15 May 2013, Laurent Pinchart wrote:
> > > > Add GPIO controller nodes to the r8a7790 core device tree.
> > > > 
> > > > Signed-off-by: Laurent Pinchart 
> > > > <laurent.pinchart+renesas@ideasonboard.com>
> > > > ---
> > > > 
> > > >  arch/arm/boot/dts/r8a7790.dtsi | 54 +++++++++++++++++++++++++++++++++
> > > >  1 file changed, 54 insertions(+)
> > > 
> > > Also here a couple of things are missing (presumably, for other SoCs you
> > > need the same):
> >
> > Thank you for testing the patch set. I'll integrate the modification in
> > the next version for r8a7778, r8a7779 and r8a7790.
> > 
> > As the gpio-ranges property specifies the number of GPIOs handled by the
> > controller, what's your opinion on dropping the #gpio-lines property from
> > the bindings ?
> 
> Well, gpio-ranges can contain several ranges, so, in a generic case you
> would have to sum them up to get a total count. But if you're ok with that
> or if your platforms only have 1 gpio gange per controller, of course,
> that property seems redundant.

The gpio-rcar driver exposes a single range on all the current platforms. I 
don't really foresee any change there in the future, but I might be 
overlooking something. Magnus, any opinion ?

> OTOH it is already used in r8a7779.dtsi, and thus already belongs to the
> ABI... Not sure whether removing it would be accepted.

It's only used in my DT bindings proposal that haven't been merged yet, so 
that shouldn't be an issue :-)
Guennadi Liakhovetski May 18, 2013, 7:50 a.m. UTC | #4
On Sat, 18 May 2013, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Saturday 18 May 2013 08:57:41 Guennadi Liakhovetski wrote:
> > On Sat, 18 May 2013, Laurent Pinchart wrote:
> > > On Friday 17 May 2013 14:26:48 Guennadi Liakhovetski wrote:
> > > > On Wed, 15 May 2013, Laurent Pinchart wrote:
> > > > > Add GPIO controller nodes to the r8a7790 core device tree.
> > > > > 
> > > > > Signed-off-by: Laurent Pinchart 
> > > > > <laurent.pinchart+renesas@ideasonboard.com>
> > > > > ---
> > > > > 
> > > > >  arch/arm/boot/dts/r8a7790.dtsi | 54 +++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 54 insertions(+)
> > > > 
> > > > Also here a couple of things are missing (presumably, for other SoCs you
> > > > need the same):
> > >
> > > Thank you for testing the patch set. I'll integrate the modification in
> > > the next version for r8a7778, r8a7779 and r8a7790.
> > > 
> > > As the gpio-ranges property specifies the number of GPIOs handled by the
> > > controller, what's your opinion on dropping the #gpio-lines property from
> > > the bindings ?
> > 
> > Well, gpio-ranges can contain several ranges, so, in a generic case you
> > would have to sum them up to get a total count. But if you're ok with that
> > or if your platforms only have 1 gpio gange per controller, of course,
> > that property seems redundant.
> 
> The gpio-rcar driver exposes a single range on all the current platforms. I 
> don't really foresee any change there in the future, but I might be 
> overlooking something. Magnus, any opinion ?
> 
> > OTOH it is already used in r8a7779.dtsi, and thus already belongs to the
> > ABI... Not sure whether removing it would be accepted.
> 
> It's only used in my DT bindings proposal that haven't been merged yet, so 
> that shouldn't be an issue :-)

Then yes, sure, you can add it any time in the future if needed, removing 
is more difficult ;-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
diff mbox

Patch

diff --git a/arch/arm/boot/dts/r8a7790.dtsi 
b/arch/arm/boot/dts/r8a7790.dtsi
index 674ee39..cd1a04f 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -51,6 +51,7 @@ 
 		interrupts = <0 4 0x4>;
 		#gpio-cells = <2>;
 		gpio-controller;
+		gpio-ranges = <&pfc 0 0 32>;
 	};
 
 	gpio1: gpio@ffc41000 {
@@ -60,6 +61,7 @@ 
 		interrupts = <0 5 0x4>;
 		#gpio-cells = <2>;
 		gpio-controller;
+		gpio-ranges = <&pfc 0 32 32>;
 	};
 
 	gpio2: gpio@ffc42000 {
@@ -69,6 +71,7 @@ 
 		interrupts = <0 6 0x4>;
 		#gpio-cells = <2>;
 		gpio-controller;
+		gpio-ranges = <&pfc 0 64 32>;
 	};
 
 	gpio3: gpio@ffc43000 {
@@ -78,6 +81,7 @@ 
 		interrupts = <0 7 0x4>;
 		#gpio-cells = <2>;
 		gpio-controller;
+		gpio-ranges = <&pfc 0 96 32>;
 	};
 
 	gpio4: gpio@ffc44000 {
@@ -87,6 +91,7 @@ 
 		interrupts = <0 8 0x4>;
 		#gpio-cells = <2>;
 		gpio-controller;
+		gpio-ranges = <&pfc 0 128 32>;
 	};
 
 	gpio5: gpio@ffc45000 {
@@ -96,6 +101,7 @@ 
 		interrupts = <0 9 0x4>;
 		#gpio-cells = <2>;
 		gpio-controller;
+		gpio-ranges = <&pfc 0 160 32>;
 	};
 
 	timer {
@@ -118,6 +124,7 @@ 
 	pfc: pfc@e6060000 {
 		compatible = "renesas,pfc-r8a7790";
 		reg = <0 0xe6060000 0 0x250>;
+		#gpio-range-cells = <3>;
 	};
 
 	/* No MMC_CAP_UHS_DDR50 (dual data rate) capability on r8a7790! */