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

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

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