public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pcnet32.c: MAC address may be in CSR registers
@ 2001-02-14 21:49 Eli Carter
  2001-02-14 23:50 ` Eli Carter
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Carter @ 2001-02-14 21:49 UTC (permalink / raw)
  To: Richard B. Johnson, tsbogend, Alan Cox; +Cc: linux-kernel, Eli Carter

[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]

All,

<aside>
Thomas Bogendoerfer is listed as maintainer. 
Richard, I know you've done some work with this driver so I thought you
might be interested.
Alan, I'd like to see this find its way into the official version(s), so
feedback would be appreciated if you don't apply it.  (In your copious
spare time, of course. ;) )
</aside>

I'm dealing with an AMD chip that does not have the station address in
the PROM at the base address, but resides in the "Physical Address
Registers" in the chip (thanks to the bootloader in my case).  This
patch makes the driver try those registers if the station address read
from the PROM is 00:00:00:00:00:00.
I think others dealing with similar setups may find this helpful.  The
other lance-derived drivers might benefit from a similar patch, but I
don't have that hardware for testing.

(The diff is against 2.2.18 and applies cleanly.)

If this is not acceptible or could be improved, please reply with
feedback.

TIA,

Eli 
--------------------.              Rule of Accuracy: When working toward
Eli Carter          |               the solution of a problem, it always 
eli.carter@inet.com `--------------------- helps if you know the answer.

[-- Attachment #2: patch-pcnet32-mac --]
[-- Type: text/plain, Size: 1481 bytes --]

diff -u -r1.1.1.6 pcnet32.c
--- linux/drivers/net/pcnet32.c	2001/01/20 11:10:30	1.1.1.6
+++ linux/drivers/net/pcnet32.c	2001/02/14 21:43:28
@@ -648,10 +648,32 @@
 
     printk(KERN_INFO "%s: %s at %#3lx,", dev->name, chipname, ioaddr);
 
-    /* There is a 16 byte station address PROM at the base address.
-     The first six bytes are the station address. */
-    for (i = 0; i < 6; i++)
-      printk(" %2.2x", dev->dev_addr[i] = inb(ioaddr + i));
+    /* In most chips, there is a station address PROM at the base address.
+     * However, if that does not have a valid address, try the "Physical
+     * Address Registers" CSR12-CSR14
+     * Currently, we only check for 00:00:00:00:00:00 as invalid.
+     */
+    {
+    int valid_station=0;
+	/* There is a 16 byte station address PROM at the base address.
+	 The first six bytes are the station address. */
+	for (i = 0; i < 6; i++) {
+	    unsigned int addr = inb(ioaddr + i);
+	    valid_station |= addr;
+	    dev->dev_addr[i] = addr;
+	}
+	if( !valid_station ) {
+	    for (i = 0; i < 3; i++) {
+		unsigned int v;
+		v = a->read_csr(ioaddr, i+12);
+		/* There may be endianness issues here. */
+		dev->dev_addr[2*i] = v & 0x0ff;
+		dev->dev_addr[2*i+1] = (v >> 8) & 0x0ff;
+	    }
+	}
+	for (i = 0; i < 6; i++)
+	    printk(" %2.2x", dev->dev_addr[i] );
+    }
 
     if (((chip_version + 1) & 0xfffe) == 0x2624) { /* Version 0x2623 or 0x2624 */
         i = a->read_csr(ioaddr, 80) & 0x0C00;  /* Check tx_start_pt */

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pcnet32.c: MAC address may be in CSR registers
  2001-02-14 21:49 [PATCH] pcnet32.c: MAC address may be in CSR registers Eli Carter
@ 2001-02-14 23:50 ` Eli Carter
  2001-02-14 23:55   ` Alan Cox
  2001-02-15 13:42   ` Richard B. Johnson
  0 siblings, 2 replies; 8+ messages in thread
From: Eli Carter @ 2001-02-14 23:50 UTC (permalink / raw)
  To: Richard B. Johnson, tsbogend, Alan Cox, Peter Missel
  Cc: linux-kernel, Eli Carter

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

Eli Carter wrote:
> I'm dealing with an AMD chip that does not have the station address in
> the PROM at the base address, but resides in the "Physical Address
> Registers" in the chip (thanks to the bootloader in my case).  This
> patch makes the driver try those registers if the station address read
> from the PROM is 00:00:00:00:00:00.
> I think others dealing with similar setups may find this helpful.  The
> other lance-derived drivers might benefit from a similar patch, but I
> don't have that hardware for testing.

<aside>
Added Peter since he's given feedback on a past pcnet32 patch.
</aside>

Two patches, one for 2.2.18 (patch-pcnet32-mac22), and one for 2.4.1
(patch-pcnet32-mac24) which should each apply cleanly.

Changes:
- Moved address validity check to a function.  (Should this go somewhere
other drivers can call it?)
- Check the second address and set the address to 00:00:00:00:00:00 if
it fails
- Check the address at open time as well, failing with -EINVAL.

I think that should address Alan's initial feedback.

What else?

TIA,

Eli
--------------------.              Rule of Accuracy: When working toward
Eli Carter          |               the solution of a problem, it always 
eli.carter@inet.com `--------------------- helps if you know the answer.

[-- Attachment #2: patch-pcnet32-mac24 --]
[-- Type: text/plain, Size: 2664 bytes --]

--- linux/drivers/net/pcnet32.c.2.4.1	Wed Feb 14 16:49:31 2001
+++ linux/drivers/net/pcnet32.c	Wed Feb 14 17:37:23 2001
@@ -283,6 +283,7 @@
     struct net_device *next;
 };
 
+static int  is_valid_ether_addr( char* );
 static int  pcnet32_probe_vlbus(int cards_found);
 static int  pcnet32_probe_pci(struct pci_dev *, const struct pci_device_id *);
 static int  pcnet32_probe1(unsigned long, unsigned char, int, int, struct pci_dev *);
@@ -437,6 +438,18 @@
 
 \f
 
+/* Check that the ethernet station address is not 00:00:00:00:00:00 and is also
+ * not a multicast address
+ * Return true if the address is valid.
+ */
+int is_valid_ether_addr( char* address )
+{
+    int i,isvalid=0;
+    for( i=0; i<6; i++)
+	isvalid |= address[i]; 
+    return isvalid && !(address[0]&1);
+}
+
 /* only probes for non-PCI devices, the rest are handled by pci_register_driver via pcnet32_probe_pci*/
 static int __init pcnet32_probe_vlbus(int cards_found)
 {
@@ -624,10 +637,33 @@
 
     printk(KERN_INFO "%s: %s at %#3lx,", dev->name, chipname, ioaddr);
 
-    /* There is a 16 byte station address PROM at the base address.
-       The first six bytes are the station address. */
-    for (i = 0; i < 6; i++)
-	printk(" %2.2x", dev->dev_addr[i] = inb(ioaddr + i));
+    /* In most chips, there is a station address PROM at the base address.
+     * However, if that does not have a valid address, try the "Physical
+     * Address Registers" CSR12-CSR14
+     */
+    {
+	/* There is a 16 byte station address PROM at the base address.
+	 The first six bytes are the station address. */
+	for (i = 0; i < 6; i++) {
+	    dev->dev_addr[i] = inb(ioaddr + i);
+	}
+	if( !is_valid_ether_addr(dev->dev_addr) ) {
+	    /* also double check this station address */
+	    for (i = 0; i < 3; i++) {
+		unsigned int val;
+		val = a->read_csr(ioaddr, i+12) & 0x0ffff;
+		/* There may be endianness issues here. */
+		dev->dev_addr[2*i] = val & 0x0ff;
+		dev->dev_addr[2*i+1] = (val >> 8) & 0x0ff;
+	    }
+	    /* if this is not valid either, force to 00:00:00:00:00:00 */
+	    if( !is_valid_ether_addr(dev->dev_addr) )
+		for (i = 0; i < 6; i++)
+		    dev->dev_addr[i]=0;
+	}
+	for (i = 0; i < 6; i++)
+	    printk(" %2.2x", dev->dev_addr[i] );
+    }
 
     if (((chip_version + 1) & 0xfffe) == 0x2624) { /* Version 0x2623 or 0x2624 */
 	i = a->read_csr(ioaddr, 80) & 0x0C00;  /* Check tx_start_pt */
@@ -774,6 +810,10 @@
 		    lp->shared_irq ? SA_SHIRQ : 0, lp->name, (void *)dev)) {
 	return -EAGAIN;
     }
+
+    /* Check for a valid station address */
+    if( !is_valid_ether_addr(dev->dev_addr) )
+	return -EINVAL;
 
     /* Reset the PCNET32 */
     lp->a.reset (ioaddr);

[-- Attachment #3: patch-pcnet32-mac22 --]
[-- Type: text/plain, Size: 2564 bytes --]

diff -u -r1.1.1.6 pcnet32.c
--- linux/drivers/net/pcnet32.c	2001/01/20 11:10:30	1.1.1.6
+++ linux/drivers/net/pcnet32.c	2001/02/14 23:30:51
@@ -281,6 +281,7 @@
 #endif    
 };
 
+static int is_valid_ether_addr( char* );
 int  pcnet32_probe(struct device *);
 static int  pcnet32_probe1(struct device *, unsigned long, unsigned char, int, int);
 static int  pcnet32_open(struct device *);
@@ -442,6 +443,18 @@
 
 \f
 
+/* Check that the ethernet station address is not 00:00:00:00:00:00 and is also
+ * not a multicast address
+ * Return true if the address is valid.
+ */
+int is_valid_ether_addr( char* address )
+{
+    int i,isvalid=0;
+    for( i=0; i<6; i++)
+	isvalid |= address[i]; 
+    return isvalid && !(address[0]&1);
+}
+
 int __init pcnet32_probe (struct device *dev)
 {
     unsigned long ioaddr = dev ? dev->base_addr: 0;
@@ -648,10 +661,33 @@
 
     printk(KERN_INFO "%s: %s at %#3lx,", dev->name, chipname, ioaddr);
 
-    /* There is a 16 byte station address PROM at the base address.
-     The first six bytes are the station address. */
-    for (i = 0; i < 6; i++)
-      printk(" %2.2x", dev->dev_addr[i] = inb(ioaddr + i));
+    /* In most chips, there is a station address PROM at the base address.
+     * However, if that does not have a valid address, try the "Physical
+     * Address Registers" CSR12-CSR14
+     */
+    {
+	/* There is a 16 byte station address PROM at the base address.
+	 The first six bytes are the station address. */
+	for (i = 0; i < 6; i++) {
+	    dev->dev_addr[i] = inb(ioaddr + i);
+	}
+	if( !is_valid_ether_addr(dev->dev_addr) ) {
+	    /* also double check this station address */
+	    for (i = 0; i < 3; i++) {
+		unsigned int val;
+		val = a->read_csr(ioaddr, i+12) & 0x0ffff;
+		/* There may be endianness issues here. */
+		dev->dev_addr[2*i] = val & 0x0ff;
+		dev->dev_addr[2*i+1] = (val >> 8) & 0x0ff;
+	    }
+	    /* if this is not valid either, force to 00:00:00:00:00:00 */
+	    if( !is_valid_ether_addr(dev->dev_addr) )
+		for (i = 0; i < 6; i++)
+		    dev->dev_addr[i]=0;
+	}
+	for (i = 0; i < 6; i++)
+	    printk(" %2.2x", dev->dev_addr[i] );
+    }
 
     if (((chip_version + 1) & 0xfffe) == 0x2624) { /* Version 0x2623 or 0x2624 */
         i = a->read_csr(ioaddr, 80) & 0x0C00;  /* Check tx_start_pt */
@@ -796,6 +832,10 @@
 		    lp->shared_irq ? SA_SHIRQ : 0, lp->name, (void *)dev)) {
 	return -EAGAIN;
     }
+
+    /* Check for a valid station address */
+    if( !is_valid_ether_addr(dev->dev_addr) )
+	return -EINVAL;
 
     /* Reset the PCNET32 */
     lp->a.reset (ioaddr);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pcnet32.c: MAC address may be in CSR registers
  2001-02-14 23:50 ` Eli Carter
@ 2001-02-14 23:55   ` Alan Cox
  2001-02-15 15:55     ` Eli Carter
  2001-02-15 13:42   ` Richard B. Johnson
  1 sibling, 1 reply; 8+ messages in thread
From: Alan Cox @ 2001-02-14 23:55 UTC (permalink / raw)
  To: Eli Carter
  Cc: Richard B. Johnson, tsbogend, Alan Cox, Peter Missel,
	linux-kernel, Eli Carter

> +int is_valid_ether_addr( char* address )
> +{
> +    int i,isvalid=0;
> +    for( i=0; i<6; i++)
> +	isvalid |= address[i]; 
> +    return isvalid && !(address[0]&1);
> +}

static and why not

static inline int is_valid_ea(u8 *addr)
{
	return memcmp(addr, "\000\000\000\000\000\000", 6) && !(addr[0]&1);
}

That all assembles to nice inline code 8)

Looks ok to me, Im picking holes now


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pcnet32.c: MAC address may be in CSR registers
  2001-02-14 23:50 ` Eli Carter
  2001-02-14 23:55   ` Alan Cox
@ 2001-02-15 13:42   ` Richard B. Johnson
  1 sibling, 0 replies; 8+ messages in thread
From: Richard B. Johnson @ 2001-02-15 13:42 UTC (permalink / raw)
  To: Eli Carter; +Cc: tsbogend, Alan Cox, Peter Missel, linux-kernel

On Wed, 14 Feb 2001, Eli Carter wrote:

> Eli Carter wrote:
> > I'm dealing with an AMD chip that does not have the station address in
> > the PROM at the base address, but resides in the "Physical Address
> > Registers" in the chip (thanks to the bootloader in my case).  This
> > patch makes the driver try those registers if the station address read
> > from the PROM is 00:00:00:00:00:00.
> > I think others dealing with similar setups may find this helpful.  The
> > other lance-derived drivers might benefit from a similar patch, but I
> > don't have that hardware for testing.
> 
> <aside>
> Added Peter since he's given feedback on a past pcnet32 patch.
> </aside>
> 
> Two patches, one for 2.2.18 (patch-pcnet32-mac22), and one for 2.4.1
> (patch-pcnet32-mac24) which should each apply cleanly.
> 
> Changes:
> - Moved address validity check to a function.  (Should this go somewhere
> other drivers can call it?)
> - Check the second address and set the address to 00:00:00:00:00:00 if
> it fails
> - Check the address at open time as well, failing with -EINVAL.
> 
> I think that should address Alan's initial feedback.
> 
> What else?
> 
> TIA,

Thanks for copying me on this. If, in the future, it seems reasonable,
I will supply a patch for the new read/write SEEPROM stuff needed for
embedded systems.

If you need to check if this device has a SEEPROM. If it doesn't,
you need to set up about 15 registers that would have been set
upon hard-reset, by the contents of the SEEPROM. Otherwise you
have a very dumb poorly performing chip.


Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

"Memory is like gasoline. You use it up when you are running. Of
course you get it all back when you reboot..."; Actual explanation
obtained from the Micro$oft help desk.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pcnet32.c: MAC address may be in CSR registers
  2001-02-14 23:55   ` Alan Cox
@ 2001-02-15 15:55     ` Eli Carter
  2001-02-15 16:49       ` Alan Cox
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Carter @ 2001-02-15 15:55 UTC (permalink / raw)
  To: Alan Cox
  Cc: Richard B. Johnson, tsbogend, Peter Missel, linux-kernel,
	Eli Carter

Alan Cox wrote:
> 
> > +int is_valid_ether_addr( char* address )
> > +{
> > +    int i,isvalid=0;
> > +    for( i=0; i<6; i++)
> > +     isvalid |= address[i];
> > +    return isvalid && !(address[0]&1);
> > +}
> 
> static and why not

oops, I *meant* static... doesn't gcc do mind reading?  ;)  (I had
static in the declaration, but forgot it on the definition.)

> static inline int is_valid_ea(u8 *addr)
> {
>         return memcmp(addr, "\000\000\000\000\000\000", 6) && !(addr[0]&1);
> }
> 
> That all assembles to nice inline code 8)

Hmm... well, if we're going for _those_ optimizations, shouldn't it be:
	return !(addr[0]&1) && memcmp(addr, "\000\000\000\000\000\000", 6);
so we do the cheaper test first and thus possibly avoid needing to do
the more expensive test? :)

Tell ya what, put that in <linux/etherdevice.h> (if that's the right
place) and then everyone can use it.  ;)  (I'd rather keep the longer
function name... "ea" isn't very helpful to the newer hackers among
us...)

> Looks ok to me, Im picking holes now

:)  That's encouraging.  I still feel like I'm scaling the learning
curve, and I'm feeling rather "green".

Peter pointed out that the contents of the CSR12-14 registers are
initialized from the EEPROM, so reading the EEPROM is superfluous--we
should just read the CSRs and not read the EEPROM.  I think he has a
point, so I'll make that change and submit yet another patch pair.  

Alan, do you want me to put your inline version in <linux/etherdevice.h>
while I'm at it, or what?

Comments?

Eli
--------------------.              Rule of Accuracy: When working toward
Eli Carter          |               the solution of a problem, it always 
eli.carter@inet.com `--------------------- helps if you know the answer.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pcnet32.c: MAC address may be in CSR registers
  2001-02-15 15:55     ` Eli Carter
@ 2001-02-15 16:49       ` Alan Cox
  2001-02-15 19:16         ` Eli Carter
  2001-02-15 20:31         ` Eli Carter
  0 siblings, 2 replies; 8+ messages in thread
From: Alan Cox @ 2001-02-15 16:49 UTC (permalink / raw)
  To: Eli Carter
  Cc: Alan Cox, Richard B. Johnson, tsbogend, Peter Missel,
	linux-kernel, Eli Carter

> Peter pointed out that the contents of the CSR12-14 registers are
> initialized from the EEPROM, so reading the EEPROM is superfluous--we
> should just read the CSRs and not read the EEPROM.  I think he has a
> point, so I'll make that change and submit yet another patch pair.  

I'd rather keep the existing initialisation behaviour of using the eeprom
for 2.2. There are also some power management cases where I am not sure
the values are restored on the pcnet/pci.

For 2.2 conservatism is the key. For 2.4 by all means default to CSR12-14 and
print a warning if they dont match the eeprom value and we'll see what it
shows

> Alan, do you want me to put your inline version in <linux/etherdevice.h>
> while I'm at it, or what?

Sure

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pcnet32.c: MAC address may be in CSR registers
  2001-02-15 16:49       ` Alan Cox
@ 2001-02-15 19:16         ` Eli Carter
  2001-02-15 20:31         ` Eli Carter
  1 sibling, 0 replies; 8+ messages in thread
From: Eli Carter @ 2001-02-15 19:16 UTC (permalink / raw)
  To: Alan Cox
  Cc: Richard B. Johnson, tsbogend, Peter Missel, linux-kernel,
	eli.carter

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

Alan Cox wrote:
> I'd rather keep the existing initialisation behaviour of using the eeprom
> for 2.2. There are also some power management cases where I am not sure
> the values are restored on the pcnet/pci.
> 
> For 2.2 conservatism is the key. For 2.4 by all means default to CSR12-14 and
> print a warning if they dont match the eeprom value and we'll see what it
> shows
> 
> > Alan, do you want me to put your inline version in <linux/etherdevice.h>
> > while I'm at it, or what?
> 
> Sure

Here is the 2.2.18 patch... I'll send the 2.4.1 patch shortly.

This one still uses the PROM since we are going for least change in
initialization.
is_valid_ether_addr() is static inline in <linux/etherdevice.h>

Is this one satisfactory?

Eli
--------------------.              Rule of Accuracy: When working toward
Eli Carter          |               the solution of a problem, it always 
eli.carter@inet.com `--------------------- helps if you know the answer.

[-- Attachment #2: patch-pcnet32-mac22 --]
[-- Type: text/plain, Size: 2356 bytes --]

--- linux/drivers/net/pcnet32.c	2001/01/20 11:10:30	1.1.1.6
+++ linux/drivers/net/pcnet32.c	2001/02/15 19:08:55
@@ -648,10 +648,33 @@
 
     printk(KERN_INFO "%s: %s at %#3lx,", dev->name, chipname, ioaddr);
 
-    /* There is a 16 byte station address PROM at the base address.
-     The first six bytes are the station address. */
-    for (i = 0; i < 6; i++)
-      printk(" %2.2x", dev->dev_addr[i] = inb(ioaddr + i));
+    /* In most chips, there is a station address PROM at the base address.
+     * However, if that does not have a valid address, try the "Physical
+     * Address Registers" CSR12-CSR14
+     */
+    {
+	/* There is a 16 byte station address PROM at the base address.
+	 The first six bytes are the station address. */
+	for (i = 0; i < 6; i++) {
+	    dev->dev_addr[i] = inb(ioaddr + i);
+	}
+	if( !is_valid_ether_addr(dev->dev_addr) ) {
+	    /* also double check this station address */
+	    for (i = 0; i < 3; i++) {
+		unsigned int val;
+		val = a->read_csr(ioaddr, i+12) & 0x0ffff;
+		/* There may be endianness issues here. */
+		dev->dev_addr[2*i] = val & 0x0ff;
+		dev->dev_addr[2*i+1] = (val >> 8) & 0x0ff;
+	    }
+	    /* if this is not valid either, force to 00:00:00:00:00:00 */
+	    if( !is_valid_ether_addr(dev->dev_addr) )
+		for (i = 0; i < 6; i++)
+		    dev->dev_addr[i]=0;
+	}
+	for (i = 0; i < 6; i++)
+	    printk(" %2.2x", dev->dev_addr[i] );
+    }
 
     if (((chip_version + 1) & 0xfffe) == 0x2624) { /* Version 0x2623 or 0x2624 */
         i = a->read_csr(ioaddr, 80) & 0x0C00;  /* Check tx_start_pt */
@@ -796,6 +819,10 @@
 		    lp->shared_irq ? SA_SHIRQ : 0, lp->name, (void *)dev)) {
 	return -EAGAIN;
     }
+
+    /* Check for a valid station address */
+    if( !is_valid_ether_addr(dev->dev_addr) )
+	return -EINVAL;
 
     /* Reset the PCNET32 */
     lp->a.reset (ioaddr);
--- linux/include/linux/etherdevice.h	2001/01/19 19:25:31	1.1.1.1
+++ linux/include/linux/etherdevice.h	2001/02/15 19:08:55
@@ -51,6 +51,14 @@
 				unsigned char *src, int length, int base);
 #endif
 
+/* Check that the ethernet address (MAC) is not 00:00:00:00:00:00 and is not
+ * a multicast address.  Return true if the address is valid.
+ */
+static __inline__ int is_valid_ether_addr( u8 *addr )
+{
+    return !(addr[0]&1) && memcmp( addr, "\0\0\0\0\0\0", 6);
+}
+
 #endif
 
 #endif	/* _LINUX_ETHERDEVICE_H */

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pcnet32.c: MAC address may be in CSR registers
  2001-02-15 16:49       ` Alan Cox
  2001-02-15 19:16         ` Eli Carter
@ 2001-02-15 20:31         ` Eli Carter
  1 sibling, 0 replies; 8+ messages in thread
From: Eli Carter @ 2001-02-15 20:31 UTC (permalink / raw)
  To: Alan Cox; +Cc: Richard B. Johnson, tsbogend, Peter Missel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

Alan Cox wrote:
> 
> > Peter pointed out that the contents of the CSR12-14 registers are
> > initialized from the EEPROM, so reading the EEPROM is superfluous--we
> > should just read the CSRs and not read the EEPROM.  I think he has a
> > point, so I'll make that change and submit yet another patch pair.
> 
> I'd rather keep the existing initialisation behaviour of using the eeprom
> for 2.2. There are also some power management cases where I am not sure
> the values are restored on the pcnet/pci.
> 
> For 2.2 conservatism is the key. For 2.4 by all means default to CSR12-14 and
> print a warning if they dont match the eeprom value and we'll see what it
> shows
> 
> > Alan, do you want me to put your inline version in <linux/etherdevice.h>
> > while I'm at it, or what?
> 
> Sure

Ok, here is the 2.4.1 version.  Note that it is slightly different from
the 2.2.18 version.  This one uses the CSRs and complains if the PROM is
different (but uses the CSRs anyway since those can be overridden by a
bootloader, etc.).

I don't have a 2.4.x tree handy that I can compile at the moment, but I
tested this change in a 2.2.x kernel, so it should be ok.

Is this patch satisfactory?

Eli
--------------------.              Rule of Accuracy: When working toward
Eli Carter          |               the solution of a problem, it always 
eli.carter@inet.com `--------------------- helps if you know the answer.

[-- Attachment #2: patch-pcnet32-mac24 --]
[-- Type: text/plain, Size: 2422 bytes --]

--- linux/drivers/net/pcnet32.c.2.4.1	Wed Feb 14 16:49:31 2001
+++ linux/drivers/net/pcnet32.c	Thu Feb 15 13:48:05 2001
@@ -624,10 +624,35 @@
 
     printk(KERN_INFO "%s: %s at %#3lx,", dev->name, chipname, ioaddr);
 
-    /* There is a 16 byte station address PROM at the base address.
-       The first six bytes are the station address. */
+    /* In most chips, after a chip reset, the ethernet address is read from the
+     * station address PROM at the base address and programmed into the
+     * "Physical Address Registers" CSR12-14.
+     * As a precautionary measure, we read the PROM values and complain if
+     * they disagree with the CSRs.  Either way, we use the CSR values, and
+     * double check that they are valid.
+     */
+    for (i = 0; i < 3; i++) {
+	unsigned int val;
+	val = a->read_csr(ioaddr, i+12) & 0x0ffff;
+	/* There may be endianness issues here. */
+	dev->dev_addr[2*i] = val & 0x0ff;
+	dev->dev_addr[2*i+1] = (val >> 8) & 0x0ff;
+    }
+    {
+	u8 promaddr[6];
+	for (i = 0; i < 6; i++) {
+	    promaddr[i] = inb(ioaddr + i);
+	}
+	if( memcmp( promaddr, dev->dev_addr, 6) )
+	    printk(" warning: PROM address does not match CSR address");
+    }
+    /* if the ethernet address is not valid, force to 00:00:00:00:00:00 */
+    if( !is_valid_ether_addr(dev->dev_addr) )
+	for (i = 0; i < 6; i++)
+	    dev->dev_addr[i]=0;
+
     for (i = 0; i < 6; i++)
-	printk(" %2.2x", dev->dev_addr[i] = inb(ioaddr + i));
+	printk(" %2.2x", dev->dev_addr[i] );
 
     if (((chip_version + 1) & 0xfffe) == 0x2624) { /* Version 0x2623 or 0x2624 */
 	i = a->read_csr(ioaddr, 80) & 0x0C00;  /* Check tx_start_pt */
@@ -774,6 +799,10 @@
 		    lp->shared_irq ? SA_SHIRQ : 0, lp->name, (void *)dev)) {
 	return -EAGAIN;
     }
+
+    /* Check for a valid station address */
+    if( !is_valid_ether_addr(dev->dev_addr) )
+	return -EINVAL;
 
     /* Reset the PCNET32 */
     lp->a.reset (ioaddr);
--- linux/include/linux/etherdevice.h.2.4.1	Thu Feb 15 11:25:46 2001
+++ linux/include/linux/etherdevice.h	Thu Feb 15 13:18:20 2001
@@ -45,6 +45,14 @@
 	memcpy (dest->data, src, len);
 }
 
+/* Check that the ethernet address (MAC) is not 00:00:00:00:00:00 and is not
+ * a multicast address.  Return true if the address is valid.
+ */
+static __inline__ int is_valid_ether_addr( u8 *addr )
+{
+	return !(addr[0]&1) && memcmp( addr, "\0\0\0\0\0\0", 6);
+}
+
 #endif
 
 #endif	/* _LINUX_ETHERDEVICE_H */

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2001-02-15 20:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2001-02-14 21:49 [PATCH] pcnet32.c: MAC address may be in CSR registers Eli Carter
2001-02-14 23:50 ` Eli Carter
2001-02-14 23:55   ` Alan Cox
2001-02-15 15:55     ` Eli Carter
2001-02-15 16:49       ` Alan Cox
2001-02-15 19:16         ` Eli Carter
2001-02-15 20:31         ` Eli Carter
2001-02-15 13:42   ` Richard B. Johnson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox