Re: [PATCH] pinctrl: qcom: Clear latched interrupt status when changing IRQ type

[Stephen Boyd]

Quoting Stephan Gerhold (2025-03-12 06:19:27)
> When submitting the TLMM test driver, Bjorn reported that some of the test
> cases are failing for GPIOs that not are backed by PDC (i.e. "non-wakeup"
> GPIOs that are handled directly in pinctrl-msm). Basically, lingering
> latched interrupt state is still being delivered at IRQ request time, e.g.:
>
>   ok 1 tlmm_test_silent_rising
>   tlmm_test_silent_falling: ASSERTION FAILED at drivers/pinctrl/qcom/tlmm-test.c:178

I wish it was called pinctrl-msm-test.c but oh well!

>   Expected atomic_read(&priv->intr_count) == 0, but
>       atomic_read(&priv->intr_count) == 1 (0x1)
>   not ok 2 tlmm_test_silent_falling
>   tlmm_test_silent_low: ASSERTION FAILED at drivers/pinctrl/qcom/tlmm-test.c:178
>   Expected atomic_read(&priv->intr_count) == 0, but
>       atomic_read(&priv->intr_count) == 1 (0x1)
>   not ok 3 tlmm_test_silent_low
>   ok 4 tlmm_test_silent_high
>
> Whether to report interrupts that came in while the IRQ was unclaimed
> doesn't seem to be well-defined in the Linux IRQ API. However, looking
> closer at these specific cases, we're actually reporting events that do not
> match the interrupt type requested by the driver:
>
>  1. After "ok 1 tlmm_test_silent_rising", the GPIO is in low state and
>     configured for IRQF_TRIGGER_RISING.
>
>  2. (a) In preparation for "tlmm_test_silent_falling", the GPIO is switched
>         to high state. The rising interrupt gets latched.

Is the interrupt unmasked here while the test is driving the GPIO line
high and the interrupt trigger is IRQF_TRIGGER_RISING? If so, this is
correct behavior.

Why wouldn't the trigger be set to IRQF_TRIGGER_FALLING, then the GPIO
driven high, and then the GPIO driven low for the test to confirm
falling edges work?

Have you seen the big comment in msm_gpio_irq_mask() and how it says we
want to latch edge interrupts even when the interrupt is masked?

>     (b) The GPIO is re-configured for IRQF_TRIGGER_FALLING, but the latched
>         interrupt isn't cleared.
>     (c) The IRQ handler is called for the latched interrupt, but there
>         wasn't any falling edge.
>
>  3. (a) For "tlmm_test_silent_low", the GPIO remains in high state.
>     (b) The GPIO is re-configured for IRQF_TRIGGER_LOW. This seems to
>         result in a phantom interrupt that gets latched.
>     (c) The IRQ handler is called for the latched interrupt, but the GPIO
>         isn't in low state.

Is the test causing phantom behavior by writing to the interrupt
hardware?

>
>  4. (a) For "tlmm_test_silent_high", the GPIO is switched to low state.
>     (b) This doesn't result in a latched interrupt, because RAW_STATUS_EN
>         was cleared when masking the level-triggered interrupt.
>
> Fix this by clearing the interrupt state whenever making any changes to the
> interrupt configuration. This includes previously disabled interrupts, but
> also any changes to interrupt polarity or detection type.

How do we avoid the case where an interrupt happens to come in while the
polarity is being changed? Won't we ignore such an interrupt now? If
these are edge interrupts that's quite bad because we may never see the
interrupt again.

I think we erred on the side of caution here and let extra edge
interrupts through because a rising or falling edge usually means the
interrupt handler just wants to run when there's some event and it will
do the work to find out if it was spurious or not. It's been years
though so I may have forgotten how this hardware works. It just makes me
very nervous that we're going to miss edges now that we always clear the
interrupt.