Re: [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled

[Wolfgang Grandegger <wg@grandegger.com>]

Hi Bhupesh,

On 02/22/2012 12:11 PM, Bhupesh SHARMA wrote:
>> -----Original Message-----
>> From: linux-can-owner@vger.kernel.org [mailto:linux-can-
>> owner@vger.kernel.org] On Behalf Of Wolfgang Grandegger
>> Sent: Wednesday, February 22, 2012 4:29 PM
>> To: Alexander Stein
>> Cc: Marc Kleine-Budde; linux-can@vger.kernel.org
>> Subject: Re: [PATCH 1/2] pch_can: Stop netif queue if last message
>> buffer is filled
>>
>> On 02/21/2012 05:30 PM, Alexander Stein wrote:
>>> Hello Wolfgang,
>>>
>>> Am Dienstag, 21. Februar 2012, 17:25:57 schrieb Wolfgang Grandegger:
>>>> Hi Alexander,
>>>>
>>>> On 02/21/2012 05:00 PM, Alexander Stein wrote:
>>>>> If all Tx buffers are empty and we start queuing frames (more than
>> Tx
>>>>> buffer size) beginning at the last message buffer we will override
>> the
>>>>> first inserted entry if it doesn't get sent meanwhile, e.g. low
>> priority
>>>>> or ACK error. There is no stop condition when all buffers are used.
>>>>> We will get get awoken by pch_can_tx_complete when the last entry
>> is sent.
>>>>>
>>>>> Signed-off-by: Alexander Stein <alexander.stein@systec-
>> electronic.com>
>>>>> ---
>>>>>  drivers/net/can/pch_can.c |    4 +---
>>>>>  1 files changed, 1 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
>>>>> index 6edc25e..3af5341 100644
>>>>> --- a/drivers/net/can/pch_can.c
>>>>> +++ b/drivers/net/can/pch_can.c
>>>>> @@ -903,9 +903,7 @@ static netdev_tx_t pch_xmit(struct sk_buff
>> *skb, struct net_device *ndev)
>>>>>
>>>>>  	tx_obj_no = priv->tx_obj;
>>>>>  	if (priv->tx_obj == PCH_TX_OBJ_END) {
>>>>> -		if (ioread32(&priv->regs->treq2) & PCH_TREQ2_TX_MASK)
>>>>> -			netif_stop_queue(ndev);
>>>>> -
>>>>> +		netif_stop_queue(ndev);
>>>>>  		priv->tx_obj = PCH_TX_OBJ_START;
>>>>>  	} else {
>>>>>  		priv->tx_obj++;
>>>>
>>>> The pch_can uses a C_CAN core (and for maintenance reasons we would
>> like
>>>> to get ride of it sooner than later). In that driver the queue is
>>>> stopped here:
>>>>
>>>> http://lxr.linux.no/#linux+v3.2.7/drivers/net/can/c_can/c_can.c#L508
>>>
>>> Does the C_CAN core have a real fifo? The message "fifo" in PCH CAN
>> just uses the first message object. So you will reorder message under
>> some conditions if used like a real fifo.
>>
>> My understanding is that the CAN controller cores are identical. What
>> the driver actually uses might be different. Out of order transmission
>> of message are *not* allowed and the software must take care. With a
>> real fifo that might be tricky, indeed.
> 
> The C_CAN core has no real FIFO. The same needs to be implemented via SW.
> This is necessary because the message objects are filled/emptied as per the
> message object priority. As soon as you mark the Message Object 0 as "free"
> incoming messages will be stored here (if you are using Msg 0 for Rx purposes),
> otherwise this message object will be Tx'ed first (if you are using Msg 0 for
> Tx purposes).
> 
> Note: The C_CAN UM talks about a possibility of configuring a FIFO for Rx objects
> but you need SW mechanisms to again maintain "in-order" reception.

Thanks for clarification.

>>>> Therefore I'm not sure if your fix is enough (without looking into
>> the
>>>> manual). I could send you a "pch_pci" driver patch for C_CAN, which
>>>> would allow you to use it. What kernel version are you using?
>>>
>>> We are currently using 2.6.39.
>>
>> OK, I'm going to prepare a patch for that kernel version first.
>>
>> More soon...
>>
> 
> I have made a series of changes in my C_CAN driver which I need to merge
> and submit to mainline.
> 
> Will do that in coming days. It is on my TO-DO list for long now :(

Well, that's almost normal.

Wolfgang.