Thread (17 messages) flat view 17 messages, 7 authors, 2006-09-20

Re: Fw: [PATCH] Remove powerpc specific parts of 3c509 driver

From: Matt Sealey <hidden>
Date: 2006-09-19 19:00:03
Also in: netdev

Some northbridges and PCI bridges have "clever" byteswapping in 
hardware, maybe this is just an effect of that. In theory depending on 
the host bridge, you should pass in big endian data and have it swap or 
not swap, not pick that way in the driver, UNLESS your driver expects 
bigendian data, in which case on a bigendian platform you can tell it to 
write without swapping. Voila, two functions.

However the existance of these PCI bridges these days? I haven't seen 
one in years, and when I have nobody has ever enabled the magic swappy 
thing as it's unreliable and can't always tell how you present the data.

One wishes that there was a ntoh and hton style macro in standard use 
for PCI access.. hang on though that jsut wouldn't work would it.

-- 
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

Linas Vepstas wrote:
On Tue, Sep 19, 2006 at 02:54:33PM +1000, Stephen Rothwell wrote:
quoted
On powerpc and ppc, insl_ns and insl are identical as are outsl_ns and
outsl, so remove the conditional use of insl_ns and outsl_ns.
The rest of this patch might indeed be correct, but the above comment 
bothers me. The "ns" versions of routines are supposed to be
non-byte-swapped versions of the insl/outsl routines (which would
byte-swap on big-endian archs such as powerpc.)
quoted
diff --git a/drivers/net/3c509.c b/drivers/net/3c509.c
index cbdae54..add6381 100644
--- a/drivers/net/3c509.c
+++ b/drivers/net/3c509.c
@@ -879,11 +879,7 @@ #endif
 	outw(skb->len, ioaddr + TX_FIFO);
 	outw(0x00, ioaddr + TX_FIFO);
 	/* ... and the packet rounded to a doubleword. */
-#ifdef  __powerpc__
-	outsl_ns(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
-#else
 	outsl(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
-#endif
Dohh, a hack like this to work around some possbile byte-swapping
bug should never have been done in the first place :-(

However, I presume someone added the  __powerpc__ define here
because they picked up a 3c509 at a garage sale, stuck it in 
a powerpc, found out it didn't work due to a byte-swapping bug,
and then patched it as above. I'm disturbed that somehow 
outsl_ns() became identical to outsl() at some point, presumably
breaking this patch.

--linas

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help