On 1/18/2010 11:45 AM, Ben Warren wrote:
int misc_init_r(void) { +#if defined(CONFIG_CMD_NET)
- uchar mac_id[6];
- if (!eth_getenv_enetaddr("ethaddr", mac_id) &&
!i2c_read_mac(mac_id))
eth_setenv_enetaddr("ethaddr", mac_id);It's really not a good idea to programatically set an environment variable like this. Also, there's already a well-defined and documented protocol for how MAC addresses should be read and the precedence between EEPROM storage and environment storage. Please find a way to fit into that model. Since you don't have network support yet, I have no idea which network controller you're using, and recommend waiting until this is really needed.
I am referring to doc/README.enetaddr in the uboot source. This document says "If the hardware design mandates that the MAC address is stored in some special place (like EEPROM etc...), then the board specific init code (such as the board-specific misc_init_r() function) is responsible for locating the MAC address(es) and initializing 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."
Coming back to the code if (!eth_getenv_enetaddr("ethaddr", mac_id) && !i2c_read_mac(mac_id)) eth_setenv_enetaddr("ethaddr", mac_id);
first I read the env variable "ethaddr" and skip setting it if it already exists then I read mac id from i2c memory and store the same in env variable
Please let me know if I am missing anything.
Although the patch is not really needed right now, it will be needed soon. I am planning to send the net support patch during the next merging window. Since the current patch does not have any side effects, it can be included as of now.
if (chip->cpufreq == -1)@@ -156,6 +215,13 @@ int do_chip_config(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) else printf("DDR Type = Not Known\n");
if (!i2c_read_mac(mac)) {sprintf(i2c_mac, "%pM", mac);printf("Ethaddr (from i2c mem) = %s\n", i2c_mac);Maybe I'm missing some context, but why's the sprintf needed?
Yes, sprintf is not needed here Thanks for pointing out
Regards Vipin