[U-Boot] [PATCH 08/11] net: sunxi: Allow sunxi boards to set the MAC from an EEPROM

[Joe Hershberger]

On Thu, Nov 10, 2016 at 6:11 AM, Olliver Schinagl <oliver at schinagl.nl> wrote:
> Hi Michal,
>
>
> On 10-11-16 12:51, Michal Simek wrote:
>>
>> On 8.11.2016 16:54, Olliver Schinagl wrote:
>>>
>>> This patch uses the newly introduced Kconfig options to use the net_op
>>> read_rom_hwaddr to retrieve the MAC from an EEPROM.
>>> This will be especially useful for the Olimex OLinuXino series of sunxi
>>> boards as they all have an 2k i2c eeprom chip.
>>>
>>> The MAC address in the eeprom is ignored (if enabled) if the CRC8 check
>>> fails.
>>>
>>> This new functionality allows for querying multiple MAC addresses. The
>>> first (supported) device being probed gets the first address, the second
>>> the second etc. If a generated MAC address is desired, set it to all 0
>>> (and if crc8 is configured also add that) for the adapter.
>>>
>>> Signed-off-by: Olliver Schinagl <oliver at schinagl.nl>
>>> ---
>>>   board/sunxi/Kconfig |  4 ++++
>>>   board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++-
>>>   2 files changed, 42 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>> index e1d4ab1..6b8ac99 100644
>>> --- a/board/sunxi/Kconfig
>>> +++ b/board/sunxi/Kconfig
>>> @@ -414,6 +414,7 @@ config I2C0_ENABLE
>>>     config I2C1_ENABLE
>>>         bool "Enable I2C/TWI controller 1"
>>> +       default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
>>>         default n
>>>         select CMD_I2C
>>>         ---help---
>>> @@ -421,6 +422,7 @@ config I2C1_ENABLE
>>>     config I2C2_ENABLE
>>>         bool "Enable I2C/TWI controller 2"
>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
>>>         default n
>>>         select CMD_I2C
>>>         ---help---
>>> @@ -428,6 +430,7 @@ config I2C2_ENABLE
>>>     if MACH_SUN6I || MACH_SUN7I
>>>   config I2C3_ENABLE
>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
>>>         bool "Enable I2C/TWI controller 3"
>>>         default n
>>>         select CMD_I2C
>>> @@ -447,6 +450,7 @@ endif
>>>     if MACH_SUN7I
>>>   config I2C4_ENABLE
>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
>>>         bool "Enable I2C/TWI controller 4"
>>>         default n
>>>         select CMD_I2C
>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>> index 71124f4..f1e64cd 100644
>>> --- a/board/sunxi/board.c
>>> +++ b/board/sunxi/board.c
>>> @@ -12,6 +12,7 @@
>>>    */
>>>     #include <common.h>
>>> +#include <i2c.h>
>>>   #include <mmc.h>
>>>   #include <axp_pmic.h>
>>>   #include <asm/arch/clock.h>
>>> @@ -31,6 +32,7 @@
>>>   #include <crc.h>
>>>   #include <environment.h>
>>>   #include <libfdt.h>
>>> +#include <linux/crc8.h>
>>>   #include <nand.h>
>>>   #include <net.h>
>>>   #include <sy8106a.h>
>>> @@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char
>>> *enetaddr, uint8_t cnt)
>>>         memcpy(enetaddr, mac_addr, ARP_HLEN);
>>>   }
>>>   +static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t
>>> cnt)
>>> +{
>>> +       uint8_t eeprom[ARP_HLEN + 1] = { 0x00 };
>>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) &&
>>> defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
>>> +       int old_i2c_bus;
>>> +
>>> +       old_i2c_bus = i2c_get_bus_num();
>>> +       if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS)
>>> +               i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
>>> +       /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */
>>> +       if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
>>> +                    CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN
>>> + 2)),
>>> +                    CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
>>> +                    eeprom, ARP_HLEN + 1)) {
>>> +               i2c_set_bus_num(old_i2c_bus);
>>> +               puts("Could not read the EEPROM; EEPROM missing?\n");
>>> +               return;
>>> +       }
>>> +       i2c_set_bus_num(old_i2c_bus);
>>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
>>> +       if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) {
>>> +               puts("CRC error on MAC address from EEPROM.\n");
>>> +               return;
>>> +       }
>>> +#endif
>>> +#endif
>>> +
>>> +       memcpy(enetaddr, eeprom, ARP_HLEN);
>>> +}
>>> +
>>
>> ok. I have briefly looked at the whole series and I think that this
>> should be done in the core because this should be shared across all
>> drivers.
>> Because what you have above make in general sense for every board which
>> contain mac address in eeprom.
>> That's why I would create
>>
>> eeprom_read_rom_etheaddr() in core which will do stuff as you have above
>> and in driver we will simply assign it to read_rom_hwaddr in drivers or
>> by default for all with options to rewrite it.
>> This function will be empty when !NET_ETHADDR_EEPROM.
>>
>> By this or similar way you open this to all ethernet drivers to read mac
>> just through enabling Kconfig.
>>
>> IMHO doesn't make sense to c&p the same logic over all ethernet drivers.
>
>
> Initially, I do agree very much. But when I first wrote this last year,
> there was no other driver yet etc. It is very very generic so maybe make
> this a weak function up one level, and let the driver override it even?
>
> Makes using the eeprom framework later easier too. I'll cook something up.
> Good idea!

Do you think it is valuable enough to apply as is and retrofit later,
or just wait on this series?

Thanks,
-Joe