Dear Mike Frysinger,
In message 200902021937.26246.vapier@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