On 10.11.2016 13:31, Olliver Schinagl wrote: > On 10-11-16 13:26, Michal Simek wrote: >> On 10.11.2016 13:08, Olliver Schinagl wrote: >>> Hi Michal, >>> >>> On 10-11-16 12:37, Michal Simek wrote: >>>> On 8.11.2016 16:54, Olliver Schinagl wrote: >>>>> This patch-series introduces methods to retrieve the MAC address >>>>> from an >>>>> onboard EEPROM using the read_rom_hwaddr hook. >>>>> >>>>> The reason we might want to read the MAC address from an EEPROM >>>>> instead of >>>>> storing the entire environment is mostly a size thing. Our default >>>>> environment >>>>> already is bigger then the EEPROM so it is understandable that >>>>> someone might >>>>> not give up the entire eeprom just for the u-boot environment. >>>>> Especially if >>>>> only board specific things might be stored in the eeprom (MAC, >>>>> serial, product >>>>> number etc). Additionally it is a board thing and a MAC address >>>>> might be >>>>> programmed at the factory before ever seeing any software. >>>>> >>>>> The current idea of the eeprom layout, is to skip the first 8 bytes, >>>>> so that >>>>> other information can be stored there if needed, for example a header >>>>> with some >>>>> magic to identify the EEPROM. Or equivalent purposes. >>>>> >>>>> After those 8 bytes the MAC address follows the first macaddress. The >>>>> macaddress >>>>> is appended by a CRC8 byte and then padded to make for nice 8 bytes. >>>>> Following >>>>> the first macaddress one can store a second, or a third etc etc mac >>>>> addresses. >>>>> >>>>> The CRC8 is optional (via a define) but is strongly recommended to >>>>> have. It >>>>> helps preventing user error and more importantly, checks if the bytes >>>>> read are >>>>> actually a user inserted address. E.g. only writing 1 macaddress into >>>>> the eeprom >>>>> but trying to consume 2. >>>>> >>>>> Hans de Goede and I talked about retrieving the MAC from the EEPROM >>>>> for sunxi >>>>> based boards a while ago, but hopefully this patch makes possible to >>>>> have >>>>> something slightly more generic, even if only the configuration >>>>> options. >>>>> Additionally the EEPROM layout could be recommended by u-boot as well. >>>>> >>>>> Since the Olimex OLinuXino sunxi boards all seem to have an eeprom, I >>>>> started >>>>> my work on one of these and tested the implementation with one of our >>>>> own real >>>>> MAC addresses. >>>>> >>>>> What still needs disussing I think, is the patches that remove the >>>>> 'setup_environment' function in board.c. I think we have put >>>>> functionality in >>>>> boards.c which does not belong. Injecting ethernet addresses into the >>>>> environment instead of using the net_op hooks as well as parsing the >>>>> fdt to get >>>>> overrides from. I think especially this last point should be done at >>>>> a higher >>>>> level, if possible at all. >>>>> >>>>> I explicitly did not use the wiser eth_validate_ethaddr_str(), >>>>> eth_parse_enetaddr() and the ARP_HLEN define as it was quite painful >>>>> (dependancies) to get these functions into the tools. I would suggest >>>>> to merge >>>>> as is, and if someone wants to improve these simple tools to use >>>>> these functions >>>>> to happily do so. >>>>> >>>>> These patches where tested on Olimex OLinuXino Lime1 (A10/A20), Lime2 >>>>> (NAND >>>>> and eMMC) and A20-OLinuXino-MICRO-4G variants and have been in use >>>>> internally on our production systems since v2 of this patch set. >>>>> >>>>> As a recommendation, I would suggest for the zynq to adopt the config >>>>> defines, >>>>> as they are nice and generic and suggest to also implement the 8 byte >>>>> offset, >>>>> crc8 and pad bytes. >>>> Yes, Zynq and ZynqMP is using this feature. I am also aware about using >>>> qspi OTP region for storing information like this. >>> I saw, which triggered me here. What the Znyq currently does it uses its >>> own private CONFIG setting: >>> >>> +int zynq_board_read_rom_ethaddr(unsigned char *ethaddr) >>> +{ >>> +#if defined(CONFIG_ZYNQ_GEM_EEPROM_ADDR) && \ >>> + defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET) && \ >>> + defined(CONFIG_ZYNQ_EEPROM_BUS) >>> + i2c_set_bus_num(CONFIG_ZYNQ_EEPROM_BUS); >>> + >>> + if (eeprom_read(CONFIG_ZYNQ_GEM_EEPROM_ADDR, >>> + CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET, >>> + ethaddr, 6)) >>> + printf("I2C EEPROM MAC address read failed\n"); >>> +#endif >>> + >>> + return 0; >>> +} >>> >>> which are ZNYQ specific. In my patchset I give them very generic names >>> as they can be used by anybody really. >>> >>> Once Maxime's patches have merged and stabilized, i'd even say to switch >>> over to the eeprom framework. >> Can you give me that link to these patches? > Well [0] is your own patch, so that is easy :) [1] is Maxime's work. But > with your generic comment, this entire function probably can simply go > then. The only thing I haven't figured out/thought through yet, if > eeprom reading fails, we want to fall back to the old board specific > method. But I think I know what might do there ...
Joe recently applied one our patch http://lists.denx.de/pipermail/u-boot/2016-November/271662.html which in case of NET_RAMDOM_ETHADDR generates random address. I don't have in my mind exact calling sequence but I expect when eeprom read failed then random eth is generated if selected or warning, etc. Thanks, Michal _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot