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

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

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

On 06/23/2014 09:26 AM, qiang.zhao@freescale.com wrote:
> 
> On 06/23/2014 03:18 PM, Marc Kleine-Budde wrote:
> 
>>
>> On 06/23/2014 09:11 AM, Zhao Qiang wrote:
>>> when flexcan is not physically linked, command 'cantest' will trigger
>>> an err_irq, add err_irq handler for it.
>>>
>>> Signed-off-by: Zhao Qiang <B45475@freescale.com>
>>> ---
>>> Changes for v2:
>>> 	- use a space instead of tab
>>> 	- use flexcan_poll_state instead of print Changes for v3:
>>> 	- return IRQ_HANDLED if err is triggered
>>> 	- stop transmitted packets when there is an err_interrupt
>>>
>>>  drivers/net/can/flexcan.c | 35 ++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 34 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>> index f425ec2..6802a25 100644
>>> --- a/drivers/net/can/flexcan.c
>>> +++ b/drivers/net/can/flexcan.c
>>> @@ -208,6 +208,7 @@ struct flexcan_priv {
>>>  	void __iomem *base;
>>>  	u32 reg_esr;
>>>  	u32 reg_ctrl_default;
>>> +	unsigned int err_irq;
>>>
>>>  	struct clk *clk_ipg;
>>>  	struct clk *clk_per;
>>> @@ -744,6 +745,24 @@ static irqreturn_t flexcan_irq(int irq, void
>> *dev_id)
>>>  	return IRQ_HANDLED;
>>>  }
>>>
>>> +static irqreturn_t flexcan_err_irq(int irq, void *dev_id) {
>>> +	struct net_device *dev = dev_id;
>>> +	struct flexcan_priv *priv = netdev_priv(dev);
>>> +	struct flexcan_regs __iomem *regs = priv->base;
>>> +	u32 reg_ctrl, reg_esr;
>>> +
>>> +	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.

What about calling the normal interrupt if er err_irq occurs, as this
function will take care of both normal and error interrupts anyway?

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