Lite5200 FEC Driver on linux 2.6 broken?

Lite5200 FEC Driver on linux 2.6 broken?

[roger blofeld]

I'm trying to get a simple app running on a Lite5200 and am having a
TCP/IP problem. Using tcpdump/ethereal I have learned that the Lite5200
ethernet works fine until the client sends a TCP retransmission
request. Then the Lite5200 replies with packets having incorrect
checksums. Each successive Lite5200 packet appears to also lose two
bytes of data (i.e., the packet size is correct, but the data is not
the same for each retransmission because two bytes have been dropped
from the start of the payload).

Could this be a problem with the FEC driver? Bestcomm? How can I figure
out what is broken here? Any help gladly accepted!
 
I started with Sylvain Munaut's kernel at
http://www.246tnt.com/mpc52xx/ and have also tried starting with the
linux-2.5 tree and pulling Sylvain's tree into it to see if newer
kernels worked better. I also tried using the recently updated bestcomm
interface from denx.de with these kernels with the same result.

Thanks
-roger



		
__________________________________ 
Do you Yahoo!? 
Check out the new Yahoo! Front Page. 
www.yahoo.com 
 

Re: Lite5200 FEC Driver on linux 2.6 broken?

[Dale Farnsworth]

On Mon, Nov 15, 2004 at 04:32:43PM +0000, roger blofeld wrote:
> I'm trying to get a simple app running on a Lite5200 and am having a
> TCP/IP problem. Using tcpdump/ethereal I have learned that the Lite5200
> ethernet works fine until the client sends a TCP retransmission
> request. Then the Lite5200 replies with packets having incorrect
> checksums. Each successive Lite5200 packet appears to also lose two
> bytes of data (i.e., the packet size is correct, but the data is not
> the same for each retransmission because two bytes have been dropped
> from the start of the payload).
> 
> Could this be a problem with the FEC driver? Bestcomm? How can I figure
> out what is broken here? Any help gladly accepted!
>  
> I started with Sylvain Munaut's kernel at
> http://www.246tnt.com/mpc52xx/ and have also tried starting with the
> linux-2.5 tree and pulling Sylvain's tree into it to see if newer
> kernels worked better. I also tried using the recently updated bestcomm
> interface from denx.de with these kernels with the same result.

Try the following patch on top of Sylvain's tree.  I sent it (and others)
to Sylvain a while back, but he must be busy with other things.

-Dale

--- linux-2.5-mpc52xx-devel/drivers/net/fec_mpc52xx/fec.c	2004-10-12 11:26:55.000000000 -0700
+++ linux-2.5-mpc52xx-devel-df/drivers/net/fec_mpc52xx/fec.c	2004-11-12 10:20:18.000000000 -0700
@@ -202,6 +202,41 @@
 	return -EAGAIN;
 }
 
+/* The BestComm hardware requires data to be 32-bit aligned.
+ * We also pad to minimum ethernet packet length, ETH_ZLEN.
+ */
+static inline struct sk_buff *fec_skb_align_and_pad(struct sk_buff *skb)
+{
+	void *data = skb->data;
+	int len = skb->len;
+	int pad = (int)data & 0x3;
+	struct sk_buff *nskb;
+	int nlen;
+
+	if (pad == 0)
+		return skb;
+
+	if (!skb_cloned(skb)) {
+		skb_push(skb, pad);
+		memmove(skb->data, data, len);
+		skb_trim(skb, len);
+		skb = skb_padto(skb, ETH_ZLEN);
+		return skb;
+	}
+
+	/* ensure skb_padto doesn't have to reallocate */
+	nlen = (len >= ETH_ZLEN) ? len : ETH_ZLEN;
+
+	nskb = alloc_skb(nlen, GFP_ATOMIC);
+	if (nskb) {
+		skb_put(nskb, len);
+		memcpy(nskb->data, data, len);
+		nskb = skb_padto(nskb, ETH_ZLEN);
+	}
+	kfree_skb(skb);
+	return nskb;
+}
+
 /* This will only be invoked if your driver is _not_ in XOFF state.
  * What this means is that you need not check it, and that this
  * invariant will hold if you make sure that the netif_*_queue()
@@ -210,30 +245,16 @@
 static int fec_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct fec_priv *priv = (struct fec_priv *)dev->priv;
-	int pad;
 	void *data;
-	short length;
 
 	if (sdma_queue_full(priv->tx_sdma))
 		panic("MPC52xx transmit queue overrun\n");
 
-	/* the BestComm hardware requires data to be 32-bit aligned */
-	pad = (int)skb->data & 0x3;
-	if (pad) {
-		void *old_data = skb->data;
-		length = skb->len;
-
-		skb_push(skb, pad);
-		memcpy(skb->data, old_data, length);
-		skb_trim(skb, length);
-	}
-
-	/* Zero out up to the minimum length ethernet packet size,
-	 * so we don't inadvertently expose sensitive data
-	 */
-	skb = skb_padto(skb, ETH_ZLEN);
-	if (skb == 0)
+	skb = fec_skb_align_and_pad(skb);
+	if (!skb) {
+		priv->stats.tx_dropped++;
 		return 0;
+	}
 
 	spin_lock_irq(&priv->lock);
 	dev->trans_start = jiffies;
@@ -294,7 +315,8 @@
 			break;
 
 		rskb = sdma_retrieve_buffer(priv->rx_sdma, &length);
-		skb_trim(rskb, length);
+		/* length included sizeof CRC32 */
+		skb_trim(rskb, length - sizeof(u32));
 
 		/* allocate replacement skb */
 		skb = dev_alloc_skb(FEC_RX_BUFFER_SIZE);
@@ -309,6 +331,7 @@
 			priv->stats.rx_dropped++;
 
 			skb_trim(rskb, 0);
+			skb = rskb;
 		}
 
 		skb->dev = dev;

Re: Lite5200 FEC Driver on linux 2.6 broken? (fixed)

[roger blofeld]

Bravo! Retransmissions now have the correct checksums. Thanks
-roger


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

Re: Lite5200 FEC Driver on linux 2.6 broken? (again)

[roger blofeld]

Retransmissions from the lite5200 work OK now, but the occasional ACK
packet is sent from the lite5200 with a bad checksum causing
retransmissions. Perhaps another patch would fix this problem too?
-roger


		
__________________________________ 
Do you Yahoo!? 
The all-new My Yahoo! - Get yours free! 
http://my.yahoo.com 
 

Re: Lite5200 FEC Driver on linux 2.6 broken? (again)

[Dale Farnsworth]

On Tue, Nov 16, 2004 at 10:26:09AM -0800, roger blofeld wrote:
> Retransmissions from the lite5200 work OK now, but the occasional ACK
> packet is sent from the lite5200 with a bad checksum causing
> retransmissions. Perhaps another patch would fix this problem too?

I too have seen occasional errors, but I don't think it's a software
issue.

-Dale

Re: Lite5200 FEC Driver on linux 2.6 broken? (again)

[roger blofeld]

--- Dale Farnsworth <dale@farnsworth.org> wrote:

> On Tue, Nov 16, 2004 at 10:26:09AM -0800, roger blofeld wrote:
> > Retransmissions from the lite5200 work OK now, but the occasional
> ACK
> > packet is sent from the lite5200 with a bad checksum causing
> > retransmissions. Perhaps another patch would fix this problem too?
> 
> I too have seen occasional errors, but I don't think it's a software
> issue.
> 
> -Dale
> 
One more clue to what is happening: Whenever a packet is transmitted
with a bad checksum, the tail end of the data is in error. For example,
a packet of 0x442 bytes (0x42 header, 0x400 data) is corrupt from byte
0x360 on. Corruption always happens at a multiple of 0x10 from the
start of the packet. If it is software, perhaps we don't wait long
enough for DMA completion, or DMA signals completion too soon? Or maybe
the buffer is re-used or stomped on? Seems like too large of a
corruption to be due to cache.
-roger


		
__________________________________ 
Do you Yahoo!? 
The all-new My Yahoo! - Get yours free! 
http://my.yahoo.com 
 

Re: Lite5200 FEC Driver on linux 2.6 broken? (fixed again)

[roger blofeld]

Adding a call 

    flush_dcache_range((u32)skb->data, (u32)skb->data + skb->len);

 just before spin_lock_irq() in fec_hard_start_xmit() removes the
remaining transmission checksum errors for me.
-rb


		
__________________________________ 
Do you Yahoo!? 
Meet the all-new My Yahoo! - Try it today! 
http://my.yahoo.com 
 

Re: Lite5200 FEC Driver on linux 2.6 broken? (fixed again)

[Sylvain Munaut]

roger blofeld wrote:

>Adding a call 
>
>    flush_dcache_range((u32)skb->data, (u32)skb->data + skb->len);
>
> just before spin_lock_irq() in fec_hard_start_xmit() removes the
>remaining transmission checksum errors for me.
>-rb
>
>  
>
Mmmm.

That should not be needed.

What tree are you exactly using ?
In mainstream XLB snooping is not configured so if you have the 
mainstream + added DMA,
the problem may come from this.


    Sylvain

Re: Lite5200 FEC Driver on linux 2.6 broken? (fixed again)

[roger blofeld]

>What tree are you exactly using ?
>In mainstream XLB snooping is not configured so if you have the 
>mainstream + added DMA,
>the problem may come from this.
>
>
>    Sylvain

I did a bk clone of the linux-2.5 tree (2.6.10-rc2), then a bk pull of
your tree. You're probably right about the snooping. Since the driver
is working at the moment, I'll wait until your tree is updated before
doing more testing.
Thanks
-rb


		
__________________________________ 
Do you Yahoo!? 
The all-new My Yahoo! - Get yours free! 
http://my.yahoo.com 
 

Re: Lite5200 FEC Driver on linux 2.6 broken? (fixed again)

[Sylvain Munaut]

roger blofeld wrote:

>>What tree are you exactly using ?
>>In mainstream XLB snooping is not configured so if you have the 
>>mainstream + added DMA,
>>the problem may come from this.
>>
>>
>>   Sylvain
>>    
>>
>
>I did a bk clone of the linux-2.5 tree (2.6.10-rc2), then a bk pull of
>your tree. You're probably right about the snooping. Since the driver
>is working at the moment, I'll wait until your tree is updated before
>doing more testing.
>Thanks
>-rb
>  
>
Wow, that must have led to a bunch of conflicts no ?

Now, it's more up-to-date and cloning mainstream and pulling should be ok.
Note that my tree is now :

bk://bkbits.246tNt.com/linux-2.5-mpc52xx

(no -devel, I merged the two in only one)

The flush_dcache_range is not present because if it still fails I'd like 
to understand why.
I should not be needed or maybe I'm missing something. If still doesn't 
work, I'll
add it with a note.


    Sylvain

Re: Lite5200 FEC Driver on linux 2.6 (updated)

[roger blofeld]

Sylvain,
 Using your latest tree still shows the packet checksum errors, despite
XLB snooping being enabled. I think that the cache flush is required or
that the skb pages must be marked coherent somehow. I note that the 8xx
fec.c file also has a cache flush, so it isn't that strange.
-rb


		
__________________________________ 
Do you Yahoo!? 
Meet the all-new My Yahoo! - Try it today! 
http://my.yahoo.com 
 

Re: MPC5200 Cache coherency with BestComm issue (was: Lite5200 FEC Driver on linux 2.6 (updated))

[Sylvain Munaut]

Hi roger

> Using your latest tree still shows the packet checksum errors, despite
>XLB snooping being enabled.
>
>I think that the cache flush is required
>
It really shouldn't be.
Take a look at 
http://www.freescale.com/files/microcontrollers/doc/app_note/AN2604.pdf,
page 3, 4th paragraph

quote : <<< In case the memory location interested by BestComm accesses 
are cached in
copy-back mode by the core, a "Snooping" activity can be forced to the 
core, so that an
'address retry' will be issued and the core will first flush it's cache 
line before allowing
BestComm to retry it's transaction. >>>


Note that performance wise, adding it is probably a good idea. It will 
have to be flushed
eventually  and forcing BestComm to retry a transaction is just waste of 
XLB cycles ...


>or
>that the skb pages must be marked coherent somehow.
>
Yes they must. I haven't checked if they were but since this requirement 
is common
for multiple ppc, I'd guess they are.


>I note that the 8xx
>fec.c file also has a cache flush, so it isn't that strange.
>  
>
I don't know 8xx in details, but in arch/ppc/Kconfig, 8xx defines
NOT_COHERENT_CACHE symbol. So they probably excpect it not to be.



    Sylvain

Re: MPC5200 Cache coherency with BestComm issue

[roger blofeld]

Sylvain,
 By experimenting I have found that the BestComm and FEC work without
the cache flush provided CPU_FTR_MAYBE_CAN_NAP is removed from the
cputable (nap disables snooping) and that CPU_FTR_NEED_COHERENT is
added to the cputable (turns on "M" bit in BAT/PTE so that the XLB has
a chance of seeing a global transaction).

I don't know if that will work for all G2_LE cores, but it seems
required for the 5200.
-rb

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

Re: MPC5200 Cache coherency with BestComm issue

[Sylvain Munaut]

Hi Roger

> By experimenting I have found that the BestComm and FEC work without
>the cache flush provided CPU_FTR_MAYBE_CAN_NAP is removed from the
>cputable (nap disables snooping) and that CPU_FTR_NEED_COHERENT is
>added to the cputable (turns on "M" bit in BAT/PTE so that the XLB has
>a chance of seeing a global transaction).
>  
>
Thanks for the info.

The nap thing was indeed obvious ... The proper way to deactivate it 
would be in lite5200.c
I added the powersave_nap = 1; as an example on how to allow it to nap. 
I'll probably
comment it out by default with a note.

For the CPU_FTR_NEED_COHERENT, are you sure it's required ?
 From my understanding of the G2Core manual, the M bit must be set if 
you want the G2Core
to assert the global signal when it access memory. Here, we don't care 
since only
the G2Core is snooping on the bus. So that should not be required ...


    Sylvain

Re: Lite5200 FEC Driver on linux 2.6 broken?

[Sylvain Munaut]

Hi all,

>Try the following patch on top of Sylvain's tree.  I sent it (and others)
>to Sylvain a while back, but he must be busy with other things.
>  
>
Yes Dale, thanks.

I was busy on other things then took some weeks off. But I'm back now 
and I've started merging what was on the
-devel tree to a new updated tree.

Still testing but should be ready soon. You'll probably be the first to 
know ;)



    Sylvain

MPC5200 Cache coherency with BestComm issue

[roger blofeld]

>Hi Roger
>
>> By experimenting I have found that the BestComm and FEC work without
>>the cache flush provided CPU_FTR_MAYBE_CAN_NAP is removed from the
>>cputable (nap disables snooping) and that CPU_FTR_NEED_COHERENT is
>>added to the cputable (turns on "M" bit in BAT/PTE so that the XLB
has
>>a chance of seeing a global transaction).
>>  
>>
>Thanks for the info.
>
>The nap thing was indeed obvious ... The proper way to deactivate it 
>would be in lite5200.c
>I added the powersave_nap = 1; as an example on how to allow it to
nap. 
>I'll probably comment it out by default with a note.
>
>For the CPU_FTR_NEED_COHERENT, are you sure it's required ?
 >From my understanding of the G2Core manual, the M bit must be set if 
>you want the G2Core
>to assert the global signal when it access memory. Here, we don't care

>since only the G2Core is snooping on the bus. So that should not be
required ...
>
>
>    Sylvain

Sylvain,
 You are correct. The CPU_FTR_NEED_COHERENT is not required. Perhaps
the solution is to only turn on powersave_nap if CONFIG_PPC_BESTCOMM is
not selected.
-rb




		
__________________________________ 
Do you Yahoo!? 
Meet the all-new My Yahoo! - Try it today! 
http://my.yahoo.com 
 

Re: MPC5200 Cache coherency with BestComm issue

[Sylvain Munaut]

Hi Roger

>
>Sylvain,
> You are correct. The CPU_FTR_NEED_COHERENT is not required. Perhaps
>the solution is to only turn on powersave_nap if CONFIG_PPC_BESTCOMM is
>not selected.
>  
>
Pull the latest tree. I've just commented it out with a note.
I didn't rip it off so that people copying lite5200.c for their own 
board know how to activate
it if they need to ...
It can also be controlled from userspace /proc/sys/kernel/powersave-nap

You just need to be aware of the consequences ( like USB not working 
either ! ) if you want
to save power.


    Sylvain