diff mbox

[02/11] fs_enet: Add FEC TX Alignment workaround for MPC5121

Message ID 1263932653-3634-3-git-send-email-agust@denx.de (mailing list archive)
State Not Applicable
Headers show

Commit Message

Anatolij Gustschin Jan. 19, 2010, 8:24 p.m. UTC 
From: John Rigby <jcrigby@gmail.com>

The FEC on 5121 has problems with misaligned tx buffers.
The RM says any alignment is ok but empirical results
show that packet buffers ending in 0x1E will sometimes
hang the FEC.  Other bad alignment does not hang but will
cause silent TX failures resulting in about a 1% packet
loss as tested by ping -f from a remote host.

This patch is a work around that copies every tx packet
to an aligned skb before sending.

Signed-off-by: John Rigby <jrigby@freescale.com>
Signed-off-by: Piotr Ziecik <kosmo@semihalf.com>
Signed-off-by: Wolfgang Denk <wd@denx.de>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: <netdev@vger.kernel.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: John Rigby <jcrigby@gmail.com>
---
 drivers/net/fs_enet/fs_enet-main.c |   39 ++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

Comments

David Miller Jan. 19, 2010, 8:37 p.m. UTC | #1 
Well... where are the other 9 patches?
Stephen Rothwell Jan. 19, 2010, 11:42 p.m. UTC | #2 
Hi Dave,

On Tue, 19 Jan 2010 12:37:06 -0800 (PST) David Miller <davem@davemloft.net> wrote:
>
> Well... where are the other 9 patches?

I think the other 9 are not network related, just PowerPC related.
David Miller Jan. 20, 2010, 4:04 a.m. UTC | #3 
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 20 Jan 2010 10:42:25 +1100

> On Tue, 19 Jan 2010 12:37:06 -0800 (PST) David Miller <davem@davemloft.net> wrote:
>>
>> Well... where are the other 9 patches?
> 
> I think the other 9 are not network related, just PowerPC related.

Ok, but that's very confusing.

If it's meant to go into two different trees split the
patches up into two different series.
Wolfram Sang Jan. 20, 2010, 10:22 a.m. UTC | #4 
> diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
> index 909b78d..a391219 100644
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -587,6 +587,33 @@ void fs_cleanup_bds(struct net_device *dev)
>  
>  /**********************************************************************************/
>  
> +static struct sk_buff *tx_skb_align_workaround(struct net_device *dev,
> +					       struct sk_buff *skb)
> +{
> +	struct sk_buff *new_skb;
> +
> +	/* Alloc new skb */
> +	new_skb = dev_alloc_skb(ENET_RX_FRSIZE + 32);
> +	if (!new_skb) {
> +		printk(KERN_WARNING DRV_MODULE_NAME
> +				": %s Memory squeeze, dropping tx packet.\n",
> +								dev->name);

I understand this type of printk fits to the rest of the code, I just wonder if
new (and patched) printouts shouldn't be converted to dev_*? Might be one line
then, and more consistent to what other drivers produce.
diff mbox

Patch

diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index 909b78d..a391219 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -587,6 +587,33 @@  void fs_cleanup_bds(struct net_device *dev)
 
 /**********************************************************************************/
 
+static struct sk_buff *tx_skb_align_workaround(struct net_device *dev,
+					       struct sk_buff *skb)
+{
+	struct sk_buff *new_skb;
+
+	/* Alloc new skb */
+	new_skb = dev_alloc_skb(ENET_RX_FRSIZE + 32);
+	if (!new_skb) {
+		printk(KERN_WARNING DRV_MODULE_NAME
+				": %s Memory squeeze, dropping tx packet.\n",
+								dev->name);
+		return NULL;
+	}
+
+	/* Make sure new skb is properly aligned */
+	skb_align(new_skb, 32);
+
+	/* Copy data to new skb ... */
+	skb_copy_from_linear_data(skb, new_skb->data, skb->len);
+	skb_put(new_skb, skb->len);
+
+	/* ... and free an old one */
+	dev_kfree_skb_any(skb);
+
+	return new_skb;
+}
+
 static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct fs_enet_private *fep = netdev_priv(dev);
@@ -595,6 +622,18 @@  static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	u16 sc;
 	unsigned long flags;
 
+	if (fep->fec.fec_id == FS_ENET_MPC5121_FEC) {
+		skb = tx_skb_align_workaround(dev, skb);
+		if (!skb) {
+			/*
+			 * We have lost packet due to memory allocation error
+			 * in tx_skb_align_workaround(). Hopefully original skb
+			 * is still valid, so try transmit it later.
+			 */
+			return NETDEV_TX_BUSY;
+		}
+	}
+
 	spin_lock_irqsave(&fep->tx_lock, flags);
 
 	/*