[U-Boot] [PATCH 01/27 v2] Blackfin: bfin_mac: force board_get_enetaddr() usage

Wolfgang Denk wd at denx.de
Thu Feb 12 08:33:46 CET 2009


Dear Mike,

In message <200902112029.30937.vapier at 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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Only a fool fights in a burning house.
	-- Kank the Klingon, "Day of the Dove", stardate unknown


More information about the U-Boot mailing list