Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47417 )
Change subject: mb/google/hatch: Configure IRQs as level triggered for HID over I2C ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47417/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/47417/2/src/mainboard/google/hatch/... PS2, Line 182: PAD_CFG_GPI_APIC(GPP_D16, NONE, PLTRST, LEVEL, NONE),
Ah, if that's the case, we should probably skip that code on other platforms.
No, we still need that code because we are relying on the double inversion. Like in the example you gave for deltaur above.
So, here is a brief history of how things happened for GPIO-based IOAPIC interrupts: 1. Before APL, pads were configured without inversion and the filter for active low was applied at the IOAPIC. 2. In APL, ITSS had the restriction that it only triggered on active high signals. So, to support this, pads that were active low signals had to be configured for inversion. But, because of this inversion, IOAPIC entries would have to be inverted as well. So, ITSS was configured to always invert a signal if it is configured as inverted at the pad. This allowed IOAPIC to see the signal as it is on the input pad. 3. Other Intel platforms (SKL, KBL, CNL, WHL, CML, etc.) do not have this restriction of active high signals for ITSS. However, it was confusing to configure different platforms at the pad differently (just like the loop we went through). So, to make this consistent across all Intel platforms, pads are always configured with inversion if the input signal is active low. ITSS is configured to invert the signal again if it is inverted at the pad. This allows IOAPIC to see the signal as it is at input. This also helped with the case of dual routing for IOAPIC and SCI. But, there have been few misses where pads were configured without inversion even though the signal is active low. Technically, it works just fine because we guarantee that IOAPIC always sees the input signal as is either with no inversion or with double inversion(one at pad and other at ITSS).
We should update the comment in gpio.c to reflect the reason behind the inversion for all platforms. Also, I think we should write some documentation on guidance for configuring pad and devicetree entries.