On 15.11.2016 04:27, Joe Hershberger wrote:
On Thu, Nov 10, 2016 at 6:11 AM, Olliver Schinagl oliver@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@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_I2Cdiff --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