Re: [PATCH 11/15] can: rx-fifo: Add support for can state tracking and error polling

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

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

On 11/03/2014 02:09 PM, David Jander wrote:
> On Mon, 03 Nov 2014 13:58:43 +0100
> Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> 
>> On 11/03/2014 01:51 PM, David Jander wrote:
>>>> On 10/10/2014 05:46 PM, David Jander wrote:
>>>>> The interrupt handler should store the error flags if needed and call
>>>>> can_rx_fifo_irq_error() if there was an error flag set. This will trigger
>>>>> a napi-poll that will call poll_can_state() and poll_bus_error()
>>>>> callbacks. Those should handle can state machine and send error frames
>>>>> as needed.
>>>>>
>>>>> Signed-off-by: David Jander <david@protonic.nl>
>>>>> ---
>>>>>  drivers/net/can/dev.c   | 23 ++++++++++++++++++++---
>>>>>  include/linux/can/dev.h |  9 +++++++++
>>>>>  2 files changed, 29 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>>>>> index fc35d28..dac7579 100644
>>>>> --- a/drivers/net/can/dev.c
>>>>> +++ b/drivers/net/can/dev.c
>>>>> @@ -352,6 +352,9 @@ static int can_rx_fifo_napi_poll(struct napi_struct
>>>>> *napi, int quota) unsigned int tail;
>>>>>  
>>>>>  restart_poll:
>>>>> +	if (work_done < quota)
>>>>> +		work_done += fifo->poll_can_state(fifo);
>>>>> +
>>>>
>>>> Do we need two callbacks in the poll-loop? I think one should be enough.
>>>> The driver can call as many non-rx related functions as needed then.
>>>> E.g. state change, bus error handling, tx-completion handling, etc...
>>>
>>> Ok. I split it into two handlers, because I saw most drivers doing first
>>> state-handling then message reception and last error handling... in this
>>> particular order. If that order is not important, we can use one poll()
>>> function for both state- and error-handling.
>>> In that case... should the poll() handler be called _before_ or _after_
>>> handling the mailboxes?
>>
>> Following the general rule zero-one-infinity ("Allow none of foo, one of
>> foo, or any number of foo" [1]) one callback should be fine. As we are
>> optimizing for not loosing RX CAN frames I tend to say it should come
>> after rx.
> 
> Hahaha, ok, I got it :)
> Any idea on how to call this function? Just "poll" seems a little bit
> under-documented...

Yes, what about extra_poll, poll_extra?

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