[PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Mario Limonciello]

Commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at
least once on boot") introduced logic to trigger edge-based GPIO
interrupts during initialization to ensure proper initial state setup
when firmware doesn't initialize it.

However, according to the Microsoft GPIO documentation, triggering GPIO
interrupts during initialization should only happen for interrupts
marked as ActiveBoth (both IRQF_TRIGGER_RISING and IRQF_TRIGGER_FALLING)
and only when the associated GPIO line is already asserted (logic level
low).

The current implementation incorrectly triggers:
1. Any edge-triggered interrupt (RISING-only or FALLING-only)
2. RISING interrupts when value is high and FALLING when value is low

This causes problems at bootup for single-edge interrupts that
don't follow the ActiveBoth pattern.

Fix this by:
- Only triggering when BOTH rising and falling edges are configured
- Only triggering when the GPIO line is asserted (value == 0)

Reported-by: Francesco Lauritano <francesco.lauritano1@protonmail.com>
Closes: https://lore.kernel.org/all/6iFCwGH2vssb7NRUTWGpkubGMNbgIlBHSz40z8ZsezjxngXpoiiRiJaijviNvhiDAGIr43bfUmdxLmxYoHDjyft4DgwFc3Pnu5hzPguTa0s=@protonmail.com/
Tested-by: Marco Scardovi <mscardovi95@gmail.com>
Fixes: ca876c7483b69 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
Suggested-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpio/gpiolib-acpi-core.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index 09f860200a059..eb8a40cfb7a98 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -233,12 +233,23 @@ static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
 
 	event->irq_requested = true;
 
-	/* Make sure we trigger the initial state of edge-triggered IRQs */
+	/*
+	 * Make sure we trigger the initial state of ActiveBoth IRQs.
+	 *
+	 * According to the Microsoft GPIO documentation, triggering GPIO
+	 * interrupts marked as ActiveBoth during initialization is correct
+	 * as long as the associated GPIO line is already "asserted"
+	 * (logic level low). We should not trigger edge-based GPIO
+	 * interrupts not marked as ActiveBoth.
+	 *
+	 * See: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
+	 * Section: "GPIO controllers and ActiveBoth interrupts"
+	 */
 	if (acpi_gpio_need_run_edge_events_on_boot() &&
-	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
+	    ((event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) ==
+	     (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
 		value = gpiod_get_raw_value_cansleep(event->desc);
-		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
-		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
+		if (value == 0)
 			event->handler(event->irq, event);
 	}
 }
-- 
2.43.0

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Mika Westerberg]

On Tue, Apr 28, 2026 at 09:52:39PM -0500, Mario Limonciello wrote:
> Commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at
> least once on boot") introduced logic to trigger edge-based GPIO
> interrupts during initialization to ensure proper initial state setup
> when firmware doesn't initialize it.
> 
> However, according to the Microsoft GPIO documentation, triggering GPIO
> interrupts during initialization should only happen for interrupts
> marked as ActiveBoth (both IRQF_TRIGGER_RISING and IRQF_TRIGGER_FALLING)
> and only when the associated GPIO line is already asserted (logic level
> low).
> 
> The current implementation incorrectly triggers:
> 1. Any edge-triggered interrupt (RISING-only or FALLING-only)
> 2. RISING interrupts when value is high and FALLING when value is low
> 
> This causes problems at bootup for single-edge interrupts that
> don't follow the ActiveBoth pattern.
> 
> Fix this by:
> - Only triggering when BOTH rising and falling edges are configured
> - Only triggering when the GPIO line is asserted (value == 0)
> 
> Reported-by: Francesco Lauritano <francesco.lauritano1@protonmail.com>
> Closes: https://lore.kernel.org/all/6iFCwGH2vssb7NRUTWGpkubGMNbgIlBHSz40z8ZsezjxngXpoiiRiJaijviNvhiDAGIr43bfUmdxLmxYoHDjyft4DgwFc3Pnu5hzPguTa0s=@protonmail.com/
> Tested-by: Marco Scardovi <mscardovi95@gmail.com>
> Fixes: ca876c7483b69 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
> Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
> Suggested-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Andy Shevchenko]

On Wed, Apr 29, 2026 at 07:32:11AM +0200, Mika Westerberg wrote:
> On Tue, Apr 28, 2026 at 09:52:39PM -0500, Mario Limonciello wrote:
> > Commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at
> > least once on boot") introduced logic to trigger edge-based GPIO
> > interrupts during initialization to ensure proper initial state setup
> > when firmware doesn't initialize it.
> > 
> > However, according to the Microsoft GPIO documentation, triggering GPIO
> > interrupts during initialization should only happen for interrupts
> > marked as ActiveBoth (both IRQF_TRIGGER_RISING and IRQF_TRIGGER_FALLING)
> > and only when the associated GPIO line is already asserted (logic level
> > low).
> > 
> > The current implementation incorrectly triggers:
> > 1. Any edge-triggered interrupt (RISING-only or FALLING-only)
> > 2. RISING interrupts when value is high and FALLING when value is low
> > 
> > This causes problems at bootup for single-edge interrupts that
> > don't follow the ActiveBoth pattern.
> > 
> > Fix this by:
> > - Only triggering when BOTH rising and falling edges are configured
> > - Only triggering when the GPIO line is asserted (value == 0)
> > 
> > Reported-by: Francesco Lauritano <francesco.lauritano1@protonmail.com>
> > Closes: https://lore.kernel.org/all/6iFCwGH2vssb7NRUTWGpkubGMNbgIlBHSz40z8ZsezjxngXpoiiRiJaijviNvhiDAGIr43bfUmdxLmxYoHDjyft4DgwFc3Pnu5hzPguTa0s=@protonmail.com/
> > Tested-by: Marco Scardovi <mscardovi95@gmail.com>
> > Fixes: ca876c7483b69 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
> > Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
> > Suggested-by: Armin Wolf <W_Armin@gmx.de>
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 09:52:39PM -0500, Mario Limonciello wrote:
> Commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at
> least once on boot") introduced logic to trigger edge-based GPIO
> interrupts during initialization to ensure proper initial state setup
> when firmware doesn't initialize it.
> 
> However, according to the Microsoft GPIO documentation, triggering GPIO
> interrupts during initialization should only happen for interrupts
> marked as ActiveBoth (both IRQF_TRIGGER_RISING and IRQF_TRIGGER_FALLING)
> and only when the associated GPIO line is already asserted (logic level
> low).
> 
> The current implementation incorrectly triggers:
> 1. Any edge-triggered interrupt (RISING-only or FALLING-only)
> 2. RISING interrupts when value is high and FALLING when value is low
> 
> This causes problems at bootup for single-edge interrupts that
> don't follow the ActiveBoth pattern.
> 
> Fix this by:
> - Only triggering when BOTH rising and falling edges are configured
> - Only triggering when the GPIO line is asserted (value == 0)

Good catch!

...

>  	if (acpi_gpio_need_run_edge_events_on_boot() &&
> -	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> +	    ((event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) ==
> +	     (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {

Use _MASK in both cases.

	    ((event->irqflags & IRQF_TRIGGER_MASK) == IRQF_TRIGGER_MASK)) {

>  		value = gpiod_get_raw_value_cansleep(event->desc);
> -		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
> -		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
> +		if (value == 0)
>  			event->handler(event->irq, event);
>  	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Andy Shevchenko]

On Wed, Apr 29, 2026 at 10:10:00AM +0300, Andy Shevchenko wrote:
> On Tue, Apr 28, 2026 at 09:52:39PM -0500, Mario Limonciello wrote:

...

> > -	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> > +	    ((event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) ==
> > +	     (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> 
> Use _MASK in both cases.
> 
> 	    ((event->irqflags & IRQF_TRIGGER_MASK) == IRQF_TRIGGER_MASK)) {

Ah, I thought it was only for edge ones, seems we can't use it in the second part.

The other definitions are for TYPE, there we have IRQ_TYPE_EDGE_BOTH.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Hans de Goede]

Hi Mario,

Thank you for fixing this.

On 29-Apr-26 04:52, Mario Limonciello wrote:
> Commit ca876c7483b6 ("gpiolib-acpi: make sure we trigger edge events at
> least once on boot") introduced logic to trigger edge-based GPIO
> interrupts during initialization to ensure proper initial state setup
> when firmware doesn't initialize it.
> 
> However, according to the Microsoft GPIO documentation, triggering GPIO
> interrupts during initialization should only happen for interrupts
> marked as ActiveBoth (both IRQF_TRIGGER_RISING and IRQF_TRIGGER_FALLING)
> and only when the associated GPIO line is already asserted (logic level
> low).
> 
> The current implementation incorrectly triggers:
> 1. Any edge-triggered interrupt (RISING-only or FALLING-only)
> 2. RISING interrupts when value is high and FALLING when value is low
> 
> This causes problems at bootup for single-edge interrupts that
> don't follow the ActiveBoth pattern.
> 
> Fix this by:
> - Only triggering when BOTH rising and falling edges are configured
> - Only triggering when the GPIO line is asserted (value == 0)
> 
> Reported-by: Francesco Lauritano <francesco.lauritano1@protonmail.com>
> Closes: https://lore.kernel.org/all/6iFCwGH2vssb7NRUTWGpkubGMNbgIlBHSz40z8ZsezjxngXpoiiRiJaijviNvhiDAGIr43bfUmdxLmxYoHDjyft4DgwFc3Pnu5hzPguTa0s=@protonmail.com/
> Tested-by: Marco Scardovi <mscardovi95@gmail.com>
> Fixes: ca876c7483b69 ("gpiolib-acpi: make sure we trigger edge events at least once on boot")
> Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
> Suggested-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpio/gpiolib-acpi-core.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
> index 09f860200a059..eb8a40cfb7a98 100644
> --- a/drivers/gpio/gpiolib-acpi-core.c
> +++ b/drivers/gpio/gpiolib-acpi-core.c
> @@ -233,12 +233,23 @@ static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
>  
>  	event->irq_requested = true;
>  
> -	/* Make sure we trigger the initial state of edge-triggered IRQs */
> +	/*
> +	 * Make sure we trigger the initial state of ActiveBoth IRQs.
> +	 *
> +	 * According to the Microsoft GPIO documentation, triggering GPIO
> +	 * interrupts marked as ActiveBoth during initialization is correct
> +	 * as long as the associated GPIO line is already "asserted"
> +	 * (logic level low). We should not trigger edge-based GPIO
> +	 * interrupts not marked as ActiveBoth.
> +	 *
> +	 * See: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
> +	 * Section: "GPIO controllers and ActiveBoth interrupts"
> +	 */
>  	if (acpi_gpio_need_run_edge_events_on_boot() &&
> -	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> +	    ((event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) ==
> +	     (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>  		value = gpiod_get_raw_value_cansleep(event->desc);
> -		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
> -		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
> +		if (value == 0)
>  			event->handler(event->irq, event);
>  	}
>  }

One nitpick, which can be a follow-up patch since Andy has already picked this
one up.

I think that now that the second if condition has been simplified to just
value == 0, it can be added to the first if as " && value == 0" dropping
the nested if.

Regards,

Hans

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Andy Shevchenko]

On Wed, Apr 29, 2026 at 11:48:13AM +0200, Hans de Goede wrote:
> On 29-Apr-26 04:52, Mario Limonciello wrote:

...

> > +	/*
> > +	 * Make sure we trigger the initial state of ActiveBoth IRQs.
> > +	 *
> > +	 * According to the Microsoft GPIO documentation, triggering GPIO
> > +	 * interrupts marked as ActiveBoth during initialization is correct
> > +	 * as long as the associated GPIO line is already "asserted"
> > +	 * (logic level low). We should not trigger edge-based GPIO
> > +	 * interrupts not marked as ActiveBoth.
> > +	 *
> > +	 * See: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
> > +	 * Section: "GPIO controllers and ActiveBoth interrupts"
> > +	 */
> >  	if (acpi_gpio_need_run_edge_events_on_boot() &&
> > -	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> > +	    ((event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) ==
> > +	     (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
> >  		value = gpiod_get_raw_value_cansleep(event->desc);
> > -		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
> > -		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
> > +		if (value == 0)
> >  			event->handler(event->irq, event);
> >  	}
> >  }
> 
> One nitpick, which can be a follow-up patch since Andy has already picked this
> one up.
> 
> I think that now that the second if condition has been simplified to just
> value == 0, it can be added to the first if as " && value == 0" dropping
> the nested if.

But we need to get that value first (unconditionally!). I think it wouldn't
be desirable change.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Hans de Goede]

Hi,

On 29-Apr-26 12:24, Andy Shevchenko wrote:
> On Wed, Apr 29, 2026 at 11:48:13AM +0200, Hans de Goede wrote:
>> On 29-Apr-26 04:52, Mario Limonciello wrote:
> 
> ...
> 
>>> +	/*
>>> +	 * Make sure we trigger the initial state of ActiveBoth IRQs.
>>> +	 *
>>> +	 * According to the Microsoft GPIO documentation, triggering GPIO
>>> +	 * interrupts marked as ActiveBoth during initialization is correct
>>> +	 * as long as the associated GPIO line is already "asserted"
>>> +	 * (logic level low). We should not trigger edge-based GPIO
>>> +	 * interrupts not marked as ActiveBoth.
>>> +	 *
>>> +	 * See: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
>>> +	 * Section: "GPIO controllers and ActiveBoth interrupts"
>>> +	 */
>>>  	if (acpi_gpio_need_run_edge_events_on_boot() &&
>>> -	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>>> +	    ((event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) ==
>>> +	     (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>>>  		value = gpiod_get_raw_value_cansleep(event->desc);
>>> -		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
>>> -		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
>>> +		if (value == 0)
>>>  			event->handler(event->irq, event);
>>>  	}
>>>  }
>>
>> One nitpick, which can be a follow-up patch since Andy has already picked this
>> one up.
>>
>> I think that now that the second if condition has been simplified to just
>> value == 0, it can be added to the first if as " && value == 0" dropping
>> the nested if.
> 
> But we need to get that value first (unconditionally!). I think it wouldn't
> be desirable change.

Ah right, I read over the line getting the value I thought this was already
done, never mind:

FWIW:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Marco Scardovi]

On 4/29/26 13:01, Hans de Goede wrote:
> Hi,
>
> On 29-Apr-26 12:24, Andy Shevchenko wrote:
>> On Wed, Apr 29, 2026 at 11:48:13AM +0200, Hans de Goede wrote:
>>> On 29-Apr-26 04:52, Mario Limonciello wrote:
>> ...
>>
>>>> +	/*
>>>> +	 * Make sure we trigger the initial state of ActiveBoth IRQs.
>>>> +	 *
>>>> +	 * According to the Microsoft GPIO documentation, triggering GPIO
>>>> +	 * interrupts marked as ActiveBoth during initialization is correct
>>>> +	 * as long as the associated GPIO line is already "asserted"
>>>> +	 * (logic level low). We should not trigger edge-based GPIO
>>>> +	 * interrupts not marked as ActiveBoth.
>>>> +	 *
>>>> +	 * See: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/general-purpose-i-o--gpio-
>>>> +	 * Section: "GPIO controllers and ActiveBoth interrupts"
>>>> +	 */
>>>>   	if (acpi_gpio_need_run_edge_events_on_boot() &&
>>>> -	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>>>> +	    ((event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) ==
>>>> +	     (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
>>>>   		value = gpiod_get_raw_value_cansleep(event->desc);
>>>> -		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
>>>> -		    ((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
>>>> +		if (value == 0)
>>>>   			event->handler(event->irq, event);
>>>>   	}
>>>>   }
>>> One nitpick, which can be a follow-up patch since Andy has already picked this
>>> one up.
>>>
>>> I think that now that the second if condition has been simplified to just
>>> value == 0, it can be added to the first if as " && value == 0" dropping
>>> the nested if.
>> But we need to get that value first (unconditionally!). I think it wouldn't
>> be desirable change.
> Ah right, I read over the line getting the value I thought this was already
> done, never mind:
>
> FWIW:
>
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>
> Regards,
>
> Hans

Hi everyone,

I tested it personally against both kernel 7.0 and 7.1 and can say it works.

Are there any news on how it is proceeding? Somewhere I can check the 
progress?

Best regards,

Marco "scardracs" Scardovi

Re: [PATCH] gpiolib: acpi: Only trigger ActiveBoth interrupts on boot

[Andy Shevchenko]

On Sun, May 03, 2026 at 04:18:57PM +0200, Marco Scardovi wrote:
> On 4/29/26 13:01, Hans de Goede wrote:
> > On 29-Apr-26 12:24, Andy Shevchenko wrote:
> > > On Wed, Apr 29, 2026 at 11:48:13AM +0200, Hans de Goede wrote:
> > > > On 29-Apr-26 04:52, Mario Limonciello wrote:

...

> > Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> 
> I tested it personally against both kernel 7.0 and 7.1 and can say it works.
> 
> Are there any news on how it is proceeding? Somewhere I can check the
> progress?

It is in the Linux Next already and after a few days or a couple of weeks
I will send a PR to upper maintainers to pass it towards upstream.

TL;DR: It should appear in one of the v7.1-rcX.

-- 
With Best Regards,
Andy Shevchenko