Subject: Re: [PATCH 1/4] [POWERPC] Implement GPIO API embryo

From: David Gibson <[email-masked]>
Date: Sun, 23 Dec 2007 14:40:46 +1100

On Sun, Dec 23, 2007 at 03:49:30AM +0100, Segher Boessenkool wrote:
> > +int __of_parse_gpio_bank_pin(struct device_node *np, int index,
> > +			     int bank_width, int max_bank)
> > +{
> > +	int bank;
> > +	int pin;
> > +	const u32 *gpios;
> > +
> > +	/*
> > +	 * We can get there only if of_get_gpio() succeeded, thus
> > +	 * no need checking for "gpios" existence.
> > +	 */
> > +	gpios = of_get_property(np, "gpios", NULL);
> > +	bank = gpios[index * 2];
> > +	pin = gpios[index * 2 + 1];
> 
> If you stick with the #gpio-cells plan, here is where you should use it.

I think part of what's happening here is due to the patch's history.
The "bank pin" information was always of a format local to the
controller, but originally the size wasn't explicitly stated in the
tree - it was just implicit in the type of gpio controller.  I
suggested that #gpio-cells be added, which it has been, but it looks
like the code hasn't been updated to use it everywhere.  Obviously a
driver for a particular gpio controller would generally need to assert
that the controller's #gpio-cells has the correct value for this
controller type, after which code like the above would be acceptable.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson