Re: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan

[Marc Kleine-Budde <mkl@pengutronix.de>]

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

On 06/23/2014 10:15 AM, qiang.zhao@freescale.com wrote:
[...]
>>>>> +	reg_esr = flexcan_read(&regs->esr);
>>>>> +	reg_ctrl = flexcan_read(&regs->ctrl);
>>>>> +	if (reg_esr & FLEXCAN_ESR_TX_WRN) {
>>>>
>>>> When does the hardware trigger the interrupt?
>>>
>>> When there is no wire link between tx and rx, tx start transfer and
>> doesn’t get the ack.
>>
>> You are testing for the warning interrupt, not for the
>> FLEXCAN_ESR_ACK_ERR (which is triggered there isn't any ACK).
>>
>>>>> +		flexcan_write(reg_esr & ~FLEXCAN_ESR_TX_WRN, &regs->esr);
>>>>> +		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
>>>>> +		netif_stop_queue(dev);
>>>>
>>>> Why are you stopping the txqueue?
>>>
>>> There is no wire link, tx can't transfer successfully.
>>
>> You are testing for the warning interrupt, which is triggered if the
>> error counter increases from 95 to 96. And the error counter can increase
>> due to several reasons. No link is only one of them. If the CAN core
>> cannot transmit new packages any more the flow control in the driver will
>> take care.
> 
> When Tx error counter increases from 95 to 96, there must be issue for tx,
> So why can't I stop the txqueue? 

Why do you want to stop the queue? It's compliant with the CAN spec to
keep sending CAN frames if the error counter increases to 96. If there
isn't any problem with the CAN bus anymore, the TX error counters will
decrease with every successfully transmitted CAN frame.

> You said that there are several reasons, would you like to take some examples?

See CAN Error Confinement Rules in
http://www.can-wiki.info/doku.php?id=can_faq_erors

>> What about calling the normal interrupt if er err_irq occurs, as this
>> function will take care of both normal and error interrupts anyway?
> 
> Calling the normal interrupt doesn't work.

Why?

flexcan_irq() if FLEXCAN_CTRL_TWRN_MSK is set and will schedule the NAPI
routine:

> 	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
> 	    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
> 	    flexcan_has_and_handle_berr(priv, reg_esr)) {
[...]
> 		napi_schedule(&priv->napi);
> 	}

Marc

-- 
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: 242 bytes --]