Re: [PATCH v8] can: grcan: Add device driver for GRCAN and GRHCAN cores

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

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

On 11/14/2012 04:07 PM, Andreas Larsson wrote:
> On 2012-11-14 12:22, Marc Kleine-Budde wrote:
>> On 11/14/2012 12:02 PM, Andreas Larsson wrote:
>>> On 2012-11-14 09:43, Marc Kleine-Budde wrote:
>>>> Handle incoming events (rx or tx-complete) until:
>>>> a) number of handled events == budget
>>>> or
>>>> b) no more events pending.
>>>>
>>>>      while (work_done < budget && interrupts_pending()) {
>>>>          work_done += handle_rx(budget - work_done);
>>>>          work_done += handle_tx(budget - work_done);
>>>>      }
>>>
>>> That could starve handle_tx completely though under high rx pressure,
>>> but I can prevent that by making sure that half of the budget is held
>>> back in the first call to handle_rx.
>>
>> What about making the budget big enough to handle both rx and tx in one
>> napi call. Have a look at the marvell driver [1] for inspiration.
> 
> Even if I set the budget to something large, unless I limit the rx side
> in some way it could go on multiple rounds around the circular buffer
> until it have used all the budget. So in some way or another,
> grcan_receive must be hindered from using all the budget.
> 
> Sorry, but I am not sure what aspect of the marvell driver poll handling
> you want me to mimic. Without having analyzed exactly how the queueing
> works yet, it seems to make sure that every function that is called from
> the poll function gets ample opportunity to do work by not letting one
> function hogging all the budget. If you want me to mimic it in the
> aspect of doing work in series of 16 or something like that, sure, no
> problem. If it is something else you want to point to, please let me
> know what.
> 
> 
> The simplest way to make sure that both tx and rx gets to run is to take
> inspiration from the ethoc driver [1] and just let the tx side get just
> as much budget as the rx side and be done with it. With the echo frames
> for can, the budget should be halved for each I guess to make sure no
> more frames are delivered than the budget, but apart from that I don't
> see the problem with such a simple approach.

Good point, if there already is an implementation, do it that way.

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