[PATCH] smsc911x: add fifo byteswap support

[PATCH] smsc911x: add fifo byteswap support

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

This patch adds fifo byteswap support to the smsc911x driver.

The smsc911x hardware supports both big and little and endian
hardware configurations, and the linux smsc911x driver currently
detects word order.

For correct operation on big endian platforms lacking swapped
byte lanes the following patch is needed. Only fifo data is
swapped, register data does not require any swapping. 

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 Platform data for the rsk7203 board will be posted separately
 to the linux-sh in the near future.

 drivers/net/smsc911x.c   |   13 +++++++++++++
 include/linux/smsc911x.h |    1 +
 2 files changed, 14 insertions(+)

--- 0001/drivers/net/smsc911x.c
+++ work/drivers/net/smsc911x.c	2009-04-21 20:06:35.000000000 +0900
@@ -47,6 +47,7 @@
 #include <linux/bitops.h>
 #include <linux/irq.h>
 #include <linux/io.h>
+#include <linux/swab.h>
 #include <linux/phy.h>
 #include <linux/smsc911x.h>
 #include "smsc911x.h"
@@ -175,6 +176,12 @@ static inline void
 smsc911x_tx_writefifo(struct smsc911x_data *pdata, unsigned int *buf,
 		      unsigned int wordcount)
 {
+	if (pdata->config.flags & SMSC911X_SWAP_FIFO) {
+		while (wordcount--)
+			smsc911x_reg_write(pdata, TX_DATA_FIFO, swab32(*buf++));
+		return;
+	}
+
 	if (pdata->config.flags & SMSC911X_USE_32BIT) {
 		writesl(pdata->ioaddr + TX_DATA_FIFO, buf, wordcount);
 		return;
@@ -194,6 +201,12 @@ static inline void
 smsc911x_rx_readfifo(struct smsc911x_data *pdata, unsigned int *buf,
 		     unsigned int wordcount)
 {
+	if (pdata->config.flags & SMSC911X_SWAP_FIFO) {
+		while (wordcount--)
+			*buf++ = swab32(smsc911x_reg_read(pdata, RX_DATA_FIFO));
+		return;
+	}
+
 	if (pdata->config.flags & SMSC911X_USE_32BIT) {
 		readsl(pdata->ioaddr + RX_DATA_FIFO, buf, wordcount);
 		return;
--- 0001/include/linux/smsc911x.h
+++ work/include/linux/smsc911x.h	2009-04-21 20:07:57.000000000 +0900
@@ -46,5 +46,6 @@ struct smsc911x_platform_config {
 #define SMSC911X_FORCE_INTERNAL_PHY		(BIT(2))
 #define SMSC911X_FORCE_EXTERNAL_PHY 		(BIT(3))
 #define SMSC911X_SAVE_MAC_ADDRESS		(BIT(4))
+#define SMSC911X_SWAP_FIFO			(BIT(5))
 
 #endif /* __LINUX_SMSC911X_H__ */

Re: [PATCH] smsc911x: add fifo byteswap support

[Steve.Glendinning]

Magnus Damm <magnus.damm@gmail.com> wrote on 22/04/2009 15:55:53:

> From: Magnus Damm <damm@igel.co.jp>
> 
> This patch adds fifo byteswap support to the smsc911x driver.
> 
> The smsc911x hardware supports both big and little and endian
> hardware configurations, and the linux smsc911x driver currently
> detects word order.
> 
> For correct operation on big endian platforms lacking swapped
> byte lanes the following patch is needed. Only fifo data is
> swapped, register data does not require any swapping. 
> 
> Signed-off-by: Magnus Damm <damm@igel.co.jp>

I guess this is to work around a problem with hardware that's
already in production?

The best solution would be to swap the byte lanes in hardware,
as the device's endian swapping features only affect registers
(not the FIFOs).  The very latest devices in this family
(such as the LAN9221) can swap both independently, but this
driver has to also support older parts.

Performance will be suboptimal with this software byteswapping,
so I think we should also add a comment to stress that this is
a "last resort" workaround for broken hardware.

Acked-by: Steve Glendinning <steve.glendinning@smsc.com>

Re: [PATCH] smsc911x: add fifo byteswap support

[Paul Mundt]

On Wed, Apr 22, 2009 at 04:21:32PM +0100, Steve.Glendinning@smsc.com wrote:
> Magnus Damm <magnus.damm@gmail.com> wrote on 22/04/2009 15:55:53:
> 
> > From: Magnus Damm <damm@igel.co.jp>
> > 
> > This patch adds fifo byteswap support to the smsc911x driver.
> > 
> > The smsc911x hardware supports both big and little and endian
> > hardware configurations, and the linux smsc911x driver currently
> > detects word order.
> > 
> > For correct operation on big endian platforms lacking swapped
> > byte lanes the following patch is needed. Only fifo data is
> > swapped, register data does not require any swapping. 
> > 
> > Signed-off-by: Magnus Damm <damm@igel.co.jp>
> 
> I guess this is to work around a problem with hardware that's
> already in production?
> 
Good guess ;-)

> The best solution would be to swap the byte lanes in hardware,
> as the device's endian swapping features only affect registers
> (not the FIFOs).  The very latest devices in this family
> (such as the LAN9221) can swap both independently, but this
> driver has to also support older parts.
> 
> Performance will be suboptimal with this software byteswapping,
> so I think we should also add a comment to stress that this is
> a "last resort" workaround for broken hardware.
> 
> Acked-by: Steve Glendinning <steve.glendinning@smsc.com>

Performance is not much of a consideration here, it's basically between
this and no ethernet at all. On the plus side, at least not many boards
will need this, I believe it's only 2 generations of microcontroller
evaluation boards that suffer from this particular bit of idiocy on our
side.

Re: [PATCH] smsc911x: add fifo byteswap support

[Steve.Glendinning]

Paul Mundt <lethal@linux-sh.org> wrote on 23/04/2009 03:09:25:

> > The best solution would be to swap the byte lanes in hardware,
> > as the device's endian swapping features only affect registers
> > (not the FIFOs).  The very latest devices in this family
> > (such as the LAN9221) can swap both independently, but this
> > driver has to also support older parts.
> > 
> > Performance will be suboptimal with this software byteswapping,
> > so I think we should also add a comment to stress that this is
> > a "last resort" workaround for broken hardware.
> > 
> > Acked-by: Steve Glendinning <steve.glendinning@smsc.com>
> 
> Performance is not much of a consideration here, it's basically between
> this and no ethernet at all. On the plus side, at least not many boards
> will need this, I believe it's only 2 generations of microcontroller
> evaluation boards that suffer from this particular bit of idiocy on our
> side.

I agree, it's a handy feature to have when you're stuck with broken
hardware.  I'd like everyone who enables it to KNOW that their hardware
is broken however, so hopefully they push back on their hardware guys
to correct it in a later revision.

I'd like to avoid people getting a broken platform working using this,
then later complaining about performance!

Steve

Re: [PATCH] smsc911x: add fifo byteswap support

[Magnus Damm]

On Thu, Apr 23, 2009 at 12:21 AM,  <Steve.Glendinning@smsc.com> wrote:
> Magnus Damm <magnus.damm@gmail.com> wrote on 22/04/2009 15:55:53:
>> This patch adds fifo byteswap support to the smsc911x driver.

> Performance will be suboptimal with this software byteswapping,
> so I think we should also add a comment to stress that this is
> a "last resort" workaround for broken hardware.

Good idea. How about adding the following to the header file right
before the SMSC911X_SWAP_FIFO definition:

/*
 * SMSC911X_SWAP_FIFO:
 * Enables software byte swap for fifo data. Should only be used as a
 * "last resort" in the case of big endian mode on boards with incorrectly
 * routed data bus to older devices such as LAN9118. Newer devices such as
 * LAN9221 handle this in hardware.
 */

Let me know if you want to improve the comment somehow, otherwise I'll
just update the patch and repost a V2.

Cheers,

/ magnus

Re: [PATCH] smsc911x: add fifo byteswap support

[Steve.Glendinning]

Hi Magnus,

Slight correction: the newer devices *can* handle this in hardware, there 
are registers to control this swapping but the driver doesn't currently 
use them.  If I see a suitably broken design with a newer device I'll 
submit a patch to add this.

Looks great otherwise, thanks for adding the "health warning" :-)

Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: steve.glendinning@smsc.com



Magnus Damm <magnus.damm@gmail.com> wrote on 24/04/2009 02:15:03:

> On Thu, Apr 23, 2009 at 12:21 AM,  <Steve.Glendinning@smsc.com> wrote:
> > Magnus Damm <magnus.damm@gmail.com> wrote on 22/04/2009 15:55:53:
> >> This patch adds fifo byteswap support to the smsc911x driver.
> 
> > Performance will be suboptimal with this software byteswapping,
> > so I think we should also add a comment to stress that this is
> > a "last resort" workaround for broken hardware.
> 
> Good idea. How about adding the following to the header file right
> before the SMSC911X_SWAP_FIFO definition:
> 
> /*
>  * SMSC911X_SWAP_FIFO:
>  * Enables software byte swap for fifo data. Should only be used as a
>  * "last resort" in the case of big endian mode on boards with 
incorrectly
>  * routed data bus to older devices such as LAN9118. Newer devices such 
as
>  * LAN9221 handle this in hardware.
>  */
> 
> Let me know if you want to improve the comment somehow, otherwise I'll
> just update the patch and repost a V2.
> 
> Cheers,
> 
> / magnus