* [PATCH] spi: Try to get ACPI GPIO IRQ earlier
@ 2025-11-02 19:09 Hans de Goede
2025-11-03 9:35 ` Andy Shevchenko
2025-11-06 11:34 ` Mark Brown
0 siblings, 2 replies; 11+ messages in thread
From: Hans de Goede @ 2025-11-02 19:09 UTC (permalink / raw)
To: Mark Brown; +Cc: Hans de Goede, Andy Shevchenko, linux-spi, linux-acpi, stable
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.
Link: https://bbs.archlinux.org/viewtopic.php?id=302348
Fixes: d24cfee7f63d ("spi: Fix acpi deferred irq probe")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hansg@kernel.org>
---
drivers/spi/spi.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 2e0647a06890..8588e8562220 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2851,6 +2851,16 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,
acpi_set_modalias(adev, acpi_device_hid(adev), spi->modalias,
sizeof(spi->modalias));
+ /*
+ * This gets re-tried in spi_probe() for -EPROBE_DEFER handling in case
+ * the GPIO controller does not have a driver yet. This needs to be done
+ * here too, because this call sets the GPIO direction and/or bias.
+ * Setting these needs to be done even if there is no driver, in which
+ * case spi_probe() will never get called.
+ */
+ if (spi->irq < 0)
+ spi->irq = acpi_dev_gpio_irq_get(adev, 0);
+
acpi_device_set_enumerated(adev);
adev->power.flags.ignore_parent = true;
--
2.51.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-02 19:09 [PATCH] spi: Try to get ACPI GPIO IRQ earlier Hans de Goede
@ 2025-11-03 9:35 ` Andy Shevchenko
2025-11-03 9:57 ` Hans de Goede
2025-11-06 11:34 ` Mark Brown
1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2025-11-03 9:35 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mark Brown, Andy Shevchenko, linux-spi, linux-acpi, stable
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?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-03 9:35 ` Andy Shevchenko
@ 2025-11-03 9:57 ` Hans de Goede
2025-11-03 10:13 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2025-11-03 9:57 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mark Brown, Andy Shevchenko, linux-spi, linux-acpi, stable
Hi,
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.
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 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.
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-03 9:57 ` Hans de Goede
@ 2025-11-03 10:13 ` Andy Shevchenko
2025-11-03 10:20 ` Hans de Goede
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2025-11-03 10:13 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mark Brown, Andy Shevchenko, linux-spi, linux-acpi, stable
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-03 10:13 ` Andy Shevchenko
@ 2025-11-03 10:20 ` Hans de Goede
2025-11-03 13:15 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2025-11-03 10:20 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mark Brown, Andy Shevchenko, linux-spi, linux-acpi, stable
Hi,
On 3-Nov-25 11:13 AM, Andy Shevchenko wrote:
> 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?
I guess so.
>> 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.
I agree in principle, but this is a quick and safe way to fix
the regression, where as the generic fix you describe below is
likely months away and also has significant risks of causing
regressions in various places, see below.
>> 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).
I guess that you want to do the direction and bias init on all
GPIOs listed in _CRS, at least for devices with status == present ?
I was wondering about the same thing, but ACPI tables are full
of, well, erm, garbage in various places so I'm afraid that doing
this for all GPIO _CRS resources is likely to cause a whole lot
of pain.
Typically the firmware already sets up the direction + bias
of all used pins. I'm pretty sure the BIOS-es have some GPIO
init table especially for this somewhere.
Now those init-tables may have bugs, but I'm seriously worried
about the implication of doing the direction + bios setup for
all _CRS GPIO entries. I have simply seen too much non sense
devices (with _STA returning 0x0f) listing GPIOs also actually
used elsewhere to think this is a good idea.
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-03 10:20 ` Hans de Goede
@ 2025-11-03 13:15 ` Andy Shevchenko
2025-11-06 10:16 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2025-11-03 13:15 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mark Brown, Andy Shevchenko, linux-spi, linux-acpi, stable
On Mon, Nov 03, 2025 at 11:20:30AM +0100, Hans de Goede wrote:
> On 3-Nov-25 11:13 AM, Andy Shevchenko wrote:
> > 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?
>
> I guess so.
>
> >> 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.
>
> I agree in principle, but this is a quick and safe way to fix
> the regression, where as the generic fix you describe below is
> likely months away and also has significant risks of causing
> regressions in various places, see below.
Perhaps we should add a TODO / FIXME there as well?
So at least we will know that this is not a proper solution.
> >> 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).
>
> I guess that you want to do the direction and bias init on all
> GPIOs listed in _CRS, at least for devices with status == present ?
For the devices that are serial busses only (I²C, SPI, UART).
> I was wondering about the same thing, but ACPI tables are full
> of, well, erm, garbage in various places so I'm afraid that doing
> this for all GPIO _CRS resources is likely to cause a whole lot
> of pain.
>
> Typically the firmware already sets up the direction + bias
> of all used pins. I'm pretty sure the BIOS-es have some GPIO
> init table especially for this somewhere.
>
> Now those init-tables may have bugs, but I'm seriously worried
> about the implication of doing the direction + bios setup for
> all _CRS GPIO entries. I have simply seen too much non sense
> devices (with _STA returning 0x0f) listing GPIOs also actually
> used elsewhere to think this is a good idea.
Btw, other GPIO initialisation issues we have been solving by adding quirks.
Why this one can't be targeted with the same approach?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-03 13:15 ` Andy Shevchenko
@ 2025-11-06 10:16 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-11-06 10:16 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mark Brown, Andy Shevchenko, linux-spi, linux-acpi, stable
On Mon, Nov 03, 2025 at 03:15:15PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 03, 2025 at 11:20:30AM +0100, Hans de Goede wrote:
> > On 3-Nov-25 11:13 AM, Andy Shevchenko wrote:
> > > 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?
> >
> > I guess so.
> >
> > >> 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.
> >
> > I agree in principle, but this is a quick and safe way to fix
> > the regression, where as the generic fix you describe below is
> > likely months away and also has significant risks of causing
> > regressions in various places, see below.
>
> Perhaps we should add a TODO / FIXME there as well?
> So at least we will know that this is not a proper solution.
Assuming it's added
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
as a quick fix of the regression. But in long term I would like to see the real
solution here.
> > >> 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).
> >
> > I guess that you want to do the direction and bias init on all
> > GPIOs listed in _CRS, at least for devices with status == present ?
>
> For the devices that are serial busses only (I²C, SPI, UART).
>
> > I was wondering about the same thing, but ACPI tables are full
> > of, well, erm, garbage in various places so I'm afraid that doing
> > this for all GPIO _CRS resources is likely to cause a whole lot
> > of pain.
> >
> > Typically the firmware already sets up the direction + bias
> > of all used pins. I'm pretty sure the BIOS-es have some GPIO
> > init table especially for this somewhere.
> >
> > Now those init-tables may have bugs, but I'm seriously worried
> > about the implication of doing the direction + bios setup for
> > all _CRS GPIO entries. I have simply seen too much non sense
> > devices (with _STA returning 0x0f) listing GPIOs also actually
> > used elsewhere to think this is a good idea.
>
> Btw, other GPIO initialisation issues we have been solving by adding quirks.
> Why this one can't be targeted with the same approach?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-02 19:09 [PATCH] spi: Try to get ACPI GPIO IRQ earlier Hans de Goede
2025-11-03 9:35 ` Andy Shevchenko
@ 2025-11-06 11:34 ` Mark Brown
2025-11-06 12:23 ` Hans de Goede
1 sibling, 1 reply; 11+ messages in thread
From: Mark Brown @ 2025-11-06 11:34 UTC (permalink / raw)
To: Hans de Goede; +Cc: Andy Shevchenko, linux-spi, linux-acpi, stable
On Sun, 02 Nov 2025 20:09:21 +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.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/1] spi: Try to get ACPI GPIO IRQ earlier
commit: 3cd2018e15b3d66d2187d92867e265f45ad79e6f
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-06 11:34 ` Mark Brown
@ 2025-11-06 12:23 ` Hans de Goede
2025-11-06 13:05 ` Mark Brown
2025-11-06 13:09 ` Andy Shevchenko
0 siblings, 2 replies; 11+ messages in thread
From: Hans de Goede @ 2025-11-06 12:23 UTC (permalink / raw)
To: Mark Brown; +Cc: Andy Shevchenko, linux-spi, linux-acpi, stable
Hi Mark,
On 6-Nov-25 12:34 PM, Mark Brown wrote:
> On Sun, 02 Nov 2025 20:09:21 +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.
>>
>> [...]
>
> Applied to
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
>
> Thanks!
>
> [1/1] spi: Try to get ACPI GPIO IRQ earlier
> commit: 3cd2018e15b3d66d2187d92867e265f45ad79e6f
Thank you.
I believe that Andy's Reviewed-by was intended for a v2 with extending
the comment with an extra paragraph with something like:
"TODO: ideally the setup of the GPIO should be handled in a generic manner
in the ACPI/gpiolib core code".
Since you've already merged this now l'll prepare a follow-up patch
to extend the comment with that info.
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-06 12:23 ` Hans de Goede
@ 2025-11-06 13:05 ` Mark Brown
2025-11-06 13:09 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: Mark Brown @ 2025-11-06 13:05 UTC (permalink / raw)
To: Hans de Goede; +Cc: Andy Shevchenko, linux-spi, linux-acpi, stable
[-- Attachment #1: Type: text/plain, Size: 431 bytes --]
On Thu, Nov 06, 2025 at 01:23:21PM +0100, Hans de Goede wrote:
> I believe that Andy's Reviewed-by was intended for a v2 with extending
> the comment with an extra paragraph with something like:
Possibly, but that got sent well after I'd got things into CI anyway and
I didn't see it till after the CI had run.
> Since you've already merged this now l'll prepare a follow-up patch
> to extend the comment with that info.
Sure.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] spi: Try to get ACPI GPIO IRQ earlier
2025-11-06 12:23 ` Hans de Goede
2025-11-06 13:05 ` Mark Brown
@ 2025-11-06 13:09 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-11-06 13:09 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mark Brown, Andy Shevchenko, linux-spi, linux-acpi, stable
On Thu, Nov 06, 2025 at 01:23:21PM +0100, Hans de Goede wrote:
> On 6-Nov-25 12:34 PM, Mark Brown wrote:
> > On Sun, 02 Nov 2025 20:09:21 +0100, Hans de Goede wrote:
[...]
> > Applied to
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> >
> > Thanks!
> >
> > [1/1] spi: Try to get ACPI GPIO IRQ earlier
> > commit: 3cd2018e15b3d66d2187d92867e265f45ad79e6f
>
> Thank you.
>
> I believe that Andy's Reviewed-by was intended for a v2 with extending
> the comment with an extra paragraph with something like:
>
> "TODO: ideally the setup of the GPIO should be handled in a generic manner
> in the ACPI/gpiolib core code".
Yes.
> Since you've already merged this now l'll prepare a follow-up patch
> to extend the comment with that info.
Thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-11-06 13:09 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-02 19:09 [PATCH] spi: Try to get ACPI GPIO IRQ earlier Hans de Goede
2025-11-03 9:35 ` Andy Shevchenko
2025-11-03 9:57 ` Hans de Goede
2025-11-03 10:13 ` Andy Shevchenko
2025-11-03 10:20 ` Hans de Goede
2025-11-03 13:15 ` Andy Shevchenko
2025-11-06 10:16 ` Andy Shevchenko
2025-11-06 11:34 ` Mark Brown
2025-11-06 12:23 ` Hans de Goede
2025-11-06 13:05 ` Mark Brown
2025-11-06 13:09 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).