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

On 11/03/2014 01:46 PM, David Jander wrote:
> On Mon, 03 Nov 2014 12:16:24 +0100
> Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> 
>> 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.
> 
> Ok. We just need to make enough noise, so that the potential driver's author
> sees it before release... a WARN*() should do.

ACK, and it will generate a stack-trace, too. However I don't want to
have crashing machines at $CUSTOMER, if there is a subtle bug, what's
not triggered regularly.

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