Re: [PATCH] gpiolib: fix chip->base handling in of_gpio_simple_xlate()

[Arnd Bergmann <arnd@arndb.de>]

On Monday 23 July 2012, Daniel Mack wrote:
> (Cc: Arnd)
> 
> On 22.07.2012 19:10, Daniel Mack wrote:
> > of_gpio_simple_xlate() is called for each chip when a GPIO is looked up.
> > When registering several chips off the same DT node (with different pin
> > offsets) however, the lookup fails as the GPIO number passed in to
> > of_gpio_simple_xlate() is likely higher than the chip's ->ngpio value.
> > 
> > Fix that by taking into account the chip's ->base value, and return the
> > relative offset of the pin inside the chip.
> > 
> > Signed-off-by: Daniel Mack <zonque@gmail.com>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Linus Walleij <linus.walleij@stericsson.com>
> > ---
> > 
> > I'm currently porting the PXA pieces over to DT, and stumbled over what
> > looks like an obvious bug to me. Correct me if I'm mistaken, but I see
> > no reason why one shouldn't be able to instanciate several GPIO chips
> > from a single DT node.

But why would you do that? Both the "gpiochip" and its DT representation
attempt to represent the hardware structure. If they don't match, then
I'd assume one of them is wrong ;-)

> > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> > index d18068a..51bc232 100644
> > --- a/drivers/gpio/gpiolib-of.c
> > +++ b/drivers/gpio/gpiolib-of.c
> > @@ -147,13 +147,13 @@ int of_gpio_simple_xlate(struct gpio_chip *gc,
> >  	if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
> >  		return -EINVAL;
> >  
> > -	if (gpiospec->args[0] >= gc->ngpio)
> > +	if (gpiospec->args[0] >= gc->ngpio + gc->base)
> >  		return -EINVAL;
> >  
> >  	if (flags)
> >  		*flags = gpiospec->args[1];
> >  
> > -	return gpiospec->args[0];
> > +	return gpiospec->args[0] - gc->base;
> >  }
> >  EXPORT_SYMBOL(of_gpio_simple_xlate);

Where would that gc->base come from?

	Arnd