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

[Olliver Schinagl]

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!

Olliver
>
> Thanks,
> Michal
>
>
>