[U-Boot] [PATCH v3] Retrieve MAC address from EEPROM

[Michal Simek]

On 10.11.2016 13:43, Olliver Schinagl wrote:
> On 10-11-16 13:37, Michal Simek wrote:
>> 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.
> In the case of sunxi, we generate a MAC address based off the CPU serial
> number and device ID, which is more predictable and doesn't change
> compared to the random MAC. The random MAC would then in turn be a
> fallback if we still have a 00:00:00:00:00 ethernet address.
> 
> So 3 steps, check EEPROM (i2c only for now, SPI etc can be added later
> with the eeprom uclass)
> call board specific hook
> if empty, generate random (if configured)

yep - not a problem with it.

Thanks,
Michal