Dear Mike,
In message 200902112029.30937.vapier@gentoo.org you wrote:
please checkout the macaddr branch of the blackfin repo ... there's about 60 changes cookin in there that touch every arch and common/boards/drivers. i'd like to get you to eye em over first before i spam the list ;).
Thanks.
net: new utility functions for working with enetaddr's:
Please ditch str_enetaddr() and rename str_enetaddr_r() into str_enetaddr(); using a static buffer for the return value is nothing I'd like to see in the code. It is trivial for the caller to allocate a buffer on the stack.
eth_parse_enetaddr() should be "int"" and allow for error checking (to catch at least simple format errors).
*: get mac address from environment
I think these N patches should be squashed into one, ot at least a much smaller number of patches.
*: do not initialize bi_enet*addr in global data
Ditto.
kup4k/kup4x: rename load_sernum_ethaddr() to kup_load_sernum_ethaddr() tqm8xx: rename load_sernum_ethaddr() to tqc_load_sernum_ethaddr()
NAK for two reasons:
- I see no reason for such a change, and especially not for adding an empty function load_sernum_ethaddr(). [And you do not give any explanation why you're making such a change either.] - You add the calls to the new kup_load_sernum_ethaddr() / tqc_load_sernum_ethaddr() functions right in the middle of a list of declarations. That's not acceptable.
ppc: do not initialize bi_enet*addr in global data:
... Also stop calling load_sernum_ethaddr() since all boards now implement this as a stub.
Makes no sense to me. I don't even see what such a stub would be needed for or where it was called. Just looks like a waste of memory footprint to me. NAK.
drop now unused load_sernum_ethaddr() function:
Ah. You propbably want to change the order of your changes so you can avoid these intermediate steps. But still please do not rename load_sernum_ethaddr() - there is no need for more complex names.
Thanks for all the work!
Best regards,
Wolfgang Denk