Re: [PATCH] pinctrl: baytrail: explicitly set gpio chip base

[Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>]

On Thu, Apr 30, 2015 at 11:51:41PM +0200, Antonio Ospite wrote:
> Hi Mika,
> 
> On Thu, 30 Apr 2015 14:02:37 +0300
> Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> 
> > On Wed, Apr 29, 2015 at 11:02:42AM +0200, Antonio Ospite wrote:
> > > Having the gpio chip base set explicitly makes it easier to compare the
> > > GPIOs definitions with the ones found on some Android kernels.
> > > 
> > > Signed-off-by: Antonio Ospite <ao2@xxxxxx>
> > > Cc: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> > > Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > > Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > > Cc: linux-gpio@xxxxxxxxxxxxxxx
> > > ---
> > >  drivers/pinctrl/intel/pinctrl-baytrail.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c
> > > index 2062c22..4b2f594 100644
> > > --- a/drivers/pinctrl/intel/pinctrl-baytrail.c
> > > +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
> > > @@ -548,6 +548,7 @@ static int byt_gpio_probe(struct platform_device *pdev)
> > >  	struct acpi_device *acpi_dev;
> > >  	struct pinctrl_gpio_range *range;
> > >  	acpi_handle handle = ACPI_HANDLE(dev);
> > > +	int base_offset;
> > >  	int ret;
> > >  
> > >  	if (acpi_bus_get_device(handle, &acpi_dev))
> > > @@ -559,12 +560,14 @@ static int byt_gpio_probe(struct platform_device *pdev)
> > >  		return -ENOMEM;
> > >  	}
> > >  
> > > +	base_offset = 0;
> > >  	for (range = byt_ranges; range->name; range++) {
> > >  		if (!strcmp(acpi_dev->pnp.unique_id, range->name)) {
> > >  			vg->chip.ngpio = range->npins;
> > >  			vg->range = range;
> > >  			break;
> > >  		}
> > > +		base_offset += range->npins;
> > >  	}
> > >  
> > >  	if (!vg->chip.ngpio || !vg->range)
> > > @@ -590,7 +593,7 @@ static int byt_gpio_probe(struct platform_device *pdev)
> > >  	gc->get = byt_gpio_get;
> > >  	gc->set = byt_gpio_set;
> > >  	gc->dbg_show = byt_gpio_dbg_show;
> > > -	gc->base = -1;
> > > +	gc->base = base_offset;
> > 
> > But this changes the base from being dynamically allocated to always
> > start from 0, right?
> >
> 
> Yes, that's the point: to have exactly the same bases as in the Android
> kernels found in the wild, that is:
> 
>   $ ls -1 /sys/class/gpio
>   export
>   gpiochip0
>   gpiochip102
>   gpiochip130
>   unexport
> 
> This makes it easier for me to verify the mappings when trying to make
> my tablet work with the mainline kernel.
> 
> I might as well keep the change local, but I thought that others may
> find it useful.
> 
> Are there drawbacks of such fixed scheme?

Well, if you happen to have another GPIO chip (a GPIO expander for
example) and it somehow gets loaded before this driver. It may take the
range you have reserved for the BYT driver.

Not sure how realistic case that is, though...
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html