Re: [PATCH] c_can: Add support for eg20t (pch_can)

[Alexander Stein <alexander.stein@systec-electronic.com>]

On Monday 07 April 2014 18:27:11, Thomas Gleixner wrote:
> > > So it would be interesting to see, whether this event is aligned to
> > > the lost packets you are hunting:
> > > 
> > >     P1 -> P2 -> WARN -> P4
> > > 
> > > A few trace printks should tell us pretty fast.
> > 
> > I slightly modified my patch (see below) to see which obj is marked
> > with NEWDAT wrongly.  It turns out it is spread over many message
> > objects, but object 1 is notably the one most occuring.  Can it
> > happen, that NEWDAT1 already shows the bit while the NEWDAT bit is
> > not updated yet?  Or does commit c0a9f4d39 affect this status
> > because INTPEND is cleared before reading the message object now?
> 
> That's what we do not know. On D_CAN it result in hardware silently
> dropping packets when we reneable the lower message buffers by
> clearing the NEWDAT bit. So C_CAN might show some different behaviour
> by giving us objects with the NEWDAT bit cleared.
> 
> That's why I came up with the patch which disables the RX Split. With 
> CONFIG_CAN_C_CAN_STRICT_FRAME_ORDERING=n we clear NEWDAT and INTPEND
> immediately. That's what the docs recommend (including the eg20t one).
> We trade the lost packets against the potential reordering.
> 
> What really buggers me is that this does not work with your
> hardware. I'll whip up a tracing patch later tonight.

So I noticed that with CONFIG_CAN_C_CAN_STRICT_FRAME_ORDERING=n (NEWDAT and INTPEND cleared immediately) the "odd behavior" occurs pretty often.
Before
> c_can_rx_object_get(dev, obj);
NEWDAT is still set, but afterwards it is already cleared in C_CAN_NEWDAT1_REG.
I guess the NEWDAT is already cleared in the message object before reading from the message object with this line
> ctrl = priv->read_reg(priv, C_CAN_IFACE(MSGCTRL_REG, IF_RX));

So I removed that IF_MCONT_NEWDAT check (see below) and I didn't lose any message, but get 5 duplicates in 250000 messages (no reordering though).

Setting CONFIG_CAN_C_CAN_STRICT_FRAME_ORDERING=y with that patch I lose ~650msg per 250000 frames.

Regards,
Alexander

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 0e9f974..dd75360 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -831,14 +831,6 @@ static int c_can_read_objects(struct net_device *dev, struct c_can_priv *priv,
 			continue;
 		}
 
-		/*
-		 * This really should not happen, but this covers some
-		 * odd HW behaviour. Do not remove that unless you
-		 * want to brick your machine.
-		 */
-		if (!(ctrl & IF_MCONT_NEWDAT))
-			continue;
-
 		/* read the data from the message object */
 		c_can_read_msg_object(dev, IF_RX, ctrl);
 

-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082