Re: [PATCH 09/15] can: rx-fifo: Add can_rx_fifo_reset() function

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

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

On 10/10/2014 05:46 PM, David Jander wrote:
> This function needs to be called every time the CAN controller is reset
> and before interrupts are enabled. Otherwise the irq-offload loop gets
> out of sync. Detect this and BUG() to the user if he forgot.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> ---
>  drivers/net/can/dev.c   | 20 ++++++++++++++++++--
>  include/linux/can/dev.h |  1 +
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 3e1160a..fc35d28 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -487,8 +487,17 @@ int can_rx_fifo_irq_offload(struct can_rx_fifo *fifo)
>  		fifo->second_first = true;
>  	}
>  
> -	if (received)
> -		napi_schedule(&fifo->napi);
> +	if (received) {
> +		can_rx_fifo_napi_schedule(fifo);
> +	} else {
> +		/*
> +		 * This should only happen if the CAN conroller was reset, but
> +		 * can_rx_fifo_reset() was not called. BUG();
> +		 */
> +		netdev_warn(fifo->dev, "%s: No messages found,"
> +			    " RX-FIFO out of sync?\n", __func__);
> +		BUG();

Can you replace the BUG() (which crashes the system) by something less
fatal? E.g. by some of the WARN function [1]? Does it make sense to call
the fifo_reset() function after this? We'll loose up to a FIFO size
amount of CAN frames, but the system stays hopefully working.

Marc

[1] http://lxr.free-electrons.com/source/include/asm-generic/bug.h#L84

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