Re: MCP251xFD runs interrupt handler twice per message.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17.01.2022 11:55:31, Markus Mirevik wrote:
> > > > You mean mcp251xfd_irq() is started twice?
> > >
> > > Yes.
> > 
> > That's not the way it's supposed to be. Maybe it's a limitation of the am335x
> > IRQ controller.
> 
> What kind of limitation could that be?

Some kind of limitation of the hardware. Don't know, SoC may have all
kinds of bugs :)

[...]

> > > > Can you test again with IRQ_TYPE_LEVEL_LOW and no rx-int-gpios.
> > > > Please instrument the beginning and the returns of the
> > > > mcp251xfd_irq() function to check if it's really started twice.
> > > > Please print the
> > > > priv->regs_status.intf in every loop. Can you record the gpio2_5
> > > > priv->with a
> > > > scope.
> > >
> > > Yes I can but what do you mean with Instrument?
> > 
> > That's a fancy way of saying: please add printk() :)
> > 
> > > And no scope until Monday.
> > 
> > no problem
> > 
> > > > Make sure that you don't send any CAN frames as reaction of reception.
> > >
> > > How do I make sure of that? :/
> > 
> > The Linux kernel doesn't send any CAN frames on its own. You need a user
> > space program to do so. Make sure you don't have any user space program
> > running that's sending CAN frames.
> > 
> 
> No user space program running that would send can frames. And no can frames received on the other side.  
> 
> With IRQ_TYPE_LEVEL_LOW and the rx-int-gpios disabled I have added a
> call to netdev_info at the top of the function:

For better readability register values are usually printed in hex
(0x%08x).

> ...
>         netdev_info(priv->ndev, "mcp251xfd_irq mcp251xfd_regs_status.intf: %d", priv->regs_status.intf );

Note: the "intf" contains old information here, because in this run of
the interrupt handler the intf has not been read so far.

>         if (priv->rx_int) {
> ...
> 
> and here:
> ... 
>                if (!(intf_pending)) {
>                         netdev_info(priv->ndev, "!intf_pending mcp251xfd_regs_status.intf: %d", priv->regs_status.intf );
>                         return handled;
>                 }
>                 netdev_info(priv->ndev, "intf_pending mcp251xfd_regs_status.intf: %d", priv->regs_status.intf );
> ...
> 
> Resulting in this when receiving one can message: 
> 
> [  987.486691] mcp251xfd spi1.1 can1: mcp251xfd_irq mcp251xfd_regs_status.intf: -1088815100
> [  987.494987] mcp251xfd spi1.1 can1: intf_pending mcp251xfd_regs_status.intf:  -1088815098
> [  987.503239] mcp251xfd spi1.1 can1: !intf_pending mcp251xfd_regs_status.intf: -1088815100

2nd loop starts here:

> [  987.511423] mcp251xfd spi1.1 can1: mcp251xfd_irq mcp251xfd_regs_status.intf: -1088815100
> [  987.519603] mcp251xfd spi1.1 can1: !intf_pending mcp251xfd_regs_status.intf: -1088815100
> 
> mcp251xfd_regs_status.intf translated to binary 
> 1	1111 1111 1111 1111 1111 1111 1111 1111 1011 1111 0001 1010 0000 0000 0000 0100
> 2	1111 1111 1111 1111 1111 1111 1111 1111 1011 1111 0001 1010 0000 0000 0000 0110
> 3	1111 1111 1111 1111 1111 1111 1111 1111 1011 1111 0001 1010 0000 0000 0000 0100

2nd loop starts here:

> 4	1111 1111 1111 1111 1111 1111 1111 1111 1011 1111 0001 1010 0000 0000 0000 0100
> 5	1111 1111 1111 1111 1111 1111 1111 1111 1011 1111 0001 1010 0000 0000 0000 0100
> 
> I have also attached a scope picture, this is without the printouts.

good idea, the printk usually takes much time.

That scope picture looks good! Can you include both runs of the IRQ
handler in one scope picture? I'm interested if there is activity or a
glich on IRQ line.

After you've that picture, you can try to catch glitches on the IRQ
line. If your scope allows this, setup a trigger on the INT channel that
triggers if the channel is low for considerably less then it takes to
clear the IRQ.

Currently it takes about 160µs to clear the IRQ, so setup a trigger for
less than 50µs.

> And if I'm reading it correctly it confirms that it runs twice. 

ACK

> 1 read pending IRQs [*]
> -RXIF-
> 2 read RX-FIFO level [*]
> 3 read RX'ed CAN frames [*]
> 4 and 5 mark RX'ed CAN frames as read [*]
> (This is the 5.10.79 version that uses regmap_update_bits which I
>  assume uses one read and one write instruction.)

ACK - in the CRC mode (which is default) the regmap_update_bits has not
been optimized, it does a read/modify/write.

> -loop-
> 6 read pending IRQs [*]
> -Exit-
> 7 read pending IRQs [*]
> 
> Confirmd with printouts:
> [   26.990324] mcp251xfd spi1.1 can1: mcp251xfd_irq()
> [   26.995173] mcp251xfd spi1.1 can1: * Bulk read MCP251XFD_REG_INT
> [   27.001371] mcp251xfd spi1.1 can1: Interrupts pending, handling.
> [   27.007435] mcp251xfd spi1.1 can1: * Read RX MCP251XFD_REG_FIFOSTA
> [   27.013676] mcp251xfd spi1.1 can1: * Bulk read. Register: 1248 count: 5
> [   27.020226] mcp251xfd spi1.1 can1: * Update bits MCP251XFD_REG_FIFOCON_UINC
> [   27.027318] mcp251xfd spi1.1 can1: * Bulk read MCP251XFD_REG_INT
> [   27.033378] mcp251xfd spi1.1 can1: No interrupts pending, returning.
> [   27.039805] mcp251xfd spi1.1 can1: mcp251xfd_irq()
> [   27.044617] mcp251xfd spi1.1 can1: * Bulk read MCP251XFD_REG_INT
> [   27.050693] mcp251xfd spi1.1 can1: No interrupts pending, returning.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux