On Wed, Mar 1, 2023 at 12:04 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxx>
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
