Skip to contentSkip to conversation

pinctrl: bcm2835: Clear the event latch register when disabling interrupts

PATCH In review Jonathan Bell 3 participants 3 messages Jun 30, 2015 – Jul 16, 2015 1 message not in the archive

Subsystems (4)
GPIO Pin control Broadcom BCM2711/BCM2835 Arm architecture Broadcom BCM281XX/BCM11XXX/BCM216XX Arm architecture
lore
Share & tools
lore

Jonathan Bell

author
#0 lore raw

PATCH 1/1 pinctrl: bcm2835: Clear the event latch register when disabling interrupts

It's possible to hit a race condition if interrupts are generated on a GPIO pin when the IRQ line in question is being disabled.

If the interrupt is freed, bcm2835_gpio_irq_disable() is called which disables the event generation sources (edge, level). If an event occurred between the last disabling of hard IRQs and the write to the event source registers, a bit would be set in the GPIO event detect register (GPEDSn) which goes unacknowledged by bcm2835_gpio_irq_handler() so Linux complains loudly.

There is no per-GPIO mask register, so when disabling GPIO interrupts write 1 to the relevant bit in GPEDSn to clear out any stale events.

Signed-off-by Jonathan Bell <[email protected]>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/pinctrl/bcm/pinctrl-bcm2835.c +2 -0
index efcf2a2..547b925 100644
@@ -473,6 +473,8 @@ static void bcm2835_gpio_irq_disable(struct irq_data *data)
 
     spin_lock_irqsave(&pc->irq_lock[bank], flags);
     bcm2835_gpio_irq_config(pc, gpio, false);
+    /* Clear events that were latched prior to clearing event sources */
+    bcm2835_gpio_set_bit(pc, GPEDS0, gpio);
     clear_bit(offset, &pc->enabled_irq_map[bank]);
     spin_unlock_irqrestore(&pc->irq_lock[bank], flags);
 }
Signature · 1 line
1.8.1.2

A message that is not in the archive (sent off-list or not yet received): [email protected]

Acked-by
#2 lore raw
Quoted 10 lines · Jonathan Bell

It's possible to hit a race condition if interrupts are generated on a GPIO pin when the IRQ line in question is being disabled.

If the interrupt is freed, bcm2835_gpio_irq_disable() is called which disables the event generation sources (edge, level). If an event occurred between the last disabling of hard IRQs and the write to the event source registers, a bit would be set in the GPIO event detect register (GPEDSn) which goes unacknowledged by bcm2835_gpio_irq_handler() so Linux complains loudly.

Quoted 2 lines

There is no per-GPIO mask register, so when disabling GPIO interrupts write 1 to the relevant bit in GPEDSn to clear out any stale events.

Acked-by Stephen Warren <[email protected]>

(Sorry for the slow response; I was on vacation)

#3 lore raw
In reply to Jonathan Bell
Quoted 10 lines · Jonathan Bell

It's possible to hit a race condition if interrupts are generated on a GPIO pin when the IRQ line in question is being disabled.

If the interrupt is freed, bcm2835_gpio_irq_disable() is called which disables the event generation sources (edge, level). If an event occurred between the last disabling of hard IRQs and the write to the event source registers, a bit would be set in the GPIO event detect register (GPEDSn) which goes unacknowledged by bcm2835_gpio_irq_handler() so Linux complains loudly.

Quoted 4 lines

There is no per-GPIO mask register, so when disabling GPIO interrupts write 1 to the relevant bit in GPEDSn to clear out any stale events.

Signed-off-by Jonathan Bell <[email protected]>

Patch applied for fixes with Stephen's ACK, I had to rebase it because of something, maybe whitespace mangling.

Yours,
Linus Walleij