AM4377: Duplicate calls made to GPIO ISR

Jeremy Dexter

Part Number: AM4377

We are developing firmware for a custom AM437x based board. We are not using the TI SDK, but rather the Linux based firmware is created using Buildroot and a standard kernel. While upgrading our previous kernel 4.19.45 to 5.7.1 I noticed an issue with a Wiznet Ethernet driver where the ISR was often being called in cases where no physical interrupt had occurred. Our device uses an active high, level sensitive interrupt signal which is connected to GPIO 2.25. It is handled by a threaded ISR.

The problem appears to be related to some changes made in gpio-omap.c. After our threaded ISR has run and has cleared the physical interrupt, omap_gpio_unmask_irq() is called to re-enable the GPIO interrupt. The current code clears the interrupt status register, then enables the interrupt for the appropriate pin (sets bit 25 in GPIO_LEVELDETECT1 register). However, this appears to immediately set the associated bit in the GPIO_IRQSTS_0 register, even though the physical interrupt line state at the time (as indicated in the GPIO_DATAIN register) is low (inactive). This results in a spurious call to the ISR. This appears to be incorrect chip behaviour, but it didn't cause spurious interrupts with the 4.19.45 code because it did these two operations in the reverse order, ie. enabled the interrupt then cleared the interrupt status.

So my workaround at the moment is to move the omap_set_gpio_triggering() call to be before the call to omap_clear_gpio_irqstatus().

The commit where the issue appears to have been introduced is https://github.com/torvalds/linux/commit/c859e0d479b3b4f6132fc12637c51e01492f31f6

My questions are: Is the observed register behaviour expected? Is the workaround valid, and if so can the standard driver be updated in due course?

thanks

Jeremy

over 3 years ago

0 RonB over 3 years ago

TI__Mastermind 30276 points

Jeremy,

Our official support is for the kernel in the SDK available for the part you are using. So for AM437x this is currently v4.19.

Given this is an upstream kernel issue, I've requested support from our developers to see if there is any feedback we can provide.

I hope to get back to you within a couple of days.

Thanks.

0 RonB over 3 years ago in reply to RonB

TI__Mastermind 30276 points

Jeremy,

Could you please try the change below:

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 2a4a11634dd1..41ec54c3609f 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -896,6 +896,8 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
         raw_spin_lock_irqsave(&bank->lock, flags);
         omap_set_gpio_irqenable(bank, offset, 1);

+       if (trigger)
+               omap_set_gpio_triggering(bank, offset, trigger);
         /*
          * For level-triggered GPIOs, clearing must be done after the source
          * is cleared, thus after the handler has run. OMAP4 needs this done
@@ -905,9 +907,6 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
             trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
                 omap_clear_gpio_irqstatus(bank, offset);

-       if (trigger)
-               omap_set_gpio_triggering(bank, offset, trigger);
-
         raw_spin_unlock_irqrestore(&bank->lock, flags);
  }

Assumption - clearing IRQ status may require IRQ type configured.

Thanks.

0 Jeremy Dexter over 3 years ago in reply to RonB

Prodigy 50 points

Thanks Ron

Yes this change resolves the issue. I'd call it a workaround, as I don't think configuring the IRQ should cause an interrupt to be generated, given that the GPIO input state was and remains low (inactive).

I'd be happy if this change could make its way upstream.

regards

Jeremy

Processors

Processors forum

AM4377: Duplicate calls made to GPIO ISR