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

Eli Carter (eli.carter@inet.com)
Thu, 15 Feb 2001 09:55:34 -0600


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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/