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

[Mike Frysinger]

On Thursday 29 January 2009 17:18:00 Wolfgang Denk wrote:
> In message <200901291648.02480.vapier at gentoo.org> you wrote:
> > > > > 	- misc_init_r() [or similar] sets up ethaddr in env if it isnt
> > > > > 	  set already and sets bi_enetaddr in global data
> > > > > 	- board_eth_init() calls the driver init
> > > > > 	  (bfin_EMAC_initialize() in your case)
> > > > > 	- driver init looks up ethaddr in env or bi_enetaddr
> > >
> > > What is wrong with using bi_enetaddr? What sort of "handling/parsing
> > > code" (in addition to a plain simple memcpy(...,6) is needed?
> >
> > converting the envvar to the raw 6 bytes and back again is duplicated all
> > over the tree.  and you suggest that both the board-specific misc_initr()
> > and the driver init should handle this.
>
> No, I don't. I suggest that it gets done once (for example in
> misc_initr()), and that you then use the binary data in bi_enetaddr.

you said:
        - driver init looks up ethaddr in env or bi_enetaddr

at any rate, i'm fine with having the driver assume bi_enetaddr is sane.  so 
the series of patches i just posted starts unifying the things i whined about 
earlier and does what you suggested.

unfortunately, with this small review i noticed another layer of confusion ;).  
every ethernet device is represented as struct eth_device, and that device has 
an enetaddr member.  the board makes sure ethaddr is set in the environment 
during misc init.  then when the network is actually used, the eth layer calls 
the board init which calls the driver init which registers a new eth_device.  
then the eth layer sets up dev->enetaddr based on the appropriate ethaddr env 
var.  so imo, no eth driver should be touching the global data (and thus the 
bi_enetaddr's contained in there).

going back a step, i dont think the board itself should be touching the global 
bi_enetaddr.  when the board sets the ethaddr env var, the common code in 
cmd_nvedit.c syncs the value to the global data.

if i were to document this mess, where would be the best place ?  start a new 
doc/README.enetaddr as i dont see any document that covers the eth layer ?  
that way in the future we can all easily agree on how things should be done.
-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/20090129/84af7670/attachment.pgp