Skip to contentSkip to conversation

Yellowfin bug fix for Symbios cards

PATCH Applied Val Henson 2 participants 5 messages Oct 19, 2001 Network drivers

lore
Share & tools
lore

Val Henson

author
#0 lore raw

PATCH 1/1 Yellowfin bug fix for Symbios cards

Short version: My last patch to the yellowfin driver forced full-duplex on for Symbios 10/100 cards, causing severe packet loss on hubs and some switches.

Long version: Reading the MAC address from the EEPROM didn't work on the Symbios card, so I turned on the IsGigabit flag to read it correctly. This also forces full-duplex on, which is wrong. So I added a flag controlling only the MAC address reading behavior and turned off the IsGigabit flag for Symbios cards.

Patch (against latest 2.4) is at the end of this mail.

Thanks to Tim Moloney for reporting this bug.

-VAL

drivers/net/yellowfin.c +7 -4
diff -Nru a/drivers/net/yellowfin.c b/drivers/net/yellowfin.c
@@ -38,10 +38,13 @@
 	* Fix three endian-ness bugs
 	* Support dual function SYM53C885E ethernet chip
 	
+	LK1.1.5 ([email protected]):
+	* Fix forced full-duplex bug I introduced
+	
 */
 
 #define DRV_NAME	"yellowfin"
-#define DRV_VERSION	"1.05+LK1.1.3"
+#define DRV_VERSION	"1.05+LK1.1.5"
 #define DRV_RELDATE	"May 10, 2001"
 
 #define PFX DRV_NAME ": "
@@ -256,7 +259,7 @@
 };
 enum capability_flags {
 	HasMII=1, FullTxStatus=2, IsGigabit=4, HasMulticastBug=8, FullRxStatus=16,
-	HasMACAddrBug=32,			/* Only on early revs.  */
+	HasMACAddrBug=32, DontUseEeprom=64, /* Only on early revs.  */
 };
 /* The PCI I/O space extent. */
 #define YELLOWFIN_SIZE 0x100
@@ -282,7 +285,7 @@
 	 PCI_IOTYPE, YELLOWFIN_SIZE,
 	 FullTxStatus | IsGigabit | HasMulticastBug | HasMACAddrBug},
 	{"Symbios SYM83C885", { 0x07011000, 0xffffffff},
-	 PCI_IOTYPE, YELLOWFIN_SIZE, HasMII | IsGigabit | FullTxStatus },
+	 PCI_IOTYPE, YELLOWFIN_SIZE, HasMII | DontUseEeprom },
 	{0,},
 };
 
@@ -453,7 +456,7 @@
 #endif
 	irq = pdev->irq;
 
-	if (drv_flags & IsGigabit)
+	if (drv_flags & DontUseEeprom)
 		for (i = 0; i < 6; i++)
 			dev->dev_addr[i] = inb(ioaddr + StnAddr + i);
 	else {
Applied
#1 lore raw
Quoted 5 lines · Val Henson

Long version: Reading the MAC address from the EEPROM didn't work on the Symbios card, so I turned on the IsGigabit flag to read it correctly. This also forces full-duplex on, which is wrong. So I added a flag controlling only the MAC address reading behavior and turned off the IsGigabit flag for Symbios cards.

Thanks, applied.

Any idea where the MAC address comes from on the Symbios card?

Standard net driver policy is to read the MAC address from the original source at probe time, typically EEPROM but sometimes in a boot PROM, firmware console memory, or cardbus CIS. It is generally preferred to -not- read the MAC address from the card registers unless you absolutely have to, since card's copy of the MAC address is easily changeable or corrupted by rebooting from Windows into Linux or similar things (MacOS into Linux).

Jeff

Signature · 3 lines
Jeff Garzik      | Only so many songs can be sung
Building 1024    | with two lips, two lungs, and one tongue.
MandrakeSoft     |         - nomeansno
author
#2 lore raw
In reply to Val Henson
Quoted 10 lines · Jeff Garzik
Quoted 5 lines · Val Henson

Long version: Reading the MAC address from the EEPROM didn't work on the Symbios card, so I turned on the IsGigabit flag to read it correctly. This also forces full-duplex on, which is wrong. So I added a flag controlling only the MAC address reading behavior and turned off the IsGigabit flag for Symbios cards.

Thanks, applied.

Any idea where the MAC address comes from on the Symbios card?

I believe our bootloader (Smon) initializes the MAC address.

Quoted 7 lines

Standard net driver policy is to read the MAC address from the original source at probe time, typically EEPROM but sometimes in a boot PROM, firmware console memory, or cardbus CIS. It is generally preferred to -not- read the MAC address from the card registers unless you absolutely have to, since card's copy of the MAC address is easily changeable or corrupted by rebooting from Windows into Linux or similar things (MacOS into Linux).

Hm, good point. I should figure out why read_eeprom isn't working and fix that instead. Maybe the driver should be changed to attempt to read the MAC from the eeprom and then read from the registers if that fails, instead of relying on flags.

By the way, the only other OS that runs on our hardware is VxWorks, and possibly a few of the more obscure RTOS's...

-VAL

#3 lore raw
In reply to Val Henson
Quoted 4 lines · Val Henson

Hm, good point. I should figure out why read_eeprom isn't working and fix that instead. Maybe the driver should be changed to attempt to read the MAC from the eeprom and then read from the registers if that fails, instead of relying on flags.

Yeah. There is at least one other driver (pcnet32?) that does something like this...

probe:

	dev->dev_addr[] = read_eeprom();
	if (!is_valid_ether_addr(dev->dev_addr))
		{ read from card registers }
Signature · 3 lines
Jeff Garzik      | Only so many songs can be sung
Building 1024    | with two lips, two lungs, and one tongue.
MandrakeSoft     |         - nomeansno
author
#4 lore raw
In reply to Val Henson
Quoted 6 lines · Jeff Garzik
Quoted 4 lines · Val Henson

Hm, good point. I should figure out why read_eeprom isn't working and fix that instead. Maybe the driver should be changed to attempt to read the MAC from the eeprom and then read from the registers if that fails, instead of relying on flags.

Quoted 7 lines

Yeah. There is at least one other driver (pcnet32?) that does something like this...

probe:

	dev->dev_addr[] = read_eeprom();
	if (!is_valid_ether_addr(dev->dev_addr))
		{ read from card registers }

Well, I asked our hardware engineers, and the reason read the MAC address from the EEPROM is that we don't have an EEPROM on our boards. Instead, the bootloader reads some stuff out of NVRAM and writes it into the MAC address registers of the ethernet chip.

I think the above scheme is a good way to read the MAC, but I'd want to test it on an actual Yellowfin gigabit card before submitting it. Anyone out here willing to test?

-VAL