2002-10-28 18:50:43

by Zwane Mwaikambo

Subject: [PATCH][2.5] 3c509 increase udelay in *read_eeprom

Hi Jeff,
This is David's patch, find his reasoning and patch below.

"... I had to set the udelay() call parameters to 2000 in read_eeprom()
and 4000 in id_read_eeprom() to get the system to boot reliably with 2
3c509's in it. If I didn't set these values high enough, I got an oops
about 1/3 of the time when I booted....somehow (I'm guessing) it just
took the cards longer to initialize/respond when there were two of them
on the bus.

I know the possibility of this (and the fix, setting the values higher) is
mentioned in Becker's 3c509 instructions, but I wanted to relay my
experience to you as well. Since AFAIK these subroutines are only called
at initialization time (we don't need to read the EEPROM after init), what
would be the harm of setting these values higher - at least 1000 for both,
say - in the standard driver? Certainly a millisecond or two means nothing
at boot time, and if it prevents even a few machines from mysteriously
oopsing when they're started, it's a win overall ..."

Index: linux-2.5.44/drivers/net/3c509.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.44/drivers/net/3c509.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 3c509.c
--- linux-2.5.44/drivers/net/3c509.c 19 Oct 2002 21:12:02 -0000 1.1.1.1
+++ linux-2.5.44/drivers/net/3c509.c 28 Oct 2002 18:49:03 -0000
@@ -51,11 +51,13 @@
- Full duplex support
v1.19 16Oct2002 Zwane Mwaikambo <[email protected]>
- Additional ethtool features
+ v1.19a 28Oct2002 Davud Ruggiero <[email protected]>
+ - Increase *read_eeprom udelay to workaround oops with 2 cards.
*/

#define DRV_NAME "3c509"
-#define DRV_VERSION "1.19"
-#define DRV_RELDATE "16Oct2002"
+#define DRV_VERSION "1.19a"
+#define DRV_RELDATE "28Oct2002"

/* A few values that may be tweaked. */

@@ -571,7 +573,7 @@
{
outw(EEPROM_READ + index, ioaddr + 10);
/* Pause for at least 162 us. for the read to take place. */
- udelay (500);
+ udelay (2000);
return inw(ioaddr + 12);
}

@@ -585,7 +587,7 @@
outb(EEPROM_READ + index, id_port);

/* Pause for at least 162 us. for the read to take place. */
- udelay (500);
+ udelay (4000);

for (bit = 15; bit >= 0; bit--)
word = (word << 1) + (inb(id_port) & 0x01);


--
function.linuxpower.ca

2002-10-28 19:04:12

by Jeff Garzik

Subject: Re: [PATCH][2.5] 3c509 increase udelay in *read_eeprom

Zwane Mwaikambo wrote:

>Hi Jeff,
>This is David's patch, find his reasoning and patch below.
>
>"... I had to set the udelay() call parameters to 2000 in read_eeprom()
>and 4000 in id_read_eeprom() to get the system to boot reliably with 2
>3c509's in it. If I didn't set these values high enough, I got an oops
>about 1/3 of the time when I booted....somehow (I'm guessing) it just
>took the cards longer to initialize/respond when there were two of them
>on the bus.
>
>I know the possibility of this (and the fix, setting the values higher) is
>mentioned in Becker's 3c509 instructions, but I wanted to relay my
>experience to you as well. Since AFAIK these subroutines are only called
>at initialization time (we don't need to read the EEPROM after init), what
>would be the harm of setting these values higher - at least 1000 for both,
>say - in the standard driver? Certainly a millisecond or two means nothing
>at boot time, and if it prevents even a few machines from mysteriously
>oopsing when they're started, it's a win overall ..."
>
>


lol... big udelays are almost always wrong.

First, long delays lock out everybody, thus you should do operations
that require long waits via a timer or schedule_timeout() in process
context.
Second, udelay of 1000 or greater is a bug, use mdelay() instead.