[PATCH] flexcan: Acknowledge all interrupt sources in the IRQ handler

[PATCH] flexcan: Acknowledge all interrupt sources in the IRQ handler

[Lothar Waßmann]

Otherwise the handler will get stuck in an endless IRQ loop when an
interrupt condition occurs that is not being acked (e.g. TWRN)

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/net/can/flexcan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index e023379..ea8f04d 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -577,7 +577,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 
 	reg_iflag1 = flexcan_read(&regs->iflag1);
 	reg_esr = flexcan_read(&regs->esr);
-	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
+	flexcan_write(reg_esr, &regs->esr);	/* ACK all IRQs */
 
 	/*
 	 * schedule NAPI in case of:
-- 
1.5.6.5

Re: [PATCH] flexcan: Acknowledge all interrupt sources in the IRQ handler

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

On 12/09/2011 02:47 PM, Lothar Waßmann wrote:
> Otherwise the handler will get stuck in an endless IRQ loop when an
> interrupt condition occurs that is not being acked (e.g. TWRN)

On which CPU do you have this problem?

Seems that mx25/35 behave a bit different than mx28. But I had no time
to dig into this, yet. BTW Wolfgang is just reworking error handling,
can you please test his patches he recently posted on linux-can.

cheers, Marc


> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  drivers/net/can/flexcan.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index e023379..ea8f04d 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -577,7 +577,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  
>  	reg_iflag1 = flexcan_read(&regs->iflag1);
>  	reg_esr = flexcan_read(&regs->esr);
> -	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
> +	flexcan_write(reg_esr, &regs->esr);	/* ACK all IRQs */
>  
>  	/*
>  	 * schedule NAPI in case of:


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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] flexcan: Acknowledge all interrupt sources in the IRQ handler

[Lothar Waßmann]

Hi,

Marc Kleine-Budde writes:
> On 12/09/2011 02:47 PM, Lothar Waßmann wrote:
> > Otherwise the handler will get stuck in an endless IRQ loop when an
> > interrupt condition occurs that is not being acked (e.g. TWRN)
> 
> On which CPU do you have this problem?
> 
on i.MX28.

> Seems that mx25/35 behave a bit different than mx28. But I had no time
> to dig into this, yet. BTW Wolfgang is just reworking error handling,
> can you please test his patches he recently posted on linux-can.
> 
The ESR of i.MX25 is completely identical to the i.MX28.
You should be able to reproduce the problem when trying to send a
message to a CAN interface with the transceiver disabled.
You will get a BIT0_ERR and the TWRN bit will be asserted and never
cleared leading to an endless interrupt loop.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

Re: [PATCH] flexcan: Acknowledge all interrupt sources in the IRQ handler

[Wolfgang Grandegger]

On 12/09/2011 02:59 PM, Lothar Waßmann wrote:
> Hi,
> 
> Marc Kleine-Budde writes:
>> On 12/09/2011 02:47 PM, Lothar Waßmann wrote:
>>> Otherwise the handler will get stuck in an endless IRQ loop when an
>>> interrupt condition occurs that is not being acked (e.g. TWRN)
>>
>> On which CPU do you have this problem?
>>
> on i.MX28.

Yes, it is definitely needed on i.MX28 and I already have it in my
series. See:

https://gitorious.org/~wgrandegger/linux-can/wg-linux-can-next/commit/8ad94fa0dd7f7728824fa8fd4479390ac3f189c7

BTW: at similar patch was already sent by Reuben Dowle.

>> Seems that mx25/35 behave a bit different than mx28. But I had no time
>> to dig into this, yet. BTW Wolfgang is just reworking error handling,
>> can you please test his patches he recently posted on linux-can.
>>
> The ESR of i.MX25 is completely identical to the i.MX28.
> You should be able to reproduce the problem when trying to send a
> message to a CAN interface with the transceiver disabled.
> You will get a BIT0_ERR and the TWRN bit will be asserted and never
> cleared leading to an endless interrupt loop.

If you are right, the code was never working on i.MX25/35... which I doubt.

Wolfgang.

Re: [PATCH] flexcan: Acknowledge all interrupt sources in the IRQ handler

[Wolfgang Grandegger]

On 12/09/2011 03:35 PM, Wolfgang Grandegger wrote:
> On 12/09/2011 02:59 PM, Lothar Waßmann wrote:
>> Hi,
>>
>> Marc Kleine-Budde writes:
>>> On 12/09/2011 02:47 PM, Lothar Waßmann wrote:
>>>> Otherwise the handler will get stuck in an endless IRQ loop when an
>>>> interrupt condition occurs that is not being acked (e.g. TWRN)
>>>
>>> On which CPU do you have this problem?
>>>
>> on i.MX28.
> 
> Yes, it is definitely needed on i.MX28 and I already have it in my
> series. See:
> 
> https://gitorious.org/~wgrandegger/linux-can/wg-linux-can-next/commit/8ad94fa0dd7f7728824fa8fd4479390ac3f189c7
> 
> BTW: at similar patch was already sent by Reuben Dowle.
> 
>>> Seems that mx25/35 behave a bit different than mx28. But I had no time
>>> to dig into this, yet. BTW Wolfgang is just reworking error handling,
>>> can you please test his patches he recently posted on linux-can.
>>>
>> The ESR of i.MX25 is completely identical to the i.MX28.
>> You should be able to reproduce the problem when trying to send a
>> message to a CAN interface with the transceiver disabled.
>> You will get a BIT0_ERR and the TWRN bit will be asserted and never
>> cleared leading to an endless interrupt loop.
> 
> If you are right, the code was never working on i.MX25/35... which I doubt.

I know remember that the old code was working on a MX35PDK board.
Actually I did the porting some time ago. That means that we have to
deal with two variants of the Flexcan controller, unfortunately:

mx25/35: State changes *and* bus errors are both signalled via ESR ERR
	bit. The TWRN, RWRN and BOFF bits seem not to be used. Therefore
	bus error reporting cannot be enabled/disabled individually.

mx28/mx53?: Bus error are signalled via ESR ERR bit and state changes
	via TWRN, RWRN and BOFF bit. Therefore bus error reporting
	*can* be enabled/disabled individually as implemented by this
	patch:

	https://gitorious.org/~wgrandegger/linux-can/wg-linux-can-next/commit/f0829d269a451c8abb99435d5a1a63cb18566668

I will try to get a MX35PDK board for further evaluation and development.

Wolfgang.