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

[Andy Shevchenko <andy@xxxxxxxxxx>]

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