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

Eli Carter
2001-02-15T15:56:24Z

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?