Re: [PATCH 03/12] fs_enet: Add FEC TX Alignment workaround for MPC5121.

2009-05-07 Thread John Rigby
I was having deja-vu with this and realized that I have fixed at least some
of the objections to this patch.

Wolfgang you may want to look at the patch in my 5121 git tree here:

http://git.denx.de/?p=linux-mpc512x.git;a=commit;h=2950be3be42af7449941c3340998c27ef918f10f

It does runtime tx packet alignment  It also has fewer ifdefs and trys to
share more code.  It also has a header that explains everything including
that fact that there is not a runtime conflict sine the only other ppc that
has fec is 8xx which is not in the same family.

On Wed, May 6, 2009 at 4:42 PM, Grant Likely grant.lik...@secretlab.cawrote:

 On Wed, May 6, 2009 at 4:12 PM, Wolfgang Denk w...@denx.de wrote:
  Dear Grant Likely,
 
  In message fa686aa40905061337w6aa82f5aj787618ba108e5...@mail.gmail.com
 you wrote:
 
   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.
 
  OUCH!
 
  Yes :-(
 
   +#else
   +#define tx_skb_align_workaround(dev, skb) (skb)
   +#endif
 
  Another use of #ifdef blocks.  What is the multiplatform impact?
 
  Hm... Can you recommend a better way to solve the problem? Suggestions
  are welcome.

 I'd rather see a runtime selectable workaround.  ie. enable it based
 on the compatible property.

 g.

 --
 Grant Likely, B.Sc., P.Eng.
 Secret Lab Technologies Ltd.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 03/12] fs_enet: Add FEC TX Alignment workaround for MPC5121.

2009-05-06 Thread Wolfgang Denk
From: John Rigby jri...@freescale.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 jri...@freescale.com
Signed-off-by: Piotr Ziecik ko...@semihalf.com
Signed-off-by: Wolfgang Denk w...@denx.de
Cc: net...@vger.kernel.org
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: John Rigby jcri...@gmail.com
---
 drivers/net/fs_enet/Kconfig|3 ++
 drivers/net/fs_enet/fs_enet-main.c |   41 
 drivers/net/fs_enet/mac-fec.c  |2 +-
 3 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/drivers/net/fs_enet/Kconfig b/drivers/net/fs_enet/Kconfig
index fc073b5..79afd07 100644
--- a/drivers/net/fs_enet/Kconfig
+++ b/drivers/net/fs_enet/Kconfig
@@ -8,6 +8,9 @@ config FS_ENET_MPC5121_FEC
def_bool y if (FS_ENET  PPC_MPC512x)
select FS_ENET_HAS_FEC
 
+config FS_ENET_FEC_TX_ALIGN_WORKAROUND
+   def_bool y if FS_ENET_MPC5121_FEC
+
 config FS_ENET_HAS_SCC
bool Chip has an SCC usable for ethernet
depends on FS_ENET  (CPM1 || CPM2)
diff --git a/drivers/net/fs_enet/fs_enet-main.c 
b/drivers/net/fs_enet/fs_enet-main.c
index 4170d33..c83ffc3 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -594,6 +594,37 @@ void fs_cleanup_bds(struct net_device *dev)
 
 
/**/
 
+#ifdef CONFIG_FS_ENET_FEC_TX_ALIGN_WORKAROUND
+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;
+}
+#else
+#define tx_skb_align_workaround(dev, skb) (skb)
+#endif
+
 static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
struct fs_enet_private *fep = netdev_priv(dev);
@@ -602,6 +633,16 @@ static int fs_enet_start_xmit(struct sk_buff *skb, struct 
net_device *dev)
u16 sc;
unsigned long flags;
 
+   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);
 
/*
diff --git a/drivers/net/fs_enet/mac-fec.c b/drivers/net/fs_enet/mac-fec.c
index b069088..3e86498 100644
--- a/drivers/net/fs_enet/mac-fec.c
+++ b/drivers/net/fs_enet/mac-fec.c
@@ -311,7 +311,7 @@ static void restart(struct net_device *dev)
 * Enable big endian.
 */
 #ifndef CONFIG_FS_ENET_MPC5121_FEC
-   /* Don't care about SDMA FC. */
+   /* Don't care about SDMA Function Code. */
FW(fecp, fun_code, 0x7800);
 #else
FS(fecp, dma_control, 0xC000);
-- 
1.6.0.6

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 03/12] fs_enet: Add FEC TX Alignment workaround for MPC5121.

2009-05-06 Thread Grant Likely
On Wed, May 6, 2009 at 2:15 PM, Wolfgang Denk w...@denx.de wrote:
 From: John Rigby jri...@freescale.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.

OUCH!

 diff --git a/drivers/net/fs_enet/fs_enet-main.c 
 b/drivers/net/fs_enet/fs_enet-main.c
 index 4170d33..c83ffc3 100644
 --- a/drivers/net/fs_enet/fs_enet-main.c
 +++ b/drivers/net/fs_enet/fs_enet-main.c
 @@ -594,6 +594,37 @@ void fs_cleanup_bds(struct net_device *dev)

  /**/

 +#ifdef CONFIG_FS_ENET_FEC_TX_ALIGN_WORKAROUND
 +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;
 +}
 +#else
 +#define tx_skb_align_workaround(dev, skb) (skb)
 +#endif

Another use of #ifdef blocks.  What is the multiplatform impact?

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 03/12] fs_enet: Add FEC TX Alignment workaround for MPC5121.

2009-05-06 Thread Wolfgang Denk
Dear Grant Likely,

In message fa686aa40905061337w6aa82f5aj787618ba108e5...@mail.gmail.com you 
wrote:

  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.

 OUCH!

Yes :-(

  +#else
  +#define tx_skb_align_workaround(dev, skb) (skb)
  +#endif

 Another use of #ifdef blocks.  What is the multiplatform impact?

Hm... Can you recommend a better way to solve the problem? Suggestions
are welcome.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
You don't have to worry about me. I might have been born yesterday...
but I stayed up all night.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 03/12] fs_enet: Add FEC TX Alignment workaround for MPC5121.

2009-05-06 Thread Grant Likely
On Wed, May 6, 2009 at 4:12 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Grant Likely,

 In message fa686aa40905061337w6aa82f5aj787618ba108e5...@mail.gmail.com you 
 wrote:

  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.

 OUCH!

 Yes :-(

  +#else
  +#define tx_skb_align_workaround(dev, skb) (skb)
  +#endif

 Another use of #ifdef blocks.  What is the multiplatform impact?

 Hm... Can you recommend a better way to solve the problem? Suggestions
 are welcome.

I'd rather see a runtime selectable workaround.  ie. enable it based
on the compatible property.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev