Bogus, broken changes: was Req. for testing: new 8139too updates

[Donald Becker]

On Thu, 30 Mar 2000, Jeff Garzik wrote:

> Attached is a patch against 2.3.99-pre4-pre1 development kernel, which
> updates the 8139too network driver to be smarter about autonegotiation
> and media handling, LED handling, and a lot of other changes.

 #include <linux/etherdevice.h>
+#include <linux/delay.h>
 #include <asm/io.h>
 
Do not use delay.h unless you absolutely cannot avoid it.  I removed
everyplace I could for good reasons.

 /* A few user-configurable values. */
+/* media options */
+static int media[] = {-1, -1, -1, -1, -1, -1, -1, -1};
+

This is a new, inconsistent parameter name.
You are creating a new interface, when a defined one already exists.

 #define RX_BUF_LEN (8192 << RX_BUF_LEN_IDX)
+#define RX_BUF_PAD 16
+#define RX_BUF_TOT_LEN (RX_BUF_LEN + RX_BUF_PAD)

This should not be in the "user adjustable compile-time setting".
It is a hardware detail and is not tunable.

-/* The following settings are log_2(bytes)-4:  0 == 16 bytes .. 6==1024. */
+/* The following settings are log_2(bytes)-4:  0 == 16 bytes .. 6==1024, 7==end of packet. */
 #define RX_FIFO_THRESH	4	/* Rx buffer level before first PCI xfer.  */
-#define RX_DMA_BURST	4	/* Maximum PCI burst, '4' is 256 bytes */
-#define TX_DMA_BURST	4	/* Calculate as 16<<val. */
+#define RX_DMA_BURST	7	/* Maximum PCI burst, '7' is unlimited */
+#define TX_DMA_BURST	4	/* Maximum PCI burst, '4' is 256 */

Please keep to 80 column lines.  You removed the calculation comment.
And increasing the burst size on the pre-PCI-2.1 chips might lead to
problems.

+	/* Soft reset the chip. */
+	RTL_W8 (ChipCmd, (RTL_R8 (ChipCmd) & ChipCmdClear) | CmdReset);

This retriggers autonegotiation.  This is very bad in some environments.
For instance, if you are talking to a CISCO switch you won't be able to
communicate for at least a minute.  This will look exactly like a driver
bug.


+	RTL_W16_F (FIFOTMS, RTL_R16 (FIFOTMS) & ~(1<<7));
+	RTL_W16_F (NWayAdvert, RTL_R16 (NWayAdvert) |
+		  (1<<13)|(1<<8)|(1<<7)|(1<<6)|(1<<5)|(1<<0));
+	RTL_W16_F (BasicModeCtrl, RTL_R16 (BasicModeCtrl) |
+		(1<<13)|(1<<12)|(1<<9)|(1<<8));
+	RTL_W8_F (MediaStatus, RTL_R8 (MediaStatus) | (1<<7) | (1<<6));

More horrible crap that won't possibly work with the older chips.  This also
breaks 'mii-diag'.  (Hey, you flamed me just weeks ago about magic constants.
Here you are doing something far worse -- these are independent bits that
are commonly modified.

Donald Becker
Scyld Computing Corporation, becker@scyld.com

 | To unsubscribe, send mail to Majordomo@cesdis.gsfc.nasa.gov, and within the
 |  body of the mail, include only the text:
 |   unsubscribe this-list-name youraddress@wherever.org
 | You will be unsubscribed as speedily as possible.