GPIO level IRQ fires twice each time.

GPIO level IRQ fires twice each time.

[Markus Mirevik]

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);

I have instrumented some of the handling in /kernel/irq/chip.c and /kernel/irq/handle.c 
And this maybe doesn't say anything but it looks like this:

[  290.229920] IRQ:19 mask_irq
[  290.233098] IRQ:19 Enter. Caller is handle_level_irq+0xe4/0x1dc
[  290.241851] IRQ:64 mask_irq
[  290.245014] IRQ:64 Enter. Caller is handle_level_irq+0xe4/0x1dc
[  290.253777] IRQ:64 Leave
[  290.253784] IRQ:19 Leave
[  290.258868] IRQ:19 unmask_irq
[  290.262785] Enter mcp251xfd_irq

-First enter. Here the interrupt is handled and the interruptline goes inactive. 
-We do not return until rx-int is inactive. AND rx-int and int goes inactive simultaneous. (when only receiving.)
-I've also tried a msleep(1000) here to make sure there is enough time for the int to go inactive. 

[  290.265966] Leave mcp251xfd_irq (RX)
[  290.269167] IRQ:64 unmask_irq
[  290.275740] IRQ:19 mask_irq
[  290.278900] IRQ:19 Enter. Caller is handle_level_irq+0xe4/0x1dc
[  290.287648] IRQ:64 mask_irq
[  290.290806] IRQ:64 Enter. Caller is handle_level_irq+0xe4/0x1dc
[  290.299547] IRQ:64 Leave
[  290.299552] IRQ:19 Leave
[  290.304633] IRQ:19 unmask_irq
[  290.308503] Enter mcp251xfd_irq

-This time rx-int is low and the SPI read of the interrupt registers show no int is pending.

[  290.311515] Leave mcp251xfd_irq (Normal)
[  290.314669] IRQ:64 unmask_irq




cat /proc/interrupts shows this after receiving one frame:

16:      10389      INTC  68 Level     clockevent
 17:          0      INTC   3 Level     arm-pmu
 19:          2      INTC  96 Level     44e07000.gpio
 20:        510      INTC  72 Level     44e09000.serial
 21:       3239      INTC  70 Level     44e0b000.i2c
 24:          0      INTC  75 Level     rtc0
 25:          0      INTC  76 Level     rtc0
 28:          0      INTC  71 Level     4802a000.i2c
 34:          0      INTC  98 Level     4804c000.gpio
 35:       2692      INTC  64 Level     mmc0
 37:          0      INTC 125 Level     481a0000.spi
 40:          0      INTC  32 Level     481ac000.gpio
 41:          0      INTC  62 Level     481ae000.gpio
 42:        315      INTC  28 Level     mmc1
 43:          0      INTC 111 Level     48310000.rng
 45:          0      INTC  41 Level     4a100000.ethernet
 46:          0      INTC  42 Level     4a100000.ethernet
 48:        967      INTC  12 Level     49000000.dma_ccint
 50:          1      INTC  14 Level     49000000.dma_ccerrint
 59:          0      INTC   7 Level     tps65217-irq
 62:          0  tps65217   2 Edge      tps65217_pwrbutton
 63:          0  481ac000.gpio   5 Level     spi1.0
 64:          2  44e07000.gpio  22 Level     spi1.1
 65:          0  44e07000.gpio   6 Edge      48060000.mmc cd


Where
19:          2      INTC  96 Level     44e07000.gpio
64:          2  44e07000.gpio  22 Level     spi1.1
Is the problem. 



This is the dts part of the mcp2518fd's:
/*
 * Device tree overlay for mcp2518fd on spi1.0 and spi1.1
 */

#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/interrupt-controller/irq.h>

&am33xx_pinmux{
        pinctrl_spi1_pins: pinctrl_spi1_pins {
                pinctrl-single,pins = <
                        AM33XX_IOPAD(0x990, PIN_INPUT | MUX_MODE3) /* (A13) mcasp0_aclkx.spi1_sclk */
                        AM33XX_IOPAD(0x994, PIN_INPUT | MUX_MODE3) /* (B13) mcasp0_fsx.spi1_d0 */
                        AM33XX_IOPAD(0x998, PIN_INPUT | MUX_MODE3) /* (D12) mcasp0_axr0.spi1_d1 */
                        AM33XX_IOPAD(0x96c, PIN_OUTPUT_PULLUP | MUX_MODE5) /* (E17) uart0_rtsn.spi1_cs0         CleANopen       LEFT*/
                        AM33XX_IOPAD(0x9b0, PIN_OUTPUT_PULLUP | MUX_MODE4) /* (A15) xdma_event_intr0.spi1_cs1   SAWM CAN        RIGHT*/
                >;
        };

        can0_int_pins: can0_int_pins {
                pinctrl-single,pins = <
                /*CleANopen*/
                AM33XX_IOPAD(0x89c, PIN_INPUT_PULLUP | MUX_MODE7) /* (T6) gpmc_be0n_cle.gpio2[5]        nINT            */
                AM33XX_IOPAD(0x968, PIN_INPUT_PULLUP | MUX_MODE7) /* (E18) uart0_ctsn.gpio1[8]          nINT1           */
                >;
        };

        can1_int_pins: can1_int_pins {
                pinctrl-single,pins = <
                /*SAWM CAN*/
                AM33XX_IOPAD(0x820, PIN_INPUT_PULLUP | MUX_MODE7) /* (U10) gpmc_ad8.gpio0[22]   nINT            */
                AM33XX_IOPAD(0x8c8, PIN_INPUT_PULLUP | MUX_MODE7) /* (U3) lcd_data10.gpio2[16]  nINT1           */
                >;
        };
};



/{
        /* external 40M oscillator of mcp2518fd on SPI1.0 */
        mcp2518fd_can0_osc: mcp2518fd_can0_osc {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <40000000>;
        };
};


/{
        /* external 40M oscillator of mcp2518fd on SPI1.1 */
        mcp2518fd_can1_osc: mcp2518fd_can1_osc {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <40000000>;
        };
};

/* the spi config of the can-controller itself binding everything together */
&spi1{
    #address-cells = <1>;
    #size-cells = <0>;

    status = "okay";
    pinctrl-names = "default";
    pinctrl-0 = <&pinctrl_spi1_pins>;

    /*CleANopen*/
    can@0 {
        compatible = "microchip,mcp2518fd";
        reg = <0>;
        clocks = <&mcp2518fd_can0_osc>;
        pinctrl-names = "default";
        pinctrl-0 = <&can0_int_pins>;
        spi-max-frequency = <20000000>;

        interrupt-parent = <&gpio2>;
        interrupts = <5 IRQ_TYPE_LEVEL_LOW>;

        interrupts-extended = <&gpio2 5 IRQ_TYPE_LEVEL_LOW>;
        microchip,rx-int-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
    };

    can@1 {
        compatible = "microchip,mcp2518fd";
        reg = <1>;
        clocks = <&mcp2518fd_can1_osc>;
        pinctrl-names = "default";
        pinctrl-0 = <&can1_int_pins>;
        spi-max-frequency = <20000000>;

        interrupt-parent = <&gpio0>;
        interrupts = <22 IRQ_TYPE_LEVEL_LOW>;

        interrupts-extended = <&gpio0 22 IRQ_TYPE_LEVEL_LOW>;
        microchip,rx-int-gpios = <&gpio2 16 GPIO_ACTIVE_LOW>;
    };
};


Any thought on why I see this behavior?

Regards
Markus 

Re: GPIO level IRQ fires twice each time.

[Kent Gibson]

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.

Sv: GPIO level IRQ fires twice each time.

[Markus Mirevik]

> 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.

Regards
Markus 

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

[Kent Gibson]

On Mon, Jan 24, 2022 at 07:12:31AM +0000, 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.
> 

 Why is a missed edge inevitable?

Cheers,
Kent.

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

[Lars-Peter Clausen]

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/commit/drivers/gpio/gpio-zynq.c?id=6dd859508336f0fd078fd62f3b9fe42a32aa38e2

- Lars

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

[Lars-Peter Clausen]

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/commit/drivers/gpio/gpio-zynq.c?id=6dd859508336f0fd078fd62f3b9fe42a32aa38e2
>
> - Lars
>
Sorry, I meant `Depending on how the am335x interrupt controller works...`

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

[Grygorii Strashko]

Hi Markus,

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/commit/drivers/gpio/gpio-zynq.c?id=6dd859508336f0fd078fd62f3b9fe42a32aa38e2
>>
>> - 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.

-- 
Best regards,
Grygorii, Ukraine

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

[Markus Mirevik]

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

Re: GPIO level IRQ fires twice each time.

[Markus Schneider-Pargmann]

Hi everyone,

On Thu, Apr 14, 2022 at 08:39:21AM +0000, Markus Mirevik wrote:
> 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!!

I just found this thread searching for the exact same issue, and the
patch works for me as well. Is there a patch somewhere already? Checking
6.1-rc it doesn't seem to have been submitted?

I am not familiar with the gpio-omap driver at the moment, so can we
submit this patch or is it just a hack?

Best,
Markus