Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Mon, Nov 03, 2025 at 10:57:44AM +0100, Hans de Goede wrote:
> On 3-Nov-25 10:35 AM, Andy Shevchenko wrote:
> > On Sun, Nov 02, 2025 at 08:09:21PM +0100, Hans de Goede wrote:
> >> Since commit d24cfee7f63d ("spi: Fix acpi deferred irq probe"), the
> >> acpi_dev_gpio_irq_get() call gets delayed till spi_probe() is called
> >> on the SPI device.
> >>
> >> If there is no driver for the SPI device then the move to spi_probe()
> >> results in acpi_dev_gpio_irq_get() never getting called. This may
> >> cause problems by leaving the GPIO pin floating because this call is
> >> responsible for setting up the GPIO pin direction and/or bias according
> >> to the values from the ACPI tables.
> >>
> >> Re-add the removed acpi_dev_gpio_irq_get() in acpi_register_spi_device()
> >> to ensure the GPIO pin is always correctly setup, while keeping the
> >> acpi_dev_gpio_irq_get() call added to spi_probe() to deal with
> >> -EPROBE_DEFER returns caused by the GPIO controller not having a driver
> >> yet.
> > 
> > Even before following the link to some papering over module via the link below
> > I wondered, if the I²C case should be covered as well. The
> > https://github.com/alexpevzner/hotfix-kvadra-touchpad refers to I²C enabled
> > touchpads.
> > 
> >> Link: https://bbs.archlinux.org/viewtopic.php?id=302348

...

> > I'm not against the SPI fix, but is it confirmed that it really fixes the issue?
> 
> Yes Mark and I got an offlist email bisecting this to the:
> 
> Fixes: d24cfee7f63d ("spi: Fix acpi deferred irq probe")
> 
> commit (on a stable kernel series) and a later email confirming that this
> patch fixes it.

Shouldn't we use Closes in this case instead of Link?

> It seems that leaving the fingerprint reader enable pin (the first GPIO
> listed in _CRS which is an output only pin, is likely the enable pin)
> floating is causing these issues. So in a way the acpi_dev_gpio_irq_get()
> fixing this by forcing the enable pin to no longer float is a bit of
> luck. But things did work before d24cfee7f63d ("spi: Fix acpi deferred irq probe")
> so we need this to fix a regression

Yeah, fixing a regression is good thing, but not papering over the issue.

> and as you indicate it seems
> like a good idea in general and maybe we should also do this for i2c.

> As for doing something similar for I2C devices, that is an interesting
> question. Even though it is possible I'm not aware of any i2c-devices
> which have a userspace driver like SPI/USB fingerprint readers do,
> so on i2c I would expect probe() to always get called. So I'm not sure
> it is necessary there.

Reading the problem statement (the second paragraph) I lean towards
a generic solution residing somewhere in drivers/acpi/scan.c (like
acpi_init_device_object() / acpi_bus_attach() calls), although I don't
see a quick way how to achieve this. It seems would require a bit of
refactoring to allow ACPI glue code to call specific subsystem calls
or making a GPIOLIB to provide some "early" initialisation flow(s).


-- 
With Best Regards,
Andy Shevchenko