On Mon, Feb 20, 2023 at 04:23:33PM +0100, Hans de Goede wrote: > On 2/20/23 14:34, Andy Shevchenko wrote: > > On Sat, Feb 18, 2023 at 11:32:33AM +0100, Hans de Goede 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 . > > > > I'm wondering if we can teach acpi_gpio_in_ignore_list() to handle this. > > You mean have it call acpi_quirk_skip_gpio_event_handlers(), or you mean > extend the DMI matchs inside drivers/gpio/gpiolib-acpi.c to cover these > cases ? > > These devices with severely broken DSDTs already need a bunch of > other acpi handling quirks. So the idea is to re-use the existing > quirk mechanism for these to avoid having to have DMI match table > entries for a single model in various different places. I don't like growing amount of compile dependencies between these modules. (Yes, I'm aware about stubs.) Can we maybe move other quirks out from gpiolib-acpi.c to something like PDx86 or another existing board files (with quirks)? > > P.S. Why do we lock an IRQ before checking acpi_gpio_in_ignore_list() and > > why do we not free that if the IRQ is in ignore list? > > The idea was to do the test after other things which can fail, so that > if there are other reasons to skip the GPIO we don't do the test + > dev_xxx msg. But you are right, we should either unlock it when ignoring > it, or move the acpi_gpio_in_ignore_list() list check up. > > I guess just moving the check up is better, shall I prepare a patch for this? Yes, please. -- With Best Regards, Andy Shevchenko