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

[qiang.zhao at freescale.com]

On 06/23/2014 03:37 PM, Marc Kleine-Budde wrote:

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl at pengutronix.de]
> Sent: Monday, June 23, 2014 3:37 PM
> To: Zhao Qiang-B45475; linuxppc-dev at lists.ozlabs.org; linux-
> can at vger.kernel.org; wg at grandegger.com; Wood Scott-B07421
> Subject: Re: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
> 
> On 06/23/2014 09:26 AM, qiang.zhao at 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 at 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.

When Tx error counter increases from 95 to 96, there must be issue for tx,
So why can't I stop the txqueue? 
You said that there are several reasons, would you like to take some examples?

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

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