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

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200902021937.26246.vapier at gentoo.org> you wrote:
>
> > Please change:
> >
> > 	If the hardware design mandates that the MAC address is stored
> > 	in some special place, like EEPROM etc., then the board
> > 	specific init code (like the board-specific misc_init_r()
> > 	function) is responsible for locating the MAC address(es) and
> > 	initialize the respective environment variable(s) from it.
> >
> >         Note that this shall be done if, and only if, the environment
> >         does not already contain these environment variables, i. e.
> >         existing variable definitions must not be overwritten.
> >
> > 	The envrionment handling code (function setevn()) will update
> > 	the global data accordingly.
>
> it will update the global data, but nowhere will it initially seed it.  i'm
> thinking env_init() should be a unified entry point that first calls down to 
> the specific env storage (eeprom/flash/nand/etc...) and then initializes the 
> relevant bi_enetaddr members of the global data structure.

No, that would mix functions in a bad way as it creates new
dependencies on what can or must be done when. env_init() in sone
thing (initializes the environment data), but global data init is
another issue.

Maybe we should try and collect the global data initialization into
one place - but I have to admit that I don't know if or how easily
this can be done.

> > Please change:
> >
> > 	During runtime, the ethernet layer will use the global data
> > 	to sync ...
>
> documenting it this way wont change the code ;).  the ethernet code does not 
> use the global data in any way.  look at eth_initialize() in net/eth.c.  i
> imagine part of the reason for this is that working with multiple ethernet 
> devices is pretty ugly atm.  the static list of CONFIG_HAS_ETH{1,2,3,...} 
> makes working with more than one device very messy.  the ethernet code today 
> though builds the string dynamically "eth%iaddr" and so can handle arbitrary 
> number of devices without needing any changes.

Well, I think we agree that the current state is a mess.  Documenting
the mess makes it easier to add to it. But should we not try to clean
up  instead?  I.e. document what we think should be done, and fix the
implementation accordingly?

> this also applies to the cascading of setenv() into the global data.  that
> code only handles bi_enetaddr and none of the bi_enetNaddr ... doing 'set 
> eth1addr ..." will not update bi_enet1addr ...

Agreed - that needs to be fixed, too.

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
Vulcans do not approve of violence.
	-- Spock, "Journey to Babel", stardate 3842.4