Re: [PATCH v4] pinctrl: baytrail: Clear direct_irq_en flag on broken configs

[Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>]

On Wed, Jan 12, 2022 at 10:02:00PM +0100, Hans de Goede wrote:
> Some boards set the direct_irq_en flag in the conf0 register without
> setting the correct trigger bits. The direct_irq_en flag just means that
> the GPIO will send IRQs directly to the APIC instead of going through
> the shared interrupt for the GPIO controller, in order for the pin to be
> able to actually generate IRQs the trigger flags must configure the IRQ
> as a level-high or level-low active IRQ.
> 
> Note testing shows that using edge trigger add the conf0 register level
> does NOT work, instead edge triggering should be set at the IO-APIC level.
> 
> I believe that the direct_irq_en flag connects the output of the GPIO's IRQ
> trigger block, which normally sets the status flag in the IRQ status reg at
> 0x800 to one of the IO-APIC pins according to the direct IRQ mux.
> 
> This means that the TRIG_LVL bit *must* be set, so that the GPIO's input
> value is directly passed (1:1 or inverted) to the IO-APIC pin, if TRIG_LVL
> is not set, selecting edge mode operation then on the first edge the
> selected IO-APIC pin goes high, but since no write-to-clear write will be
> done to the IRQ status reg at 0x800, the detected edge condition will never
> get cleared.
> 
> This APIC pin stuck high condition can be observed with the pin configured
> as level-high active, in the form of an interrupt storm. Clearing the
> TRIG_MASK bits of conf0 stops the storm, reconfiguring them as edge again
> results in a storm again as soon as the edge is triggered once.
> 
> Detect invalid trigger flags, log a FW_BUG warning when encountering this
> and clear the direct_irq_en flag so that a driver can actually use the pin
> as IRQ through gpiod_to_irq().
> 
> Specifically this allows the edt-ft5x06 touchscreen driver to use
> INT33FC:02 pin 3 as touchscreen IRQ on the Nextbook Ares 8 tablet,
> accompanied by the following new log message
> 
> byt_gpio INT33FC:02: [Firmware Bug]: pin 3: direct_irq_en set without trigger, clearing
> 
> The new byt_direct_irq_sanity_check() function also checks that the
> pin is actually appointed to one of the 16 direct-IRQs which the GPIO
> controller supports and on success prints debug messages like these:
> 
> byt_gpio INT33FC:02: Pin 0: uses direct IRQ 0 (IO-APIC 67)
> byt_gpio INT33FC:02: Pin 15: uses direct IRQ 2 (IO-APIC 69)
> 
> This is useful to figure out the GPIO pin belonging to ACPI
> resources like this one: "Interrupt () { 0x00000043 }" or
> the other way around.
> 
> The strict checking of valid trigger flags this introduces does result in
> FW_BUG messages on quite a few devices. E.g. on the Yoga Tablet 2 1051L:
>  byt_gpio INT33FC:00: [Firmware Bug]: pin 92: direct_irq_en set but no IRQ assigned, clearing
>  byt_gpio INT33FC:00: [Firmware Bug]: pin 93: direct_irq_en set but no IRQ assigned, clearing
>   These 2 also have mux set to 7 and fall + rise + level trigger bits set,
>   presumably something has written 0xffffffff to their conf0 registers
>  byt_gpio INT33FC:02: Pin 3: uses direct IRQ 1 (IO-APIC 68)
>  byt_gpio INT33FC:02: [Firmware Bug]: pin 3: direct_irq_en set without trigger (conf0: 2803cc00h), clearing
>   Most tablets seem to have this, looking at DSDTs this seems intended for
>   use with an I2C HID sensor-hub and is still set on devices without one.
> 
> To make sure this does not cause any regressions this has been tested,
> including checking disabled direct-IRQs are not used in the DSDT,
> on the following devices:
> 
> Asus ME176C
> Asus TF103C
> Chuwi Vi10 (with its Windows BIOS)
> HP x2 10-n000nd
> Lenovo Yoga Tablet 2 1050L (Android version, without EC, with buggy DSDT)
> Lenovo Yoga Tablet 2 1051L (Windows version, with EC)
> 
> Suggested-by: Andy Shevchenko <andy@xxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Acked-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>