Hi, On 2/20/23 16:35, Andy Shevchenko wrote: > 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.) gpiolib-acpi.c already depends on CONFIG_ACPI and is not build when this is not set. So this does not add any new dependencies. IOW I don't see the problem here ? (also for this reason there is no stub for the new acpi_quirk_skip_gpio_event_handlers() helper) > Can we maybe move other quirks out from gpiolib-acpi.c to something like > PDx86 or another existing board files (with quirks)? I don't really see a clean way to move these. >>> 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. Ok will do. Regards, hans
