Re: [PATCH 1/5] net: can: flexcan: provide propper return code in ISR

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

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

On 07/25/2014 08:16 PM, Matthias Klein wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> flexcan_irq() always returns IRQ_HANDLED no matter if it actually did
> something or not. If the CAN interface is brought up without BERR
> reporting on a SoC which has the FLEXCAN_HAS_BROKEN_ERR_STATE feature
> then ERR reporting is activated but not really handled. That means on an
> open bus one receives a lot of STF_ERR and the only thing that happens
> is that the number of interrupts is incremented.

Yes....although the interrupt is ack'ed.

> This patch ensures that in such a case the core has a chance to detect
> such (or similar) misbehavior and disable the interrupt line.

The problem is, without the enabled bus errors, you have no chance to
detect the warning interrupt, as it is not connected on some SoCs.

However, the series berr_limit addresses the problem, too.

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Matthias Klein <matthias.klein@optimeas.de>
> ---
>  drivers/net/can/flexcan.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index e381142..f677b49 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -641,6 +641,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	u32 reg_iflag1, reg_esr;
> +	int worked = 0;
>  
>  	reg_iflag1 = flexcan_read(&regs->iflag1);
>  	reg_esr = flexcan_read(&regs->esr);
> @@ -667,6 +668,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
>  		       &regs->ctrl);
>  		napi_schedule(&priv->napi);
> +		worked = 1;
>  	}
>  
>  	/* FIFO overflow */
> @@ -674,6 +676,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
>  		dev->stats.rx_over_errors++;
>  		dev->stats.rx_errors++;
> +		worked = 1;
>  	}
>  
>  	/* transmission complete interrupt */
> @@ -683,9 +686,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		can_led_event(dev, CAN_LED_EVENT_TX);
>  		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
>  		netif_wake_queue(dev);
> +		worked = 1;
>  	}
> -
> -	return IRQ_HANDLED;
> +	if (worked)
> +		return IRQ_HANDLED;
> +	else
> +		return IRQ_NONE;
>  }
>  
>  static void flexcan_set_bittiming(struct net_device *dev)
> 

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