
On Tue, Jan 26, 2016 at 10:35 AM, Olliver Schinagl o.schinagl@ultimaker.com wrote:
Hey Joe,
On 26-01-16 17:23, Joe Hershberger wrote:
On Tue, Jan 26, 2016 at 10:10 AM, Olliver Schinagl o.schinagl@ultimaker.com wrote:
Hey Joe
On 26-01-16 01:32, Joe Hershberger wrote:
On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl o.schinagl@ultimaker.com wrote:
This patch uses the newly introduced Kconfig options to set the MAC address 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 is in the eeprom is ignored if there is already a MAC
"The MAC address in the eeprom is ignored..."
oops :) fixed for v3
address in the environment or (if enabled) the CRC8 check fails.
Signed-off-by: Olliver Schinagl o.schinagl@ultimaker.com
board/sunxi/Kconfig | 4 ++++ board/sunxi/board.c | 36 ++++++++++++++++++++++++++++++++++++ net/Kconfig | 1 + 3 files changed, 41 insertions(+)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index 2dd9d3b..a2da3a6 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -339,18 +339,21 @@ config I2C0_ENABLE
config I2C1_ENABLE bool "Enable I2C/TWI controller 1"
default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1) default n ---help--- See I2C0_ENABLE help text.
config I2C2_ENABLE bool "Enable I2C/TWI controller 2"
default y if NET_ETHADDR_EEPROM_I2C_BUS = 2 default n ---help--- See I2C0_ENABLE help text.
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 ---help---
@@ -359,6 +362,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 ---help---
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 6ac398c..28310a2 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> @@ -23,6 +24,7 @@ #include <asm/arch/usb_phy.h> #include <asm/gpio.h> #include <asm/io.h> +#include <linux/crc8.h> #include <nand.h> #include <net.h>
@@ -510,6 +512,37 @@ void get_board_serial(struct tag_serialnr *serialnr) } #endif
+#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C) +static int read_mac_from_eeprom(uint8_t *mac_addr) +{
uint8_t eeprom[7];
int old_i2c_bus;
old_i2c_bus = i2c_get_bus_num();
i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
CONFIG_NET_ETHADDR_EEPROM_OFFSET,
CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
eeprom, 7)) {
i2c_set_bus_num(old_i2c_bus);
puts("Could not read the EEPROM; EEPROM missing?\n");
return -1;
}
i2c_set_bus_num(old_i2c_bus);
+#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
if (crc8(eeprom, 6) != eeprom[6]) {
puts("CRC error on MAC address from EEPROM.\n");
return -1;
}
+#endif
memcpy(mac_addr, eeprom, 6);
return 0;
+} +#else +static int read_mac_from_eeprom(uint8_t *mac_addr) { return -1; } +#endif
- #if !defined(CONFIG_SPL_BUILD) #include <asm/arch/spl.h>
@@ -553,6 +586,9 @@ int misc_init_r(void) } #endif
if ((!getenv("ethaddr")) && (!read_mac_from_eeprom(mac_addr)))
eth_setenv_enetaddr("ethaddr", mac_addr);
It seems a bit odd to actually write this to the environment instead of it being a read_rom_hwaddr operation for the Ethernet driver that this board uses.
Do we need a different board-level ROM callback? Maybe the drivers used in such situations can be the only ones to support it.
I'm hoping your thinking out-loud, as I'm not too familiar with all the
Indeed. I was thinking out loud.
backing code. But if I understand you correctly, you suggest to implement a new callback for ethernet drivers to get mac's from eeprom? Anyhow, I just
Not for now... at least not in the core code.
followed suit with how the address gets set in case of when it is not in the environment and we need to generate a random one.
Sure. I don't think it's the same case though. We already have the entry for the driver to read the rom ethaddr. I think you should use that.
I think I lost you here, what function/example can you point me to that uses this already so I can spy/learn from?
I think you'll be the first driver ported to use this (it's driver model only).
It should be fairly straightforward. I've mocked up a simple version in the Zynq GEM driver as an example.
diff --git a/arch/arm/mach-zynq/include/mach/sys_proto.h b/arch/arm/mach-zynq/include/mach/sys_proto.h index 882beab..44c9b50 100644 --- a/arch/arm/mach-zynq/include/mach/sys_proto.h +++ b/arch/arm/mach-zynq/include/mach/sys_proto.h @@ -19,6 +19,8 @@ extern int zynq_slcr_get_mio_pin_status(const char *periph); extern void zynq_ddrc_init(void); extern unsigned int zynq_get_silicon_version(void);
+int zynq_board_read_rom_ethaddr(unsigned char *ethaddr); + /* Driver extern functions */ extern void ps7_init(void);
diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index 414f530..ca617a5 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -122,6 +122,16 @@ int board_eth_init(bd_t *bis) return ret; }
+#ifdef CONFIG_TARGET_ZYNQ_ZC770 +int zynq_board_read_rom_ethaddr(unsigned char *ethaddr) +{ + // TODO: Read from eeprom + memset(ethaddr, 0, ARP_HLEN); + + return 0; +} +#endif + int dram_init(void) { #if CONFIG_IS_ENABLED(OF_CONTROL) diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 7059c84..068f3d0 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -588,6 +588,23 @@ static void zynq_gem_halt(struct udevice *dev) ZYNQ_GEM_NWCTRL_TXEN_MASK, 0); }
+__weak int zynq_board_read_rom_ethaddr(unsigned char *ethaddr) +{ + return -ENOSYS; +} + +static int zynq_gem_read_rom_mac(struct udevice *dev) +{ + int retval; + struct eth_pdata *pdata = dev_get_platdata(dev); + + retval = zynq_board_read_rom_ethaddr(pdata->enetaddr); + if (retval == -ENOSYS) + retval = 0; + + return retval; +} + static int zynq_gem_miiphy_read(struct mii_dev *bus, int addr, int devad, int reg) { @@ -661,6 +678,7 @@ static const struct eth_ops zynq_gem_ops = { .free_pkt = zynq_gem_free_pkt, .stop = zynq_gem_halt, .write_hwaddr = zynq_gem_setup_mac, + .read_rom_hwaddr = zynq_gem_read_rom_mac, };
static int zynq_gem_ofdata_to_platdata(struct udevice *dev) --
Hopefully this makes it more clear what I have in mind.
Thanks, -Joe
olliver
I was reading the u-boot documentation and though the assumed method was to use the environment and to only use other methods (such as the eeprom method) where it is absolutely a must. In this case the eeprom is not directly related to the ethernet, it's a generic eeprom.
Sure. I think you should add support to whatever driver your board uses (it's a driver model driver, right?) for a weak board function. That way that one driver is responsible for that behavior because of your board's need. Any other boards that use that driver just don't supply that function and it is a no-op. I'd rather be able to easily follow where that ethaddr is getting configured instead of using misc_init_r() and having the env populate magically. This also means you'll be in a context to write the ethaddr to the pdata struct instead of the env.
But then again, I am new to this stuff so I can be gladly very wrong here :)
Seems it's not a case that comes up, so it's fairly new territory.
ret = sunxi_get_sid(sid); if (ret == 0 && sid[0] != 0 && sid[3] != 0) { if (!getenv("ethaddr")) {
diff --git a/net/Kconfig b/net/Kconfig index aced51e..0f4cc5a 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -8,6 +8,7 @@ menuconfig NET if NET
config NET_ETHADDR_EEPROM
depends on ARCH_SUNXI bool "Get ethaddr from eeprom" help Selecting this will try to get the Ethernet address from an
onboard
Thanks, -Joe
-- Met vriendelijke groeten, Kind regards, 与亲切的问候
Olliver Schinagl Software Engineer Research & Development Ultimaker B.V.