Subject: Re: [PATCH v6] flexcan: add err_irq handler for flexcan

From: Marc Kleine-Budde <[email-masked]>
Date: Thu, 03 Jul 2014 12:13:24 +0200

On 07/03/2014 11:22 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 <[email-masked]>
> ---
> 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 
> Changes for v4:
> 	- call flexcan_irq 
> Changes for v5:
> 	- move err_int_handling code from flexcan_irq to flexcan_err_irq
> 	- call flexcan_err_irq from flexcan_irq
> Changes for v6:
> 	- move RX_IRQ handling back to flexcan_irq
> 
>  drivers/net/can/flexcan.c | 52 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f425ec2..f6c92bc 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;
> +	int err_irq;
>  
>  	struct clk *clk_ipg;
>  	struct clk *clk_per;
> @@ -690,6 +691,28 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
>  	return work_done;
>  }
>  
> +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;
> +	irqreturn_t ret = IRQ_NONE;
> +
> +	reg_esr = flexcan_read(&regs->esr);
> +	reg_ctrl = flexcan_read(&regs->ctrl);
> +
> +	if (reg_esr & FLEXCAN_ESR_ALL_INT) {
> +		flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
> +		ret = IRQ_HANDLED;
> +	}
> +
> +	if (reg_esr & FLEXCAN_ESR_ERR_INT)
> +		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);

Now you just ACK the interrupts, but you're not triggering the NAPI,
where the actual error interrupts are handled.

In the current interrupt handler we are looking at two registers for
interrupts, that is "iflag1" and "esr". "iflag1" is triggered on RX,
RX-FIFO overflow and TX-complete. "esr" is triggered by errors in
general. In the interrupt handler we distinguish between bus errors
(FLEXCAN_ESR_ERR_BUS) and errors effecting a change of state
(FLEXCAN_ESR_ERR_STATE).

The next step is to figure out which bits in which register trigger the
error irq. Then move the code handling this register into the error irq
handler. As the seconds half of the error irq handling takes place in
the napi function, you have to duplicate the 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   |