On Thu, Nov 10, 2016 at 6:11 AM, Olliver Schinagl <oli...@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 <oli...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot