2001-02-14 21:51:28

by Eli Carter

[permalink] [raw]
Subject: [PATCH] pcnet32.c: MAC address may be in CSR registers

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 */


Attachments:
patch-pcnet32-mac (1.45 kB)

2001-02-14 23:50:58

by Eli Carter

[permalink] [raw]
Subject: Re: [PATCH] pcnet32.c: MAC address may be in CSR registers

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 @@



+/* 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);


Attachments:
patch-pcnet32-mac24 (2.60 kB)
patch-pcnet32-mac22 (2.50 kB)
Download all attachments

2001-02-14 23:56:58

by Alan

[permalink] [raw]
Subject: Re: [PATCH] pcnet32.c: MAC address may be in CSR registers

> +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

2001-02-15 13:43:53

by Richard B. Johnson

[permalink] [raw]
Subject: Re: [PATCH] pcnet32.c: MAC address may be in CSR registers

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.


2001-02-15 15:56:24

by Eli Carter

[permalink] [raw]
Subject: Re: [PATCH] pcnet32.c: MAC address may be in CSR registers

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
[email protected] `--------------------- helps if you know the answer.

2001-02-15 16:52:02

by Alan

[permalink] [raw]
Subject: Re: [PATCH] pcnet32.c: MAC address may be in CSR registers

> 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

2001-02-15 19:16:56

by Eli Carter

[permalink] [raw]
Subject: Re: [PATCH] pcnet32.c: MAC address may be in CSR registers

--- 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 */


Attachments:
patch-pcnet32-mac22 (2.30 kB)

2001-02-15 20:31:20

by Eli Carter

[permalink] [raw]
Subject: Re: [PATCH] pcnet32.c: MAC address may be in CSR registers

--- 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 */


Attachments:
patch-pcnet32-mac24 (2.37 kB)