Sv: Sv: GPIO level IRQ fires twice each time.

[Markus Mirevik <markus.mirevik@xxxxxxxxxxxxxx>]

Hi Grygorii

> On 24/01/2022 09:58, Lars-Peter Clausen wrote:
> > On 1/24/22 08:56, Lars-Peter Clausen wrote:
> >> On 1/24/22 08:12, Markus Mirevik wrote:
> >>>> On Fri, Jan 21, 2022 at 09:03:43AM +0000, Markus Mirevik wrote:
> >>>>> I have a problem with a custom bord based on SoC am335x and a
> >>>>> driver
> >>>> utilizing a GPIO line for interrupts.
> >>>>> I have two mcp2518fd chip connected on one SPI line and everything
> >>>> works, but it's hogs a lot of CPU.
> >>>>> In the current setup only one chip is connected and it only receives
> packets.
> >>>>>
> >>>>> The mcp2518fd is connected with 2 interrupt lines one "main" and
> >>>>> one for
> >>>> rx frames.
> >>>>> The problem is that for every frame received the interrupt handler
> >>>>> is run
> >>>> twice, which is kind of expensive since it's a SPI call to the chip
> >>>> to check interrupt registers.
> >>>>> To me it looks like the interrupt is fired again as soon as it's unmasked.
> >>>> Either because it's queued? or maybe not cleared internally?
> >>>>> I have scoped the interrupt signal and its real good without any
> glitches.
> >>>>>
> >>>>> I'm currently running a yocto build:
> >>>>> Linux botekcc 5.10.79-yocto-tiny #1 SMP Tue Nov 16 03:57:43 UTC
> >>>>> 2021 armv7l armv7l armv7l GNU/Linux
> >>>>>
> >>>>> But the mcp251xfd driver is from net-next/master
> >>>>>
> >>>>> mcp251xfd_irq is the irqhandler for the mcp2518fd and is added like
> this:
> >>>>> err = request_threaded_irq(spi->irq, NULL, mcp251xfd_irq,
> >>>>>                                     IRQF_SHARED | IRQF_ONESHOT,
> >>>>> dev_name(&spi->dev), priv);
> >>>>>
> >>>> You haven't set a IRQF_TRIGGER flag, so you are getting the
> >>>> "as-already- configured" behaviour, which on your setup is both
> edges?
> >>>> Try adding IRQF_TRIGGER_RISING, IRQF_TRIGGER_FALLING,
> >>>> IRQF_TRIGGER_HIGH or IRQF_TRIGGER_LOW, as appropriate to your
> use
> >>>> case, to your flags.
> >>>>
> >>>> Cheers,
> >>>> Kent.
> >>> I have tried with the IRQF_TRIGGGER_LOW flag as well. With same
> result. i.e the interrupt is fired again as soon as the handler is ready. Even if
> the interrupt line is deactivated.
> >>> However if I change the trigger to edge falling the interrupt will only fire
> once. But his will inevitably lead to a missed edge eventually.
> >>
> >> Depending on how the mcp2518 GPIO controller works internally its
> >> driver might have to use the handle_fasteoi_irq() flow to avoid this.
> >> It is not uncommon to have hardware which needs a level IRQ acked
> >> after the interrupt handler has run, rather than before like the
> >> handle_level_irq() does. E.g.
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/co
> >> mmit/drivers/gpio/gpio-
> zynq.c?id=6dd859508336f0fd078fd62f3b9fe42a32aa
> >> 38e2
> >>
> >> - Lars
> >>
> > Sorry, I meant `Depending on how the am335x interrupt controller
> > works...`
> >
> 
> 
> Could  you try to test below diff (may not be applied cleanly):
> 
> 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.
> 

I  have moved that part suggested in your diff and it looks like it has solved the problem! 

The interrupt associated with the GPIO module still fires twice

(cat /proc/interrupts)
19:     147227      INTC  96 Level     44e07000.gpio

But the interrupt in the mcp251xfd driver is only fired once. 

64:      76673  44e07000.gpio  22 Level     spi1.1

And since it's the spi call that worries me the most I think this is fine. 
Thank you!!

 BR
Markus Mirevik

> --
> Best regards,
> Grygorii, Ukraine