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

[Mike Frysinger <vapier@gentoo.org>]

On Monday 02 February 2009 16:04:34 Wolfgang Denk wrote:
> In message Mike Frysinger you wrote:
> > -------
> >  Usage
> > -------
> >
> > During board init (like the board-specific misc_init_r() function),
> > boards should take care of locating the MAC address, initializing the
> > environment, and seeding the global data.
>
> 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.

> > During runtime, the ethernet layer will use the environment variables to
> > sync the MAC addresses to the ethernet structures.  All ethernet driver
> > code should then only use the enetaddr member of the eth_device
> > structure.
>
> 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.

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 ...

> > Any other common code that wishes to access the MAC address should then
> > query the global data directly.  No one should be looking in the
> > environment for any addresses.
>
> Any code that wishes to access the MAC address should then query the
> global data or the environment, depending on whatever (binary or
> string representation) seems more appropriate.

relying on global data only means code can avoid having to handle unset 
variables.  global data will always have something in there.  the 
str_enetaddr() utility function makes this easy ...

> > 	* bool eth_getenv_enetaddr(char *name, uchar *enetaddr);
>
> Make this "int" - we don't use "bool" in U-Boot. This is C code.

bool is a C type ... not that i'll bitch enough to keep it :p

i've merged your other changes locally
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090202/d5a1667a/attachment-0001.pgp