All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] sunxi: net: Use net_ops hooks to get the MAC
@ 2016-11-25 15:38 Olliver Schinagl
  2016-11-25 15:38 ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Olliver Schinagl
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-25 15:38 UTC (permalink / raw)
  To: u-boot

This patch series is split off from my previous series,
[PATCH v3] Retrieve MAC address from EEPROM
and only adds, uses and fixes the read/write rom_hwaddr hooks.

I changed things around as suggested by Hans. As far as I can tell this
should work now as you described. The fdt_fixup_ethernet function should
populate the environment as expected.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook
  2016-11-25 15:38 [U-Boot] [PATCH] sunxi: net: Use net_ops hooks to get the MAC Olliver Schinagl
@ 2016-11-25 15:38 ` Olliver Schinagl
  2016-11-25 15:46   ` [U-Boot] [PATCH] Use eth_ops hooks to set the MAC address Olliver Schinagl
  2016-11-27 17:02   ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Simon Glass
  2016-11-25 15:38 ` [U-Boot] [PATCH 2/6] net: sunxi-emac: Write HW address via function Olliver Schinagl
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-25 15:38 UTC (permalink / raw)
  To: u-boot

Add the read_rom_hwaddr net_op hook so that it can be called from boards
to read the mac from a ROM chip.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/net/designware.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 9e6d726..3f2f67c 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -230,6 +230,21 @@ static int _dw_write_hwaddr(struct dw_eth_dev *priv, u8 *mac_id)
 	return 0;
 }
 
+__weak int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
+{
+	return -ENOSYS;
+}
+
+static int designware_eth_read_rom_hwaddr(struct udevice *dev)
+{
+	struct eth_pdata *pdata = dev_get_platdata(dev);
+
+	if (!dev)
+		return -ENOSYS;
+
+	return dw_board_read_rom_hwaddr(pdata->enetaddr, dev->seq);
+}
+
 static void dw_adjust_link(struct eth_mac_regs *mac_p,
 			   struct phy_device *phydev)
 {
@@ -685,6 +700,7 @@ static const struct eth_ops designware_eth_ops = {
 	.free_pkt		= designware_eth_free_pkt,
 	.stop			= designware_eth_stop,
 	.write_hwaddr		= designware_eth_write_hwaddr,
+	.read_rom_hwaddr	= designware_eth_read_rom_hwaddr,
 };
 
 static int designware_eth_ofdata_to_platdata(struct udevice *dev)
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 2/6] net: sunxi-emac: Write HW address via function
  2016-11-25 15:38 [U-Boot] [PATCH] sunxi: net: Use net_ops hooks to get the MAC Olliver Schinagl
  2016-11-25 15:38 ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Olliver Schinagl
@ 2016-11-25 15:38 ` Olliver Schinagl
  2016-11-30 21:10   ` Joe Hershberger
  2017-03-27 16:51   ` [U-Boot] " Joe Hershberger
  2016-11-25 15:38 ` [U-Boot] [PATCH 3/6] net: sunxi-emac: Add write_hwaddr net_op hook Olliver Schinagl
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-25 15:38 UTC (permalink / raw)
  To: u-boot

Currently the mac address is programmed directly in _sunxi_emac_eth_init
making it a one time inflexible operation. By moving it into a separate
function, we can now use this more flexibly.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/net/sunxi_emac.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/sunxi_emac.c b/drivers/net/sunxi_emac.c
index 11cd0ea..99339db 100644
--- a/drivers/net/sunxi_emac.c
+++ b/drivers/net/sunxi_emac.c
@@ -327,6 +327,20 @@ static void emac_reset(struct emac_eth_dev *priv)
 	udelay(200);
 }
 
+static int _sunxi_write_hwaddr(struct emac_eth_dev *priv, u8 *enetaddr)
+{
+	struct emac_regs *regs = priv->regs;
+	u32 enetaddr_lo, enetaddr_hi;
+
+	enetaddr_lo = enetaddr[2] | (enetaddr[1] << 8) | (enetaddr[0] << 16);
+	enetaddr_hi = enetaddr[5] | (enetaddr[4] << 8) | (enetaddr[3] << 16);
+
+	writel(enetaddr_hi, &regs->mac_a1);
+	writel(enetaddr_lo, &regs->mac_a0);
+
+	return 0;
+}
+
 static int _sunxi_emac_eth_init(struct emac_eth_dev *priv, u8 *enetaddr)
 {
 	struct emac_regs *regs = priv->regs;
@@ -350,10 +364,7 @@ static int _sunxi_emac_eth_init(struct emac_eth_dev *priv, u8 *enetaddr)
 	/* Set up EMAC */
 	emac_setup(priv);
 
-	writel(enetaddr[0] << 16 | enetaddr[1] << 8 | enetaddr[2],
-	       &regs->mac_a1);
-	writel(enetaddr[3] << 16 | enetaddr[4] << 8 | enetaddr[5],
-	       &regs->mac_a0);
+	_sunxi_write_hwaddr(priv, enetaddr);
 
 	mdelay(1);
 
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 3/6] net: sunxi-emac: Add write_hwaddr net_op hook
  2016-11-25 15:38 [U-Boot] [PATCH] sunxi: net: Use net_ops hooks to get the MAC Olliver Schinagl
  2016-11-25 15:38 ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Olliver Schinagl
  2016-11-25 15:38 ` [U-Boot] [PATCH 2/6] net: sunxi-emac: Write HW address via function Olliver Schinagl
@ 2016-11-25 15:38 ` Olliver Schinagl
  2016-11-30 21:32   ` Joe Hershberger
  2016-11-25 15:38 ` [U-Boot] [PATCH 4/6] net: sunxi-emac: Add read_rom_hwaddr " Olliver Schinagl
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-25 15:38 UTC (permalink / raw)
  To: u-boot

Expose enetaddr writing via net_ops so that it can be hooked into.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/net/sunxi_emac.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/sunxi_emac.c b/drivers/net/sunxi_emac.c
index 99339db..bba8630 100644
--- a/drivers/net/sunxi_emac.c
+++ b/drivers/net/sunxi_emac.c
@@ -554,6 +554,17 @@ static void sunxi_emac_eth_stop(struct udevice *dev)
 	/* Nothing to do here */
 }
 
+static int sunxi_emac_eth_write_hwaddr(struct udevice *dev)
+{
+	struct eth_pdata *pdata = dev_get_platdata(dev);
+	struct emac_eth_dev *priv = dev_get_priv(dev);
+
+	if (!dev)
+		return -ENOSYS;
+
+	return _sunxi_write_hwaddr(priv, pdata->enetaddr);
+}
+
 static int sunxi_emac_eth_probe(struct udevice *dev)
 {
 	struct eth_pdata *pdata = dev_get_platdata(dev);
@@ -570,6 +581,7 @@ static const struct eth_ops sunxi_emac_eth_ops = {
 	.send			= sunxi_emac_eth_send,
 	.recv			= sunxi_emac_eth_recv,
 	.stop			= sunxi_emac_eth_stop,
+	.write_hwaddr		= sunxi_emac_eth_write_hwaddr,
 };
 
 static int sunxi_emac_eth_ofdata_to_platdata(struct udevice *dev)
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 4/6] net: sunxi-emac: Add read_rom_hwaddr net_op hook
  2016-11-25 15:38 [U-Boot] [PATCH] sunxi: net: Use net_ops hooks to get the MAC Olliver Schinagl
                   ` (2 preceding siblings ...)
  2016-11-25 15:38 ` [U-Boot] [PATCH 3/6] net: sunxi-emac: Add write_hwaddr net_op hook Olliver Schinagl
@ 2016-11-25 15:38 ` Olliver Schinagl
  2016-11-30 21:34   ` Joe Hershberger
  2016-11-25 15:38 ` [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address Olliver Schinagl
  2016-11-25 15:38 ` [U-Boot] [PATCH 6/6] net: sunxi: Enable eeprom on OLinuXino Lime boards Olliver Schinagl
  5 siblings, 1 reply; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-25 15:38 UTC (permalink / raw)
  To: u-boot

Add the read_rom_hwaddr net_op hook so that it can be called from boards
to read the mac from a ROM chip.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/net/sunxi_emac.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/sunxi_emac.c b/drivers/net/sunxi_emac.c
index bba8630..7d45f8b 100644
--- a/drivers/net/sunxi_emac.c
+++ b/drivers/net/sunxi_emac.c
@@ -565,6 +565,21 @@ static int sunxi_emac_eth_write_hwaddr(struct udevice *dev)
 	return _sunxi_write_hwaddr(priv, pdata->enetaddr);
 }
 
+__weak int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
+{
+	return -ENOSYS;
+}
+
+static int sunxi_emac_eth_read_rom_hwaddr(struct udevice *dev)
+{
+	struct eth_pdata *pdata = dev_get_platdata(dev);
+
+	if (!dev)
+		return -ENOSYS;
+
+	return sunxi_emac_board_read_rom_hwaddr(pdata->enetaddr, dev->seq);
+}
+
 static int sunxi_emac_eth_probe(struct udevice *dev)
 {
 	struct eth_pdata *pdata = dev_get_platdata(dev);
@@ -582,6 +597,7 @@ static const struct eth_ops sunxi_emac_eth_ops = {
 	.recv			= sunxi_emac_eth_recv,
 	.stop			= sunxi_emac_eth_stop,
 	.write_hwaddr		= sunxi_emac_eth_write_hwaddr,
+	.read_rom_hwaddr	= sunxi_emac_eth_read_rom_hwaddr,
 };
 
 static int sunxi_emac_eth_ofdata_to_platdata(struct udevice *dev)
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address
  2016-11-25 15:38 [U-Boot] [PATCH] sunxi: net: Use net_ops hooks to get the MAC Olliver Schinagl
                   ` (3 preceding siblings ...)
  2016-11-25 15:38 ` [U-Boot] [PATCH 4/6] net: sunxi-emac: Add read_rom_hwaddr " Olliver Schinagl
@ 2016-11-25 15:38 ` Olliver Schinagl
  2016-11-30 21:36   ` Joe Hershberger
  2016-11-25 15:38 ` [U-Boot] [PATCH 6/6] net: sunxi: Enable eeprom on OLinuXino Lime boards Olliver Schinagl
  5 siblings, 1 reply; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-25 15:38 UTC (permalink / raw)
  To: u-boot

Add board hooks allowing to get ethernet addresses in a board specific
manner. Currently this is done by generating a MAC address from
the SID and injecting the ethernet device number in the first octet.

This usually happens as a fallback, if either the eeprom fails to set a
MAC address or the FDT forces an override.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
 board/sunxi/board.c                         | 161 +++++++++++++++-------------
 net/eth_legacy.c                            |   1 +
 3 files changed, 98 insertions(+), 75 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h b/arch/arm/include/asm/arch-sunxi/sys_proto.h
index a373319..fad7c48 100644
--- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
+++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
@@ -30,4 +30,15 @@ void eth_init_board(void);
 static inline void eth_init_board(void) {}
 #endif
 
+int board_get_enetaddr(const int i, unsigned char *mac_addr);
+
+#if CONFIG_SUNXI_EMAC
+int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
+#endif
+
+#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
+int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
+#endif
+
+
 #endif
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 5365638..4aeab51 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -21,6 +21,7 @@
 #include <asm/arch/gpio.h>
 #include <asm/arch/mmc.h>
 #include <asm/arch/spl.h>
+#include <asm/arch/sys_proto.h>
 #include <asm/arch/usb_phy.h>
 #ifndef CONFIG_ARM64
 #include <asm/armv7.h>
@@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
 }
 #endif
 
+int sunxi_get_board_serial(unsigned int *serial)
+{
+	int ret;
+
+	ret = sunxi_get_sid(serial);
+	if (!ret || serial[0])
+		return -ENOSYS;
+
+	/*
+	 * The single words 1 - 3 of the SID have quite a few bits
+	 * which are the same on many models, so we take a crc32
+	 * of all 3 words, to get a more unique value.
+	 *
+	 * Note we only do this on newer SoCs as we cannot change
+	 * the algorithm on older SoCs since those have been using
+	 * fixed mac-addresses/serial based on only using word 3 for a
+	 * long time and changing a fixed mac-address/serial with an
+	 * u-boot update is not good.
+	 */
+#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
+    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
+    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
+	serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
+#endif
+
+	return 0;
+}
+
 #ifdef CONFIG_SERIAL_TAG
 void get_board_serial(struct tag_serialnr *serialnr)
 {
@@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr *serialnr)
 #endif
 
 /*
+ * Generate a MAC address based on device index and the serial number.
+ * The first half of the of the first octet holds the eth index.
+ *
+ * In the second octet we forcefully mark the MAC address to a locally
+ * administered MAC address.
+ *
+ */
+int board_get_enetaddr(const int index, unsigned char *enetaddr)
+{
+	uint8_t mac_addr[ARP_HLEN] = { 0x00 };
+	unsigned int serial[4];
+	int ret;
+
+	if ((index < 0) || !enetaddr)
+		return -ENOSYS;
+
+	ret = sunxi_get_board_serial(serial);
+	if (!ret)
+		return ret;
+
+	/* Ensure the NIC specific bytes of the mac are not all 0 */
+	if ((serial[3] & 0xffffff) == 0)
+		serial[3] |= 0x800000;
+
+	mac_addr[0] = (index << 4);
+	mac_addr[1] = (serial[0] >>  0) & 0xff;
+	mac_addr[2] = (serial[3] >> 24) & 0xff;
+	mac_addr[3] = (serial[3] >> 16) & 0xff;
+	mac_addr[4] = (serial[3] >>  8) & 0xff;
+	mac_addr[5] = (serial[3] >>  0) & 0xff;
+
+	set_local_ethaddr(mac_addr);
+	memcpy(enetaddr, mac_addr, ARP_HLEN);
+
+	return 0;
+}
+
+int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
+{
+	return board_get_enetaddr(id, enetaddr);
+}
+
+int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
+{
+	return board_get_enetaddr(id, enetaddr);
+}
+
+/*
  * Check the SPL header for the "sunxi" variant. If found: parse values
  * that might have been passed by the loader ("fel" utility), and update
  * the environment accordingly.
@@ -617,77 +694,10 @@ static void parse_spl_header(const uint32_t spl_addr)
 	setenv_hex("fel_scriptaddr", spl->fel_script_address);
 }
 
-/*
- * Note this function gets called multiple times.
- * It must not make any changes to env variables which already exist.
- */
-static void setup_environment(const void *fdt)
-{
-	char serial_string[17] = { 0 };
-	unsigned int sid[4];
-	uint8_t mac_addr[6];
-	char ethaddr[16];
-	int i, ret;
-
-	ret = sunxi_get_sid(sid);
-	if (ret == 0 && sid[0] != 0) {
-		/*
-		 * The single words 1 - 3 of the SID have quite a few bits
-		 * which are the same on many models, so we take a crc32
-		 * of all 3 words, to get a more unique value.
-		 *
-		 * Note we only do this on newer SoCs as we cannot change
-		 * the algorithm on older SoCs since those have been using
-		 * fixed mac-addresses based on only using word 3 for a
-		 * long time and changing a fixed mac-address with an
-		 * u-boot update is not good.
-		 */
-#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
-    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
-    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
-		sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
-#endif
-
-		/* Ensure the NIC specific bytes of the mac are not all 0 */
-		if ((sid[3] & 0xffffff) == 0)
-			sid[3] |= 0x800000;
-
-		for (i = 0; i < 4; i++) {
-			sprintf(ethaddr, "ethernet%d", i);
-			if (!fdt_get_alias(fdt, ethaddr))
-				continue;
-
-			if (i == 0)
-				strcpy(ethaddr, "ethaddr");
-			else
-				sprintf(ethaddr, "eth%daddr", i);
-
-			if (getenv(ethaddr))
-				continue;
-
-			/* Non OUI / registered MAC address */
-			mac_addr[0] = (i << 4) | 0x02;
-			mac_addr[1] = (sid[0] >>  0) & 0xff;
-			mac_addr[2] = (sid[3] >> 24) & 0xff;
-			mac_addr[3] = (sid[3] >> 16) & 0xff;
-			mac_addr[4] = (sid[3] >>  8) & 0xff;
-			mac_addr[5] = (sid[3] >>  0) & 0xff;
-
-			eth_setenv_enetaddr(ethaddr, mac_addr);
-		}
-
-		if (!getenv("serial#")) {
-			snprintf(serial_string, sizeof(serial_string),
-				"%08x%08x", sid[0], sid[3]);
-
-			setenv("serial#", serial_string);
-		}
-	}
-}
-
 int misc_init_r(void)
 {
 	__maybe_unused int ret;
+	unsigned int serial[4];
 
 	setenv("fel_booted", NULL);
 	setenv("fel_scriptaddr", NULL);
@@ -697,7 +707,14 @@ int misc_init_r(void)
 		parse_spl_header(SPL_ADDR);
 	}
 
-	setup_environment(gd->fdt_blob);
+	if (sunxi_get_board_serial(serial)) {
+		char serial_string[17] = { 0 };
+
+		snprintf(serial_string, sizeof(serial_string),
+			 "%08x%08x", serial[0], serial[3]);
+
+		setenv("serial#", serial_string);
+	}
 
 #ifndef CONFIG_MACH_SUN9I
 	ret = sunxi_usb_phy_probe();
@@ -713,12 +730,6 @@ int ft_board_setup(void *blob, bd_t *bd)
 {
 	int __maybe_unused r;
 
-	/*
-	 * Call setup_environment again in case the boot fdt has
-	 * ethernet aliases the u-boot copy does not have.
-	 */
-	setup_environment(blob);
-
 #ifdef CONFIG_VIDEO_DT_SIMPLEFB
 	r = sunxi_simplefb_setup(blob);
 	if (r)
diff --git a/net/eth_legacy.c b/net/eth_legacy.c
index d6d7cee..b8b1e3b 100644
--- a/net/eth_legacy.c
+++ b/net/eth_legacy.c
@@ -10,6 +10,7 @@
 #include <command.h>
 #include <environment.h>
 #include <net.h>
+#include <i2c.h>
 #include <phy.h>
 #include <linux/errno.h>
 #include "eth_internal.h"
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 6/6] net: sunxi: Enable eeprom on OLinuXino Lime boards
  2016-11-25 15:38 [U-Boot] [PATCH] sunxi: net: Use net_ops hooks to get the MAC Olliver Schinagl
                   ` (4 preceding siblings ...)
  2016-11-25 15:38 ` [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address Olliver Schinagl
@ 2016-11-25 15:38 ` Olliver Schinagl
  2016-11-30 21:37   ` Joe Hershberger
  2017-03-27 16:51   ` [U-Boot] " Joe Hershberger
  5 siblings, 2 replies; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-25 15:38 UTC (permalink / raw)
  To: u-boot

This patch enables the I2C EEPROM to be probed for a MAC address on the
OLinuXino Lime1 and Lime2 boards. Other boards surely qualify as well
but where not tested yet.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 configs/A10-OLinuXino-Lime_defconfig  | 4 ++++
 configs/A20-OLinuXino-Lime2_defconfig | 4 ++++
 configs/A20-OLinuXino-Lime_defconfig  | 4 ++++
 configs/A20-OLinuXino_MICRO_defconfig | 4 ++++
 4 files changed, 16 insertions(+)

diff --git a/configs/A10-OLinuXino-Lime_defconfig b/configs/A10-OLinuXino-Lime_defconfig
index 04b720d..c2d76c0 100644
--- a/configs/A10-OLinuXino-Lime_defconfig
+++ b/configs/A10-OLinuXino-Lime_defconfig
@@ -13,6 +13,10 @@ CONFIG_SPL=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_FPGA is not set
+CONFIG_NET_ETHADDR_EEPROM=y
+CONFIG_NET_ETHADDR_EEPROM_I2C=y
+CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
+CONFIG_I2C1_ENABLE=y
 CONFIG_AXP_ALDO3_VOLT=2800
 CONFIG_AXP_ALDO4_VOLT=2800
 CONFIG_USB_EHCI_HCD=y
diff --git a/configs/A20-OLinuXino-Lime2_defconfig b/configs/A20-OLinuXino-Lime2_defconfig
index 4751fe0..af50660 100644
--- a/configs/A20-OLinuXino-Lime2_defconfig
+++ b/configs/A20-OLinuXino-Lime2_defconfig
@@ -18,6 +18,10 @@ CONFIG_CMD_USB_MASS_STORAGE=y
 CONFIG_DFU_RAM=y
 CONFIG_RTL8211X_PHY_FORCE_MASTER=y
 CONFIG_ETH_DESIGNWARE=y
+CONFIG_NET_ETHADDR_EEPROM=y
+CONFIG_NET_ETHADDR_EEPROM_I2C=y
+CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
+CONFIG_I2C1_ENABLE=y
 CONFIG_AXP_ALDO3_VOLT=2800
 CONFIG_AXP_ALDO4_VOLT=2800
 CONFIG_USB_EHCI_HCD=y
diff --git a/configs/A20-OLinuXino-Lime_defconfig b/configs/A20-OLinuXino-Lime_defconfig
index 024dc2d..0ef4a52 100644
--- a/configs/A20-OLinuXino-Lime_defconfig
+++ b/configs/A20-OLinuXino-Lime_defconfig
@@ -12,6 +12,10 @@ CONFIG_SPL=y
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_FPGA is not set
 CONFIG_ETH_DESIGNWARE=y
+CONFIG_NET_ETHADDR_EEPROM=y
+CONFIG_NET_ETHADDR_EEPROM_I2C=y
+CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
+CONFIG_I2C1_ENABLE=y
 CONFIG_AXP_ALDO3_VOLT=2800
 CONFIG_AXP_ALDO4_VOLT=2800
 CONFIG_USB_EHCI_HCD=y
diff --git a/configs/A20-OLinuXino_MICRO_defconfig b/configs/A20-OLinuXino_MICRO_defconfig
index 5809345..f0f2d16 100644
--- a/configs/A20-OLinuXino_MICRO_defconfig
+++ b/configs/A20-OLinuXino_MICRO_defconfig
@@ -15,6 +15,10 @@ CONFIG_SPL=y
 # CONFIG_CMD_FLASH is not set
 # CONFIG_CMD_FPGA is not set
 CONFIG_ETH_DESIGNWARE=y
+CONFIG_NET_ETHADDR_EEPROM=y
+CONFIG_NET_ETHADDR_EEPROM_I2C=y
+CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
+CONFIG_I2C1_ENABLE=y
 CONFIG_AXP_ALDO3_VOLT=2800
 CONFIG_AXP_ALDO4_VOLT=2800
 CONFIG_USB_EHCI_HCD=y
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH] Use eth_ops hooks to set the MAC address
  2016-11-25 15:38 ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Olliver Schinagl
@ 2016-11-25 15:46   ` Olliver Schinagl
  2016-11-27 17:02   ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Simon Glass
  1 sibling, 0 replies; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-25 15:46 UTC (permalink / raw)
  To: u-boot

This patch series uses the read/write rom_hwaddr for sunxi based 
devices. It is split off from, but still dependant on my other 
patch-series, Retrieve MAC address from EEPROM.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook
  2016-11-25 15:38 ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Olliver Schinagl
  2016-11-25 15:46   ` [U-Boot] [PATCH] Use eth_ops hooks to set the MAC address Olliver Schinagl
@ 2016-11-27 17:02   ` Simon Glass
  2016-11-28 10:38     ` Olliver Schinagl
  1 sibling, 1 reply; 24+ messages in thread
From: Simon Glass @ 2016-11-27 17:02 UTC (permalink / raw)
  To: u-boot

Hi,

On 25 November 2016 at 08:38, Olliver Schinagl <oliver@schinagl.nl> wrote:
> Add the read_rom_hwaddr net_op hook so that it can be called from boards
> to read the mac from a ROM chip.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  drivers/net/designware.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> index 9e6d726..3f2f67c 100644
> --- a/drivers/net/designware.c
> +++ b/drivers/net/designware.c
> @@ -230,6 +230,21 @@ static int _dw_write_hwaddr(struct dw_eth_dev *priv, u8 *mac_id)
>         return 0;
>  }
>
> +__weak int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
> +{
> +       return -ENOSYS;
> +}
> +
> +static int designware_eth_read_rom_hwaddr(struct udevice *dev)
> +{
> +       struct eth_pdata *pdata = dev_get_platdata(dev);
> +
> +       if (!dev)
> +               return -ENOSYS;
> +
> +       return dw_board_read_rom_hwaddr(pdata->enetaddr, dev->seq);
> +}
> +
>  static void dw_adjust_link(struct eth_mac_regs *mac_p,
>                            struct phy_device *phydev)
>  {
> @@ -685,6 +700,7 @@ static const struct eth_ops designware_eth_ops = {
>         .free_pkt               = designware_eth_free_pkt,
>         .stop                   = designware_eth_stop,
>         .write_hwaddr           = designware_eth_write_hwaddr,
> +       .read_rom_hwaddr        = designware_eth_read_rom_hwaddr,
>  };
>
>  static int designware_eth_ofdata_to_platdata(struct udevice *dev)

You should not call board code from a driver. But since this is a
sunxi driver, why not move all the code that reads the serial number
into this file?

Regards,
Simon

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook
  2016-11-27 17:02   ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Simon Glass
@ 2016-11-28 10:38     ` Olliver Schinagl
  2016-11-29 21:41       ` Simon Glass
  0 siblings, 1 reply; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-28 10:38 UTC (permalink / raw)
  To: u-boot

On 27-11-16 18:02, Simon Glass wrote:
> Hi,
>
> On 25 November 2016 at 08:38, Olliver Schinagl <oliver@schinagl.nl> wrote:
>> Add the read_rom_hwaddr net_op hook so that it can be called from boards
>> to read the mac from a ROM chip.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>   drivers/net/designware.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
>> index 9e6d726..3f2f67c 100644
>> --- a/drivers/net/designware.c
>> +++ b/drivers/net/designware.c
>> @@ -230,6 +230,21 @@ static int _dw_write_hwaddr(struct dw_eth_dev *priv, u8 *mac_id)
>>          return 0;
>>   }
>>
>> +__weak int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
>> +{
>> +       return -ENOSYS;
>> +}
>> +
>> +static int designware_eth_read_rom_hwaddr(struct udevice *dev)
>> +{
>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> +
>> +       if (!dev)
>> +               return -ENOSYS;
>> +
>> +       return dw_board_read_rom_hwaddr(pdata->enetaddr, dev->seq);
>> +}
>> +
>>   static void dw_adjust_link(struct eth_mac_regs *mac_p,
>>                             struct phy_device *phydev)
>>   {
>> @@ -685,6 +700,7 @@ static const struct eth_ops designware_eth_ops = {
>>          .free_pkt               = designware_eth_free_pkt,
>>          .stop                   = designware_eth_stop,
>>          .write_hwaddr           = designware_eth_write_hwaddr,
>> +       .read_rom_hwaddr        = designware_eth_read_rom_hwaddr,
>>   };
>>
>>   static int designware_eth_ofdata_to_platdata(struct udevice *dev)
> You should not call board code from a driver. But since this is a
> sunxi driver, why not move all the code that reads the serial number
> into this file?
Hey Simon,

unless I missunderstand, this is how Joe suggested in a while ago, and 
how it has been implemented in a few other drivers. Can you elaborate a 
bit more?

Olliver
>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook
  2016-11-28 10:38     ` Olliver Schinagl
@ 2016-11-29 21:41       ` Simon Glass
  2016-11-30  8:16         ` Olliver Schinagl
  0 siblings, 1 reply; 24+ messages in thread
From: Simon Glass @ 2016-11-29 21:41 UTC (permalink / raw)
  To: u-boot

Hi Oliver,

On 28 November 2016 at 03:38, Olliver Schinagl <oliver@schinagl.nl> wrote:
> On 27-11-16 18:02, Simon Glass wrote:
>>
>> Hi,
>>
>> On 25 November 2016 at 08:38, Olliver Schinagl <oliver@schinagl.nl> wrote:
>>>
>>> Add the read_rom_hwaddr net_op hook so that it can be called from boards
>>> to read the mac from a ROM chip.
>>>
>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>>> ---
>>>   drivers/net/designware.c | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
>>> index 9e6d726..3f2f67c 100644
>>> --- a/drivers/net/designware.c
>>> +++ b/drivers/net/designware.c
>>> @@ -230,6 +230,21 @@ static int _dw_write_hwaddr(struct dw_eth_dev *priv,
>>> u8 *mac_id)
>>>          return 0;
>>>   }
>>>
>>> +__weak int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
>>> +{
>>> +       return -ENOSYS;
>>> +}
>>> +
>>> +static int designware_eth_read_rom_hwaddr(struct udevice *dev)
>>> +{
>>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>>> +
>>> +       if (!dev)
>>> +               return -ENOSYS;
>>> +
>>> +       return dw_board_read_rom_hwaddr(pdata->enetaddr, dev->seq);
>>> +}
>>> +
>>>   static void dw_adjust_link(struct eth_mac_regs *mac_p,
>>>                             struct phy_device *phydev)
>>>   {
>>> @@ -685,6 +700,7 @@ static const struct eth_ops designware_eth_ops = {
>>>          .free_pkt               = designware_eth_free_pkt,
>>>          .stop                   = designware_eth_stop,
>>>          .write_hwaddr           = designware_eth_write_hwaddr,
>>> +       .read_rom_hwaddr        = designware_eth_read_rom_hwaddr,
>>>   };
>>>
>>>   static int designware_eth_ofdata_to_platdata(struct udevice *dev)
>>
>> You should not call board code from a driver. But since this is a
>> sunxi driver, why not move all the code that reads the serial number
>> into this file?
>
> Hey Simon,
>
> unless I missunderstand, this is how Joe suggested in a while ago, and how
> it has been implemented in a few other drivers. Can you elaborate a bit
> more?

Yes...drivers must not call into board-specific code, nor have
board-specific #defines. This limits their usefulness for other
boards.

Adding hooks like this (particularly with the word 'board' in the
name) should be avoided.

We end up with bidirectional coupling between the board and the
driver. The board should use the driver but not the other way around.
I understand that you are trying to get around this by using a weak
function, but with driver model I'm really trying hard to avoid weak
functions. They normally indicate an ad-hoc API and can generally be
avoided with a bit more design thought.

If you have a standard way of reading the serial number which is
supported by all sunxi boards, then you should be able to add your
changes to the sunxi Ethernet driver (which uses designware.c?). Then
you can leave the designware.c code alone and avoid adding a hook.

In a sense you end up subclassing the designware driver.

Also see this series which deals with a similar problem with rockchip:

http://lists.denx.de/pipermail/u-boot/2016-November/274256.html

Regards,
Simon

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook
  2016-11-29 21:41       ` Simon Glass
@ 2016-11-30  8:16         ` Olliver Schinagl
  2016-11-30 20:43           ` Joe Hershberger
  0 siblings, 1 reply; 24+ messages in thread
From: Olliver Schinagl @ 2016-11-30  8:16 UTC (permalink / raw)
  To: u-boot

Hey Simon,


On 29-11-16 22:41, Simon Glass wrote:
> Hi Oliver,
>
> On 28 November 2016 at 03:38, Olliver Schinagl <oliver@schinagl.nl> wrote:
>> On 27-11-16 18:02, Simon Glass wrote:
>>> Hi,
>>>
>>> On 25 November 2016 at 08:38, Olliver Schinagl <oliver@schinagl.nl> wrote:
>>>> Add the read_rom_hwaddr net_op hook so that it can be called from boards
>>>> to read the mac from a ROM chip.
>>>>
>>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>>>> ---
>>>>    drivers/net/designware.c | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
>>>> index 9e6d726..3f2f67c 100644
>>>> --- a/drivers/net/designware.c
>>>> +++ b/drivers/net/designware.c
>>>> @@ -230,6 +230,21 @@ static int _dw_write_hwaddr(struct dw_eth_dev *priv,
>>>> u8 *mac_id)
>>>>           return 0;
>>>>    }
>>>>
>>>> +__weak int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
>>>> +{
>>>> +       return -ENOSYS;
>>>> +}
>>>> +
>>>> +static int designware_eth_read_rom_hwaddr(struct udevice *dev)
>>>> +{
>>>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>>>> +
>>>> +       if (!dev)
>>>> +               return -ENOSYS;
>>>> +
>>>> +       return dw_board_read_rom_hwaddr(pdata->enetaddr, dev->seq);
>>>> +}
>>>> +
>>>>    static void dw_adjust_link(struct eth_mac_regs *mac_p,
>>>>                              struct phy_device *phydev)
>>>>    {
>>>> @@ -685,6 +700,7 @@ static const struct eth_ops designware_eth_ops = {
>>>>           .free_pkt               = designware_eth_free_pkt,
>>>>           .stop                   = designware_eth_stop,
>>>>           .write_hwaddr           = designware_eth_write_hwaddr,
>>>> +       .read_rom_hwaddr        = designware_eth_read_rom_hwaddr,
>>>>    };
>>>>
>>>>    static int designware_eth_ofdata_to_platdata(struct udevice *dev)
>>> You should not call board code from a driver. But since this is a
>>> sunxi driver, why not move all the code that reads the serial number
>>> into this file?
>> Hey Simon,
>>
>> unless I missunderstand, this is how Joe suggested in a while ago, and how
>> it has been implemented in a few other drivers. Can you elaborate a bit
>> more?
> Yes...drivers must not call into board-specific code, nor have
> board-specific #defines. This limits their usefulness for other
> boards.
Hmm, well as I said, I just followed Joe's suggestion with his example. 
also isn't this exactly how the zynq does it as well?
>
> Adding hooks like this (particularly with the word 'board' in the
> name) should be avoided.
>
> We end up with bidirectional coupling between the board and the
> driver. The board should use the driver but not the other way around.
> I understand that you are trying to get around this by using a weak
> function, but with driver model I'm really trying hard to avoid weak
> functions. They normally indicate an ad-hoc API and can generally be
> avoided with a bit more design thought.
>
> If you have a standard way of reading the serial number which is
> supported by all sunxi boards, then you should be able to add your
> changes to the sunxi Ethernet driver (which uses designware.c?). Then
> you can leave the designware.c code alone and avoid adding a hook.
Well yes and no. We use designware, but also sunxi_emac, and some 
sdio_realtek that does not have a driver yet. But in essence, this is 
somewhat what I do in this patch I guess. I have the weak driver 
specific function in the sunxi code.

But I think I'm starting to understand your solution and will read up on 
the rockchip patches and rewrite this bit.
>
> In a sense you end up subclassing the designware driver.
>
> Also see this series which deals with a similar problem with rockchip:
>
> http://lists.denx.de/pipermail/u-boot/2016-November/274256.html
>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook
  2016-11-30  8:16         ` Olliver Schinagl
@ 2016-11-30 20:43           ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2016-11-30 20:43 UTC (permalink / raw)
  To: u-boot

On Wed, Nov 30, 2016 at 2:16 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
> Hey Simon,
>
>
>
> On 29-11-16 22:41, Simon Glass wrote:
>>
>> Hi Oliver,
>>
>> On 28 November 2016 at 03:38, Olliver Schinagl <oliver@schinagl.nl> wrote:
>>>
>>> On 27-11-16 18:02, Simon Glass wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 25 November 2016 at 08:38, Olliver Schinagl <oliver@schinagl.nl>
>>>> wrote:
>>>>>
>>>>> Add the read_rom_hwaddr net_op hook so that it can be called from
>>>>> boards
>>>>> to read the mac from a ROM chip.
>>>>>
>>>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>>>>> ---
>>>>>    drivers/net/designware.c | 16 ++++++++++++++++
>>>>>    1 file changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
>>>>> index 9e6d726..3f2f67c 100644
>>>>> --- a/drivers/net/designware.c
>>>>> +++ b/drivers/net/designware.c
>>>>> @@ -230,6 +230,21 @@ static int _dw_write_hwaddr(struct dw_eth_dev
>>>>> *priv,
>>>>> u8 *mac_id)
>>>>>           return 0;
>>>>>    }
>>>>>
>>>>> +__weak int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
>>>>> +{
>>>>> +       return -ENOSYS;
>>>>> +}
>>>>> +
>>>>> +static int designware_eth_read_rom_hwaddr(struct udevice *dev)
>>>>> +{
>>>>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>>>>> +
>>>>> +       if (!dev)
>>>>> +               return -ENOSYS;
>>>>> +
>>>>> +       return dw_board_read_rom_hwaddr(pdata->enetaddr, dev->seq);
>>>>> +}
>>>>> +
>>>>>    static void dw_adjust_link(struct eth_mac_regs *mac_p,
>>>>>                              struct phy_device *phydev)
>>>>>    {
>>>>> @@ -685,6 +700,7 @@ static const struct eth_ops designware_eth_ops = {
>>>>>           .free_pkt               = designware_eth_free_pkt,
>>>>>           .stop                   = designware_eth_stop,
>>>>>           .write_hwaddr           = designware_eth_write_hwaddr,
>>>>> +       .read_rom_hwaddr        = designware_eth_read_rom_hwaddr,
>>>>>    };
>>>>>
>>>>>    static int designware_eth_ofdata_to_platdata(struct udevice *dev)
>>>>
>>>> You should not call board code from a driver. But since this is a
>>>> sunxi driver, why not move all the code that reads the serial number
>>>> into this file?
>>>
>>> Hey Simon,
>>>
>>> unless I missunderstand, this is how Joe suggested in a while ago, and
>>> how
>>> it has been implemented in a few other drivers. Can you elaborate a bit
>>> more?
>>
>> Yes...drivers must not call into board-specific code, nor have
>> board-specific #defines. This limits their usefulness for other
>> boards.
>
> Hmm, well as I said, I just followed Joe's suggestion with his example. also
> isn't this exactly how the zynq does it as well?

Sorry for misleading you. Simon has since convinced me that making a
separate board-specific driver that leverages the core driver's code
is a cleaner approach, and now what I recommend.

>> Adding hooks like this (particularly with the word 'board' in the
>> name) should be avoided.
>>
>> We end up with bidirectional coupling between the board and the
>> driver. The board should use the driver but not the other way around.
>> I understand that you are trying to get around this by using a weak
>> function, but with driver model I'm really trying hard to avoid weak
>> functions. They normally indicate an ad-hoc API and can generally be
>> avoided with a bit more design thought.
>>
>> If you have a standard way of reading the serial number which is
>> supported by all sunxi boards, then you should be able to add your
>> changes to the sunxi Ethernet driver (which uses designware.c?). Then
>> you can leave the designware.c code alone and avoid adding a hook.
>
> Well yes and no. We use designware, but also sunxi_emac, and some
> sdio_realtek that does not have a driver yet. But in essence, this is
> somewhat what I do in this patch I guess. I have the weak driver specific
> function in the sunxi code.
>
> But I think I'm starting to understand your solution and will read up on the
> rockchip patches and rewrite this bit.
>
>>
>> In a sense you end up subclassing the designware driver.
>>
>> Also see this series which deals with a similar problem with rockchip:
>>
>> http://lists.denx.de/pipermail/u-boot/2016-November/274256.html
>>
>> Regards,
>> Simon
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 2/6] net: sunxi-emac: Write HW address via function
  2016-11-25 15:38 ` [U-Boot] [PATCH 2/6] net: sunxi-emac: Write HW address via function Olliver Schinagl
@ 2016-11-30 21:10   ` Joe Hershberger
  2017-03-27 16:51   ` [U-Boot] " Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2016-11-30 21:10 UTC (permalink / raw)
  To: u-boot

On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
> Currently the mac address is programmed directly in _sunxi_emac_eth_init
> making it a one time inflexible operation. By moving it into a separate
> function, we can now use this more flexibly.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 3/6] net: sunxi-emac: Add write_hwaddr net_op hook
  2016-11-25 15:38 ` [U-Boot] [PATCH 3/6] net: sunxi-emac: Add write_hwaddr net_op hook Olliver Schinagl
@ 2016-11-30 21:32   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2016-11-30 21:32 UTC (permalink / raw)
  To: u-boot

On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
> Expose enetaddr writing via net_ops so that it can be hooked into.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  drivers/net/sunxi_emac.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/sunxi_emac.c b/drivers/net/sunxi_emac.c
> index 99339db..bba8630 100644
> --- a/drivers/net/sunxi_emac.c
> +++ b/drivers/net/sunxi_emac.c
> @@ -554,6 +554,17 @@ static void sunxi_emac_eth_stop(struct udevice *dev)
>         /* Nothing to do here */
>  }
>
> +static int sunxi_emac_eth_write_hwaddr(struct udevice *dev)
> +{
> +       struct eth_pdata *pdata = dev_get_platdata(dev);
> +       struct emac_eth_dev *priv = dev_get_priv(dev);
> +
> +       if (!dev)
> +               return -ENOSYS;
> +
> +       return _sunxi_write_hwaddr(priv, pdata->enetaddr);

You should check the pdata ptr value before dereferencing it.

> +}
> +
>  static int sunxi_emac_eth_probe(struct udevice *dev)
>  {
>         struct eth_pdata *pdata = dev_get_platdata(dev);
> @@ -570,6 +581,7 @@ static const struct eth_ops sunxi_emac_eth_ops = {
>         .send                   = sunxi_emac_eth_send,
>         .recv                   = sunxi_emac_eth_recv,
>         .stop                   = sunxi_emac_eth_stop,
> +       .write_hwaddr           = sunxi_emac_eth_write_hwaddr,
>  };
>
>  static int sunxi_emac_eth_ofdata_to_platdata(struct udevice *dev)
> --
> 2.10.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 4/6] net: sunxi-emac: Add read_rom_hwaddr net_op hook
  2016-11-25 15:38 ` [U-Boot] [PATCH 4/6] net: sunxi-emac: Add read_rom_hwaddr " Olliver Schinagl
@ 2016-11-30 21:34   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2016-11-30 21:34 UTC (permalink / raw)
  To: u-boot

On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
> Add the read_rom_hwaddr net_op hook so that it can be called from boards
> to read the mac from a ROM chip.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  drivers/net/sunxi_emac.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/sunxi_emac.c b/drivers/net/sunxi_emac.c
> index bba8630..7d45f8b 100644
> --- a/drivers/net/sunxi_emac.c
> +++ b/drivers/net/sunxi_emac.c
> @@ -565,6 +565,21 @@ static int sunxi_emac_eth_write_hwaddr(struct udevice *dev)
>         return _sunxi_write_hwaddr(priv, pdata->enetaddr);
>  }
>
> +__weak int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
> +{
> +       return -ENOSYS;
> +}
> +
> +static int sunxi_emac_eth_read_rom_hwaddr(struct udevice *dev)
> +{
> +       struct eth_pdata *pdata = dev_get_platdata(dev);
> +
> +       if (!dev)
> +               return -ENOSYS;
> +
> +       return sunxi_emac_board_read_rom_hwaddr(pdata->enetaddr, dev->seq);
> +}
> +
>  static int sunxi_emac_eth_probe(struct udevice *dev)
>  {
>         struct eth_pdata *pdata = dev_get_platdata(dev);
> @@ -582,6 +597,7 @@ static const struct eth_ops sunxi_emac_eth_ops = {
>         .recv                   = sunxi_emac_eth_recv,
>         .stop                   = sunxi_emac_eth_stop,
>         .write_hwaddr           = sunxi_emac_eth_write_hwaddr,
> +       .read_rom_hwaddr        = sunxi_emac_eth_read_rom_hwaddr,
>  };

I think we are already on the same page, but the solution would look
like this, but the ops themselves would be redefined in a separate
board-specific driver and would only have a read_rom_hwaddr op in that
driver, not this one. Then the board specific
"sunxi_emac_eth_read_rom_hwaddr()" would simply be in that driver, not
a weak reference to it.

>  static int sunxi_emac_eth_ofdata_to_platdata(struct udevice *dev)
> --
> 2.10.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address
  2016-11-25 15:38 ` [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address Olliver Schinagl
@ 2016-11-30 21:36   ` Joe Hershberger
  2017-04-07 13:45     ` Olliver Schinagl
  0 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2016-11-30 21:36 UTC (permalink / raw)
  To: u-boot

On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
> Add board hooks allowing to get ethernet addresses in a board specific
> manner. Currently this is done by generating a MAC address from
> the SID and injecting the ethernet device number in the first octet.
>
> This usually happens as a fallback, if either the eeprom fails to set a
> MAC address or the FDT forces an override.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>  board/sunxi/board.c                         | 161 +++++++++++++++-------------
>  net/eth_legacy.c                            |   1 +
>  3 files changed, 98 insertions(+), 75 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h b/arch/arm/include/asm/arch-sunxi/sys_proto.h
> index a373319..fad7c48 100644
> --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
> +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
> @@ -30,4 +30,15 @@ void eth_init_board(void);
>  static inline void eth_init_board(void) {}
>  #endif
>
> +int board_get_enetaddr(const int i, unsigned char *mac_addr);
> +
> +#if CONFIG_SUNXI_EMAC
> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
> +#endif
> +
> +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
> +#endif
> +
> +
>  #endif
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 5365638..4aeab51 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -21,6 +21,7 @@
>  #include <asm/arch/gpio.h>
>  #include <asm/arch/mmc.h>
>  #include <asm/arch/spl.h>
> +#include <asm/arch/sys_proto.h>
>  #include <asm/arch/usb_phy.h>
>  #ifndef CONFIG_ARM64
>  #include <asm/armv7.h>
> @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>  }
>  #endif
>
> +int sunxi_get_board_serial(unsigned int *serial)
> +{
> +       int ret;
> +
> +       ret = sunxi_get_sid(serial);
> +       if (!ret || serial[0])
> +               return -ENOSYS;
> +
> +       /*
> +        * The single words 1 - 3 of the SID have quite a few bits
> +        * which are the same on many models, so we take a crc32
> +        * of all 3 words, to get a more unique value.
> +        *
> +        * Note we only do this on newer SoCs as we cannot change
> +        * the algorithm on older SoCs since those have been using
> +        * fixed mac-addresses/serial based on only using word 3 for a
> +        * long time and changing a fixed mac-address/serial with an
> +        * u-boot update is not good.
> +        */
> +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
> +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
> +    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
> +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
> +#endif
> +
> +       return 0;
> +}
> +
>  #ifdef CONFIG_SERIAL_TAG
>  void get_board_serial(struct tag_serialnr *serialnr)
>  {
> @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr *serialnr)
>  #endif
>
>  /*
> + * Generate a MAC address based on device index and the serial number.
> + * The first half of the of the first octet holds the eth index.
> + *
> + * In the second octet we forcefully mark the MAC address to a locally
> + * administered MAC address.
> + *
> + */
> +int board_get_enetaddr(const int index, unsigned char *enetaddr)

This would be part of a board-specific eth driver.

> +{
> +       uint8_t mac_addr[ARP_HLEN] = { 0x00 };
> +       unsigned int serial[4];
> +       int ret;
> +
> +       if ((index < 0) || !enetaddr)
> +               return -ENOSYS;
> +
> +       ret = sunxi_get_board_serial(serial);
> +       if (!ret)
> +               return ret;
> +
> +       /* Ensure the NIC specific bytes of the mac are not all 0 */
> +       if ((serial[3] & 0xffffff) == 0)
> +               serial[3] |= 0x800000;
> +
> +       mac_addr[0] = (index << 4);
> +       mac_addr[1] = (serial[0] >>  0) & 0xff;
> +       mac_addr[2] = (serial[3] >> 24) & 0xff;
> +       mac_addr[3] = (serial[3] >> 16) & 0xff;
> +       mac_addr[4] = (serial[3] >>  8) & 0xff;
> +       mac_addr[5] = (serial[3] >>  0) & 0xff;
> +
> +       set_local_ethaddr(mac_addr);
> +       memcpy(enetaddr, mac_addr, ARP_HLEN);
> +
> +       return 0;
> +}
> +
> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
> +{
> +       return board_get_enetaddr(id, enetaddr);
> +}
> +
> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
> +{
> +       return board_get_enetaddr(id, enetaddr);
> +}
> +
> +/*
>   * Check the SPL header for the "sunxi" variant. If found: parse values
>   * that might have been passed by the loader ("fel" utility), and update
>   * the environment accordingly.
> @@ -617,77 +694,10 @@ static void parse_spl_header(const uint32_t spl_addr)
>         setenv_hex("fel_scriptaddr", spl->fel_script_address);
>  }
>
> -/*
> - * Note this function gets called multiple times.
> - * It must not make any changes to env variables which already exist.
> - */
> -static void setup_environment(const void *fdt)
> -{
> -       char serial_string[17] = { 0 };
> -       unsigned int sid[4];
> -       uint8_t mac_addr[6];
> -       char ethaddr[16];
> -       int i, ret;
> -
> -       ret = sunxi_get_sid(sid);
> -       if (ret == 0 && sid[0] != 0) {
> -               /*
> -                * The single words 1 - 3 of the SID have quite a few bits
> -                * which are the same on many models, so we take a crc32
> -                * of all 3 words, to get a more unique value.
> -                *
> -                * Note we only do this on newer SoCs as we cannot change
> -                * the algorithm on older SoCs since those have been using
> -                * fixed mac-addresses based on only using word 3 for a
> -                * long time and changing a fixed mac-address with an
> -                * u-boot update is not good.
> -                */
> -#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
> -    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
> -    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
> -               sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
> -#endif
> -
> -               /* Ensure the NIC specific bytes of the mac are not all 0 */
> -               if ((sid[3] & 0xffffff) == 0)
> -                       sid[3] |= 0x800000;
> -
> -               for (i = 0; i < 4; i++) {
> -                       sprintf(ethaddr, "ethernet%d", i);
> -                       if (!fdt_get_alias(fdt, ethaddr))
> -                               continue;
> -
> -                       if (i == 0)
> -                               strcpy(ethaddr, "ethaddr");
> -                       else
> -                               sprintf(ethaddr, "eth%daddr", i);
> -
> -                       if (getenv(ethaddr))
> -                               continue;
> -
> -                       /* Non OUI / registered MAC address */
> -                       mac_addr[0] = (i << 4) | 0x02;
> -                       mac_addr[1] = (sid[0] >>  0) & 0xff;
> -                       mac_addr[2] = (sid[3] >> 24) & 0xff;
> -                       mac_addr[3] = (sid[3] >> 16) & 0xff;
> -                       mac_addr[4] = (sid[3] >>  8) & 0xff;
> -                       mac_addr[5] = (sid[3] >>  0) & 0xff;
> -
> -                       eth_setenv_enetaddr(ethaddr, mac_addr);
> -               }
> -
> -               if (!getenv("serial#")) {
> -                       snprintf(serial_string, sizeof(serial_string),
> -                               "%08x%08x", sid[0], sid[3]);
> -
> -                       setenv("serial#", serial_string);
> -               }
> -       }
> -}
> -
>  int misc_init_r(void)
>  {
>         __maybe_unused int ret;
> +       unsigned int serial[4];
>
>         setenv("fel_booted", NULL);
>         setenv("fel_scriptaddr", NULL);
> @@ -697,7 +707,14 @@ int misc_init_r(void)
>                 parse_spl_header(SPL_ADDR);
>         }
>
> -       setup_environment(gd->fdt_blob);
> +       if (sunxi_get_board_serial(serial)) {
> +               char serial_string[17] = { 0 };
> +
> +               snprintf(serial_string, sizeof(serial_string),
> +                        "%08x%08x", serial[0], serial[3]);
> +
> +               setenv("serial#", serial_string);
> +       }
>
>  #ifndef CONFIG_MACH_SUN9I
>         ret = sunxi_usb_phy_probe();
> @@ -713,12 +730,6 @@ int ft_board_setup(void *blob, bd_t *bd)
>  {
>         int __maybe_unused r;
>
> -       /*
> -        * Call setup_environment again in case the boot fdt has
> -        * ethernet aliases the u-boot copy does not have.
> -        */
> -       setup_environment(blob);
> -
>  #ifdef CONFIG_VIDEO_DT_SIMPLEFB
>         r = sunxi_simplefb_setup(blob);
>         if (r)
> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> index d6d7cee..b8b1e3b 100644
> --- a/net/eth_legacy.c
> +++ b/net/eth_legacy.c
> @@ -10,6 +10,7 @@
>  #include <command.h>
>  #include <environment.h>
>  #include <net.h>
> +#include <i2c.h>
>  #include <phy.h>
>  #include <linux/errno.h>
>  #include "eth_internal.h"
> --
> 2.10.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 6/6] net: sunxi: Enable eeprom on OLinuXino Lime boards
  2016-11-25 15:38 ` [U-Boot] [PATCH 6/6] net: sunxi: Enable eeprom on OLinuXino Lime boards Olliver Schinagl
@ 2016-11-30 21:37   ` Joe Hershberger
  2017-03-27 16:51   ` [U-Boot] " Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2016-11-30 21:37 UTC (permalink / raw)
  To: u-boot

On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
> This patch enables the I2C EEPROM to be probed for a MAC address on the
> OLinuXino Lime1 and Lime2 boards. Other boards surely qualify as well
> but where not tested yet.

typo: where -> were

>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  configs/A10-OLinuXino-Lime_defconfig  | 4 ++++
>  configs/A20-OLinuXino-Lime2_defconfig | 4 ++++
>  configs/A20-OLinuXino-Lime_defconfig  | 4 ++++
>  configs/A20-OLinuXino_MICRO_defconfig | 4 ++++
>  4 files changed, 16 insertions(+)
>
> diff --git a/configs/A10-OLinuXino-Lime_defconfig b/configs/A10-OLinuXino-Lime_defconfig
> index 04b720d..c2d76c0 100644
> --- a/configs/A10-OLinuXino-Lime_defconfig
> +++ b/configs/A10-OLinuXino-Lime_defconfig
> @@ -13,6 +13,10 @@ CONFIG_SPL=y
>  # CONFIG_CMD_IMLS is not set
>  # CONFIG_CMD_FLASH is not set
>  # CONFIG_CMD_FPGA is not set
> +CONFIG_NET_ETHADDR_EEPROM=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
> +CONFIG_I2C1_ENABLE=y
>  CONFIG_AXP_ALDO3_VOLT=2800
>  CONFIG_AXP_ALDO4_VOLT=2800
>  CONFIG_USB_EHCI_HCD=y
> diff --git a/configs/A20-OLinuXino-Lime2_defconfig b/configs/A20-OLinuXino-Lime2_defconfig
> index 4751fe0..af50660 100644
> --- a/configs/A20-OLinuXino-Lime2_defconfig
> +++ b/configs/A20-OLinuXino-Lime2_defconfig
> @@ -18,6 +18,10 @@ CONFIG_CMD_USB_MASS_STORAGE=y
>  CONFIG_DFU_RAM=y
>  CONFIG_RTL8211X_PHY_FORCE_MASTER=y
>  CONFIG_ETH_DESIGNWARE=y
> +CONFIG_NET_ETHADDR_EEPROM=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
> +CONFIG_I2C1_ENABLE=y
>  CONFIG_AXP_ALDO3_VOLT=2800
>  CONFIG_AXP_ALDO4_VOLT=2800
>  CONFIG_USB_EHCI_HCD=y
> diff --git a/configs/A20-OLinuXino-Lime_defconfig b/configs/A20-OLinuXino-Lime_defconfig
> index 024dc2d..0ef4a52 100644
> --- a/configs/A20-OLinuXino-Lime_defconfig
> +++ b/configs/A20-OLinuXino-Lime_defconfig
> @@ -12,6 +12,10 @@ CONFIG_SPL=y
>  # CONFIG_CMD_FLASH is not set
>  # CONFIG_CMD_FPGA is not set
>  CONFIG_ETH_DESIGNWARE=y
> +CONFIG_NET_ETHADDR_EEPROM=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
> +CONFIG_I2C1_ENABLE=y
>  CONFIG_AXP_ALDO3_VOLT=2800
>  CONFIG_AXP_ALDO4_VOLT=2800
>  CONFIG_USB_EHCI_HCD=y
> diff --git a/configs/A20-OLinuXino_MICRO_defconfig b/configs/A20-OLinuXino_MICRO_defconfig
> index 5809345..f0f2d16 100644
> --- a/configs/A20-OLinuXino_MICRO_defconfig
> +++ b/configs/A20-OLinuXino_MICRO_defconfig
> @@ -15,6 +15,10 @@ CONFIG_SPL=y
>  # CONFIG_CMD_FLASH is not set
>  # CONFIG_CMD_FPGA is not set
>  CONFIG_ETH_DESIGNWARE=y
> +CONFIG_NET_ETHADDR_EEPROM=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
> +CONFIG_I2C1_ENABLE=y
>  CONFIG_AXP_ALDO3_VOLT=2800
>  CONFIG_AXP_ALDO4_VOLT=2800
>  CONFIG_USB_EHCI_HCD=y
> --
> 2.10.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] net: sunxi-emac: Write HW address via function
  2016-11-25 15:38 ` [U-Boot] [PATCH 2/6] net: sunxi-emac: Write HW address via function Olliver Schinagl
  2016-11-30 21:10   ` Joe Hershberger
@ 2017-03-27 16:51   ` Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2017-03-27 16:51 UTC (permalink / raw)
  To: u-boot

Hi oliver at schinagl.nl,

https://patchwork.ozlabs.org/patch/699295/ was applied to u-boot-net.git.

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] net: sunxi: Enable eeprom on OLinuXino Lime boards
  2016-11-25 15:38 ` [U-Boot] [PATCH 6/6] net: sunxi: Enable eeprom on OLinuXino Lime boards Olliver Schinagl
  2016-11-30 21:37   ` Joe Hershberger
@ 2017-03-27 16:51   ` Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2017-03-27 16:51 UTC (permalink / raw)
  To: u-boot

Hi oliver at schinagl.nl,

https://patchwork.ozlabs.org/patch/699297/ was applied to u-boot-net.git.

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address
  2016-11-30 21:36   ` Joe Hershberger
@ 2017-04-07 13:45     ` Olliver Schinagl
  2017-04-07 13:57       ` o.schinagl at ultimaker.com
  0 siblings, 1 reply; 24+ messages in thread
From: Olliver Schinagl @ 2017-04-07 13:45 UTC (permalink / raw)
  To: u-boot

Hey Joe,

On 30-11-16 22:36, Joe Hershberger wrote:
> On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.nl> wrote:
>> Add board hooks allowing to get ethernet addresses in a board specific
>> manner. Currently this is done by generating a MAC address from
>> the SID and injecting the ethernet device number in the first octet.
>>
>> This usually happens as a fallback, if either the eeprom fails to set a
>> MAC address or the FDT forces an override.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>  arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>>  board/sunxi/board.c                         | 161 +++++++++++++++-------------
>>  net/eth_legacy.c                            |   1 +
>>  3 files changed, 98 insertions(+), 75 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> index a373319..fad7c48 100644
>> --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> @@ -30,4 +30,15 @@ void eth_init_board(void);
>>  static inline void eth_init_board(void) {}
>>  #endif
>>
>> +int board_get_enetaddr(const int i, unsigned char *mac_addr);
>> +
>> +#if CONFIG_SUNXI_EMAC
>> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>> +#endif
>> +
>> +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
>> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>> +#endif
>> +
>> +
>>  #endif
>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> index 5365638..4aeab51 100644
>> --- a/board/sunxi/board.c
>> +++ b/board/sunxi/board.c
>> @@ -21,6 +21,7 @@
>>  #include <asm/arch/gpio.h>
>>  #include <asm/arch/mmc.h>
>>  #include <asm/arch/spl.h>
>> +#include <asm/arch/sys_proto.h>
>>  #include <asm/arch/usb_phy.h>
>>  #ifndef CONFIG_ARM64
>>  #include <asm/armv7.h>
>> @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>>  }
>>  #endif
>>
>> +int sunxi_get_board_serial(unsigned int *serial)
>> +{
>> +       int ret;
>> +
>> +       ret = sunxi_get_sid(serial);
>> +       if (!ret || serial[0])
>> +               return -ENOSYS;
>> +
>> +       /*
>> +        * The single words 1 - 3 of the SID have quite a few bits
>> +        * which are the same on many models, so we take a crc32
>> +        * of all 3 words, to get a more unique value.
>> +        *
>> +        * Note we only do this on newer SoCs as we cannot change
>> +        * the algorithm on older SoCs since those have been using
>> +        * fixed mac-addresses/serial based on only using word 3 for a
>> +        * long time and changing a fixed mac-address/serial with an
>> +        * u-boot update is not good.
>> +        */
>> +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
>> +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
>> +    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
>> +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
>> +#endif
>> +
>> +       return 0;
>> +}
>> +
>>  #ifdef CONFIG_SERIAL_TAG
>>  void get_board_serial(struct tag_serialnr *serialnr)
>>  {
>> @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr *serialnr)
>>  #endif
>>
>>  /*
>> + * Generate a MAC address based on device index and the serial number.
>> + * The first half of the of the first octet holds the eth index.
>> + *
>> + * In the second octet we forcefully mark the MAC address to a locally
>> + * administered MAC address.
>> + *
>> + */
>> +int board_get_enetaddr(const int index, unsigned char *enetaddr)
>
> This would be part of a board-specific eth driver.

this is being called now from sunxi_gmac.c and sunxi_emac.c and supplies 
these board specific drivers with a mac address based on the serial 
number of the board. I could move this logic over, but then i'd have to 
add it to both eth drivers. By having it in the board.c file, we have 2 
simple functions in the board-specific eth driver:


static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
{
	struct eth_pdata *pdata = dev_get_platdata(dev);

	return board_get_enetaddr(dev->seq, pdata->enetaddr);
}

So do you propose to dupilicate the code into both board specific 
drivers, have it named differently or that the shared code live elsewhere?

Olliver
>
>> +{
>> +       uint8_t mac_addr[ARP_HLEN] = { 0x00 };
>> +       unsigned int serial[4];
>> +       int ret;
>> +
>> +       if ((index < 0) || !enetaddr)
>> +               return -ENOSYS;
>> +
>> +       ret = sunxi_get_board_serial(serial);
>> +       if (!ret)
>> +               return ret;
>> +
>> +       /* Ensure the NIC specific bytes of the mac are not all 0 */
>> +       if ((serial[3] & 0xffffff) == 0)
>> +               serial[3] |= 0x800000;
>> +
>> +       mac_addr[0] = (index << 4);
>> +       mac_addr[1] = (serial[0] >>  0) & 0xff;
>> +       mac_addr[2] = (serial[3] >> 24) & 0xff;
>> +       mac_addr[3] = (serial[3] >> 16) & 0xff;
>> +       mac_addr[4] = (serial[3] >>  8) & 0xff;
>> +       mac_addr[5] = (serial[3] >>  0) & 0xff;
>> +
>> +       set_local_ethaddr(mac_addr);
>> +       memcpy(enetaddr, mac_addr, ARP_HLEN);
>> +
>> +       return 0;
>> +}
>> +
>> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
>> +{
>> +       return board_get_enetaddr(id, enetaddr);
>> +}
>> +
>> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
>> +{
>> +       return board_get_enetaddr(id, enetaddr);
>> +}
>> +
>> +/*
>>   * Check the SPL header for the "sunxi" variant. If found: parse values
>>   * that might have been passed by the loader ("fel" utility), and update
>>   * the environment accordingly.
>> @@ -617,77 +694,10 @@ static void parse_spl_header(const uint32_t spl_addr)
>>         setenv_hex("fel_scriptaddr", spl->fel_script_address);
>>  }
>>
>> -/*
>> - * Note this function gets called multiple times.
>> - * It must not make any changes to env variables which already exist.
>> - */
>> -static void setup_environment(const void *fdt)
>> -{
>> -       char serial_string[17] = { 0 };
>> -       unsigned int sid[4];
>> -       uint8_t mac_addr[6];
>> -       char ethaddr[16];
>> -       int i, ret;
>> -
>> -       ret = sunxi_get_sid(sid);
>> -       if (ret == 0 && sid[0] != 0) {
>> -               /*
>> -                * The single words 1 - 3 of the SID have quite a few bits
>> -                * which are the same on many models, so we take a crc32
>> -                * of all 3 words, to get a more unique value.
>> -                *
>> -                * Note we only do this on newer SoCs as we cannot change
>> -                * the algorithm on older SoCs since those have been using
>> -                * fixed mac-addresses based on only using word 3 for a
>> -                * long time and changing a fixed mac-address with an
>> -                * u-boot update is not good.
>> -                */
>> -#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \
>> -    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \
>> -    !defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33)
>> -               sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
>> -#endif
>> -
>> -               /* Ensure the NIC specific bytes of the mac are not all 0 */
>> -               if ((sid[3] & 0xffffff) == 0)
>> -                       sid[3] |= 0x800000;
>> -
>> -               for (i = 0; i < 4; i++) {
>> -                       sprintf(ethaddr, "ethernet%d", i);
>> -                       if (!fdt_get_alias(fdt, ethaddr))
>> -                               continue;
>> -
>> -                       if (i == 0)
>> -                               strcpy(ethaddr, "ethaddr");
>> -                       else
>> -                               sprintf(ethaddr, "eth%daddr", i);
>> -
>> -                       if (getenv(ethaddr))
>> -                               continue;
>> -
>> -                       /* Non OUI / registered MAC address */
>> -                       mac_addr[0] = (i << 4) | 0x02;
>> -                       mac_addr[1] = (sid[0] >>  0) & 0xff;
>> -                       mac_addr[2] = (sid[3] >> 24) & 0xff;
>> -                       mac_addr[3] = (sid[3] >> 16) & 0xff;
>> -                       mac_addr[4] = (sid[3] >>  8) & 0xff;
>> -                       mac_addr[5] = (sid[3] >>  0) & 0xff;
>> -
>> -                       eth_setenv_enetaddr(ethaddr, mac_addr);
>> -               }
>> -
>> -               if (!getenv("serial#")) {
>> -                       snprintf(serial_string, sizeof(serial_string),
>> -                               "%08x%08x", sid[0], sid[3]);
>> -
>> -                       setenv("serial#", serial_string);
>> -               }
>> -       }
>> -}
>> -
>>  int misc_init_r(void)
>>  {
>>         __maybe_unused int ret;
>> +       unsigned int serial[4];
>>
>>         setenv("fel_booted", NULL);
>>         setenv("fel_scriptaddr", NULL);
>> @@ -697,7 +707,14 @@ int misc_init_r(void)
>>                 parse_spl_header(SPL_ADDR);
>>         }
>>
>> -       setup_environment(gd->fdt_blob);
>> +       if (sunxi_get_board_serial(serial)) {
>> +               char serial_string[17] = { 0 };
>> +
>> +               snprintf(serial_string, sizeof(serial_string),
>> +                        "%08x%08x", serial[0], serial[3]);
>> +
>> +               setenv("serial#", serial_string);
>> +       }
>>
>>  #ifndef CONFIG_MACH_SUN9I
>>         ret = sunxi_usb_phy_probe();
>> @@ -713,12 +730,6 @@ int ft_board_setup(void *blob, bd_t *bd)
>>  {
>>         int __maybe_unused r;
>>
>> -       /*
>> -        * Call setup_environment again in case the boot fdt has
>> -        * ethernet aliases the u-boot copy does not have.
>> -        */
>> -       setup_environment(blob);
>> -
>>  #ifdef CONFIG_VIDEO_DT_SIMPLEFB
>>         r = sunxi_simplefb_setup(blob);
>>         if (r)
>> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
>> index d6d7cee..b8b1e3b 100644
>> --- a/net/eth_legacy.c
>> +++ b/net/eth_legacy.c
>> @@ -10,6 +10,7 @@
>>  #include <command.h>
>>  #include <environment.h>
>>  #include <net.h>
>> +#include <i2c.h>
>>  #include <phy.h>
>>  #include <linux/errno.h>
>>  #include "eth_internal.h"
>> --
>> 2.10.2
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address
  2017-04-07 13:45     ` Olliver Schinagl
@ 2017-04-07 13:57       ` o.schinagl at ultimaker.com
  2017-04-10 22:56         ` Joe Hershberger
  0 siblings, 1 reply; 24+ messages in thread
From: o.schinagl at ultimaker.com @ 2017-04-07 13:57 UTC (permalink / raw)
  To: u-boot

On Fri, 2017-04-07 at 15:45 +0200, Olliver Schinagl wrote:
> Hey Joe,
> 
> On 30-11-16 22:36, Joe Hershberger wrote:
> > On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.
> > nl> wrote:
> > > Add board hooks allowing to get ethernet addresses in a board
> > > specific
> > > manner. Currently this is done by generating a MAC address from
> > > the SID and injecting the ethernet device number in the first
> > > octet.
> > > 
> > > This usually happens as a fallback, if either the eeprom fails to
> > > set a
> > > MAC address or the FDT forces an override.
> > > 
> > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> > > ---
> > >  arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
> > >  board/sunxi/board.c                         | 161
> > > +++++++++++++++-------------
> > >  net/eth_legacy.c                            |   1 +
> > >  3 files changed, 98 insertions(+), 75 deletions(-)
> > > 
> > > diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h
> > > b/arch/arm/include/asm/arch-sunxi/sys_proto.h
> > > index a373319..fad7c48 100644
> > > --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
> > > +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
> > > @@ -30,4 +30,15 @@ void eth_init_board(void);
> > >  static inline void eth_init_board(void) {}
> > >  #endif
> > > 
> > > +int board_get_enetaddr(const int i, unsigned char *mac_addr);
> > > +
> > > +#if CONFIG_SUNXI_EMAC
> > > +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
> > > int id);
> > > +#endif
> > > +
> > > +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
> > > +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
> > > +#endif
> > > +
> > > +
> > >  #endif
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index 5365638..4aeab51 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -21,6 +21,7 @@
> > >  #include <asm/arch/gpio.h>
> > >  #include <asm/arch/mmc.h>
> > >  #include <asm/arch/spl.h>
> > > +#include <asm/arch/sys_proto.h>
> > >  #include <asm/arch/usb_phy.h>
> > >  #ifndef CONFIG_ARM64
> > >  #include <asm/armv7.h>
> > > @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
> > >  }
> > >  #endif
> > > 
> > > +int sunxi_get_board_serial(unsigned int *serial)
> > > +{
> > > +       int ret;
> > > +
> > > +       ret = sunxi_get_sid(serial);
> > > +       if (!ret || serial[0])
> > > +               return -ENOSYS;
> > > +
> > > +       /*
> > > +        * The single words 1 - 3 of the SID have quite a few
> > > bits
> > > +        * which are the same on many models, so we take a crc32
> > > +        * of all 3 words, to get a more unique value.
> > > +        *
> > > +        * Note we only do this on newer SoCs as we cannot change
> > > +        * the algorithm on older SoCs since those have been
> > > using
> > > +        * fixed mac-addresses/serial based on only using word 3
> > > for a
> > > +        * long time and changing a fixed mac-address/serial with
> > > an
> > > +        * u-boot update is not good.
> > > +        */
> > > +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
> > > && \
> > > +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
> > > && \
> > > +    !defined(CONFIG_MACH_SUN8I_A23) &&
> > > !defined(CONFIG_MACH_SUN8I_A33)
> > > +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
> > > +#endif
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  #ifdef CONFIG_SERIAL_TAG
> > >  void get_board_serial(struct tag_serialnr *serialnr)
> > >  {
> > > @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr
> > > *serialnr)
> > >  #endif
> > > 
> > >  /*
> > > + * Generate a MAC address based on device index and the serial
> > > number.
> > > + * The first half of the of the first octet holds the eth index.
> > > + *
> > > + * In the second octet we forcefully mark the MAC address to a
> > > locally
> > > + * administered MAC address.
> > > + *
> > > + */
> > > +int board_get_enetaddr(const int index, unsigned char *enetaddr)
> > 
> > This would be part of a board-specific eth driver.
> 
> this is being called now from sunxi_gmac.c and sunxi_emac.c and
> supplies 
> these board specific drivers with a mac address based on the serial 
> number of the board. I could move this logic over, but then i'd have
> to 
> add it to both eth drivers. By having it in the board.c file, we have
> 2 
> simple functions in the board-specific eth driver:
> 
> 
> static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
> {
> 	struct eth_pdata *pdata = dev_get_platdata(dev);
> 
> 	return board_get_enetaddr(dev->seq, pdata->enetaddr);
> }
> 
> So do you propose to dupilicate the code into both board specific 
> drivers, have it named differently or that the shared code live
> elsewhere?
Replying to myself here,

I just realized, while this bit was not accepted, the overal
implementation has changed in the set. So before, I did things wrong :)
As Simon explained last time.

To clarify, I now have added the logic to the sunxi gmac and emac board
specific drivers. But afaik they share no common code. (like
sunxi_common.c in drivers/net)

With that in mind, how we did things up until now, was to have a
fallback scenario where we use the SoC serial number to generate a MAC
address.

If this is go be done with the board specific driver, we'd need to
still however call board specific functions (sunxi_get_board_serial).

The solution I think is still one of the previously mentioned, a
sunxi_common.c which does the serial -> MAC conversion according to the
previous logic, using sunxi_get_board_serial() (which really is a SoC
specific function, rather board) or have (sunxi)_board_get_enetaddr()
in the same spot where it is now.

Writing this, I realize the sunxi_common.c approach may not be half
bad, even if it only contains a single function for now.

Olliver

> 
> Olliver
> > 
> > > +{
> > > +       uint8_t mac_addr[ARP_HLEN] = { 0x00 };
> > > +       unsigned int serial[4];
> > > +       int ret;
> > > +
> > > +       if ((index < 0) || !enetaddr)
> > > +               return -ENOSYS;
> > > +
> > > +       ret = sunxi_get_board_serial(serial);
> > > +       if (!ret)
> > > +               return ret;
> > > +
> > > +       /* Ensure the NIC specific bytes of the mac are not all 0
> > > */
> > > +       if ((serial[3] & 0xffffff) == 0)
> > > +               serial[3] |= 0x800000;
> > > +
> > > +       mac_addr[0] = (index << 4);
> > > +       mac_addr[1] = (serial[0] >>  0) & 0xff;
> > > +       mac_addr[2] = (serial[3] >> 24) & 0xff;
> > > +       mac_addr[3] = (serial[3] >> 16) & 0xff;
> > > +       mac_addr[4] = (serial[3] >>  8) & 0xff;
> > > +       mac_addr[5] = (serial[3] >>  0) & 0xff;
> > > +
> > > +       set_local_ethaddr(mac_addr);
> > > +       memcpy(enetaddr, mac_addr, ARP_HLEN);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
> > > int id)
> > > +{
> > > +       return board_get_enetaddr(id, enetaddr);
> > > +}
> > > +
> > > +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id)
> > > +{
> > > +       return board_get_enetaddr(id, enetaddr);
> > > +}
> > > +
> > > +/*
> > >   * Check the SPL header for the "sunxi" variant. If found: parse
> > > values
> > >   * that might have been passed by the loader ("fel" utility),
> > > and update
> > >   * the environment accordingly.
> > > @@ -617,77 +694,10 @@ static void parse_spl_header(const uint32_t
> > > spl_addr)
> > >         setenv_hex("fel_scriptaddr", spl->fel_script_address);
> > >  }
> > > 
> > > -/*
> > > - * Note this function gets called multiple times.
> > > - * It must not make any changes to env variables which already
> > > exist.
> > > - */
> > > -static void setup_environment(const void *fdt)
> > > -{
> > > -       char serial_string[17] = { 0 };
> > > -       unsigned int sid[4];
> > > -       uint8_t mac_addr[6];
> > > -       char ethaddr[16];
> > > -       int i, ret;
> > > -
> > > -       ret = sunxi_get_sid(sid);
> > > -       if (ret == 0 && sid[0] != 0) {
> > > -               /*
> > > -                * The single words 1 - 3 of the SID have quite a
> > > few bits
> > > -                * which are the same on many models, so we take
> > > a crc32
> > > -                * of all 3 words, to get a more unique value.
> > > -                *
> > > -                * Note we only do this on newer SoCs as we
> > > cannot change
> > > -                * the algorithm on older SoCs since those have
> > > been using
> > > -                * fixed mac-addresses based on only using word 3
> > > for a
> > > -                * long time and changing a fixed mac-address
> > > with an
> > > -                * u-boot update is not good.
> > > -                */
> > > -#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
> > > && \
> > > -    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
> > > && \
> > > -    !defined(CONFIG_MACH_SUN8I_A23) &&
> > > !defined(CONFIG_MACH_SUN8I_A33)
> > > -               sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
> > > -#endif
> > > -
> > > -               /* Ensure the NIC specific bytes of the mac are
> > > not all 0 */
> > > -               if ((sid[3] & 0xffffff) == 0)
> > > -                       sid[3] |= 0x800000;
> > > -
> > > -               for (i = 0; i < 4; i++) {
> > > -                       sprintf(ethaddr, "ethernet%d", i);
> > > -                       if (!fdt_get_alias(fdt, ethaddr))
> > > -                               continue;
> > > -
> > > -                       if (i == 0)
> > > -                               strcpy(ethaddr, "ethaddr");
> > > -                       else
> > > -                               sprintf(ethaddr, "eth%daddr", i);
> > > -
> > > -                       if (getenv(ethaddr))
> > > -                               continue;
> > > -
> > > -                       /* Non OUI / registered MAC address */
> > > -                       mac_addr[0] = (i << 4) | 0x02;
> > > -                       mac_addr[1] = (sid[0] >>  0) & 0xff;
> > > -                       mac_addr[2] = (sid[3] >> 24) & 0xff;
> > > -                       mac_addr[3] = (sid[3] >> 16) & 0xff;
> > > -                       mac_addr[4] = (sid[3] >>  8) & 0xff;
> > > -                       mac_addr[5] = (sid[3] >>  0) & 0xff;
> > > -
> > > -                       eth_setenv_enetaddr(ethaddr, mac_addr);
> > > -               }
> > > -
> > > -               if (!getenv("serial#")) {
> > > -                       snprintf(serial_string,
> > > sizeof(serial_string),
> > > -                               "%08x%08x", sid[0], sid[3]);
> > > -
> > > -                       setenv("serial#", serial_string);
> > > -               }
> > > -       }
> > > -}
> > > -
> > >  int misc_init_r(void)
> > >  {
> > >         __maybe_unused int ret;
> > > +       unsigned int serial[4];
> > > 
> > >         setenv("fel_booted", NULL);
> > >         setenv("fel_scriptaddr", NULL);
> > > @@ -697,7 +707,14 @@ int misc_init_r(void)
> > >                 parse_spl_header(SPL_ADDR);
> > >         }
> > > 
> > > -       setup_environment(gd->fdt_blob);
> > > +       if (sunxi_get_board_serial(serial)) {
> > > +               char serial_string[17] = { 0 };
> > > +
> > > +               snprintf(serial_string, sizeof(serial_string),
> > > +                        "%08x%08x", serial[0], serial[3]);
> > > +
> > > +               setenv("serial#", serial_string);
> > > +       }
> > > 
> > >  #ifndef CONFIG_MACH_SUN9I
> > >         ret = sunxi_usb_phy_probe();
> > > @@ -713,12 +730,6 @@ int ft_board_setup(void *blob, bd_t *bd)
> > >  {
> > >         int __maybe_unused r;
> > > 
> > > -       /*
> > > -        * Call setup_environment again in case the boot fdt has
> > > -        * ethernet aliases the u-boot copy does not have.
> > > -        */
> > > -       setup_environment(blob);
> > > -
> > >  #ifdef CONFIG_VIDEO_DT_SIMPLEFB
> > >         r = sunxi_simplefb_setup(blob);
> > >         if (r)
> > > diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> > > index d6d7cee..b8b1e3b 100644
> > > --- a/net/eth_legacy.c
> > > +++ b/net/eth_legacy.c
> > > @@ -10,6 +10,7 @@
> > >  #include <command.h>
> > >  #include <environment.h>
> > >  #include <net.h>
> > > +#include <i2c.h>
> > >  #include <phy.h>
> > >  #include <linux/errno.h>
> > >  #include "eth_internal.h"
> > > --
> > > 2.10.2
> > > 
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address
  2017-04-07 13:57       ` o.schinagl at ultimaker.com
@ 2017-04-10 22:56         ` Joe Hershberger
  2017-04-11 20:14           ` Olliver Schinagl
  0 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2017-04-10 22:56 UTC (permalink / raw)
  To: u-boot

On Fri, Apr 7, 2017 at 8:57 AM,  <o.schinagl@ultimaker.com> wrote:
> On Fri, 2017-04-07 at 15:45 +0200, Olliver Schinagl wrote:
>> Hey Joe,
>>
>> On 30-11-16 22:36, Joe Hershberger wrote:
>> > On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.
>> > nl> wrote:
>> > > Add board hooks allowing to get ethernet addresses in a board
>> > > specific
>> > > manner. Currently this is done by generating a MAC address from
>> > > the SID and injecting the ethernet device number in the first
>> > > octet.
>> > >
>> > > This usually happens as a fallback, if either the eeprom fails to
>> > > set a
>> > > MAC address or the FDT forces an override.
>> > >
>> > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> > > ---
>> > >  arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>> > >  board/sunxi/board.c                         | 161
>> > > +++++++++++++++-------------
>> > >  net/eth_legacy.c                            |   1 +
>> > >  3 files changed, 98 insertions(+), 75 deletions(-)
>> > >
>> > > diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> > > b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> > > index a373319..fad7c48 100644
>> > > --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> > > +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>> > > @@ -30,4 +30,15 @@ void eth_init_board(void);
>> > >  static inline void eth_init_board(void) {}
>> > >  #endif
>> > >
>> > > +int board_get_enetaddr(const int i, unsigned char *mac_addr);
>> > > +
>> > > +#if CONFIG_SUNXI_EMAC
>> > > +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
>> > > int id);
>> > > +#endif
>> > > +
>> > > +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
>> > > +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>> > > +#endif
>> > > +
>> > > +
>> > >  #endif
>> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> > > index 5365638..4aeab51 100644
>> > > --- a/board/sunxi/board.c
>> > > +++ b/board/sunxi/board.c
>> > > @@ -21,6 +21,7 @@
>> > >  #include <asm/arch/gpio.h>
>> > >  #include <asm/arch/mmc.h>
>> > >  #include <asm/arch/spl.h>
>> > > +#include <asm/arch/sys_proto.h>
>> > >  #include <asm/arch/usb_phy.h>
>> > >  #ifndef CONFIG_ARM64
>> > >  #include <asm/armv7.h>
>> > > @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>> > >  }
>> > >  #endif
>> > >
>> > > +int sunxi_get_board_serial(unsigned int *serial)
>> > > +{
>> > > +       int ret;
>> > > +
>> > > +       ret = sunxi_get_sid(serial);
>> > > +       if (!ret || serial[0])
>> > > +               return -ENOSYS;
>> > > +
>> > > +       /*
>> > > +        * The single words 1 - 3 of the SID have quite a few
>> > > bits
>> > > +        * which are the same on many models, so we take a crc32
>> > > +        * of all 3 words, to get a more unique value.
>> > > +        *
>> > > +        * Note we only do this on newer SoCs as we cannot change
>> > > +        * the algorithm on older SoCs since those have been
>> > > using
>> > > +        * fixed mac-addresses/serial based on only using word 3
>> > > for a
>> > > +        * long time and changing a fixed mac-address/serial with
>> > > an
>> > > +        * u-boot update is not good.
>> > > +        */
>> > > +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
>> > > && \
>> > > +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
>> > > && \
>> > > +    !defined(CONFIG_MACH_SUN8I_A23) &&
>> > > !defined(CONFIG_MACH_SUN8I_A33)
>> > > +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
>> > > +#endif
>> > > +
>> > > +       return 0;
>> > > +}
>> > > +
>> > >  #ifdef CONFIG_SERIAL_TAG
>> > >  void get_board_serial(struct tag_serialnr *serialnr)
>> > >  {
>> > > @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr
>> > > *serialnr)
>> > >  #endif
>> > >
>> > >  /*
>> > > + * Generate a MAC address based on device index and the serial
>> > > number.
>> > > + * The first half of the of the first octet holds the eth index.
>> > > + *
>> > > + * In the second octet we forcefully mark the MAC address to a
>> > > locally
>> > > + * administered MAC address.
>> > > + *
>> > > + */
>> > > +int board_get_enetaddr(const int index, unsigned char *enetaddr)
>> >
>> > This would be part of a board-specific eth driver.
>>
>> this is being called now from sunxi_gmac.c and sunxi_emac.c and
>> supplies
>> these board specific drivers with a mac address based on the serial
>> number of the board. I could move this logic over, but then i'd have
>> to
>> add it to both eth drivers. By having it in the board.c file, we have
>> 2
>> simple functions in the board-specific eth driver:
>>
>>
>> static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
>> {
>>       struct eth_pdata *pdata = dev_get_platdata(dev);
>>
>>       return board_get_enetaddr(dev->seq, pdata->enetaddr);
>> }
>>
>> So do you propose to dupilicate the code into both board specific
>> drivers, have it named differently or that the shared code live
>> elsewhere?
> Replying to myself here,
>
> I just realized, while this bit was not accepted, the overal
> implementation has changed in the set. So before, I did things wrong :)
> As Simon explained last time.
>
> To clarify, I now have added the logic to the sunxi gmac and emac board
> specific drivers. But afaik they share no common code. (like
> sunxi_common.c in drivers/net)
>
> With that in mind, how we did things up until now, was to have a
> fallback scenario where we use the SoC serial number to generate a MAC
> address.
>
> If this is go be done with the board specific driver, we'd need to
> still however call board specific functions (sunxi_get_board_serial).
>
> The solution I think is still one of the previously mentioned, a
> sunxi_common.c which does the serial -> MAC conversion according to the
> previous logic, using sunxi_get_board_serial() (which really is a SoC
> specific function, rather board) or have (sunxi)_board_get_enetaddr()
> in the same spot where it is now.
>
> Writing this, I realize the sunxi_common.c approach may not be half
> bad, even if it only contains a single function for now.

Sounds like the common layout has promise. Care to send an RFC for how
it would look?

Thanks,
-Joe

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address
  2017-04-10 22:56         ` Joe Hershberger
@ 2017-04-11 20:14           ` Olliver Schinagl
  0 siblings, 0 replies; 24+ messages in thread
From: Olliver Schinagl @ 2017-04-11 20:14 UTC (permalink / raw)
  To: u-boot

Hey Joe,

On 11-04-17 00:56, Joe Hershberger wrote:
> On Fri, Apr 7, 2017 at 8:57 AM,  <o.schinagl@ultimaker.com> wrote:
>> On Fri, 2017-04-07 at 15:45 +0200, Olliver Schinagl wrote:
>>> Hey Joe,
>>>
>>> On 30-11-16 22:36, Joe Hershberger wrote:
>>>> On Fri, Nov 25, 2016 at 9:38 AM, Olliver Schinagl <oliver@schinagl.
>>>> nl> wrote:
>>>>> Add board hooks allowing to get ethernet addresses in a board
>>>>> specific
>>>>> manner. Currently this is done by generating a MAC address from
>>>>> the SID and injecting the ethernet device number in the first
>>>>> octet.
>>>>>
>>>>> This usually happens as a fallback, if either the eeprom fails to
>>>>> set a
>>>>> MAC address or the FDT forces an override.
>>>>>
>>>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>>>>> ---
>>>>>   arch/arm/include/asm/arch-sunxi/sys_proto.h |  11 ++
>>>>>   board/sunxi/board.c                         | 161
>>>>> +++++++++++++++-------------
>>>>>   net/eth_legacy.c                            |   1 +
>>>>>   3 files changed, 98 insertions(+), 75 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> index a373319..fad7c48 100644
>>>>> --- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> +++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
>>>>> @@ -30,4 +30,15 @@ void eth_init_board(void);
>>>>>   static inline void eth_init_board(void) {}
>>>>>   #endif
>>>>>
>>>>> +int board_get_enetaddr(const int i, unsigned char *mac_addr);
>>>>> +
>>>>> +#if CONFIG_SUNXI_EMAC
>>>>> +int sunxi_emac_board_read_rom_hwaddr(unsigned char *enetaddr,
>>>>> int id);
>>>>> +#endif
>>>>> +
>>>>> +#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
>>>>> +int dw_board_read_rom_hwaddr(unsigned char *enetaddr, int id);
>>>>> +#endif
>>>>> +
>>>>> +
>>>>>   #endif
>>>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>>>> index 5365638..4aeab51 100644
>>>>> --- a/board/sunxi/board.c
>>>>> +++ b/board/sunxi/board.c
>>>>> @@ -21,6 +21,7 @@
>>>>>   #include <asm/arch/gpio.h>
>>>>>   #include <asm/arch/mmc.h>
>>>>>   #include <asm/arch/spl.h>
>>>>> +#include <asm/arch/sys_proto.h>
>>>>>   #include <asm/arch/usb_phy.h>
>>>>>   #ifndef CONFIG_ARM64
>>>>>   #include <asm/armv7.h>
>>>>> @@ -564,6 +565,34 @@ int g_dnl_board_usb_cable_connected(void)
>>>>>   }
>>>>>   #endif
>>>>>
>>>>> +int sunxi_get_board_serial(unsigned int *serial)
>>>>> +{
>>>>> +       int ret;
>>>>> +
>>>>> +       ret = sunxi_get_sid(serial);
>>>>> +       if (!ret || serial[0])
>>>>> +               return -ENOSYS;
>>>>> +
>>>>> +       /*
>>>>> +        * The single words 1 - 3 of the SID have quite a few
>>>>> bits
>>>>> +        * which are the same on many models, so we take a crc32
>>>>> +        * of all 3 words, to get a more unique value.
>>>>> +        *
>>>>> +        * Note we only do this on newer SoCs as we cannot change
>>>>> +        * the algorithm on older SoCs since those have been
>>>>> using
>>>>> +        * fixed mac-addresses/serial based on only using word 3
>>>>> for a
>>>>> +        * long time and changing a fixed mac-address/serial with
>>>>> an
>>>>> +        * u-boot update is not good.
>>>>> +        */
>>>>> +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I)
>>>>> && \
>>>>> +    !defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I)
>>>>> && \
>>>>> +    !defined(CONFIG_MACH_SUN8I_A23) &&
>>>>> !defined(CONFIG_MACH_SUN8I_A33)
>>>>> +       serial[3] = crc32(0, (unsigned char *)&serial[1], 12);
>>>>> +#endif
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>>   #ifdef CONFIG_SERIAL_TAG
>>>>>   void get_board_serial(struct tag_serialnr *serialnr)
>>>>>   {
>>>>> @@ -585,6 +614,54 @@ void get_board_serial(struct tag_serialnr
>>>>> *serialnr)
>>>>>   #endif
>>>>>
>>>>>   /*
>>>>> + * Generate a MAC address based on device index and the serial
>>>>> number.
>>>>> + * The first half of the of the first octet holds the eth index.
>>>>> + *
>>>>> + * In the second octet we forcefully mark the MAC address to a
>>>>> locally
>>>>> + * administered MAC address.
>>>>> + *
>>>>> + */
>>>>> +int board_get_enetaddr(const int index, unsigned char *enetaddr)
>>>> This would be part of a board-specific eth driver.
>>> this is being called now from sunxi_gmac.c and sunxi_emac.c and
>>> supplies
>>> these board specific drivers with a mac address based on the serial
>>> number of the board. I could move this logic over, but then i'd have
>>> to
>>> add it to both eth drivers. By having it in the board.c file, we have
>>> 2
>>> simple functions in the board-specific eth driver:
>>>
>>>
>>> static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
>>> {
>>>        struct eth_pdata *pdata = dev_get_platdata(dev);
>>>
>>>        return board_get_enetaddr(dev->seq, pdata->enetaddr);
>>> }
>>>
>>> So do you propose to dupilicate the code into both board specific
>>> drivers, have it named differently or that the shared code live
>>> elsewhere?
>> Replying to myself here,
>>
>> I just realized, while this bit was not accepted, the overal
>> implementation has changed in the set. So before, I did things wrong :)
>> As Simon explained last time.
>>
>> To clarify, I now have added the logic to the sunxi gmac and emac board
>> specific drivers. But afaik they share no common code. (like
>> sunxi_common.c in drivers/net)
>>
>> With that in mind, how we did things up until now, was to have a
>> fallback scenario where we use the SoC serial number to generate a MAC
>> address.
>>
>> If this is go be done with the board specific driver, we'd need to
>> still however call board specific functions (sunxi_get_board_serial).
>>
>> The solution I think is still one of the previously mentioned, a
>> sunxi_common.c which does the serial -> MAC conversion according to the
>> previous logic, using sunxi_get_board_serial() (which really is a SoC
>> specific function, rather board) or have (sunxi)_board_get_enetaddr()
>> in the same spot where it is now.
>>
>> Writing this, I realize the sunxi_common.c approach may not be half
>> bad, even if it only contains a single function for now.
> Sounds like the common layout has promise. Care to send an RFC for how
> it would look?
Not sure if we are on the same page :) but I've sent a patch set which 
includes the change. Find it on patchwork [0].

We should discuss it there I reccon, but for now, it is a single 
function for the read_rom_hwaddr hook for sunxi_emac, sunxi_gmac and 
sun8i_emac. I saw more similar code paths, such as the write_hwaddr, 
which could move here as well. I think in time with some effort a few 
more functions can go into this shared file.

The only thing I haven't figured out yet, where I did continue the 
original thread [1], is how to solve the fdt_fixup scenario.

Olliver

[0] https://patchwork.ozlabs.org/patch/749103/
[1] https://patchwork.ozlabs.org/patch/699288/
>
> Thanks,
> -Joe


-- 
Met vriendelijke groeten, Kind regards, 与亲切的问候
Olliver Schinagl
Research & Development
Ultimaker B.V.
http://www.ultimaker.com

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2017-04-11 20:14 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-25 15:38 [U-Boot] [PATCH] sunxi: net: Use net_ops hooks to get the MAC Olliver Schinagl
2016-11-25 15:38 ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Olliver Schinagl
2016-11-25 15:46   ` [U-Boot] [PATCH] Use eth_ops hooks to set the MAC address Olliver Schinagl
2016-11-27 17:02   ` [U-Boot] [PATCH 1/6] net: dw: Add read_rom_hwaddr net_op hook Simon Glass
2016-11-28 10:38     ` Olliver Schinagl
2016-11-29 21:41       ` Simon Glass
2016-11-30  8:16         ` Olliver Schinagl
2016-11-30 20:43           ` Joe Hershberger
2016-11-25 15:38 ` [U-Boot] [PATCH 2/6] net: sunxi-emac: Write HW address via function Olliver Schinagl
2016-11-30 21:10   ` Joe Hershberger
2017-03-27 16:51   ` [U-Boot] " Joe Hershberger
2016-11-25 15:38 ` [U-Boot] [PATCH 3/6] net: sunxi-emac: Add write_hwaddr net_op hook Olliver Schinagl
2016-11-30 21:32   ` Joe Hershberger
2016-11-25 15:38 ` [U-Boot] [PATCH 4/6] net: sunxi-emac: Add read_rom_hwaddr " Olliver Schinagl
2016-11-30 21:34   ` Joe Hershberger
2016-11-25 15:38 ` [U-Boot] [PATCH 5/6] arm: sunxi: Use board hooks to obtain MAC address Olliver Schinagl
2016-11-30 21:36   ` Joe Hershberger
2017-04-07 13:45     ` Olliver Schinagl
2017-04-07 13:57       ` o.schinagl at ultimaker.com
2017-04-10 22:56         ` Joe Hershberger
2017-04-11 20:14           ` Olliver Schinagl
2016-11-25 15:38 ` [U-Boot] [PATCH 6/6] net: sunxi: Enable eeprom on OLinuXino Lime boards Olliver Schinagl
2016-11-30 21:37   ` Joe Hershberger
2017-03-27 16:51   ` [U-Boot] " Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.