Skip to content

Commit e225128

Browse files
stephan-ghlinusw
authored andcommitted
pinctrl: qcom: Clear latched interrupt status when changing IRQ type
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 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. (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. 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. With this change, all 16 test cases are now passing for the non-wakeup GPIOs in the TLMM. Cc: stable@vger.kernel.org Fixes: cf9d052 ("pinctrl: qcom: Don't clear pending interrupts when enabling") Reported-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> Closes: https://lore.kernel.org/r/20250227-tlmm-test-v1-1-d18877b4a5db@oss.qualcomm.com/ Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org> Tested-by: Bjorn Andersson <andersson@kernel.org> Reviewed-by: Bjorn Andersson <andersson@kernel.org> Link: https://lore.kernel.org/20250312-pinctrl-msm-type-latch-v1-1-ce87c561d3d7@linaro.org Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
1 parent 2ef8547 commit e225128

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

drivers/pinctrl/qcom/pinctrl-msm.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,8 +1045,7 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
10451045
const struct msm_pingroup *g;
10461046
u32 intr_target_mask = GENMASK(2, 0);
10471047
unsigned long flags;
1048-
bool was_enabled;
1049-
u32 val;
1048+
u32 val, oldval;
10501049

10511050
if (msm_gpio_needs_dual_edge_parent_workaround(d, type)) {
10521051
set_bit(d->hwirq, pctrl->dual_edge_irqs);
@@ -1108,8 +1107,7 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
11081107
* internal circuitry of TLMM, toggling the RAW_STATUS
11091108
* could cause the INTR_STATUS to be set for EDGE interrupts.
11101109
*/
1111-
val = msm_readl_intr_cfg(pctrl, g);
1112-
was_enabled = val & BIT(g->intr_raw_status_bit);
1110+
val = oldval = msm_readl_intr_cfg(pctrl, g);
11131111
val |= BIT(g->intr_raw_status_bit);
11141112
if (g->intr_detection_width == 2) {
11151113
val &= ~(3 << g->intr_detection_bit);
@@ -1162,9 +1160,11 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
11621160
/*
11631161
* The first time we set RAW_STATUS_EN it could trigger an interrupt.
11641162
* Clear the interrupt. This is safe because we have
1165-
* IRQCHIP_SET_TYPE_MASKED.
1163+
* IRQCHIP_SET_TYPE_MASKED. When changing the interrupt type, we could
1164+
* also still have a non-matching interrupt latched, so clear whenever
1165+
* making changes to the interrupt configuration.
11661166
*/
1167-
if (!was_enabled)
1167+
if (val != oldval)
11681168
msm_ack_intr_status(pctrl, g);
11691169

11701170
if (test_bit(d->hwirq, pctrl->dual_edge_irqs))

0 commit comments

Comments
 (0)