Messages in this thread | | | Date | Thu, 15 Feb 2001 09:55:34 -0600 | From | Eli Carter <> | 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 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/
| |