All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled
@ 2012-02-21 16:00 Alexander Stein
  2012-02-21 16:00 ` [PATCH 2/2] pch_can: (re)start Tx queue at first entry Alexander Stein
  2012-02-21 16:25 ` [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled Wolfgang Grandegger
  0 siblings, 2 replies; 7+ messages in thread
From: Alexander Stein @ 2012-02-21 16:00 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde; +Cc: linux-can, Alexander Stein

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++;
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] pch_can: (re)start Tx queue at first entry
  2012-02-21 16:00 [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled Alexander Stein
@ 2012-02-21 16:00 ` Alexander Stein
  2012-02-21 16:25 ` [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled Wolfgang Grandegger
  1 sibling, 0 replies; 7+ messages in thread
From: Alexander Stein @ 2012-02-21 16:00 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde; +Cc: linux-can, Alexander Stein

If we restart and our queue stopped at the last entry we will already
stop it again after the first message inserted.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 drivers/net/can/pch_can.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 3af5341..8d617c7 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -817,6 +817,9 @@ static void pch_can_start(struct net_device *ndev)
 	pch_can_set_tx_all(priv, 1);
 	pch_can_set_rx_all(priv, 1);
 
+	/* Reset Tx Obj to 1st one */
+	priv->tx_obj = PCH_TX_OBJ_START;
+
 	/* Setting the CAN to run mode. */
 	pch_can_set_run_mode(priv, PCH_CAN_RUN);
 
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled
  2012-02-21 16:00 [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled Alexander Stein
  2012-02-21 16:00 ` [PATCH 2/2] pch_can: (re)start Tx queue at first entry Alexander Stein
@ 2012-02-21 16:25 ` Wolfgang Grandegger
  2012-02-21 16:30   ` Alexander Stein
  1 sibling, 1 reply; 7+ messages in thread
From: Wolfgang Grandegger @ 2012-02-21 16:25 UTC (permalink / raw)
  To: Alexander Stein; +Cc: Marc Kleine-Budde, linux-can

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

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?

Thanks,

Wolfgang.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled
  2012-02-21 16:25 ` [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled Wolfgang Grandegger
@ 2012-02-21 16:30   ` Alexander Stein
  2012-02-22 10:58     ` Wolfgang Grandegger
  0 siblings, 1 reply; 7+ messages in thread
From: Alexander Stein @ 2012-02-21 16:30 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: Marc Kleine-Budde, linux-can

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.

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

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
August-Bebel-Str. 29
D-07973 Greiz

Tel: +49-3661-6279-0, Fax: +49-3661-6279-99
eMail:    Alexander.Stein@systec-electronic.com
Internet: http://www.systec-electronic.com

Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Jena, HRB 205563

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled
  2012-02-21 16:30   ` Alexander Stein
@ 2012-02-22 10:58     ` Wolfgang Grandegger
  2012-02-22 11:11       ` Bhupesh SHARMA
  0 siblings, 1 reply; 7+ messages in thread
From: Wolfgang Grandegger @ 2012-02-22 10:58 UTC (permalink / raw)
  To: Alexander Stein; +Cc: Marc Kleine-Budde, linux-can

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.

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

Wolfgang.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled
  2012-02-22 10:58     ` Wolfgang Grandegger
@ 2012-02-22 11:11       ` Bhupesh SHARMA
  2012-02-22 13:05         ` Wolfgang Grandegger
  0 siblings, 1 reply; 7+ messages in thread
From: Bhupesh SHARMA @ 2012-02-22 11:11 UTC (permalink / raw)
  To: Wolfgang Grandegger, Alexander Stein
  Cc: Marc Kleine-Budde, linux-can@vger.kernel.org

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

> >> 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 :(

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled
  2012-02-22 11:11       ` Bhupesh SHARMA
@ 2012-02-22 13:05         ` Wolfgang Grandegger
  0 siblings, 0 replies; 7+ messages in thread
From: Wolfgang Grandegger @ 2012-02-22 13:05 UTC (permalink / raw)
  To: Bhupesh SHARMA
  Cc: Alexander Stein, Marc Kleine-Budde, linux-can@vger.kernel.org

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-02-22 13:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-21 16:00 [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled Alexander Stein
2012-02-21 16:00 ` [PATCH 2/2] pch_can: (re)start Tx queue at first entry Alexander Stein
2012-02-21 16:25 ` [PATCH 1/2] pch_can: Stop netif queue if last message buffer is filled Wolfgang Grandegger
2012-02-21 16:30   ` Alexander Stein
2012-02-22 10:58     ` Wolfgang Grandegger
2012-02-22 11:11       ` Bhupesh SHARMA
2012-02-22 13:05         ` Wolfgang Grandegger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.