Patchwork powerpc: Specify GPIO number base for controller in DT

login
register
mail settings
Submitter Wolfgang Ocker
Date Oct. 23, 2008, 12:27 p.m.
Message ID <1224764850.4082.54.camel@galileo.recco.de>
Download mbox | patch
Permalink /patch/5452/
State Changes Requested
Headers show

Comments

Wolfgang Ocker - Oct. 23, 2008, 12:27 p.m.
The GPIOLIB allows the specification of a base gpio number for a
controller. That is not possible using OF. Instead, free gpio numbers
are assigned.

In order to allow static, predefined gpio numbers, a base property in
the gpio controller node specifies the first gpio number.

Signed-off-by: Wolfgang Ocker <weo@reccoware.de>
---
Kumar Gala - Oct. 23, 2008, 2:18 p.m.
On Oct 23, 2008, at 7:27 AM, Wolfgang Ocker wrote:

> The GPIOLIB allows the specification of a base gpio number for a
> controller. That is not possible using OF. Instead, free gpio numbers
> are assigned.
>
> In order to allow static, predefined gpio numbers, a base property in
> the gpio controller node specifies the first gpio number.
>
> Signed-off-by: Wolfgang Ocker <weo@reccoware.de>
> ---
>
> --- linux-2.6.27-rc7/drivers/of/gpio.c.of_gpiospecbase	2008-09-22  
> 00:29:55.000000000 +0200
> +++ linux-2.6.27-rc7/drivers/of/gpio.c	2008-09-29 13:50:28.000000000  
> +0200
> @@ -164,6 +164,8 @@
> 	int ret = -ENOMEM;
> 	struct of_gpio_chip *of_gc = &mm_gc->of_gc;
> 	struct gpio_chip *gc = &of_gc->gc;
> +	const int *basep;
> +	int baselen;
>
> 	gc->label = kstrdup(np->full_name, GFP_KERNEL);
> 	if (!gc->label)
> @@ -173,7 +175,11 @@
> 	if (!mm_gc->regs)
> 		goto err1;
>
> -	gc->base = -1;
> +	basep = of_get_property(np, "base", &baselen);
> +	if (!basep || baselen != sizeof(*basep))
> +		gc->base = -1;
> +	else
> +		gc->base = *basep;
>
> 	if (!of_gc->xlate)
> 		of_gc->xlate = of_gpio_simple_xlate;
> --- linux-2.6.27-rc7/Documentation/powerpc/booting-without- 
> of.txt.spi_gpio_doc	2008-09-29 14:14:08.000000000 +0200
> +++ linux-2.6.27-rc7/Documentation/powerpc/booting-without-of.txt	 
> 2008-09-29 14:24:26.000000000 +0200
> @@ -2586,6 +2588,7 @@
> 		#gpio-cells = <2>;
> 		compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank";
> 		reg = <0x1400 0x18>;
> +		base = < 0x20 >;
> 		gpio-controller;
> 	};

We need the define what base is somewhere

- k
Matt Sealey - Oct. 23, 2008, 9:35 p.m.
Kumar Gala wrote:
> 
> On Oct 23, 2008, at 7:27 AM, Wolfgang Ocker wrote:
> 
>> The GPIOLIB allows the specification of a base gpio number for a
>> controller. That is not possible using OF. Instead, free gpio numbers
>> are assigned.
>>
>> In order to allow static, predefined gpio numbers, a base property in
>> the gpio controller node specifies the first gpio number.

See my latest mail.

I don't think it's enough to say which pin the GPIOs exposed 
start at; you need some sort of mask, or array of applicable 
GPIOs so that GPIOLIB can check which perhaps 3 pins out of a 
possible 32 are allocated to a controller and usable (these 
may be pin 5, pin 9 and pin 20, so a "base" of pin 5 would be 
outrageously inadequate).
Wolfgang Ocker - Oct. 24, 2008, 4:10 p.m.
On Thu, 2008-10-23 at 16:35 -0500, Matt Sealey wrote:
> > On Oct 23, 2008, at 7:27 AM, Wolfgang Ocker wrote: 
> >> The GPIOLIB allows the specification of a base gpio number for a
> >> controller. That is not possible using OF. Instead, free gpio numbers
> >> are assigned.
> >>
> >> In order to allow static, predefined gpio numbers, a base property in
> >> the gpio controller node specifies the first gpio number.
> 
> See my latest mail.
> 
> I don't think it's enough to say which pin the GPIOs exposed 
> start at; you need some sort of mask, or array of applicable 
> GPIOs so that GPIOLIB can check which perhaps 3 pins out of a 
> possible 32 are allocated to a controller and usable (these 
> may be pin 5, pin 9 and pin 20, so a "base" of pin 5 would be 
> outrageously inadequate).

I'm not sure what you mean. That "base" is used in gpiochip_add() when
registering the gpio controller. There is no magic with a mask or so.

What do I miss?

Thanks,
Wolfgang
Anton Vorontsov - Oct. 24, 2008, 4:45 p.m.
On Thu, Oct 23, 2008 at 02:27:30PM +0200, Wolfgang Ocker wrote:
> The GPIOLIB allows the specification of a base gpio number for a
> controller. That is not possible using OF. Instead, free gpio numbers
> are assigned.
> 
> In order to allow static, predefined gpio numbers, a base property in
> the gpio controller node specifies the first gpio number.
> 
> Signed-off-by: Wolfgang Ocker <weo@reccoware.de>

[...]
> --- linux-2.6.27-rc7/Documentation/powerpc/booting-without-of.txt.spi_gpio_doc	2008-09-29 14:14:08.000000000 +0200
> +++ linux-2.6.27-rc7/Documentation/powerpc/booting-without-of.txt	2008-09-29 14:24:26.000000000 +0200
> @@ -2586,6 +2588,7 @@
>  		#gpio-cells = <2>;
>  		compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank";
>  		reg = <0x1400 0x18>;
> +		base = < 0x20 >;

The base has nothing to do with the hardware description, thus
device tree should not include it. Why exactly you need this?
I'm sure there is another way to solve the problem (whatever it
is).

Patch

--- linux-2.6.27-rc7/drivers/of/gpio.c.of_gpiospecbase	2008-09-22 00:29:55.000000000 +0200
+++ linux-2.6.27-rc7/drivers/of/gpio.c	2008-09-29 13:50:28.000000000 +0200
@@ -164,6 +164,8 @@ 
 	int ret = -ENOMEM;
 	struct of_gpio_chip *of_gc = &mm_gc->of_gc;
 	struct gpio_chip *gc = &of_gc->gc;
+	const int *basep;
+	int baselen;
 
 	gc->label = kstrdup(np->full_name, GFP_KERNEL);
 	if (!gc->label)
@@ -173,7 +175,11 @@ 
 	if (!mm_gc->regs)
 		goto err1;
 
-	gc->base = -1;
+	basep = of_get_property(np, "base", &baselen);
+	if (!basep || baselen != sizeof(*basep))
+		gc->base = -1;
+	else
+		gc->base = *basep;
 
 	if (!of_gc->xlate)
 		of_gc->xlate = of_gpio_simple_xlate;
--- linux-2.6.27-rc7/Documentation/powerpc/booting-without-of.txt.spi_gpio_doc	2008-09-29 14:14:08.000000000 +0200
+++ linux-2.6.27-rc7/Documentation/powerpc/booting-without-of.txt	2008-09-29 14:24:26.000000000 +0200
@@ -2586,6 +2588,7 @@ 
 		#gpio-cells = <2>;
 		compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank";
 		reg = <0x1400 0x18>;
+		base = < 0x20 >;
 		gpio-controller;
 	};