flexcan: Acknowledge all interrupt sources in the IRQ handler

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

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:

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

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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html