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

2016-11-10 Thread 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 

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

2016-11-10 Thread Michal Simek
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 

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

2016-11-10 Thread Olliver Schinagl

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 

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

2016-11-10 Thread Olliver Schinagl

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 ...


Olliver

[0] http://git.denx.de/?p=u-boot.git;a=commit;h=6919b4bf363574
[1] https://www.mail-archive.com/u-boot@lists.denx.de/msg230179.html


Thanks,
Michal



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-11-10 Thread Michal Simek
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?

Thanks,
Michal

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-11-10 Thread Olliver Schinagl

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.


Thanks,
Michal




___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-11-10 Thread Michal Simek
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.

Thanks,
Michal


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot