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

[Michal Simek]

On 15.11.2016 04:27, Joe Hershberger wrote:
> 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?

TBH I would split this series to two to get Sunxi part in and this get
later.

Thanks,
Michal