diff mbox series

[RFT,11/21] platform: x86: android-tablets: don't access GPIOLIB private members

Message ID 20230905185309.131295-12-brgl@bgdev.pl (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series gpio: convert users to gpio_device_find() and remove gpiochip_find() | expand

Commit Message

Bartosz Golaszewski Sept. 5, 2023, 6:52 p.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We're slowly removing cases of abuse of the GPIOLIB public API. One of
the biggest issues is looking up and accessing struct gpio_chip whose
life-time is tied to the provider and which can disappear from under any
user at any given moment. We have provided new interfaces that use the
opaque struct gpio_device which is reference counted and will soon be
thorougly protected with appropriate locking.

Stop using old interfaces in this driver and switch to safer
alternatives.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 .../platform/x86/x86-android-tablets/core.c   | 38 ++++++++++---------
 1 file changed, 20 insertions(+), 18 deletions(-)

Comments

Hans de Goede Sept. 6, 2023, 1:01 p.m. UTC | #1
Hi Bartosz,

On 9/5/23 20:52, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> We're slowly removing cases of abuse of the GPIOLIB public API. One of
> the biggest issues is looking up and accessing struct gpio_chip whose
> life-time is tied to the provider and which can disappear from under any
> user at any given moment. We have provided new interfaces that use the
> opaque struct gpio_device which is reference counted and will soon be
> thorougly protected with appropriate locking.
> 
> Stop using old interfaces in this driver and switch to safer
> alternatives.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

First of all sorry for the issues this hack-ish kernel module
is causing for cleaning up gpiolib APIs.

I don't know how close a look you took at the code, so first of
all let me try to briefly explain what this hackish kernel module
is for:

There are some x86_64/ACPI tablets which shipped with Android as
factory OS. On these tablets the device-specific (BSP style)
kernel has things like the touchscreen driver simply having
a hardcoded I2C bus-number + I2C client address. Combined
with also hardcoded GPIO numbers (using the old number base APIs)
for any GPIOs it needs.

So the original Android kernels do not need the devices
to be properly described in ACPI and the ACPI tables are
just one big copy and paste job from some BSP which do
not accurately describe the hardware at all.

x86-android-tablets.ko identifies affected models by their
DMI strings and then manually instantiates things like
i2c-clients for the touchscreen, accelerometer and also
other stuff. Yes this is ugly but it allows mainline kernels
to run pretty well on these devices since other then
the messed up ACPI tables these are pretty standard x86/ACPI
tablets.

I hope this explains the hacks, now on to the problems
these are causing:

> ---
>  .../platform/x86/x86-android-tablets/core.c   | 38 ++++++++++---------
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
> index 2fd6060a31bb..687f84cd193c 100644
> --- a/drivers/platform/x86/x86-android-tablets/core.c
> +++ b/drivers/platform/x86/x86-android-tablets/core.c
> @@ -12,6 +12,7 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/gpio/machine.h>
>  #include <linux/irq.h>
> @@ -21,27 +22,28 @@
>  #include <linux/string.h>
>  
>  #include "x86-android-tablets.h"
> -/* For gpiochip_get_desc() which is EXPORT_SYMBOL_GPL() */
> -#include "../../../gpio/gpiolib.h"
> -#include "../../../gpio/gpiolib-acpi.h"
> -
> -static int gpiochip_find_match_label(struct gpio_chip *gc, void *data)
> -{
> -	return gc->label && !strcmp(gc->label, data);
> -}
>  
>  int x86_android_tablet_get_gpiod(const char *label, int pin, struct gpio_desc **desc)
>  {
> +	struct gpio_device *gdev;
>  	struct gpio_desc *gpiod;
> -	struct gpio_chip *chip;
>  
> -	chip = gpiochip_find((void *)label, gpiochip_find_match_label);
> -	if (!chip) {
> -		pr_err("error cannot find GPIO chip %s\n", label);
> +	/*
> +	 * FIXME: handle GPIOs correctly! This driver should really use struct
> +	 * device and GPIO lookup tables.
> +	 *
> +	 * WONTDO: We do leak this reference, but the whole approach to getting
> +	 * GPIOs in this driver is such an abuse of the GPIOLIB API that it
> +	 * doesn't make it much worse and it's the only way to keep the
> +	 * interrupt requested later functional...
> +	 */
> +	gdev = gpio_device_find_by_label(label);
> +	if (!gdev) {
> +		pr_err("error cannot find GPIO device %s\n", label);
>  		return -ENODEV;
>  	}
>  
> -	gpiod = gpiochip_get_desc(chip, pin);
> +	gpiod = gpio_device_get_desc(gdev, pin);
>  	if (IS_ERR(gpiod)) {
>  		pr_err("error %ld getting GPIO %s %d\n", PTR_ERR(gpiod), label, pin);
>  		return PTR_ERR(gpiod);


So rather then the above I think what needs to happen here
(and I can hopefully make some time for that this weekend) is:

1. Have the x86-android-tablets code instantiate a
   "x86-android-tablets" platform-dev
2. Have the code generate a gpiod_lookup_table for all GPIOs
   for which it currently uses x86_android_tablet_get_gpiod()
   with the .dev_id set to "x86-android-tablets"
3. Use regular gpiod_get() on the "x86-android-tablets" pdev
   to get the desc.

I think this should solve all the issues with 
x86_android_tablet_get_gpiod() poking inside
gpiolib external since now it is only using
public gpiolib APIs, right ?

One question about 2. there are 2 ways to do this:

i. Have the module_init() function loop over all
x86_dev_info members which will result in calling
x86_android_tablet_get_gpiod() and have it generate
one big gpiod_lookup_table for all GPIOs needed
in one go. At which point x86_android_tablet_get_gpiod()
goes away and can be directly replaced with gpiod_get()
calls on the pdev.

ii. Keep x86_android_tablet_get_gpiod() and have it
generate a gpiod_lookup_table with just 1 entry for
the GPIO which its caller wants. Register the lookup
table, do the gpiod_get() and then immediately
unregister the lookup table again.

ii. Would be easier for me to implement, especially
since there is also some custom (board specific)
init code calling x86_android_tablet_get_gpiod()
which would require some special handling for i.

OTOH I guess some people will consider ii. somewhat
ugly, although AFAICT it is perfectly ok to use
the gpiolib lookup APIs this way.

Can you please let me known if you are ok with ii,
or if you would prefer me going with solution i. ?

That way when I can make some time to start working
on this I can pick the preferred solution right away.



> @@ -257,9 +259,9 @@ static void x86_android_tablet_cleanup(void)
>  
>  static __init int x86_android_tablet_init(void)
>  {
> +	struct gpio_device *gdev __free(gpio_device_put) = NULL;
>  	const struct x86_dev_info *dev_info;
>  	const struct dmi_system_id *id;
> -	struct gpio_chip *chip;
>  	int i, ret = 0;
>  
>  	id = dmi_first_match(x86_android_tablet_ids);
> @@ -273,13 +275,13 @@ static __init int x86_android_tablet_init(void)
>  	 * _AEI (ACPI Event Interrupt) handlers, disable these.
>  	 */
>  	if (dev_info->invalid_aei_gpiochip) {
> -		chip = gpiochip_find(dev_info->invalid_aei_gpiochip,
> -				     gpiochip_find_match_label);
> -		if (!chip) {
> +		gdev = gpio_device_find_by_label(
> +				dev_info->invalid_aei_gpiochip);
> +		if (!gdev) {
>  			pr_err("error cannot find GPIO chip %s\n", dev_info->invalid_aei_gpiochip);
>  			return -ENODEV;
>  		}
> -		acpi_gpiochip_free_interrupts(chip);
> +		acpi_gpio_device_free_interrupts(gdev);
>  	}
>  
>  	/*

After some recent improvements there is only 1 board left which sets
dev_info->invalid_aei_gpiochip and that can easily be replaced with
with adding 1 extra entry to gpiolib_acpi_quirks[] inside
drivers/gpio/gpiolib-acpi.c .

So I believe the right solution here is to just remove
dev_info->invalid_aei_gpiochip support for x86-android-tablets
all together and then at least x86-android-tablets will no
longer be making any hackish acpi_gpiochip_free_interrupts() calls.

I don't want to make any promises wrt the timing, but I should
be able to prepare a set of patches which simply removes all
the private gpiolib API use from x86-android-tablets, so that
you don't need to workaround that in this patch series.

With some luck I can have an immutable branch with 6.6-rc1 +
such a patch-series ready for you soon after 6.6-rc1 is
released.

Regards,

Hans
Bartosz Golaszewski Sept. 6, 2023, 2:27 p.m. UTC | #2
On Wed, Sep 6, 2023 at 3:01 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Bartosz,
>
> On 9/5/23 20:52, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > We're slowly removing cases of abuse of the GPIOLIB public API. One of
> > the biggest issues is looking up and accessing struct gpio_chip whose
> > life-time is tied to the provider and which can disappear from under any
> > user at any given moment. We have provided new interfaces that use the
> > opaque struct gpio_device which is reference counted and will soon be
> > thorougly protected with appropriate locking.
> >
> > Stop using old interfaces in this driver and switch to safer
> > alternatives.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> First of all sorry for the issues this hack-ish kernel module
> is causing for cleaning up gpiolib APIs.
>
> I don't know how close a look you took at the code, so first of
> all let me try to briefly explain what this hackish kernel module
> is for:
>
> There are some x86_64/ACPI tablets which shipped with Android as
> factory OS. On these tablets the device-specific (BSP style)
> kernel has things like the touchscreen driver simply having
> a hardcoded I2C bus-number + I2C client address. Combined
> with also hardcoded GPIO numbers (using the old number base APIs)
> for any GPIOs it needs.
>
> So the original Android kernels do not need the devices
> to be properly described in ACPI and the ACPI tables are
> just one big copy and paste job from some BSP which do
> not accurately describe the hardware at all.
>
> x86-android-tablets.ko identifies affected models by their
> DMI strings and then manually instantiates things like
> i2c-clients for the touchscreen, accelerometer and also
> other stuff. Yes this is ugly but it allows mainline kernels
> to run pretty well on these devices since other then
> the messed up ACPI tables these are pretty standard x86/ACPI
> tablets.
>
> I hope this explains the hacks, now on to the problems
> these are causing:

This makes sense! Maybe we'd need a good-old board file setting up all
non-described devices using the driver model?

>
> > ---
> >  .../platform/x86/x86-android-tablets/core.c   | 38 ++++++++++---------
> >  1 file changed, 20 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
> > index 2fd6060a31bb..687f84cd193c 100644
> > --- a/drivers/platform/x86/x86-android-tablets/core.c
> > +++ b/drivers/platform/x86/x86-android-tablets/core.c
> > @@ -12,6 +12,7 @@
> >
> >  #include <linux/acpi.h>
> >  #include <linux/dmi.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/gpio/driver.h>
> >  #include <linux/gpio/machine.h>
> >  #include <linux/irq.h>
> > @@ -21,27 +22,28 @@
> >  #include <linux/string.h>
> >
> >  #include "x86-android-tablets.h"
> > -/* For gpiochip_get_desc() which is EXPORT_SYMBOL_GPL() */
> > -#include "../../../gpio/gpiolib.h"
> > -#include "../../../gpio/gpiolib-acpi.h"
> > -
> > -static int gpiochip_find_match_label(struct gpio_chip *gc, void *data)
> > -{
> > -     return gc->label && !strcmp(gc->label, data);
> > -}
> >
> >  int x86_android_tablet_get_gpiod(const char *label, int pin, struct gpio_desc **desc)
> >  {
> > +     struct gpio_device *gdev;
> >       struct gpio_desc *gpiod;
> > -     struct gpio_chip *chip;
> >
> > -     chip = gpiochip_find((void *)label, gpiochip_find_match_label);
> > -     if (!chip) {
> > -             pr_err("error cannot find GPIO chip %s\n", label);
> > +     /*
> > +      * FIXME: handle GPIOs correctly! This driver should really use struct
> > +      * device and GPIO lookup tables.
> > +      *
> > +      * WONTDO: We do leak this reference, but the whole approach to getting
> > +      * GPIOs in this driver is such an abuse of the GPIOLIB API that it
> > +      * doesn't make it much worse and it's the only way to keep the
> > +      * interrupt requested later functional...
> > +      */
> > +     gdev = gpio_device_find_by_label(label);
> > +     if (!gdev) {
> > +             pr_err("error cannot find GPIO device %s\n", label);
> >               return -ENODEV;
> >       }
> >
> > -     gpiod = gpiochip_get_desc(chip, pin);
> > +     gpiod = gpio_device_get_desc(gdev, pin);
> >       if (IS_ERR(gpiod)) {
> >               pr_err("error %ld getting GPIO %s %d\n", PTR_ERR(gpiod), label, pin);
> >               return PTR_ERR(gpiod);
>
>
> So rather then the above I think what needs to happen here
> (and I can hopefully make some time for that this weekend) is:
>
> 1. Have the x86-android-tablets code instantiate a
>    "x86-android-tablets" platform-dev
> 2. Have the code generate a gpiod_lookup_table for all GPIOs
>    for which it currently uses x86_android_tablet_get_gpiod()
>    with the .dev_id set to "x86-android-tablets"
> 3. Use regular gpiod_get() on the "x86-android-tablets" pdev
>    to get the desc.
>
> I think this should solve all the issues with
> x86_android_tablet_get_gpiod() poking inside
> gpiolib external since now it is only using
> public gpiolib APIs, right ?
>
> One question about 2. there are 2 ways to do this:
>
> i. Have the module_init() function loop over all
> x86_dev_info members which will result in calling
> x86_android_tablet_get_gpiod() and have it generate
> one big gpiod_lookup_table for all GPIOs needed
> in one go. At which point x86_android_tablet_get_gpiod()
> goes away and can be directly replaced with gpiod_get()
> calls on the pdev.
>
> ii. Keep x86_android_tablet_get_gpiod() and have it
> generate a gpiod_lookup_table with just 1 entry for
> the GPIO which its caller wants. Register the lookup
> table, do the gpiod_get() and then immediately
> unregister the lookup table again.
>
> ii. Would be easier for me to implement, especially
> since there is also some custom (board specific)
> init code calling x86_android_tablet_get_gpiod()
> which would require some special handling for i.
>
> OTOH I guess some people will consider ii. somewhat
> ugly, although AFAICT it is perfectly ok to use
> the gpiolib lookup APIs this way.
>
> Can you please let me known if you are ok with ii,
> or if you would prefer me going with solution i. ?
>

I am fine with ii. I have recently sent a patch that does exactly that
in one of the SPI drivers. It's ugly but it's better than what we have
now.

> That way when I can make some time to start working
> on this I can pick the preferred solution right away.
>
>
>
> > @@ -257,9 +259,9 @@ static void x86_android_tablet_cleanup(void)
> >
> >  static __init int x86_android_tablet_init(void)
> >  {
> > +     struct gpio_device *gdev __free(gpio_device_put) = NULL;
> >       const struct x86_dev_info *dev_info;
> >       const struct dmi_system_id *id;
> > -     struct gpio_chip *chip;
> >       int i, ret = 0;
> >
> >       id = dmi_first_match(x86_android_tablet_ids);
> > @@ -273,13 +275,13 @@ static __init int x86_android_tablet_init(void)
> >        * _AEI (ACPI Event Interrupt) handlers, disable these.
> >        */
> >       if (dev_info->invalid_aei_gpiochip) {
> > -             chip = gpiochip_find(dev_info->invalid_aei_gpiochip,
> > -                                  gpiochip_find_match_label);
> > -             if (!chip) {
> > +             gdev = gpio_device_find_by_label(
> > +                             dev_info->invalid_aei_gpiochip);
> > +             if (!gdev) {
> >                       pr_err("error cannot find GPIO chip %s\n", dev_info->invalid_aei_gpiochip);
> >                       return -ENODEV;
> >               }
> > -             acpi_gpiochip_free_interrupts(chip);
> > +             acpi_gpio_device_free_interrupts(gdev);
> >       }
> >
> >       /*
>
> After some recent improvements there is only 1 board left which sets
> dev_info->invalid_aei_gpiochip and that can easily be replaced with
> with adding 1 extra entry to gpiolib_acpi_quirks[] inside
> drivers/gpio/gpiolib-acpi.c .
>
> So I believe the right solution here is to just remove
> dev_info->invalid_aei_gpiochip support for x86-android-tablets
> all together and then at least x86-android-tablets will no
> longer be making any hackish acpi_gpiochip_free_interrupts() calls.
>
> I don't want to make any promises wrt the timing, but I should
> be able to prepare a set of patches which simply removes all
> the private gpiolib API use from x86-android-tablets, so that
> you don't need to workaround that in this patch series.
>
> With some luck I can have an immutable branch with 6.6-rc1 +
> such a patch-series ready for you soon after 6.6-rc1 is
> released.
>

That would be awesome, thanks a lot!

> Regards,
>
> Hans
>
>
>

Bart

[1] https://lore.kernel.org/lkml/d57a99ce-77eb-409f-8371-95f2658fa0c0@sirena.org.uk/T/
Hans de Goede Sept. 9, 2023, 2:17 p.m. UTC | #3
Hi Bart,

On 9/6/23 16:27, Bartosz Golaszewski wrote:
> On Wed, Sep 6, 2023 at 3:01 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Bartosz,
>>
>> On 9/5/23 20:52, Bartosz Golaszewski wrote:
>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> We're slowly removing cases of abuse of the GPIOLIB public API. One of
>>> the biggest issues is looking up and accessing struct gpio_chip whose
>>> life-time is tied to the provider and which can disappear from under any
>>> user at any given moment. We have provided new interfaces that use the
>>> opaque struct gpio_device which is reference counted and will soon be
>>> thorougly protected with appropriate locking.
>>>
>>> Stop using old interfaces in this driver and switch to safer
>>> alternatives.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> First of all sorry for the issues this hack-ish kernel module
>> is causing for cleaning up gpiolib APIs.
>>
>> I don't know how close a look you took at the code, so first of
>> all let me try to briefly explain what this hackish kernel module
>> is for:
>>
>> There are some x86_64/ACPI tablets which shipped with Android as
>> factory OS. On these tablets the device-specific (BSP style)
>> kernel has things like the touchscreen driver simply having
>> a hardcoded I2C bus-number + I2C client address. Combined
>> with also hardcoded GPIO numbers (using the old number base APIs)
>> for any GPIOs it needs.
>>
>> So the original Android kernels do not need the devices
>> to be properly described in ACPI and the ACPI tables are
>> just one big copy and paste job from some BSP which do
>> not accurately describe the hardware at all.
>>
>> x86-android-tablets.ko identifies affected models by their
>> DMI strings and then manually instantiates things like
>> i2c-clients for the touchscreen, accelerometer and also
>> other stuff. Yes this is ugly but it allows mainline kernels
>> to run pretty well on these devices since other then
>> the messed up ACPI tables these are pretty standard x86/ACPI
>> tablets.
>>
>> I hope this explains the hacks, now on to the problems
>> these are causing:
> 
> This makes sense! Maybe we'd need a good-old board file setting up all
> non-described devices using the driver model?

Right, this is pretty much exactly what the x86-android-tablets
code does. Except that it does it for a bunch of boards in a single
.ko / driver. There is a lot of commonality between these boards,
so this allows sharing most of the code.

The driver uses DMI matching, with the match's driver_data pointing
to a description of which devices to instantiate and then the shared
code takes care of instantiating those.

About 90% of the data / code is __init or __initdata so both
the code to instantiate the devices as well as the per board
data is free-ed after module_init() has run.

<snip>

>> So rather then the above I think what needs to happen here
>> (and I can hopefully make some time for that this weekend) is:
>>
>> 1. Have the x86-android-tablets code instantiate a
>>    "x86-android-tablets" platform-dev
>> 2. Have the code generate a gpiod_lookup_table for all GPIOs
>>    for which it currently uses x86_android_tablet_get_gpiod()
>>    with the .dev_id set to "x86-android-tablets"
>> 3. Use regular gpiod_get() on the "x86-android-tablets" pdev
>>    to get the desc.
>>
>> I think this should solve all the issues with
>> x86_android_tablet_get_gpiod() poking inside
>> gpiolib external since now it is only using
>> public gpiolib APIs, right ?
>>
>> One question about 2. there are 2 ways to do this:
>>
>> i. Have the module_init() function loop over all
>> x86_dev_info members which will result in calling
>> x86_android_tablet_get_gpiod() and have it generate
>> one big gpiod_lookup_table for all GPIOs needed
>> in one go. At which point x86_android_tablet_get_gpiod()
>> goes away and can be directly replaced with gpiod_get()
>> calls on the pdev.
>>
>> ii. Keep x86_android_tablet_get_gpiod() and have it
>> generate a gpiod_lookup_table with just 1 entry for
>> the GPIO which its caller wants. Register the lookup
>> table, do the gpiod_get() and then immediately
>> unregister the lookup table again.
>>
>> ii. Would be easier for me to implement, especially
>> since there is also some custom (board specific)
>> init code calling x86_android_tablet_get_gpiod()
>> which would require some special handling for i.
>>
>> OTOH I guess some people will consider ii. somewhat
>> ugly, although AFAICT it is perfectly ok to use
>> the gpiolib lookup APIs this way.
>>
>> Can you please let me known if you are ok with ii,
>> or if you would prefer me going with solution i. ?
>>
> 
> I am fine with ii. I have recently sent a patch that does exactly that
> in one of the SPI drivers. It's ugly but it's better than what we have
> now.

Ok, I have just finished implementing this using the ii. method.

I'll post a patch-series for this for review right after this email.

After that series x86-android-tablets should no longer be a problem
wrt using any private gpiolib APIs.

Regards,

Hans
Andy Shevchenko Sept. 11, 2023, 10:05 a.m. UTC | #4
On Sat, Sep 09, 2023 at 04:17:53PM +0200, Hans de Goede wrote:
> On 9/6/23 16:27, Bartosz Golaszewski wrote:
> > On Wed, Sep 6, 2023 at 3:01 PM Hans de Goede <hdegoede@redhat.com> wrote:

...

> > This makes sense! Maybe we'd need a good-old board file setting up all
> > non-described devices using the driver model?
> 
> Right, this is pretty much exactly what the x86-android-tablets
> code does. Except that it does it for a bunch of boards in a single
> .ko / driver. There is a lot of commonality between these boards,
> so this allows sharing most of the code.
> 
> The driver uses DMI matching, with the match's driver_data pointing
> to a description of which devices to instantiate and then the shared
> code takes care of instantiating those.
> 
> About 90% of the data / code is __init or __initdata so both
> the code to instantiate the devices as well as the per board
> data is free-ed after module_init() has run.

...which is nicely looked and isolated hack (or quirk if you prefer)
that I like! Thanks, Hans, for maintaining that!
diff mbox series

Patch

diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index 2fd6060a31bb..687f84cd193c 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -12,6 +12,7 @@ 
 
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+#include <linux/gpio/consumer.h>
 #include <linux/gpio/driver.h>
 #include <linux/gpio/machine.h>
 #include <linux/irq.h>
@@ -21,27 +22,28 @@ 
 #include <linux/string.h>
 
 #include "x86-android-tablets.h"
-/* For gpiochip_get_desc() which is EXPORT_SYMBOL_GPL() */
-#include "../../../gpio/gpiolib.h"
-#include "../../../gpio/gpiolib-acpi.h"
-
-static int gpiochip_find_match_label(struct gpio_chip *gc, void *data)
-{
-	return gc->label && !strcmp(gc->label, data);
-}
 
 int x86_android_tablet_get_gpiod(const char *label, int pin, struct gpio_desc **desc)
 {
+	struct gpio_device *gdev;
 	struct gpio_desc *gpiod;
-	struct gpio_chip *chip;
 
-	chip = gpiochip_find((void *)label, gpiochip_find_match_label);
-	if (!chip) {
-		pr_err("error cannot find GPIO chip %s\n", label);
+	/*
+	 * FIXME: handle GPIOs correctly! This driver should really use struct
+	 * device and GPIO lookup tables.
+	 *
+	 * WONTDO: We do leak this reference, but the whole approach to getting
+	 * GPIOs in this driver is such an abuse of the GPIOLIB API that it
+	 * doesn't make it much worse and it's the only way to keep the
+	 * interrupt requested later functional...
+	 */
+	gdev = gpio_device_find_by_label(label);
+	if (!gdev) {
+		pr_err("error cannot find GPIO device %s\n", label);
 		return -ENODEV;
 	}
 
-	gpiod = gpiochip_get_desc(chip, pin);
+	gpiod = gpio_device_get_desc(gdev, pin);
 	if (IS_ERR(gpiod)) {
 		pr_err("error %ld getting GPIO %s %d\n", PTR_ERR(gpiod), label, pin);
 		return PTR_ERR(gpiod);
@@ -257,9 +259,9 @@  static void x86_android_tablet_cleanup(void)
 
 static __init int x86_android_tablet_init(void)
 {
+	struct gpio_device *gdev __free(gpio_device_put) = NULL;
 	const struct x86_dev_info *dev_info;
 	const struct dmi_system_id *id;
-	struct gpio_chip *chip;
 	int i, ret = 0;
 
 	id = dmi_first_match(x86_android_tablet_ids);
@@ -273,13 +275,13 @@  static __init int x86_android_tablet_init(void)
 	 * _AEI (ACPI Event Interrupt) handlers, disable these.
 	 */
 	if (dev_info->invalid_aei_gpiochip) {
-		chip = gpiochip_find(dev_info->invalid_aei_gpiochip,
-				     gpiochip_find_match_label);
-		if (!chip) {
+		gdev = gpio_device_find_by_label(
+				dev_info->invalid_aei_gpiochip);
+		if (!gdev) {
 			pr_err("error cannot find GPIO chip %s\n", dev_info->invalid_aei_gpiochip);
 			return -ENODEV;
 		}
-		acpi_gpiochip_free_interrupts(chip);
+		acpi_gpio_device_free_interrupts(gdev);
 	}
 
 	/*