Re: [PATCH v2 1/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Wed, Mar 1, 2023 at 12:04 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> x86 ACPI boards which ship with only Android as their factory image usually
> have pretty broken ACPI tables, relying on everything being hardcoded in
> the factory kernel image and often disabling parts of the ACPI enumeration
> kernel code to avoid the broken tables causing issues.
>
> Part of this broken ACPI code is that sometimes these boards have _AEI
> ACPI GPIO event handlers which are broken.
>
> So far this has been dealt with in the platform/x86/x86-android-tablets.c
> module, which contains various workarounds for these devices, by it calling
> acpi_gpiochip_free_interrupts() on gpiochip-s with troublesome handlers to
> disable the handlers.
>
> But in some cases this is too late, if the handlers are of the edge type
> then gpiolib-acpi.c's code will already have run them at boot.
> This can cause issues such as GPIOs ending up as owned by "ACPI:OpRegion",
> making them unavailable for drivers which actually need them.
>
> Boards with these broken ACPI tables are already listed in
> drivers/acpi/x86/utils.c for e.g. acpi_quirk_skip_i2c_client_enumeration().
> Extend the quirks mechanism for a new acpi_quirk_skip_gpio_event_handlers()
> helper, this re-uses the DMI-ids rather then having to duplicate the same
> DMI table in gpiolib-acpi.c .
>
> Also add the new ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS quirk to existing
> boards with troublesome ACPI gpio event handlers, so that the current
> acpi_gpiochip_free_interrupts() hack can be removed from
> x86-android-tablets.c .
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/x86/utils.c    | 24 +++++++++++++++++++++---
>  drivers/gpio/gpiolib-acpi.c |  3 +++
>  include/acpi/acpi_bus.h     |  5 +++++
>  3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
> index 4e816bb402f6..4a6f3a6726d0 100644
> --- a/drivers/acpi/x86/utils.c
> +++ b/drivers/acpi/x86/utils.c
> @@ -262,6 +262,7 @@ bool force_storage_d3(void)
>  #define ACPI_QUIRK_UART1_TTY_UART2_SKIP                                BIT(1)
>  #define ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY                    BIT(2)
>  #define ACPI_QUIRK_USE_ACPI_AC_AND_BATTERY                     BIT(3)
> +#define ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS                    BIT(4)
>
>  static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
>         /*
> @@ -297,7 +298,8 @@ static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
>                 },
>                 .driver_data = (void *)(ACPI_QUIRK_SKIP_I2C_CLIENTS |
>                                         ACPI_QUIRK_UART1_TTY_UART2_SKIP |
> -                                       ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY),
> +                                       ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY |
> +                                       ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS),
>         },
>         {
>                 .matches = {
> @@ -305,7 +307,8 @@ static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "TF103C"),
>                 },
>                 .driver_data = (void *)(ACPI_QUIRK_SKIP_I2C_CLIENTS |
> -                                       ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY),
> +                                       ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY |
> +                                       ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS),
>         },
>         {
>                 /* Lenovo Yoga Tablet 2 1050F/L */
> @@ -347,7 +350,8 @@ static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "M890BAP"),
>                 },
>                 .driver_data = (void *)(ACPI_QUIRK_SKIP_I2C_CLIENTS |
> -                                       ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY),
> +                                       ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY |
> +                                       ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS),
>         },
>         {
>                 /* Whitelabel (sold as various brands) TM800A550L */
> @@ -424,6 +428,20 @@ int acpi_quirk_skip_serdev_enumeration(struct device *controller_parent, bool *s
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(acpi_quirk_skip_serdev_enumeration);
> +
> +bool acpi_quirk_skip_gpio_event_handlers(void)
> +{
> +       const struct dmi_system_id *dmi_id;
> +       long quirks;
> +
> +       dmi_id = dmi_first_match(acpi_quirk_skip_dmi_ids);
> +       if (!dmi_id)
> +               return false;
> +
> +       quirks = (unsigned long)dmi_id->driver_data;
> +       return (quirks & ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS);
> +}
> +EXPORT_SYMBOL_GPL(acpi_quirk_skip_gpio_event_handlers);
>  #endif
>
>  /* Lists of PMIC ACPI HIDs with an (often better) native charger driver */
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 34ff048e70d0..7c9175619a1d 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -536,6 +536,9 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
>         if (ACPI_FAILURE(status))
>                 return;
>
> +       if (acpi_quirk_skip_gpio_event_handlers())
> +               return;

Not that I'm a fan of this, but at least it's not so ugly and invasive,
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
for GPIO ACPI library part.

>         acpi_walk_resources(handle, METHOD_NAME__AEI,
>                             acpi_gpiochip_alloc_event, acpi_gpio);
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index e44be31115a6..d69545cd6a48 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -656,6 +656,7 @@ static inline bool acpi_quirk_skip_acpi_ac_and_battery(void)
>  #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS)
>  bool acpi_quirk_skip_i2c_client_enumeration(struct acpi_device *adev);
>  int acpi_quirk_skip_serdev_enumeration(struct device *controller_parent, bool *skip);
> +bool acpi_quirk_skip_gpio_event_handlers(void);
>  #else
>  static inline bool acpi_quirk_skip_i2c_client_enumeration(struct acpi_device *adev)
>  {
> @@ -667,6 +668,10 @@ acpi_quirk_skip_serdev_enumeration(struct device *controller_parent, bool *skip)
>         *skip = false;
>         return 0;
>  }
> +static inline bool acpi_quirk_skip_gpio_event_handlers(void)
> +{
> +       return false;
> +}
>  #endif
>
>  #ifdef CONFIG_PM
> --
> 2.39.1
>


-- 
With Best Regards,
Andy Shevchenko