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

[Eli Carter <eli.carter@inet.com>]

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.