[PATCH v3] Retrieve MAC address from EEPROM
722 views
Skip to first unread message
Olliver Schinagl
Nov 8, 2016, 10:54:58 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
This patch-series introduces methods to retrieve the MAC address from an
onboard EEPROM using the read_rom_hwaddr hook.
The reason we might want to read the MAC address from an EEPROM instead of
storing the entire environment is mostly a size thing. Our default environment
already is bigger then the EEPROM so it is understandable that someone might
not give up the entire eeprom just for the u-boot environment. Especially if
only board specific things might be stored in the eeprom (MAC, serial, product
number etc). Additionally it is a board thing and a MAC address might be
programmed at the factory before ever seeing any software.
The current idea of the eeprom layout, is to skip the first 8 bytes, so that
other information can be stored there if needed, for example a header with some
magic to identify the EEPROM. Or equivalent purposes.
After those 8 bytes the MAC address follows the first macaddress. The macaddress
is appended by a CRC8 byte and then padded to make for nice 8 bytes. Following
the first macaddress one can store a second, or a third etc etc mac addresses.
The CRC8 is optional (via a define) but is strongly recommended to have. It
helps preventing user error and more importantly, checks if the bytes read are
actually a user inserted address. E.g. only writing 1 macaddress into the eeprom
but trying to consume 2.
Hans de Goede and I talked about retrieving the MAC from the EEPROM for sunxi
based boards a while ago, but hopefully this patch makes possible to have
something slightly more generic, even if only the configuration options.
Additionally the EEPROM layout could be recommended by u-boot as well.
Since the Olimex OLinuXino sunxi boards all seem to have an eeprom, I started
my work on one of these and tested the implementation with one of our own real
MAC addresses.
What still needs disussing I think, is the patches that remove the
'setup_environment' function in board.c. I think we have put functionality in
boards.c which does not belong. Injecting ethernet addresses into the
environment instead of using the net_op hooks as well as parsing the fdt to get
overrides from. I think especially this last point should be done at a higher
level, if possible at all.
I explicitly did not use the wiser eth_validate_ethaddr_str(),
eth_parse_enetaddr() and the ARP_HLEN define as it was quite painful
(dependancies) to get these functions into the tools. I would suggest to merge
as is, and if someone wants to improve these simple tools to use these functions
to happily do so.
These patches where tested on Olimex OLinuXino Lime1 (A10/A20), Lime2 (NAND
and eMMC) and A20-OLinuXino-MICRO-4G variants and have been in use
internally on our production systems since v2 of this patch set.
As a recommendation, I would suggest for the zynq to adopt the config defines,
as they are nice and generic and suggest to also implement the 8 byte offset,
crc8 and pad bytes.
P.S. I have gotten in depth/familiar with PATMAN yet :)
=======
Changes since v2:
* Drop the id byte altogether and just mark it as reserved. The 'index' can be
used to indicate the interface instead
* Addopt the read_rom_hwaddr hooks
* Renamed crc8 tool to gen_ethaddr_crc
* Improved the layout EEPROM example
* Made a function out of the hwaddress writing function in sunxi_emac so it
can be re-used as the write_hwaddr net_ops hook.
* No longer handle fdt parameters in board.c
Changes since v1:
* Do not CRC the id byte, move it just after the crc byte.
One of the reasons I decided to move it after the crc8 was mostly due to mass
generation of MAC + CRC combo's where the ID is still unknown. Also not crc-ing
the ID means that it is much easier for a user to change it (via the u-boot i2c
cmd line or from within linux) without having to worry about the crc.
* Add a generator to convert a MAC address from the input to a MAC + CRC8 on
the output.
onboard EEPROM using the read_rom_hwaddr hook.
The reason we might want to read the MAC address from an EEPROM instead of
storing the entire environment is mostly a size thing. Our default environment
already is bigger then the EEPROM so it is understandable that someone might
not give up the entire eeprom just for the u-boot environment. Especially if
only board specific things might be stored in the eeprom (MAC, serial, product
number etc). Additionally it is a board thing and a MAC address might be
programmed at the factory before ever seeing any software.
The current idea of the eeprom layout, is to skip the first 8 bytes, so that
other information can be stored there if needed, for example a header with some
magic to identify the EEPROM. Or equivalent purposes.
After those 8 bytes the MAC address follows the first macaddress. The macaddress
is appended by a CRC8 byte and then padded to make for nice 8 bytes. Following
the first macaddress one can store a second, or a third etc etc mac addresses.
The CRC8 is optional (via a define) but is strongly recommended to have. It
helps preventing user error and more importantly, checks if the bytes read are
actually a user inserted address. E.g. only writing 1 macaddress into the eeprom
but trying to consume 2.
Hans de Goede and I talked about retrieving the MAC from the EEPROM for sunxi
based boards a while ago, but hopefully this patch makes possible to have
something slightly more generic, even if only the configuration options.
Additionally the EEPROM layout could be recommended by u-boot as well.
Since the Olimex OLinuXino sunxi boards all seem to have an eeprom, I started
my work on one of these and tested the implementation with one of our own real
MAC addresses.
What still needs disussing I think, is the patches that remove the
'setup_environment' function in board.c. I think we have put functionality in
boards.c which does not belong. Injecting ethernet addresses into the
environment instead of using the net_op hooks as well as parsing the fdt to get
overrides from. I think especially this last point should be done at a higher
level, if possible at all.
I explicitly did not use the wiser eth_validate_ethaddr_str(),
eth_parse_enetaddr() and the ARP_HLEN define as it was quite painful
(dependancies) to get these functions into the tools. I would suggest to merge
as is, and if someone wants to improve these simple tools to use these functions
to happily do so.
These patches where tested on Olimex OLinuXino Lime1 (A10/A20), Lime2 (NAND
and eMMC) and A20-OLinuXino-MICRO-4G variants and have been in use
internally on our production systems since v2 of this patch set.
As a recommendation, I would suggest for the zynq to adopt the config defines,
as they are nice and generic and suggest to also implement the 8 byte offset,
crc8 and pad bytes.
P.S. I have gotten in depth/familiar with PATMAN yet :)
=======
Changes since v2:
* Drop the id byte altogether and just mark it as reserved. The 'index' can be
used to indicate the interface instead
* Addopt the read_rom_hwaddr hooks
* Renamed crc8 tool to gen_ethaddr_crc
* Improved the layout EEPROM example
* Made a function out of the hwaddress writing function in sunxi_emac so it
can be re-used as the write_hwaddr net_ops hook.
* No longer handle fdt parameters in board.c
Changes since v1:
* Do not CRC the id byte, move it just after the crc byte.
One of the reasons I decided to move it after the crc8 was mostly due to mass
generation of MAC + CRC combo's where the ID is still unknown. Also not crc-ing
the ID means that it is much easier for a user to change it (via the u-boot i2c
cmd line or from within linux) without having to worry about the crc.
* Add a generator to convert a MAC address from the input to a MAC + CRC8 on
the output.
Olliver Schinagl
Nov 8, 2016, 10:54:58 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
Expose enetaddr writing via net_ops so that it can be hooked into.
Signed-off-by: Olliver Schinagl <oli...@schinagl.nl>
---
drivers/net/sunxi_emac.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/sunxi_emac.c b/drivers/net/sunxi_emac.c
index 99339db..dcf832e 100644
--- a/drivers/net/sunxi_emac.c
+++ b/drivers/net/sunxi_emac.c
@@ -554,6 +554,14 @@ 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);
+
+ 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 +578,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
Signed-off-by: Olliver Schinagl <oli...@schinagl.nl>
---
drivers/net/sunxi_emac.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/sunxi_emac.c b/drivers/net/sunxi_emac.c
index 99339db..dcf832e 100644
--- a/drivers/net/sunxi_emac.c
+++ b/drivers/net/sunxi_emac.c
@@ -554,6 +554,14 @@ 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);
+
+ 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 +578,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
Olliver Schinagl
Nov 8, 2016, 10:54:59 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
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.
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, ®s->mac_a1);
+ writel(enetaddr_lo, ®s->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],
- ®s->mac_a1);
- writel(enetaddr[3] << 16 | enetaddr[4] << 8 | enetaddr[5],
- ®s->mac_a0);
+ _sunxi_write_hwaddr(priv, enetaddr);
mdelay(1);
--
2.10.2
making it a one time inflexible operation. By moving it into a separate
function, we can now use this more flexibly.
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, ®s->mac_a1);
+ writel(enetaddr_lo, ®s->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],
- ®s->mac_a1);
- writel(enetaddr[3] << 16 | enetaddr[4] << 8 | enetaddr[5],
- ®s->mac_a0);
+ _sunxi_write_hwaddr(priv, enetaddr);
mdelay(1);
--
2.10.2
Olliver Schinagl
Nov 8, 2016, 10:54:59 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
This patch uses the newly introduced Kconfig options to use the net_op
read_rom_hwaddr to retrieve the MAC from an EEPROM.
This will be especially useful for the Olimex OLinuXino series of sunxi
boards as they all have an 2k i2c eeprom chip.
The MAC address in the eeprom is ignored (if enabled) if the CRC8 check
fails.
This new functionality allows for querying multiple MAC addresses. The
first (supported) device being probed gets the first address, the second
the second etc. If a generated MAC address is desired, set it to all 0
(and if crc8 is configured also add that) for the adapter.
board/sunxi/Kconfig | 4 ++++
board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index e1d4ab1..6b8ac99 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -414,6 +414,7 @@ config I2C0_ENABLE
config I2C1_ENABLE
bool "Enable I2C/TWI controller 1"
+ default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
default n
select CMD_I2C
---help---
@@ -421,6 +422,7 @@ config I2C1_ENABLE
config I2C2_ENABLE
bool "Enable I2C/TWI controller 2"
+ default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
default n
select CMD_I2C
---help---
@@ -428,6 +430,7 @@ config I2C2_ENABLE
if MACH_SUN6I || MACH_SUN7I
config I2C3_ENABLE
+ default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
bool "Enable I2C/TWI controller 3"
default n
select CMD_I2C
@@ -447,6 +450,7 @@ endif
if MACH_SUN7I
config I2C4_ENABLE
+ default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
bool "Enable I2C/TWI controller 4"
default n
select CMD_I2C
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 71124f4..f1e64cd 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -12,6 +12,7 @@
*/
#include <common.h>
+#include <i2c.h>
#include <mmc.h>
#include <axp_pmic.h>
#include <asm/arch/clock.h>
@@ -31,6 +32,7 @@
#include <crc.h>
#include <environment.h>
#include <libfdt.h>
+#include <linux/crc8.h>
#include <nand.h>
#include <net.h>
#include <sy8106a.h>
@@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char *enetaddr, uint8_t cnt)
memcpy(enetaddr, mac_addr, ARP_HLEN);
}
+static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t cnt)
+{
+ uint8_t eeprom[ARP_HLEN + 1] = { 0x00 };
+#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
+ int old_i2c_bus;
+
+ old_i2c_bus = i2c_get_bus_num();
+ if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS)
+ i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
+ /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */
+ if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
+ CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN + 2)),
+ CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
+ eeprom, ARP_HLEN + 1)) {
+ i2c_set_bus_num(old_i2c_bus);
+ puts("Could not read the EEPROM; EEPROM missing?\n");
+ return;
+ }
+ i2c_set_bus_num(old_i2c_bus);
+#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
+ if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) {
+ puts("CRC error on MAC address from EEPROM.\n");
+ return;
+ }
+#endif
+#endif
+
+ memcpy(enetaddr, eeprom, ARP_HLEN);
+}
+
static int sunxi_read_rom_hwaddr(unsigned char *enetaddr)
{
static unsigned int cnt;
- _sunxi_gen_sid_hwaddr(enetaddr, cnt);
+ _sunxi_read_rom_hwaddr(enetaddr, cnt);
+
+ if (is_zero_ethaddr(enetaddr)) {
+ _sunxi_gen_sid_hwaddr(enetaddr, cnt);
+ puts("Serial# based ");
+ }
/* First call, first stored MAC address, increase for next call */
cnt++;
--
2.10.2
read_rom_hwaddr to retrieve the MAC from an EEPROM.
This will be especially useful for the Olimex OLinuXino series of sunxi
boards as they all have an 2k i2c eeprom chip.
The MAC address in the eeprom is ignored (if enabled) if the CRC8 check
fails.
This new functionality allows for querying multiple MAC addresses. The
first (supported) device being probed gets the first address, the second
the second etc. If a generated MAC address is desired, set it to all 0
(and if crc8 is configured also add that) for the adapter.
board/sunxi/Kconfig | 4 ++++
board/sunxi/board.c | 39 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index e1d4ab1..6b8ac99 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -414,6 +414,7 @@ config I2C0_ENABLE
config I2C1_ENABLE
bool "Enable I2C/TWI controller 1"
+ default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
default n
select CMD_I2C
---help---
@@ -421,6 +422,7 @@ config I2C1_ENABLE
config I2C2_ENABLE
bool "Enable I2C/TWI controller 2"
+ default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
default n
select CMD_I2C
---help---
@@ -428,6 +430,7 @@ config I2C2_ENABLE
if MACH_SUN6I || MACH_SUN7I
config I2C3_ENABLE
+ default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
bool "Enable I2C/TWI controller 3"
default n
select CMD_I2C
@@ -447,6 +450,7 @@ endif
if MACH_SUN7I
config I2C4_ENABLE
+ default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
bool "Enable I2C/TWI controller 4"
default n
select CMD_I2C
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 71124f4..f1e64cd 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -12,6 +12,7 @@
*/
#include <common.h>
+#include <i2c.h>
#include <mmc.h>
#include <axp_pmic.h>
#include <asm/arch/clock.h>
@@ -31,6 +32,7 @@
#include <crc.h>
#include <environment.h>
#include <libfdt.h>
+#include <linux/crc8.h>
#include <nand.h>
#include <net.h>
#include <sy8106a.h>
@@ -626,11 +628,46 @@ static void _sunxi_gen_sid_hwaddr(unsigned char *enetaddr, uint8_t cnt)
memcpy(enetaddr, mac_addr, ARP_HLEN);
}
+static void _sunxi_read_rom_hwaddr(unsigned char *enetaddr, uint8_t cnt)
+{
+ uint8_t eeprom[ARP_HLEN + 1] = { 0x00 };
+#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
+ int old_i2c_bus;
+
+ old_i2c_bus = i2c_get_bus_num();
+ if (old_i2c_bus != CONFIG_NET_ETHADDR_EEPROM_I2C_BUS)
+ i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
+ /* Skip in blocks of 8 (ARP + CRC8 + pad), but read 7. */
+ if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
+ CONFIG_NET_ETHADDR_EEPROM_OFFSET + (cnt * (ARP_HLEN + 2)),
+ CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
+ eeprom, ARP_HLEN + 1)) {
+ i2c_set_bus_num(old_i2c_bus);
+ puts("Could not read the EEPROM; EEPROM missing?\n");
+ return;
+ }
+ i2c_set_bus_num(old_i2c_bus);
+#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
+ if (crc8(0, eeprom, ARP_HLEN) != eeprom[ARP_HLEN]) {
+ puts("CRC error on MAC address from EEPROM.\n");
+ return;
+ }
+#endif
+#endif
+
+ memcpy(enetaddr, eeprom, ARP_HLEN);
+}
+
static int sunxi_read_rom_hwaddr(unsigned char *enetaddr)
{
static unsigned int cnt;
- _sunxi_gen_sid_hwaddr(enetaddr, cnt);
+ _sunxi_read_rom_hwaddr(enetaddr, cnt);
+
+ if (is_zero_ethaddr(enetaddr)) {
+ _sunxi_gen_sid_hwaddr(enetaddr, cnt);
+ puts("Serial# based ");
+ }
/* First call, first stored MAC address, increase for next call */
cnt++;
--
2.10.2
Olliver Schinagl
Nov 8, 2016, 10:54:59 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
Currently we inject 5 ethernet addresses into the environment, just in
case we may need them. We do this because some boards have no eeprom
(programmed) with a proper ethernet address. With the recent addition of
reading actual ethernet addresses from the eeprom via the net_op we
should not inject environment variables any more.
board/sunxi/board.c | 45 +++++++++++++--------------------------------
1 file changed, 13 insertions(+), 32 deletions(-)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 9b56a89..71124f4 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -688,15 +688,19 @@ 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)
+int misc_init_r(void)
{
char serial_string[17] = { 0 };
unsigned int sid[4];
- int ret;
+ __maybe_unused int ret;
+
+ setenv("fel_booted", NULL);
+ setenv("fel_scriptaddr", NULL);
+ /* determine if we are running in FEL mode */
+ if (!is_boot0_magic(SPL_ADDR + 4)) { /* eGON.BT0 */
+ setenv("fel_booted", "1");
+ parse_spl_header(SPL_ADDR);
+ }
ret = sunxi_get_sid(sid);
if (ret == 0 && sid[0] != 0) {
@@ -717,28 +721,11 @@ static void setup_environment(const void *fdt)
sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
#endif
- if (!getenv("serial#")) {
- snprintf(serial_string, sizeof(serial_string),
- "%08x%08x", sid[0], sid[3]);
+ snprintf(serial_string, sizeof(serial_string),
+ "%08x%08x", sid[0], sid[3]);
- setenv("serial#", serial_string);
- }
+ setenv("serial#", serial_string);
}
-}
-
-int misc_init_r(void)
-{
- __maybe_unused int ret;
-
- setenv("fel_booted", NULL);
- setenv("fel_scriptaddr", NULL);
- /* determine if we are running in FEL mode */
- if (!is_boot0_magic(SPL_ADDR + 4)) { /* eGON.BT0 */
- setenv("fel_booted", "1");
- parse_spl_header(SPL_ADDR);
- }
-
- setup_environment(gd->fdt_blob);
#ifndef CONFIG_MACH_SUN9I
ret = sunxi_usb_phy_probe();
@@ -754,12 +741,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)
--
2.10.2
case we may need them. We do this because some boards have no eeprom
(programmed) with a proper ethernet address. With the recent addition of
reading actual ethernet addresses from the eeprom via the net_op we
should not inject environment variables any more.
board/sunxi/board.c | 45 +++++++++++++--------------------------------
1 file changed, 13 insertions(+), 32 deletions(-)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 9b56a89..71124f4 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -688,15 +688,19 @@ 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)
+int misc_init_r(void)
{
char serial_string[17] = { 0 };
unsigned int sid[4];
- int ret;
+ __maybe_unused int ret;
+
+ setenv("fel_booted", NULL);
+ setenv("fel_scriptaddr", NULL);
+ /* determine if we are running in FEL mode */
+ if (!is_boot0_magic(SPL_ADDR + 4)) { /* eGON.BT0 */
+ setenv("fel_booted", "1");
+ parse_spl_header(SPL_ADDR);
+ }
ret = sunxi_get_sid(sid);
if (ret == 0 && sid[0] != 0) {
@@ -717,28 +721,11 @@ static void setup_environment(const void *fdt)
sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
#endif
- if (!getenv("serial#")) {
- snprintf(serial_string, sizeof(serial_string),
- "%08x%08x", sid[0], sid[3]);
+ snprintf(serial_string, sizeof(serial_string),
+ "%08x%08x", sid[0], sid[3]);
- setenv("serial#", serial_string);
- }
+ setenv("serial#", serial_string);
}
-}
-
-int misc_init_r(void)
-{
- __maybe_unused int ret;
-
- setenv("fel_booted", NULL);
- setenv("fel_scriptaddr", NULL);
- /* determine if we are running in FEL mode */
- if (!is_boot0_magic(SPL_ADDR + 4)) { /* eGON.BT0 */
- setenv("fel_booted", "1");
- parse_spl_header(SPL_ADDR);
- }
-
- setup_environment(gd->fdt_blob);
#ifndef CONFIG_MACH_SUN9I
ret = sunxi_usb_phy_probe();
@@ -754,12 +741,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)
--
2.10.2
Olliver Schinagl
Nov 8, 2016, 10:54:59 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de, Olliver Schinagl
This patch allows Kconfig to enable and set parameters to make it
possible to read the MAC address from an EEPROM. This patch only sets up
some environment variables, it is up to the specific boards to actually
use these defines.
Besides the various tuneables as to how to access the eeprom (bus,
address, addressing mode/length, 2 configurable that are EEPROM generic
(e.g. SPI or some other form of access) which are:
NET_ETHADDR_EEPROM_OFFSET, indicating where in the EEPROM the start of
the MAC address is. The default is 8 allowing for 8 bytes before the MAC
for other purposes (header MAGIC for example).
NET_ETHADDR_EEPROM_CRC8, indicating the MAC is appended with a CRC8-CCIT
checksum that should be verified.
Currently only I2C eeproms have been tested and thus only those options
are available, but shouldn't be a limit. NET_ETHADDR_EEPROM_SPI can be
just as created.
Signed-off-by: Olliver Schinagl <o.sch...@ultimaker.com>
---
doc/README.enetaddr | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
net/Kconfig | 58 ++++++++++++++++++++++++++++++++++++
2 files changed, 143 insertions(+)
diff --git a/doc/README.enetaddr b/doc/README.enetaddr
index 50e4899..0bf291d 100644
--- a/doc/README.enetaddr
+++ b/doc/README.enetaddr
@@ -47,6 +47,91 @@ Correct flow of setting up the MAC address (summarized):
Previous behavior had the MAC address always being programmed into hardware
in the device's init() function.
+--------
+ EEPROM
+--------
+
+When there is an EEPROM available on a board, but the EEPROM is not large enough
+to store the whole environment, it may be desired to store a MAC address in the
+onboard EEPROM. Using CONFIG_NET_ETHADDR_EEPROM enables this feature. Depending
+on the board, the EEPROM may be connected on various methods, but currently,
+only the I2C bus is available via CONFIG_NET_ETHADDR_EEPROM_I2C.
+
+The following config options are available,
+CONFIG_NET_ETHADDR_EEPROM_I2C_BUS is the I2C bus on which the eeprom is present.
+CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR sets the address of the EEPROM, which
+defaults to the very common 0x50. Small size EEPROM's generally use single byte
+addressing but larger EEPROM's may use double byte addressing, which can be
+configured using CONFIG_NET_ETHADDR_EEPROM_ADDRLEN.
+
+Within the EEPROM, the MAC address can be stored on any arbitrary offset,
+CONFIG_NET_ETHADDR_EEPROM_OFFSET sets this to 8 as a default however, allowing
+the first 8 bytes to be used for an optional data, for example a configuration
+struct where the mac address is part of.
+
+Appending the 6 (ARP_HLEN) bytes is a CRC8 byte over the previous ARP_HLEN
+bytes. Whether to check this CRC8 or not is dependent on
+CONFIG_NET_ETHADDR_EEPROM_CRC8.
+
+To keep things nicely aligned, a final 'reserved' byte is added to the mac
+address + crc8 combo.
+
+A board may want to store more information in its eeprom, using the following
+example layout, this can be achieved.
+
+struct mac_addr {
+ uint8_t mac[ARP_HLEN];
+ uint8_t crc8;
+ uint8_t reserved;
+};
+
+struct config_eeprom {
+ uint32_t magic;
+ uint8_t version;
+ uint8_t reserved[2];
+ uint8_t mac_cnt;
+ struct mac_addr[mac_cnt];
+};
+
+Filling this in:
+struct config_eeprom eeprom = {
+ .magic = { 'M', 'g', 'i', 'c' },
+ .reserved = { 0x00, 0x00 },
+ .mac_cnt = 2,
+ .mac_addr = {
+ {
+ .mac = {
+ 0x01, 0x23, 0x45,
+ 0x67, 0x89, 0xab,
+ },
+ .crc8 = 0xbe,
+ .reserved = 0x00,
+ }, {
+ .mac = {
+ 0xba, 0x98, 0x76,
+ 0x54, 0x32, 0x10,
+ },
+ .crc8 = 0x82,
+ .reserved = 0x00,
+ },
+ },
+};
+
+The eeprom content would look like this.
+
+00000000 4d 67 69 63 01 00 00 02 01 23 45 67 89 ab be 00 |Mgic.....#Eg....|
+00000010 ba 98 76 54 32 10 82 00 |..vT2...|
+
+Alternativly the i2c-tools can be used as well.
+
+i2cset I2CBUS 0x50 0x08 0x01
+i2cset I2CBUS 0x50 0x09 0x23
+i2cset I2CBUS 0x50 0x0a 0x45
+i2cset I2CBUS 0x50 0x0b 0x67
+i2cset I2CBUS 0x50 0x0c 0x89
+i2cset I2CBUS 0x50 0x0d 0xab
+i2cset I2CBUS 0x50 0x0e 0xbe
+
-------
Usage
-------
diff --git a/net/Kconfig b/net/Kconfig
index 414c549..f7ef2b7 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -7,6 +7,64 @@ menuconfig NET
if NET
+config NET_ETHADDR_EEPROM
+ bool "Get ethaddr from eeprom"
+ help
+ Selecting this will try to get the Ethernet address from an onboard
+ EEPROM and set into the environment if and only if the environment
+ does currently not already hold a MAC address. For more information
+ see doc/README.enetaddr.
+
+config NET_ETHADDR_EEPROM_I2C
+ depends on NET_ETHADDR_EEPROM
+ bool "EEPROM on I2C bus"
+ help
+ This switch enables checks for an EEPROM on the I2C bus. Naturally
+ this will only work if there is an actual EEPROM connected on the
+ I2C bus and the bus and device are properly configured via the
+ options below.
+
+config NET_ETHADDR_EEPROM_I2C_BUS
+ depends on NET_ETHADDR_EEPROM_I2C
+ int "I2C bus"
+ default 0
+ help
+ Select the bus on which the EEPROM is present, defaults to bus 0.
+
+config NET_ETHADDR_EEPROM_I2C_ADDR
+ depends on NET_ETHADDR_EEPROM_I2C
+ hex "eeprom address"
+ default 0x50
+ help
+ Select the address of the eeprom, defaults to address 0x50.
+
+config NET_ETHADDR_EEPROM_I2C_ADDRLEN
+ depends on NET_ETHADDR_EEPROM_I2C
+ int "eeprom address length"
+ default 1
+ help
+ Number of bytes to be used for the I2C address length. Typically 1,
+ 2 for large memories, 0 for register type devices with only one
+ register.
+
+config NET_ETHADDR_EEPROM_OFFSET
+ depends on NET_ETHADDR_EEPROM
+ int "EEPROM offset"
+ default 8
+ help
+ Select the byte offset of the MAC address within the page,
+ defaults to byte 8.
+
+config NET_ETHADDR_EEPROM_CRC8
+ depends on NET_ETHADDR_EEPROM
+ bool "Check CRC8 of MAC"
+ default y
+ help
+ Optionally, it is possible to run a CRC-8-CCITT check on the MAC
+ address. To do so, the MAC address is stored with a CRC8 byte append.
+ This option enables the CRC check of the MAC address against the CRC
+ byte.
+
config NET_RANDOM_ETHADDR
bool "Random ethaddr if unset"
select LIB_RAND
--
2.10.2
possible to read the MAC address from an EEPROM. This patch only sets up
some environment variables, it is up to the specific boards to actually
use these defines.
Besides the various tuneables as to how to access the eeprom (bus,
address, addressing mode/length, 2 configurable that are EEPROM generic
(e.g. SPI or some other form of access) which are:
NET_ETHADDR_EEPROM_OFFSET, indicating where in the EEPROM the start of
the MAC address is. The default is 8 allowing for 8 bytes before the MAC
for other purposes (header MAGIC for example).
NET_ETHADDR_EEPROM_CRC8, indicating the MAC is appended with a CRC8-CCIT
checksum that should be verified.
Currently only I2C eeproms have been tested and thus only those options
are available, but shouldn't be a limit. NET_ETHADDR_EEPROM_SPI can be
just as created.
Signed-off-by: Olliver Schinagl <o.sch...@ultimaker.com>
---
doc/README.enetaddr | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
net/Kconfig | 58 ++++++++++++++++++++++++++++++++++++
2 files changed, 143 insertions(+)
diff --git a/doc/README.enetaddr b/doc/README.enetaddr
index 50e4899..0bf291d 100644
--- a/doc/README.enetaddr
+++ b/doc/README.enetaddr
@@ -47,6 +47,91 @@ Correct flow of setting up the MAC address (summarized):
Previous behavior had the MAC address always being programmed into hardware
in the device's init() function.
+--------
+ EEPROM
+--------
+
+When there is an EEPROM available on a board, but the EEPROM is not large enough
+to store the whole environment, it may be desired to store a MAC address in the
+onboard EEPROM. Using CONFIG_NET_ETHADDR_EEPROM enables this feature. Depending
+on the board, the EEPROM may be connected on various methods, but currently,
+only the I2C bus is available via CONFIG_NET_ETHADDR_EEPROM_I2C.
+
+The following config options are available,
+CONFIG_NET_ETHADDR_EEPROM_I2C_BUS is the I2C bus on which the eeprom is present.
+CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR sets the address of the EEPROM, which
+defaults to the very common 0x50. Small size EEPROM's generally use single byte
+addressing but larger EEPROM's may use double byte addressing, which can be
+configured using CONFIG_NET_ETHADDR_EEPROM_ADDRLEN.
+
+Within the EEPROM, the MAC address can be stored on any arbitrary offset,
+CONFIG_NET_ETHADDR_EEPROM_OFFSET sets this to 8 as a default however, allowing
+the first 8 bytes to be used for an optional data, for example a configuration
+struct where the mac address is part of.
+
+Appending the 6 (ARP_HLEN) bytes is a CRC8 byte over the previous ARP_HLEN
+bytes. Whether to check this CRC8 or not is dependent on
+CONFIG_NET_ETHADDR_EEPROM_CRC8.
+
+To keep things nicely aligned, a final 'reserved' byte is added to the mac
+address + crc8 combo.
+
+A board may want to store more information in its eeprom, using the following
+example layout, this can be achieved.
+
+struct mac_addr {
+ uint8_t mac[ARP_HLEN];
+ uint8_t crc8;
+ uint8_t reserved;
+};
+
+struct config_eeprom {
+ uint32_t magic;
+ uint8_t version;
+ uint8_t reserved[2];
+ uint8_t mac_cnt;
+ struct mac_addr[mac_cnt];
+};
+
+Filling this in:
+struct config_eeprom eeprom = {
+ .magic = { 'M', 'g', 'i', 'c' },
+ .reserved = { 0x00, 0x00 },
+ .mac_cnt = 2,
+ .mac_addr = {
+ {
+ .mac = {
+ 0x01, 0x23, 0x45,
+ 0x67, 0x89, 0xab,
+ },
+ .crc8 = 0xbe,
+ .reserved = 0x00,
+ }, {
+ .mac = {
+ 0xba, 0x98, 0x76,
+ 0x54, 0x32, 0x10,
+ },
+ .crc8 = 0x82,
+ .reserved = 0x00,
+ },
+ },
+};
+
+The eeprom content would look like this.
+
+00000000 4d 67 69 63 01 00 00 02 01 23 45 67 89 ab be 00 |Mgic.....#Eg....|
+00000010 ba 98 76 54 32 10 82 00 |..vT2...|
+
+Alternativly the i2c-tools can be used as well.
+
+i2cset I2CBUS 0x50 0x08 0x01
+i2cset I2CBUS 0x50 0x09 0x23
+i2cset I2CBUS 0x50 0x0a 0x45
+i2cset I2CBUS 0x50 0x0b 0x67
+i2cset I2CBUS 0x50 0x0c 0x89
+i2cset I2CBUS 0x50 0x0d 0xab
+i2cset I2CBUS 0x50 0x0e 0xbe
+
-------
Usage
-------
diff --git a/net/Kconfig b/net/Kconfig
index 414c549..f7ef2b7 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -7,6 +7,64 @@ menuconfig NET
if NET
+config NET_ETHADDR_EEPROM
+ bool "Get ethaddr from eeprom"
+ help
+ Selecting this will try to get the Ethernet address from an onboard
+ EEPROM and set into the environment if and only if the environment
+ does currently not already hold a MAC address. For more information
+ see doc/README.enetaddr.
+
+config NET_ETHADDR_EEPROM_I2C
+ depends on NET_ETHADDR_EEPROM
+ bool "EEPROM on I2C bus"
+ help
+ This switch enables checks for an EEPROM on the I2C bus. Naturally
+ this will only work if there is an actual EEPROM connected on the
+ I2C bus and the bus and device are properly configured via the
+ options below.
+
+config NET_ETHADDR_EEPROM_I2C_BUS
+ depends on NET_ETHADDR_EEPROM_I2C
+ int "I2C bus"
+ default 0
+ help
+ Select the bus on which the EEPROM is present, defaults to bus 0.
+
+config NET_ETHADDR_EEPROM_I2C_ADDR
+ depends on NET_ETHADDR_EEPROM_I2C
+ hex "eeprom address"
+ default 0x50
+ help
+ Select the address of the eeprom, defaults to address 0x50.
+
+config NET_ETHADDR_EEPROM_I2C_ADDRLEN
+ depends on NET_ETHADDR_EEPROM_I2C
+ int "eeprom address length"
+ default 1
+ help
+ Number of bytes to be used for the I2C address length. Typically 1,
+ 2 for large memories, 0 for register type devices with only one
+ register.
+
+config NET_ETHADDR_EEPROM_OFFSET
+ depends on NET_ETHADDR_EEPROM
+ int "EEPROM offset"
+ default 8
+ help
+ Select the byte offset of the MAC address within the page,
+ defaults to byte 8.
+
+config NET_ETHADDR_EEPROM_CRC8
+ depends on NET_ETHADDR_EEPROM
+ bool "Check CRC8 of MAC"
+ default y
+ help
+ Optionally, it is possible to run a CRC-8-CCITT check on the MAC
+ address. To do so, the MAC address is stored with a CRC8 byte append.
+ This option enables the CRC check of the MAC address against the CRC
+ byte.
+
config NET_RANDOM_ETHADDR
bool "Random ethaddr if unset"
select LIB_RAND
--
2.10.2
Olliver Schinagl
Nov 8, 2016, 10:54:59 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
Add the read_rom_hwaddr net_op hook so that it can be called from boards
to read the mac from a ROM chip.
drivers/net/designware.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 9e6d726..aa87f30 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -230,6 +230,23 @@ 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)
+{
+ return -ENOSYS;
+}
+
+static int designware_eth_read_rom_hwaddr(struct udevice *dev)
+{
+ int retval;
+ if (retval == -ENOSYS)
+ return 0;
+
+ return retval;
+}
+
static void dw_adjust_link(struct eth_mac_regs *mac_p,
struct phy_device *phydev)
{
@@ -685,6 +702,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
to read the mac from a ROM chip.
drivers/net/designware.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 9e6d726..aa87f30 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -230,6 +230,23 @@ 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)
+{
+ return -ENOSYS;
+}
+
+static int designware_eth_read_rom_hwaddr(struct udevice *dev)
+{
+ int retval;
+ struct eth_pdata *pdata = dev_get_platdata(dev);
+
+ retval = dw_board_read_rom_hwaddr(pdata->enetaddr);
+
+ if (retval == -ENOSYS)
+ return 0;
+
+ return retval;
+}
+
static void dw_adjust_link(struct eth_mac_regs *mac_p,
struct phy_device *phydev)
{
@@ -685,6 +702,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
Olliver Schinagl
Nov 8, 2016, 10:54:59 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
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 <oli...@schinagl.nl>
---
drivers/net/sunxi_emac.c | 18 ++++++++++++++++++
to read the mac from a ROM chip.
Signed-off-by: Olliver Schinagl <oli...@schinagl.nl>
---
1 file changed, 18 insertions(+)
diff --git a/drivers/net/sunxi_emac.c b/drivers/net/sunxi_emac.c
index dcf832e..6a6611e 100644
--- a/drivers/net/sunxi_emac.c
+++ b/drivers/net/sunxi_emac.c
@@ -562,6 +562,23 @@ static int sunxi_emac_eth_write_hwaddr(struct udevice *dev)
return _sunxi_write_hwaddr(priv, pdata->enetaddr);
}
+__weak int sunxi_board_read_rom_hwaddr(unsigned char *enetaddr)
+{
+ return -ENOSYS;
+}
+
+static int sunxi_emac_eth_read_rom_hwaddr(struct udevice *dev)
+ return -ENOSYS;
+}
+
+{
+ int retval;
+ struct eth_pdata *pdata = dev_get_platdata(dev);
+
+ retval = sunxi_board_read_rom_hwaddr(pdata->enetaddr);
+ int retval;
+ struct eth_pdata *pdata = dev_get_platdata(dev);
+
+ if (retval == -ENOSYS)
+ return 0;
+
+ return retval;
+}
+
static int sunxi_emac_eth_probe(struct udevice *dev)
+ return 0;
+
+ return retval;
+}
+
{
struct eth_pdata *pdata = dev_get_platdata(dev);
@@ -579,6 +596,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,
.stop = sunxi_emac_eth_stop,
+ .read_rom_hwaddr = sunxi_emac_eth_read_rom_hwaddr,
Olliver Schinagl
Nov 8, 2016, 10:55:00 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de, Olliver Schinagl
This patch enables crc8 to be used from within the tools directory using
u-boot/crc.h.
Signed-off-by: Olliver Schinagl <o.sch...@ultimaker.com>
Reviewed-by: Joe Hershberger <joe.her...@ni.com>
---
include/u-boot/crc.h | 3 +++
tools/Makefile | 1 +
2 files changed, 4 insertions(+)
diff --git a/include/u-boot/crc.h b/include/u-boot/crc.h
index 754ac72..6764d58 100644
--- a/include/u-boot/crc.h
+++ b/include/u-boot/crc.h
@@ -9,6 +9,9 @@
#ifndef _UBOOT_CRC_H
#define _UBOOT_CRC_H
+/* lib/crc8.c */
+unsigned int crc8(unsigned int crc_start, const unsigned char *vptr, int len);
+
/* lib/crc32.c */
uint32_t crc32 (uint32_t, const unsigned char *, uint);
uint32_t crc32_wd (uint32_t, const unsigned char *, uint, uint);
diff --git a/tools/Makefile b/tools/Makefile
index 400588c..06afdb0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -191,6 +191,7 @@ fdtgrep-objs += $(LIBFDT_OBJS) fdtgrep.o
# that won't build on some weird host compiler -- though there are lots of
# exceptions for files that aren't complaint.
HOSTCFLAGS_crc32.o := -pedantic
+HOSTCFLAGS_crc8.o := -pedantic
HOSTCFLAGS_md5.o := -pedantic
HOSTCFLAGS_sha1.o := -pedantic
HOSTCFLAGS_sha256.o := -pedantic
--
2.10.2
u-boot/crc.h.
Signed-off-by: Olliver Schinagl <o.sch...@ultimaker.com>
Reviewed-by: Joe Hershberger <joe.her...@ni.com>
---
include/u-boot/crc.h | 3 +++
tools/Makefile | 1 +
2 files changed, 4 insertions(+)
diff --git a/include/u-boot/crc.h b/include/u-boot/crc.h
index 754ac72..6764d58 100644
--- a/include/u-boot/crc.h
+++ b/include/u-boot/crc.h
@@ -9,6 +9,9 @@
#ifndef _UBOOT_CRC_H
#define _UBOOT_CRC_H
+/* lib/crc8.c */
+unsigned int crc8(unsigned int crc_start, const unsigned char *vptr, int len);
+
/* lib/crc32.c */
uint32_t crc32 (uint32_t, const unsigned char *, uint);
uint32_t crc32_wd (uint32_t, const unsigned char *, uint, uint);
diff --git a/tools/Makefile b/tools/Makefile
index 400588c..06afdb0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -191,6 +191,7 @@ fdtgrep-objs += $(LIBFDT_OBJS) fdtgrep.o
# that won't build on some weird host compiler -- though there are lots of
# exceptions for files that aren't complaint.
HOSTCFLAGS_crc32.o := -pedantic
+HOSTCFLAGS_crc8.o := -pedantic
HOSTCFLAGS_md5.o := -pedantic
HOSTCFLAGS_sha1.o := -pedantic
HOSTCFLAGS_sha256.o := -pedantic
--
2.10.2
Olliver Schinagl
Nov 8, 2016, 10:55:00 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
With the newly introduced hooks, we can now set the MAC address at the
lowest level properly. The user is still free to override it via a
u-boot environment variable.
arch/arm/include/asm/arch-sunxi/sys_proto.h | 8 +++
board/sunxi/board.c | 103 +++++++++++++++++++---------
2 files changed, 80 insertions(+), 31 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..5b24b86 100644
--- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
+++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
@@ -30,4 +30,12 @@ void eth_init_board(void);
static inline void eth_init_board(void) {}
#endif
+#if CONFIG_SUNXI_EMAC
+int sunxi_board_read_rom_hwaddr(unsigned char *enetaddr);
+#endif
+
+#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
+int dw_board_read_rom_hwaddr(unsigned char *enetaddr);
+#endif
+
#endif
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 5365638..9b56a89 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>
@@ -584,6 +585,76 @@ void get_board_serial(struct tag_serialnr *serialnr)
}
#endif
+static void _sunxi_gen_sid_hwaddr(unsigned char *enetaddr, uint8_t cnt)
+{
+ uint8_t mac_addr[ARP_HLEN] = { 0x00 };
+ unsigned int sid[4];
+ int 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;
+
+ /* Non OUI / registered MAC address */
+ mac_addr[0] = (cnt << 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;
+ }
+
+ memcpy(enetaddr, mac_addr, ARP_HLEN);
+}
+
+static int sunxi_read_rom_hwaddr(unsigned char *enetaddr)
+{
+ static unsigned int cnt;
+
+ _sunxi_gen_sid_hwaddr(enetaddr, cnt);
+
+
+ if (is_zero_ethaddr(enetaddr))
+ return -ENOSYS;
+
+ printf("MAC address: %02X:%02X:%02X:%02X:%02X:%02X\n",
+ enetaddr[0], enetaddr[1], enetaddr[2],
+ enetaddr[3], enetaddr[4], enetaddr[5]);
+
+{
+ return sunxi_read_rom_hwaddr(enetaddr);
+}
+
+int dw_board_read_rom_hwaddr(unsigned char *enetaddr)
+{
+ return sunxi_read_rom_hwaddr(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
@@ -625,9 +696,7 @@ static void setup_environment(const void *fdt)
- char ethaddr[16];
- int i, ret;
+ int ret;
ret = sunxi_get_sid(sid);
if (ret == 0 && sid[0] != 0) {
@@ -648,34 +717,6 @@ static void setup_environment(const void *fdt)
- 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);
- }
2.10.2
lowest level properly. The user is still free to override it via a
u-boot environment variable.
arch/arm/include/asm/arch-sunxi/sys_proto.h | 8 +++
board/sunxi/board.c | 103 +++++++++++++++++++---------
2 files changed, 80 insertions(+), 31 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..5b24b86 100644
--- a/arch/arm/include/asm/arch-sunxi/sys_proto.h
+++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
@@ -30,4 +30,12 @@ void eth_init_board(void);
static inline void eth_init_board(void) {}
#endif
+#if CONFIG_SUNXI_EMAC
+int sunxi_board_read_rom_hwaddr(unsigned char *enetaddr);
+#endif
+
+#if defined(CONFIG_SUNXI_GMAC) || defined(CONFIG_ETH_DESIGNWARE)
+int dw_board_read_rom_hwaddr(unsigned char *enetaddr);
+#endif
+
#endif
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 5365638..9b56a89 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>
@@ -584,6 +585,76 @@ void get_board_serial(struct tag_serialnr *serialnr)
}
#endif
+static void _sunxi_gen_sid_hwaddr(unsigned char *enetaddr, uint8_t cnt)
+{
+ uint8_t mac_addr[ARP_HLEN] = { 0x00 };
+ unsigned int sid[4];
+ int 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;
+
+ /* Non OUI / registered MAC address */
+ mac_addr[0] = (cnt << 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;
+ }
+
+ memcpy(enetaddr, mac_addr, ARP_HLEN);
+}
+
+static int sunxi_read_rom_hwaddr(unsigned char *enetaddr)
+{
+ static unsigned int cnt;
+
+ _sunxi_gen_sid_hwaddr(enetaddr, cnt);
+
+ /* First call, first stored MAC address, increase for next call */
+ cnt++;
+
+ if (is_zero_ethaddr(enetaddr))
+ return -ENOSYS;
+
+ printf("MAC address: %02X:%02X:%02X:%02X:%02X:%02X\n",
+ enetaddr[0], enetaddr[1], enetaddr[2],
+ enetaddr[3], enetaddr[4], enetaddr[5]);
+
+ return 0;
+}
+
+int sunxi_board_read_rom_hwaddr(unsigned char *enetaddr)
+}
+
+{
+ return sunxi_read_rom_hwaddr(enetaddr);
+}
+
+int dw_board_read_rom_hwaddr(unsigned char *enetaddr)
+{
+ return sunxi_read_rom_hwaddr(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
@@ -625,9 +696,7 @@ static void setup_environment(const void *fdt)
{
char serial_string[17] = { 0 };
unsigned int sid[4];
- uint8_t mac_addr[6];
char serial_string[17] = { 0 };
unsigned int sid[4];
- char ethaddr[16];
- int i, ret;
+ int ret;
ret = sunxi_get_sid(sid);
if (ret == 0 && sid[0] != 0) {
sid[3] = crc32(0, (unsigned char *)&sid[1], 12);
#endif
- /* Ensure the NIC specific bytes of the mac are not all 0 */
#endif
- 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),
if (!getenv("serial#")) {
"%08x%08x", sid[0], sid[3]);
--
2.10.2
Olliver Schinagl
Nov 8, 2016, 10:55:00 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
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.
configs/A10-OLinuXino-Lime_defconfig | 3 +++
configs/A20-OLinuXino-Lime2_defconfig | 3 +++
configs/A20-OLinuXino-Lime_defconfig | 3 +++
configs/A20-OLinuXino_MICRO_defconfig | 3 +++
4 files changed, 12 insertions(+)
diff --git a/configs/A10-OLinuXino-Lime_defconfig b/configs/A10-OLinuXino-Lime_defconfig
index 04b720d..dce33c8 100644
--- a/configs/A10-OLinuXino-Lime_defconfig
+++ b/configs/A10-OLinuXino-Lime_defconfig
@@ -13,6 +13,9 @@ 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_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..f2424c8 100644
--- a/configs/A20-OLinuXino-Lime2_defconfig
+++ b/configs/A20-OLinuXino-Lime2_defconfig
@@ -18,6 +18,9 @@ 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_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..0e50d46 100644
--- a/configs/A20-OLinuXino-Lime_defconfig
+++ b/configs/A20-OLinuXino-Lime_defconfig
@@ -12,6 +12,9 @@ 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_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..b70dd2f 100644
--- a/configs/A20-OLinuXino_MICRO_defconfig
+++ b/configs/A20-OLinuXino_MICRO_defconfig
@@ -15,6 +15,9 @@ 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_AXP_ALDO3_VOLT=2800
CONFIG_AXP_ALDO4_VOLT=2800
CONFIG_USB_EHCI_HCD=y
--
2.10.2
OLinuXino Lime1 and Lime2 boards. Other boards surely qualify as well
but where not tested yet.
configs/A10-OLinuXino-Lime_defconfig | 3 +++
configs/A20-OLinuXino-Lime2_defconfig | 3 +++
configs/A20-OLinuXino-Lime_defconfig | 3 +++
configs/A20-OLinuXino_MICRO_defconfig | 3 +++
4 files changed, 12 insertions(+)
diff --git a/configs/A10-OLinuXino-Lime_defconfig b/configs/A10-OLinuXino-Lime_defconfig
index 04b720d..dce33c8 100644
--- a/configs/A10-OLinuXino-Lime_defconfig
+++ b/configs/A10-OLinuXino-Lime_defconfig
@@ -13,6 +13,9 @@ 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_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..f2424c8 100644
--- a/configs/A20-OLinuXino-Lime2_defconfig
+++ b/configs/A20-OLinuXino-Lime2_defconfig
@@ -18,6 +18,9 @@ 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_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..0e50d46 100644
--- a/configs/A20-OLinuXino-Lime_defconfig
+++ b/configs/A20-OLinuXino-Lime_defconfig
@@ -12,6 +12,9 @@ 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_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..b70dd2f 100644
--- a/configs/A20-OLinuXino_MICRO_defconfig
+++ b/configs/A20-OLinuXino_MICRO_defconfig
@@ -15,6 +15,9 @@ 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_AXP_ALDO3_VOLT=2800
CONFIG_AXP_ALDO4_VOLT=2800
CONFIG_USB_EHCI_HCD=y
--
2.10.2
Olliver Schinagl
Nov 8, 2016, 10:55:00 AM11/8/16
to Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Olliver Schinagl, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de, Olliver Schinagl
This patch adds a little tool that takes a generic MAC address and
generates a CRC byte for it. The output is the full MAC address without
any separators, ready written into an EEPROM.
tools/.gitignore | 1 +
tools/Makefile | 4 ++++
tools/gen_ethaddr_crc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+)
create mode 100644 tools/gen_ethaddr_crc.c
diff --git a/tools/.gitignore b/tools/.gitignore
index cb1e722..0d1f076 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -6,6 +6,7 @@
/fit_check_sign
/fit_info
/gen_eth_addr
+/gen_ethaddr_crc
/ifdtool
/img2srec
/kwboot
diff --git a/tools/Makefile b/tools/Makefile
index 06afdb0..4879ded 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -43,6 +43,10 @@ envcrc-objs := envcrc.o lib/crc32.o common/env_embedded.o lib/sha1.o
hostprogs-$(CONFIG_CMD_NET) += gen_eth_addr
HOSTCFLAGS_gen_eth_addr.o := -pedantic
+hostprogs-$(CONFIG_CMD_NET) += gen_ethaddr_crc
+gen_ethaddr_crc-objs := gen_ethaddr_crc.o lib/crc8.o
+HOSTCFLAGS_gen_ethaddr_crc.o := -pedantic
+
hostprogs-$(CONFIG_CMD_LOADS) += img2srec
HOSTCFLAGS_img2srec.o := -pedantic
diff --git a/tools/gen_ethaddr_crc.c b/tools/gen_ethaddr_crc.c
new file mode 100644
index 0000000..9b5bdb0
--- /dev/null
+++ b/tools/gen_ethaddr_crc.c
@@ -0,0 +1,52 @@
+/*
+ * (C) Copyright 2016
+ * Olliver Schinagl <oli...@schinagl.nl>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <ctype.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <u-boot/crc.h>
+
+#define ARP_HLEN 6 /* Length of hardware address */
+
+int main(int argc, char *argv[])
+{
+ uint_fast8_t i;
+ uint8_t mac_addr[ARP_HLEN + 1] = { 0x00 };
+
+ if (argc < 2) {
+ puts("Please supply a MAC address.");
+ return -1;
+ }
+
+ if (!((strlen(argv[1]) == 12) || (strlen(argv[1]) == 17))) {
+ puts("Please supply a valid MAC address with optionaly\n"
+ "dashes (-) or colons (:) as seperators.");
+ return -1;
+ }
+
+ i = 0;
+ while (*argv[1] != '\0') {
+ char nibble[2] = { 0x00, '\n' }; /* for strtol */
+
+ nibble[0] = *argv[1]++;
+ if (isxdigit(nibble[0])) {
+ if (isupper(nibble[0]))
+ nibble[0] = tolower(nibble[0]);
+ mac_addr[i >> 1] |= strtol(nibble, NULL, 16) << ((i % 2) ? 0 : 4) & ((i % 2) ? 0x0f : 0xf0);
+ i++;
+ }
+ }
+ mac_addr[ARP_HLEN] = crc8(0, mac_addr, ARP_HLEN);
+
+ for (i = 0; i < ARP_HLEN + 1; i++)
+ printf("%.2x", mac_addr[i]);
+ putchar('\n');
2.10.2
generates a CRC byte for it. The output is the full MAC address without
any separators, ready written into an EEPROM.
tools/.gitignore | 1 +
tools/Makefile | 4 ++++
tools/gen_ethaddr_crc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+)
create mode 100644 tools/gen_ethaddr_crc.c
diff --git a/tools/.gitignore b/tools/.gitignore
index cb1e722..0d1f076 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -6,6 +6,7 @@
/fit_check_sign
/fit_info
/gen_eth_addr
+/gen_ethaddr_crc
/ifdtool
/img2srec
/kwboot
diff --git a/tools/Makefile b/tools/Makefile
index 06afdb0..4879ded 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -43,6 +43,10 @@ envcrc-objs := envcrc.o lib/crc32.o common/env_embedded.o lib/sha1.o
hostprogs-$(CONFIG_CMD_NET) += gen_eth_addr
HOSTCFLAGS_gen_eth_addr.o := -pedantic
+hostprogs-$(CONFIG_CMD_NET) += gen_ethaddr_crc
+gen_ethaddr_crc-objs := gen_ethaddr_crc.o lib/crc8.o
+HOSTCFLAGS_gen_ethaddr_crc.o := -pedantic
+
hostprogs-$(CONFIG_CMD_LOADS) += img2srec
HOSTCFLAGS_img2srec.o := -pedantic
diff --git a/tools/gen_ethaddr_crc.c b/tools/gen_ethaddr_crc.c
new file mode 100644
index 0000000..9b5bdb0
--- /dev/null
+++ b/tools/gen_ethaddr_crc.c
@@ -0,0 +1,52 @@
+/*
+ * (C) Copyright 2016
+ * Olliver Schinagl <oli...@schinagl.nl>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <ctype.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <u-boot/crc.h>
+
+#define ARP_HLEN 6 /* Length of hardware address */
+
+int main(int argc, char *argv[])
+{
+ uint_fast8_t i;
+ uint8_t mac_addr[ARP_HLEN + 1] = { 0x00 };
+
+ if (argc < 2) {
+ puts("Please supply a MAC address.");
+ return -1;
+ }
+
+ if (!((strlen(argv[1]) == 12) || (strlen(argv[1]) == 17))) {
+ puts("Please supply a valid MAC address with optionaly\n"
+ "dashes (-) or colons (:) as seperators.");
+ return -1;
+ }
+
+ i = 0;
+ while (*argv[1] != '\0') {
+ char nibble[2] = { 0x00, '\n' }; /* for strtol */
+
+ nibble[0] = *argv[1]++;
+ if (isxdigit(nibble[0])) {
+ if (isupper(nibble[0]))
+ nibble[0] = tolower(nibble[0]);
+ mac_addr[i >> 1] |= strtol(nibble, NULL, 16) << ((i % 2) ? 0 : 4) & ((i % 2) ? 0x0f : 0xf0);
+ i++;
+ }
+ }
+ mac_addr[ARP_HLEN] = crc8(0, mac_addr, ARP_HLEN);
+
+ for (i = 0; i < ARP_HLEN + 1; i++)
+ printf("%.2x", mac_addr[i]);
+ putchar('\n');
+
+ return 0;
+}
--
+ return 0;
+}
2.10.2
Olliver Schinagl
Nov 10, 2016, 7:08:40 AM11/10/16
to Michal Simek, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
Hi Michal,
On 10-11-16 12:37, Michal Simek wrote:
> Yes, Zynq and ZynqMP is using this feature. I am also aware about using
> qspi OTP region for storing information like this.
I saw, which triggered me here. What the Znyq currently does it uses its
own private CONFIG setting:
+int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
+{
+#if defined(CONFIG_ZYNQ_GEM_EEPROM_ADDR) && \
+ defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET) && \
+ defined(CONFIG_ZYNQ_EEPROM_BUS)
+ i2c_set_bus_num(CONFIG_ZYNQ_EEPROM_BUS);
+
+ if (eeprom_read(CONFIG_ZYNQ_GEM_EEPROM_ADDR,
+ CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET,
+ ethaddr, 6))
+ printf("I2C EEPROM MAC address read failed\n");
+#endif
as they can be used by anybody really.
Once Maxime's patches have merged and stabilized, i'd even say to switch
over to the eeprom framework.
>
> Thanks,
> Michal
>
>
On 10-11-16 12:37, Michal Simek wrote:
> qspi OTP region for storing information like this.
I saw, which triggered me here. What the Znyq currently does it uses its
own private CONFIG setting:
+int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
+{
+#if defined(CONFIG_ZYNQ_GEM_EEPROM_ADDR) && \
+ defined(CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET) && \
+ defined(CONFIG_ZYNQ_EEPROM_BUS)
+ i2c_set_bus_num(CONFIG_ZYNQ_EEPROM_BUS);
+
+ if (eeprom_read(CONFIG_ZYNQ_GEM_EEPROM_ADDR,
+ CONFIG_ZYNQ_GEM_I2C_MAC_OFFSET,
+ ethaddr, 6))
+ printf("I2C EEPROM MAC address read failed\n");
+#endif
+
+ return 0;
+}
which are ZNYQ specific. In my patchset I give them very generic names
+ return 0;
+}
as they can be used by anybody really.
Once Maxime's patches have merged and stabilized, i'd even say to switch
over to the eeprom framework.
>
> Thanks,
> Michal
>
>
Olliver Schinagl
Nov 10, 2016, 7:11:08 AM11/10/16
to Michal Simek, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
Hi Michal,
On 10-11-16 12:51, Michal Simek wrote:
> On 8.11.2016 16:54, Olliver Schinagl wrote:
> ok. I have briefly looked at the whole series and I think that this
> should be done in the core because this should be shared across all
> drivers.
> Because what you have above make in general sense for every board which
> contain mac address in eeprom.
> That's why I would create
>
> eeprom_read_rom_etheaddr() in core which will do stuff as you have above
> and in driver we will simply assign it to read_rom_hwaddr in drivers or
> by default for all with options to rewrite it.
> This function will be empty when !NET_ETHADDR_EEPROM.
>
> By this or similar way you open this to all ethernet drivers to read mac
> just through enabling Kconfig.
>
> IMHO doesn't make sense to c&p the same logic over all ethernet drivers.
Initially, I do agree very much. But when I first wrote this last year,
there was no other driver yet etc. It is very very generic so maybe make
this a weak function up one level, and let the driver override it even?
Makes using the eeprom framework later easier too. I'll cook something
up. Good idea!
Olliver
>
> Thanks,
> Michal
>
>
>
On 10-11-16 12:51, Michal Simek wrote:
> On 8.11.2016 16:54, Olliver Schinagl wrote:
> should be done in the core because this should be shared across all
> drivers.
> Because what you have above make in general sense for every board which
> contain mac address in eeprom.
> That's why I would create
>
> eeprom_read_rom_etheaddr() in core which will do stuff as you have above
> and in driver we will simply assign it to read_rom_hwaddr in drivers or
> by default for all with options to rewrite it.
> This function will be empty when !NET_ETHADDR_EEPROM.
>
> By this or similar way you open this to all ethernet drivers to read mac
> just through enabling Kconfig.
>
> IMHO doesn't make sense to c&p the same logic over all ethernet drivers.
Initially, I do agree very much. But when I first wrote this last year,
there was no other driver yet etc. It is very very generic so maybe make
this a weak function up one level, and let the driver override it even?
Makes using the eeprom framework later easier too. I'll cook something
up. Good idea!
Olliver
>
> Thanks,
> Michal
>
>
>
Olliver Schinagl
Nov 10, 2016, 7:31:49 AM11/10/16
to Michal Simek, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
On 10-11-16 13:26, Michal Simek wrote:
> Can you give me that link to these patches?
Well [0] is your own patch, so that is easy :) [1] is Maxime's work. But
with your generic comment, this entire function probably can simply go
then. The only thing I haven't figured out/thought through yet, if
eeprom reading fails, we want to fall back to the old board specific
method. But I think I know what might do there ...
Olliver
[0] http://git.denx.de/?p=u-boot.git;a=commit;h=6919b4bf363574
[1] https://www.mail-archive.com/u-b...@lists.denx.de/msg230179.html
>
> Thanks,
> Michal
>
Well [0] is your own patch, so that is easy :) [1] is Maxime's work. But
with your generic comment, this entire function probably can simply go
then. The only thing I haven't figured out/thought through yet, if
eeprom reading fails, we want to fall back to the old board specific
method. But I think I know what might do there ...
Olliver
[0] http://git.denx.de/?p=u-boot.git;a=commit;h=6919b4bf363574
[1] https://www.mail-archive.com/u-b...@lists.denx.de/msg230179.html
>
> Thanks,
> Michal
>
Olliver Schinagl
Nov 10, 2016, 7:43:39 AM11/10/16
to Michal Simek, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-b...@lists.denx.de
On 10-11-16 13:37, Michal Simek wrote:
> Joe recently applied one our patch
> http://lists.denx.de/pipermail/u-boot/2016-November/271662.html
> which in case of NET_RAMDOM_ETHADDR generates random address.
> I don't have in my mind exact calling sequence but I expect when eeprom
> read failed then random eth is generated if selected or warning, etc.
In the case of sunxi, we generate a MAC address based off the CPU serial
number and device ID, which is more predictable and doesn't change
compared to the random MAC. The random MAC would then in turn be a
fallback if we still have a 00:00:00:00:00 ethernet address.
So 3 steps, check EEPROM (i2c only for now, SPI etc can be added later
with the eeprom uclass)
call board specific hook
if empty, generate random (if configured)
>
> Thanks,
> Michal
>
>
> http://lists.denx.de/pipermail/u-boot/2016-November/271662.html
> which in case of NET_RAMDOM_ETHADDR generates random address.
> I don't have in my mind exact calling sequence but I expect when eeprom
> read failed then random eth is generated if selected or warning, etc.
In the case of sunxi, we generate a MAC address based off the CPU serial
number and device ID, which is more predictable and doesn't change
compared to the random MAC. The random MAC would then in turn be a
fallback if we still have a 00:00:00:00:00 ethernet address.
So 3 steps, check EEPROM (i2c only for now, SPI etc can be added later
with the eeprom uclass)
call board specific hook
if empty, generate random (if configured)
>
> Thanks,
> Michal
>
>
Simon Glass
Nov 11, 2016, 11:17:58 AM11/11/16
to Olliver Schinagl, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, U-Boot Mailing List, Olliver Schinagl
On 8 November 2016 at 08:54, Olliver Schinagl <oli...@schinagl.nl> wrote:
> This patch enables crc8 to be used from within the tools directory using
> u-boot/crc.h.
>
> Signed-off-by: Olliver Schinagl <o.sch...@ultimaker.com>
> Reviewed-by: Joe Hershberger <joe.her...@ni.com>
> ---
> include/u-boot/crc.h | 3 +++
> tools/Makefile | 1 +
> 2 files changed, 4 insertions(+)
Reviewed-by: Simon Glass <s...@chromium.org>
> This patch enables crc8 to be used from within the tools directory using
> u-boot/crc.h.
>
> Signed-off-by: Olliver Schinagl <o.sch...@ultimaker.com>
> Reviewed-by: Joe Hershberger <joe.her...@ni.com>
> ---
> include/u-boot/crc.h | 3 +++
> tools/Makefile | 1 +
> 2 files changed, 4 insertions(+)
Simon Glass
Nov 11, 2016, 11:18:36 AM11/11/16
to Olliver Schinagl, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, U-Boot Mailing List, Olliver Schinagl
Hi Olliver, (is it one l or two?)
Is there no #define for this in standard headers?
> +
> +int main(int argc, char *argv[])
> +{
> + uint_fast8_t i;
> + uint8_t mac_addr[ARP_HLEN + 1] = { 0x00 };
Why do you need to init it?
> +
> + if (argc < 2) {
> + puts("Please supply a MAC address.");
How about a usage() function which gives generic help as well?
> + return -1;
> + }
> +
> + if (!((strlen(argv[1]) == 12) || (strlen(argv[1]) == 17))) {
> + puts("Please supply a valid MAC address with optionaly\n"
optionally. But do you mean 'Please supply a valid MAC address with
optional - or : separators?'
> + "dashes (-) or colons (:) as seperators.");
separators.
> + return -1;
> + }
> +
> + i = 0;
> + while (*argv[1] != '\0') {
How about putting this code in a separate function:
int process_mac(const char *max)
so you don't have to access argv[1] all the time. Also you can return
1 instead of -1 from main() on error.
> + char nibble[2] = { 0x00, '\n' }; /* for strtol */
> +
> + nibble[0] = *argv[1]++;
> + if (isxdigit(nibble[0])) {
> + if (isupper(nibble[0]))
> + nibble[0] = tolower(nibble[0]);
> + mac_addr[i >> 1] |= strtol(nibble, NULL, 16) << ((i % 2) ? 0 : 4) & ((i % 2) ? 0x0f : 0xf0);
How about a nibble_to_hex() function here? This is strange-looking
code. I'm wondering what you are trying to do.
> + i++;
> + }
> + }
> + mac_addr[ARP_HLEN] = crc8(0, mac_addr, ARP_HLEN);
I suggest putting this in a separate variable...
> +
> + for (i = 0; i < ARP_HLEN + 1; i++)
> + printf("%.2x", mac_addr[i]);
> + putchar('\n');
and here: printf("%.2x\n", crc)
> +
> + return 0;
> +}
> --
> 2.10.2
>
Regards,
Simon
> +
> +int main(int argc, char *argv[])
> +{
> + uint_fast8_t i;
> + uint8_t mac_addr[ARP_HLEN + 1] = { 0x00 };
> +
> + if (argc < 2) {
> + puts("Please supply a MAC address.");
> + return -1;
> + }
> +
> + if (!((strlen(argv[1]) == 12) || (strlen(argv[1]) == 17))) {
> + puts("Please supply a valid MAC address with optionaly\n"
optional - or : separators?'
> + "dashes (-) or colons (:) as seperators.");
> + return -1;
> + }
> +
> + i = 0;
> + while (*argv[1] != '\0') {
int process_mac(const char *max)
so you don't have to access argv[1] all the time. Also you can return
1 instead of -1 from main() on error.
> + char nibble[2] = { 0x00, '\n' }; /* for strtol */
> +
> + nibble[0] = *argv[1]++;
> + if (isxdigit(nibble[0])) {
> + if (isupper(nibble[0]))
> + nibble[0] = tolower(nibble[0]);
> + mac_addr[i >> 1] |= strtol(nibble, NULL, 16) << ((i % 2) ? 0 : 4) & ((i % 2) ? 0x0f : 0xf0);
code. I'm wondering what you are trying to do.
> + i++;
> + }
> + }
> + mac_addr[ARP_HLEN] = crc8(0, mac_addr, ARP_HLEN);
> +
> + for (i = 0; i < ARP_HLEN + 1; i++)
> + printf("%.2x", mac_addr[i]);
> + putchar('\n');
> +
> + return 0;
> +}
> --
> 2.10.2
>
Simon
Olliver Schinagl
Nov 15, 2016, 3:05:52 AM11/15/16
to Michal Simek, Joe Hershberger, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-boot
Hey Joe, Michal,
On 15-11-16 08:22, Michal Simek wrote:
> On 15.11.2016 04:27, Joe Hershberger wrote:
>> Do you think it is valuable enough to apply as is and retrofit later,
>> or just wait on this series?
> TBH I would split this series to two to get Sunxi part in and this get
> later.
I have both series ready and they just need to be tested. I'll test it
hopefully later today, and send the 2 seperate patch series very soon
(within a day or so, not a year :p)
Olliver
>
> Thanks,
> Michal
>
>
On 15-11-16 08:22, Michal Simek wrote:
> On 15.11.2016 04:27, Joe Hershberger wrote:
>> or just wait on this series?
> TBH I would split this series to two to get Sunxi part in and this get
> later.
I have both series ready and they just need to be tested. I'll test it
hopefully later today, and send the 2 seperate patch series very soon
(within a day or so, not a year :p)
Olliver
>
> Thanks,
> Michal
>
>
Olliver Schinagl
Nov 15, 2016, 3:10:25 AM11/15/16
to Joe Hershberger, Simon Glass, Albert Aribaud, Olliver Schinagl, U-Boot Mailing List, d...@linux-sunxi.org, Jeffy Chen, Maxime Ripard, Michal Simek, Joe Hershberger, Ian Campbell, Stefan Roese
Hey Simon, Joe,
On 15-11-16 04:31, Joe Hershberger wrote:
> There is. ARP_HLEN is defined in include/net.h
Yep, but including net.h then makes net.h very unhappy (lots of missing
things, u32 to begin with) then you add those includes and the party
continues with lots of other missing includes. I managed to get all
those includes sorted at some point, but then using the functions
initially suggested by Joe, caused a whole lot of other library and
include files to be missing. So I didn't think it was worth the effort.
Thus I suggest, merge as is, and if someone wants to start cleaning it
up (by fixing net.h) that would be awesome. The idea for this tool was
to be able to quickly generate a crc8 code :)
On 15-11-16 04:31, Joe Hershberger wrote:
Yep, but including net.h then makes net.h very unhappy (lots of missing
things, u32 to begin with) then you add those includes and the party
continues with lots of other missing includes. I managed to get all
those includes sorted at some point, but then using the functions
initially suggested by Joe, caused a whole lot of other library and
include files to be missing. So I didn't think it was worth the effort.
Thus I suggest, merge as is, and if someone wants to start cleaning it
up (by fixing net.h) that would be awesome. The idea for this tool was
to be able to quickly generate a crc8 code :)
Hans de Goede
Nov 15, 2016, 4:26:50 AM11/15/16
to Joe Hershberger, Olliver Schinagl, Ian Campbell, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-boot
Hi,
On 15-11-16 04:25, Joe Hershberger wrote:
Erm, this patch seems wrong to me: NACK, let me
explain:
1) It does not do what its commit message says, it only
removes the second step for setting ethernet addresses
from the env, but it keeps the existing code to set them
AFAICT, it only does it once now.
2) "Currently we inject 5 ethernet addresses into the environment",
this is not true, we only inject ethernet addresses into the
environment for devices which have an ethernet alias in dt,
so maximum 2 for devices with both wired ethernet and wifi
3) The second attempt at setting ethernet addresses in
the environment after loading the kernel dt is necessary
because the kernel dt may be newer and contain more
ethernet aliases, e.g. the u-boot dt may only contain the
nodes + alias for the wired, while the (newer) kernel dt
may also contain a dt-node + alias for the wireless
4) We cannot solely rely on the ethernet driver to set
mac-addresses, because the ethernet driver may not be enabled
while the kernel does have the ethernet driver enabled; and
the kernel relies on u-boot to generate fixed mac-addresses
based on the SID independent whether or not u-boot has
ethernet enabled, this is especially relevant for wifi
chips where the kernel also relies on u-boot generated
fixed mac-addresses on e.g. the recent orange-pi boards,
which come with a realtek rtl8189etv chip which does not
have a mac address programmed.
5) AFAIK the dt code for passing mac-addresses to the kernel
relies on the environment variables, so even if we get the
mac-address from a ROM we should still store it in the
environment variable.
Regards,
Hans
On 15-11-16 04:25, Joe Hershberger wrote:
> On Tue, Nov 8, 2016 at 9:54 AM, Olliver Schinagl <oli...@schinagl.nl> wrote:
>> Currently we inject 5 ethernet addresses into the environment, just in
>> case we may need them. We do this because some boards have no eeprom
>> (programmed) with a proper ethernet address. With the recent addition of
>> reading actual ethernet addresses from the eeprom via the net_op we
>> should not inject environment variables any more.
>>
>> Signed-off-by: Olliver Schinagl <oli...@schinagl.nl>
>
> Acked-by: Joe Hershberger <joe.her...@ni.com>
>> Currently we inject 5 ethernet addresses into the environment, just in
>> case we may need them. We do this because some boards have no eeprom
>> (programmed) with a proper ethernet address. With the recent addition of
>> reading actual ethernet addresses from the eeprom via the net_op we
>> should not inject environment variables any more.
>>
>> Signed-off-by: Olliver Schinagl <oli...@schinagl.nl>
>
Erm, this patch seems wrong to me: NACK, let me
explain:
1) It does not do what its commit message says, it only
removes the second step for setting ethernet addresses
from the env, but it keeps the existing code to set them
AFAICT, it only does it once now.
2) "Currently we inject 5 ethernet addresses into the environment",
this is not true, we only inject ethernet addresses into the
environment for devices which have an ethernet alias in dt,
so maximum 2 for devices with both wired ethernet and wifi
3) The second attempt at setting ethernet addresses in
the environment after loading the kernel dt is necessary
because the kernel dt may be newer and contain more
ethernet aliases, e.g. the u-boot dt may only contain the
nodes + alias for the wired, while the (newer) kernel dt
may also contain a dt-node + alias for the wireless
4) We cannot solely rely on the ethernet driver to set
mac-addresses, because the ethernet driver may not be enabled
while the kernel does have the ethernet driver enabled; and
the kernel relies on u-boot to generate fixed mac-addresses
based on the SID independent whether or not u-boot has
ethernet enabled, this is especially relevant for wifi
chips where the kernel also relies on u-boot generated
fixed mac-addresses on e.g. the recent orange-pi boards,
which come with a realtek rtl8189etv chip which does not
have a mac address programmed.
5) AFAIK the dt code for passing mac-addresses to the kernel
relies on the environment variables, so even if we get the
mac-address from a ROM we should still store it in the
environment variable.
Regards,
Hans
Olliver Schinagl
Nov 15, 2016, 4:59:45 AM11/15/16
to Simon Glass, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, U-Boot Mailing List, Olliver Schinagl
Hey Simon,
Because I did it in the net stuff as well, where I used the
'is_zero_mac()'' to indicate things where not setup properly. I guess it
can go here ...
a simple tool. I'll put it in the next version.
it's a pain to get to add it to this, hence the manual way. I'll add
some doc to the func as well.
'is_zero_mac()'' to indicate things where not setup properly. I guess it
can go here ...
>
>> +
>> + if (argc < 2) {
>> + puts("Please supply a MAC address.");
> How about a usage() function which gives generic help as well?
fair point, You caught me on being lazy :) I didn't think of it for such
>> +
>> + if (argc < 2) {
>> + puts("Please supply a MAC address.");
> How about a usage() function which gives generic help as well?
a simple tool. I'll put it in the next version.
>
>> + return -1;
>> + }
>> +
>> + if (!((strlen(argv[1]) == 12) || (strlen(argv[1]) == 17))) {
>> + puts("Please supply a valid MAC address with optionaly\n"
> optionally. But do you mean 'Please supply a valid MAC address with
> optional - or : separators?'
>
>> + "dashes (-) or colons (:) as seperators.");
> separators.
>
>> + return -1;
>> + }
>> +
>> + i = 0;
>> + while (*argv[1] != '\0') {
> How about putting this code in a separate function:
>
> int process_mac(const char *max)
>
> so you don't have to access argv[1] all the time. Also you can return
> 1 instead of -1 from main() on error.
right, no prob.
>> + return -1;
>> + }
>> +
>> + if (!((strlen(argv[1]) == 12) || (strlen(argv[1]) == 17))) {
>> + puts("Please supply a valid MAC address with optionaly\n"
> optionally. But do you mean 'Please supply a valid MAC address with
> optional - or : separators?'
>
>> + "dashes (-) or colons (:) as seperators.");
> separators.
>
>> + return -1;
>> + }
>> +
>> + i = 0;
>> + while (*argv[1] != '\0') {
> How about putting this code in a separate function:
>
> int process_mac(const char *max)
>
> so you don't have to access argv[1] all the time. Also you can return
> 1 instead of -1 from main() on error.
>
>> + char nibble[2] = { 0x00, '\n' }; /* for strtol */
>> +
>> + nibble[0] = *argv[1]++;
>> + if (isxdigit(nibble[0])) {
>> + if (isupper(nibble[0]))
>> + nibble[0] = tolower(nibble[0]);
>> + mac_addr[i >> 1] |= strtol(nibble, NULL, 16) << ((i % 2) ? 0 : 4) & ((i % 2) ? 0x0f : 0xf0);
> How about a nibble_to_hex() function here? This is strange-looking
> code. I'm wondering what you are trying to do.
It is strange, and we even have a nice function in u-boot I belive, but
>> + char nibble[2] = { 0x00, '\n' }; /* for strtol */
>> +
>> + nibble[0] = *argv[1]++;
>> + if (isxdigit(nibble[0])) {
>> + if (isupper(nibble[0]))
>> + nibble[0] = tolower(nibble[0]);
>> + mac_addr[i >> 1] |= strtol(nibble, NULL, 16) << ((i % 2) ? 0 : 4) & ((i % 2) ? 0x0f : 0xf0);
> How about a nibble_to_hex() function here? This is strange-looking
> code. I'm wondering what you are trying to do.
it's a pain to get to add it to this, hence the manual way. I'll add
some doc to the func as well.
>
>> + i++;
>> + }
>> + }
>> + mac_addr[ARP_HLEN] = crc8(0, mac_addr, ARP_HLEN);
> I suggest putting this in a separate variable...
>
>> +
>> + for (i = 0; i < ARP_HLEN + 1; i++)
>> + printf("%.2x", mac_addr[i]);
>> + putchar('\n');
> and here: printf("%.2x\n", crc)
yeah i do like the separate var for here.
>> + i++;
>> + }
>> + }
>> + mac_addr[ARP_HLEN] = crc8(0, mac_addr, ARP_HLEN);
> I suggest putting this in a separate variable...
>
>> +
>> + for (i = 0; i < ARP_HLEN + 1; i++)
>> + printf("%.2x", mac_addr[i]);
>> + putchar('\n');
> and here: printf("%.2x\n", crc)
Olliver Schinagl
Nov 15, 2016, 5:17:21 AM11/15/16
to Hans de Goede, Joe Hershberger, Ian Campbell, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-boot
Hey Hans,
I was hopeing and expecting this :)
As you will be able to tell below, I need to learn a bit more as to why
we do things and discuss this proper I guess.
On 15-11-16 10:26, Hans de Goede wrote:
> Hi,
>
> On 15-11-16 04:25, Joe Hershberger wrote:
>> On Tue, Nov 8, 2016 at 9:54 AM, Olliver Schinagl <oli...@schinagl.nl>
>> wrote:
>>> Currently we inject 5 ethernet addresses into the environment, just in
>>> case we may need them. We do this because some boards have no eeprom
>>> (programmed) with a proper ethernet address. With the recent
>>> addition of
>>> reading actual ethernet addresses from the eeprom via the net_op we
>>> should not inject environment variables any more.
>>>
>>> Signed-off-by: Olliver Schinagl <oli...@schinagl.nl>
>>
>> Acked-by: Joe Hershberger <joe.her...@ni.com>
>
>
> Erm, this patch seems wrong to me: NACK, let me
> explain:
>
> 1) It does not do what its commit message says, it only
> removes the second step for setting ethernet addresses
> from the env, but it keeps the existing code to set them
> AFAICT, it only does it once now.
>
> 2) "Currently we inject 5 ethernet addresses into the environment",
> this is not true, we only inject ethernet addresses into the
> environment for devices which have an ethernet alias in dt,
> so maximum 2 for devices with both wired ethernet and wifi
If we want the fdt to do mac things, shouldn't that be done at a higher
level? This is not really board specific is it?
rtl8189etv also read rom hook which IS board specific
What happens is (note code is mangled and might not be 100% accurate, i
reduced it the bares):
eth_read_eeprom_hwaddr(dev);
first read from the eeprom, which may return all zero's if it is
unconfigured/missconfigured or should not be used from the eeprom.
if (is_zero_ethaddr(pdata->enetaddr))
if (eth_get_ops(dev)->read_rom_hwaddr)
eth_get_ops(dev)->read_rom_hwaddr(dev);
if the eeprom failed to produce a mac, we check the read_rom_hwaddr
callback, which hooks into the driver. The driver can be overridden by a
board (such as sunxi) where the MAC is generated from the SID.
so at this point we may have a hwaddress actually obtained from the
hardware, via the eeprom (or some fixed rom even) or from the hardware
itself
next we allow 'software' overrides. e.g. u-boot env (and i think this is
where the fdt method should live as well
eth_getenv_enetaddr_by_index("eth", dev->seq, env_enetaddr);
if (!is_zero_ethaddr(env_enetaddr)) {
if (!is_zero_ethaddr(pdata->enetaddr) &&
memcmp(pdata->enetaddr, env_enetaddr, ARP_HLEN))
memcpy(pdata->enetaddr, env_enetaddr, ARP_HLEN);
// <snip> we compare the HW addr and the ENV addr. if the env is unset,
we use whatever the hardware supplied us with.
if the env is set, it overrides the HW addr.
I think next would be to check the fdt to override the env?
} else if (is_valid_ethaddr(pdata->enetaddr)) {
eth_setenv_enetaddr_by_index("eth", dev->seq, pdata->enetaddr);
Finally, we set whatever mac has come from the above probing into the
environment (if the address is actually a valid MAC).
} else if (is_zero_ethaddr(pdata->enetaddr)) {
#ifdef CONFIG_NET_RANDOM_ETHADDR
net_random_ethaddr(pdata->enetaddr);
otherwise (if configured) let u-boot generate it.
So I think here is where the fdt override should live, as it applies to
everyone, not just sunxi.
I'll post my split-up new series for review after testing it, so we can
discuss it in more detail?
Olliver
>
> Regards,
>
> Hans
>
I was hopeing and expecting this :)
As you will be able to tell below, I need to learn a bit more as to why
we do things and discuss this proper I guess.
On 15-11-16 10:26, Hans de Goede wrote:
> Hi,
>
> On 15-11-16 04:25, Joe Hershberger wrote:
>> On Tue, Nov 8, 2016 at 9:54 AM, Olliver Schinagl <oli...@schinagl.nl>
>> wrote:
>>> Currently we inject 5 ethernet addresses into the environment, just in
>>> case we may need them. We do this because some boards have no eeprom
>>> (programmed) with a proper ethernet address. With the recent
>>> addition of
>>> reading actual ethernet addresses from the eeprom via the net_op we
>>> should not inject environment variables any more.
>>>
>>> Signed-off-by: Olliver Schinagl <oli...@schinagl.nl>
>>
>> Acked-by: Joe Hershberger <joe.her...@ni.com>
>
>
> Erm, this patch seems wrong to me: NACK, let me
> explain:
>
> 1) It does not do what its commit message says, it only
> removes the second step for setting ethernet addresses
> from the env, but it keeps the existing code to set them
> AFAICT, it only does it once now.
>
> 2) "Currently we inject 5 ethernet addresses into the environment",
> this is not true, we only inject ethernet addresses into the
> environment for devices which have an ethernet alias in dt,
> so maximum 2 for devices with both wired ethernet and wifi
level? This is not really board specific is it?
>
> 3) The second attempt at setting ethernet addresses in
> the environment after loading the kernel dt is necessary
> because the kernel dt may be newer and contain more
> ethernet aliases, e.g. the u-boot dt may only contain the
> nodes + alias for the wired, while the (newer) kernel dt
> may also contain a dt-node + alias for the wireless
I agree with you here, but I still don't think this should be board specific
> 3) The second attempt at setting ethernet addresses in
> the environment after loading the kernel dt is necessary
> because the kernel dt may be newer and contain more
> ethernet aliases, e.g. the u-boot dt may only contain the
> nodes + alias for the wired, while the (newer) kernel dt
> may also contain a dt-node + alias for the wireless
>
> 4) We cannot solely rely on the ethernet driver to set
> mac-addresses, because the ethernet driver may not be enabled
> while the kernel does have the ethernet driver enabled; and
> the kernel relies on u-boot to generate fixed mac-addresses
> based on the SID independent whether or not u-boot has
> ethernet enabled, this is especially relevant for wifi
> chips where the kernel also relies on u-boot generated
> fixed mac-addresses on e.g. the recent orange-pi boards,
> which come with a realtek rtl8189etv chip which does not
> have a mac address programmed.
I agree, and I'll fix that in my new patch series proper by making
> 4) We cannot solely rely on the ethernet driver to set
> mac-addresses, because the ethernet driver may not be enabled
> while the kernel does have the ethernet driver enabled; and
> the kernel relies on u-boot to generate fixed mac-addresses
> based on the SID independent whether or not u-boot has
> ethernet enabled, this is especially relevant for wifi
> chips where the kernel also relies on u-boot generated
> fixed mac-addresses on e.g. the recent orange-pi boards,
> which come with a realtek rtl8189etv chip which does not
> have a mac address programmed.
rtl8189etv also read rom hook which IS board specific
>
> 5) AFAIK the dt code for passing mac-addresses to the kernel
> relies on the environment variables, so even if we get the
> mac-address from a ROM we should still store it in the
> environment variable.
The new patch series does that, as the net core layer does that.
> 5) AFAIK the dt code for passing mac-addresses to the kernel
> relies on the environment variables, so even if we get the
> mac-address from a ROM we should still store it in the
> environment variable.
What happens is (note code is mangled and might not be 100% accurate, i
reduced it the bares):
eth_read_eeprom_hwaddr(dev);
first read from the eeprom, which may return all zero's if it is
unconfigured/missconfigured or should not be used from the eeprom.
if (is_zero_ethaddr(pdata->enetaddr))
if (eth_get_ops(dev)->read_rom_hwaddr)
eth_get_ops(dev)->read_rom_hwaddr(dev);
if the eeprom failed to produce a mac, we check the read_rom_hwaddr
callback, which hooks into the driver. The driver can be overridden by a
board (such as sunxi) where the MAC is generated from the SID.
so at this point we may have a hwaddress actually obtained from the
hardware, via the eeprom (or some fixed rom even) or from the hardware
itself
next we allow 'software' overrides. e.g. u-boot env (and i think this is
where the fdt method should live as well
eth_getenv_enetaddr_by_index("eth", dev->seq, env_enetaddr);
if (!is_zero_ethaddr(env_enetaddr)) {
if (!is_zero_ethaddr(pdata->enetaddr) &&
memcmp(pdata->enetaddr, env_enetaddr, ARP_HLEN))
memcpy(pdata->enetaddr, env_enetaddr, ARP_HLEN);
// <snip> we compare the HW addr and the ENV addr. if the env is unset,
we use whatever the hardware supplied us with.
if the env is set, it overrides the HW addr.
I think next would be to check the fdt to override the env?
} else if (is_valid_ethaddr(pdata->enetaddr)) {
eth_setenv_enetaddr_by_index("eth", dev->seq, pdata->enetaddr);
Finally, we set whatever mac has come from the above probing into the
environment (if the address is actually a valid MAC).
} else if (is_zero_ethaddr(pdata->enetaddr)) {
#ifdef CONFIG_NET_RANDOM_ETHADDR
net_random_ethaddr(pdata->enetaddr);
otherwise (if configured) let u-boot generate it.
So I think here is where the fdt override should live, as it applies to
everyone, not just sunxi.
I'll post my split-up new series for review after testing it, so we can
discuss it in more detail?
Olliver
>
> Regards,
>
> Hans
>
Hans de Goede
Nov 15, 2016, 5:27:31 AM11/15/16
to Olliver Schinagl, Joe Hershberger, Ian Campbell, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-boot
Hi,
We want to put mac addresses into the fdt, and this is done at
a higher level, by existing dt code, which looks at ethernet
aliases in dt and then for any which are present, checks
the corresponding ethaddr env setting and if set, injects
that mac address into the dt-node the alias points to.
What is sunxi specific is setting the environment variables
based on the SID. The sunxi specific code also checks the
aliases, exactly to avoid the "inject 5 ethernet addresses"
thing you are describing, as we don't want to inject
ethernet addresses for non existent NICs as that may confuse
the user.
>> 3) The second attempt at setting ethernet addresses in
>> the environment after loading the kernel dt is necessary
>> because the kernel dt may be newer and contain more
>> ethernet aliases, e.g. the u-boot dt may only contain the
>> nodes + alias for the wired, while the (newer) kernel dt
>> may also contain a dt-node + alias for the wireless
> I agree with you here, but I still don't think this should be board specific
Instead of doing this through the environment I guess you
could have the u-boot dt code which is injecting the MACs
into the dt call some board specific callback when there
is no MAC set in the environment, and implement a weak
stub for this. Then all the sunxi environment mangling
code can go away, and sunxi can simply:
1) Try eeprom if present
2) Do the current SID based thing
>> 4) We cannot solely rely on the ethernet driver to set
>> mac-addresses, because the ethernet driver may not be enabled
>> while the kernel does have the ethernet driver enabled; and
>> the kernel relies on u-boot to generate fixed mac-addresses
>> based on the SID independent whether or not u-boot has
>> ethernet enabled, this is especially relevant for wifi
>> chips where the kernel also relies on u-boot generated
>> fixed mac-addresses on e.g. the recent orange-pi boards,
>> which come with a realtek rtl8189etv chip which does not
>> have a mac address programmed.
> I agree, and I'll fix that in my new patch series proper by making rtl8189etv also read rom hook which IS board specific
The problem is that u-boot may not have a driver for one
of the NICs at all, so no place to call the rom hook at all.
Anyways I believe this is solved by my suggestion for making
the u-boot dt code which injects the MAC call a board specific
callback when no ethaddr is set in the env.
Ok, good I just wanted to make sure that that would still happen.
>
> } else if (is_zero_ethaddr(pdata->enetaddr)) {
> #ifdef CONFIG_NET_RANDOM_ETHADDR
> net_random_ethaddr(pdata->enetaddr);
> otherwise (if configured) let u-boot generate it.
>
>
> So I think here is where the fdt override should live, as it applies to everyone, not just sunxi.
I think you're thinking too much about the case where u-boot
has an actual driver for the NIC (and that driver gets
initialized, what if it gets skipped to speed up the boot?)
and not enough about the case where there is no driver but
we still want to use u-boot's board specific MAC generation code
to provide a fixed MAC address to the kernel.
Regards,
Hans
a higher level, by existing dt code, which looks at ethernet
aliases in dt and then for any which are present, checks
the corresponding ethaddr env setting and if set, injects
that mac address into the dt-node the alias points to.
What is sunxi specific is setting the environment variables
based on the SID. The sunxi specific code also checks the
aliases, exactly to avoid the "inject 5 ethernet addresses"
thing you are describing, as we don't want to inject
ethernet addresses for non existent NICs as that may confuse
the user.
>> 3) The second attempt at setting ethernet addresses in
>> the environment after loading the kernel dt is necessary
>> because the kernel dt may be newer and contain more
>> ethernet aliases, e.g. the u-boot dt may only contain the
>> nodes + alias for the wired, while the (newer) kernel dt
>> may also contain a dt-node + alias for the wireless
> I agree with you here, but I still don't think this should be board specific
could have the u-boot dt code which is injecting the MACs
into the dt call some board specific callback when there
is no MAC set in the environment, and implement a weak
stub for this. Then all the sunxi environment mangling
code can go away, and sunxi can simply:
1) Try eeprom if present
2) Do the current SID based thing
>> 4) We cannot solely rely on the ethernet driver to set
>> mac-addresses, because the ethernet driver may not be enabled
>> while the kernel does have the ethernet driver enabled; and
>> the kernel relies on u-boot to generate fixed mac-addresses
>> based on the SID independent whether or not u-boot has
>> ethernet enabled, this is especially relevant for wifi
>> chips where the kernel also relies on u-boot generated
>> fixed mac-addresses on e.g. the recent orange-pi boards,
>> which come with a realtek rtl8189etv chip which does not
>> have a mac address programmed.
> I agree, and I'll fix that in my new patch series proper by making rtl8189etv also read rom hook which IS board specific
of the NICs at all, so no place to call the rom hook at all.
Anyways I believe this is solved by my suggestion for making
the u-boot dt code which injects the MAC call a board specific
callback when no ethaddr is set in the env.
>
> } else if (is_zero_ethaddr(pdata->enetaddr)) {
> #ifdef CONFIG_NET_RANDOM_ETHADDR
> net_random_ethaddr(pdata->enetaddr);
> otherwise (if configured) let u-boot generate it.
>
>
> So I think here is where the fdt override should live, as it applies to everyone, not just sunxi.
has an actual driver for the NIC (and that driver gets
initialized, what if it gets skipped to speed up the boot?)
and not enough about the case where there is no driver but
we still want to use u-boot's board specific MAC generation code
to provide a fixed MAC address to the kernel.
Regards,
Hans
Olliver Schinagl
Nov 15, 2016, 5:29:05 AM11/15/16
to Olliver Schinagl, Hans de Goede, Joe Hershberger, Ian Campbell, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-boot
Hey Hans,
I guess I have to contradict something here ...
On 15-11-16 11:17, Olliver Schinagl wrote:
Of course I didn't realize that the rtl8189etv does not have a u-boot
driver, and thus does not get to call its hook and thus nothing sunxi
specific will ever be invoked.
So I guess in the case of the rtl8189 we have to figure out something
(probably near the same as you did) to make it work.
It does feel somewhat nasty/hackish of course. I would expect that the
linux driver sorts this out for itself and not simply assume u-boot
supplies this info on non-existing hardware (to u-boot).
I need some time to think this over, so I'm hoping smarter people then
me come with great suggestions here :)
But for now I'm leaning to think that, the rtl8189 is special.
So is this a board specific hack, or a fdt net specific hack. I does
feel like the fdt bit I described earlier injects extra mac addresses
into the environment for these unknown hardware pieces ... But that will
need to come from board specific pieces, as the net stack never gets
invoked for these ...
I'll stop thinking outloud now and get back to work ;)
olliver
--
Met vriendelijke groeten, Kind regards, 与亲切的问候
Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.
I guess I have to contradict something here ...
On 15-11-16 11:17, Olliver Schinagl wrote:
driver, and thus does not get to call its hook and thus nothing sunxi
specific will ever be invoked.
So I guess in the case of the rtl8189 we have to figure out something
(probably near the same as you did) to make it work.
It does feel somewhat nasty/hackish of course. I would expect that the
linux driver sorts this out for itself and not simply assume u-boot
supplies this info on non-existing hardware (to u-boot).
I need some time to think this over, so I'm hoping smarter people then
me come with great suggestions here :)
But for now I'm leaning to think that, the rtl8189 is special.
So is this a board specific hack, or a fdt net specific hack. I does
feel like the fdt bit I described earlier injects extra mac addresses
into the environment for these unknown hardware pieces ... But that will
need to come from board specific pieces, as the net stack never gets
invoked for these ...
I'll stop thinking outloud now and get back to work ;)
olliver
Met vriendelijke groeten, Kind regards, 与亲切的问候
Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.
Olliver Schinagl
Nov 15, 2016, 5:35:28 AM11/15/16
to Hans de Goede, Joe Hershberger, Ian Campbell, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-boot
Hey,
In my other e-mail contradiciting myself I just found this somewhat out,
and that indeed is another usecase worth thinking about yeah. So I'll
need to re-think that part too.
And then thinks come to mind 'if there are 5 address in the eeprom, but
only 2 drivers, do the drivers get the first two?' ... I guess there
needs to be a general agreement on strange cases as such. How are
non-driver devices handled. The dts obviously is one method, but I'm
sure board manufactures will hate us for forcing board specific dts.
They want to just produce boards en-masse and may be kind enough to
supply eeprom or MAC-prom's with fixed mac addresses stored there.
I think this is an architectural based decision which deserves its own
thread?
Olliver
>
> Regards,
>
> Hans
and that indeed is another usecase worth thinking about yeah. So I'll
need to re-think that part too.
And then thinks come to mind 'if there are 5 address in the eeprom, but
only 2 drivers, do the drivers get the first two?' ... I guess there
needs to be a general agreement on strange cases as such. How are
non-driver devices handled. The dts obviously is one method, but I'm
sure board manufactures will hate us for forcing board specific dts.
They want to just produce boards en-masse and may be kind enough to
supply eeprom or MAC-prom's with fixed mac addresses stored there.
I think this is an architectural based decision which deserves its own
thread?
Olliver
>
> Regards,
>
> Hans
Hans de Goede
Nov 15, 2016, 5:50:08 AM11/15/16
to Olliver Schinagl, Olliver Schinagl, Joe Hershberger, Ian Campbell, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-boot
Hi,
We've the same with e.g. the wobo-i5 which has its emac routed to
an other set of pins then which are usually used which u-boot does
not support. Yet the kernel emac driver relies on u-boot to set a
MAC, or it will fall back to a random-MAC (which is undesirable).
Likewise if someone configures u-boot without network support
to make it boot faster, we get the same problem with the emac /
gmac driver.
The logic here really goes like this:
1) When u-boot does have network support, and picks a MAC-address
that MAC-address should be propagated to the kernel so that it
uses the same MAC (this is esp. important with switches which
allow only 1 MAC per port as a security setting).
2) 1. means that u-boot has some algorithm to pick a fixed MAC
in a SoC specific manner. Since we do not want booting with an
u-boot with networking enabled resulting in a different fixed
MAC then booting on a u-boot without networking support, this
means that this algorithm should be used even when networking
support in u-boot is disabled (e.g. the wobo-i5 case).
3) Given 2. it makes sense to simply have u-boot generate
MACs for all NICs in the system.
> I need some time to think this over, so I'm hoping smarter people then me come with great suggestions here :)
I still think that my suggestion for having fdt_fixup_ethernet()
from common/fdt_support.c call a board specific function instead
of the "continue" here:
tmp = getenv(mac);
if (!tmp)
continue;
for (j = 0; j < 6; j++) {
mac_addr[j] = tmp ?
simple_strtoul(tmp, &end, 16) : 0;
if (tmp)
tmp = (*end) ? end + 1 : end;
}
E.g:
tmp = getenv(mac);
if (!tmp) {
if (board_get_ethaddr(i, mac_addr) != 0)
continue;
} else {
for (j = 0; j < 6; j++) {
mac_addr[j] = tmp ?
simple_strtoul(tmp, &end, 16) : 0;
if (tmp)
tmp = (*end) ? end + 1 : end;
}
}
Would be a good idea, with a weak default for
int board_get_ethaddr(int index, unsigned char *mac_addr);
Which simply returns -EINVAL;
This will neatly solve all the problems discussed, you
could probably even use the same board_get_ethaddr()
in both the generic ethernet setup code you've been
working in, as well as in fdt_fixup_ethernet()
Regards,
Hans
an other set of pins then which are usually used which u-boot does
not support. Yet the kernel emac driver relies on u-boot to set a
MAC, or it will fall back to a random-MAC (which is undesirable).
Likewise if someone configures u-boot without network support
to make it boot faster, we get the same problem with the emac /
gmac driver.
The logic here really goes like this:
1) When u-boot does have network support, and picks a MAC-address
that MAC-address should be propagated to the kernel so that it
uses the same MAC (this is esp. important with switches which
allow only 1 MAC per port as a security setting).
2) 1. means that u-boot has some algorithm to pick a fixed MAC
in a SoC specific manner. Since we do not want booting with an
u-boot with networking enabled resulting in a different fixed
MAC then booting on a u-boot without networking support, this
means that this algorithm should be used even when networking
support in u-boot is disabled (e.g. the wobo-i5 case).
3) Given 2. it makes sense to simply have u-boot generate
MACs for all NICs in the system.
> I need some time to think this over, so I'm hoping smarter people then me come with great suggestions here :)
from common/fdt_support.c call a board specific function instead
of the "continue" here:
tmp = getenv(mac);
if (!tmp)
continue;
for (j = 0; j < 6; j++) {
mac_addr[j] = tmp ?
simple_strtoul(tmp, &end, 16) : 0;
if (tmp)
tmp = (*end) ? end + 1 : end;
}
E.g:
tmp = getenv(mac);
if (!tmp) {
if (board_get_ethaddr(i, mac_addr) != 0)
continue;
} else {
for (j = 0; j < 6; j++) {
mac_addr[j] = tmp ?
simple_strtoul(tmp, &end, 16) : 0;
if (tmp)
tmp = (*end) ? end + 1 : end;
}
}
Would be a good idea, with a weak default for
int board_get_ethaddr(int index, unsigned char *mac_addr);
Which simply returns -EINVAL;
This will neatly solve all the problems discussed, you
could probably even use the same board_get_ethaddr()
in both the generic ethernet setup code you've been
working in, as well as in fdt_fixup_ethernet()
Regards,
Hans
Olliver Schinagl
Nov 15, 2016, 6:58:56 AM11/15/16
to Hans de Goede, Olliver Schinagl, Joe Hershberger, Ian Campbell, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Simon Glass, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, u-boot
Hey Hans,
I have to digest all you said so far, but
3) makes sense, expect for the 'generate' part :) But I think we mean
the same.
From a manufacturing PoV. we have 2 problems. A product with ethernet
hardware needs to use registered mac addresses. So you have fixed mac
addresses you can use for your product. This needs to be facilitated in
some manner.
The MAC needs to be available to the board uniformly, regardless of the
used bootloader or OS. E.g. lets say raspberry pi 3 (without going into
actual factual details), which can boot windows or linux, both OSes need
to get the same MAC address from the board somehow. It is not uncommon
to have an EEPROM from this configuration data (even NIC's have eeproms
for this).
In any case, a manufacturer of boards wants to program a unique MAC into
every board, without having to modify each env/fdt.
The cases you describe are all valid, but or really for hobbist use in
small scales. They are likley also not handing out valid MAC addresses,
but use the 'for internal use' range.
Of course we want to facilitate both.
So I do agree that u-boot should be responsible for setting the mac of
all devices (configured or unconfigured), or 'generate' if you will.
The problem however is 'what are all NIC's in the system'. And again, if
there are MAC's in the eeprom, which device gets what eeprom.
Normally, u-boot probes the hardware, based on that you get a fixed
order of eth devices and that fixed order decides which MAC to use.
Unless we make the FDT leading here, so that the FDT decides on the
probing order.
You may have noticed however, I am in somewhat unfamiliary territory
here as to what does u-boot do now, how does the fdt fit in here.
Let me digest this all some more and mull it over.
But to summarize, ignoring the ordering of devices for now (which runs
havoc in the 'unconfigured' case to speed up booting),
check eeprom for mac
if not, call device read rom hook
which may be overriden by the board
set to env
let env override hw generated code
check fdt and let the fdt override the env
if fdt has more ethernet devices, add those to the env as well.
Ideally this is where re-ording would take place to match what the fdt
says, but we don't know of course what eth0, eth1 and eth2 are bound
too... but again, i need to mull and look at the fdt_support bit.
olliver
3) makes sense, expect for the 'generate' part :) But I think we mean
the same.
From a manufacturing PoV. we have 2 problems. A product with ethernet
hardware needs to use registered mac addresses. So you have fixed mac
addresses you can use for your product. This needs to be facilitated in
some manner.
The MAC needs to be available to the board uniformly, regardless of the
used bootloader or OS. E.g. lets say raspberry pi 3 (without going into
actual factual details), which can boot windows or linux, both OSes need
to get the same MAC address from the board somehow. It is not uncommon
to have an EEPROM from this configuration data (even NIC's have eeproms
for this).
In any case, a manufacturer of boards wants to program a unique MAC into
every board, without having to modify each env/fdt.
The cases you describe are all valid, but or really for hobbist use in
small scales. They are likley also not handing out valid MAC addresses,
but use the 'for internal use' range.
Of course we want to facilitate both.
So I do agree that u-boot should be responsible for setting the mac of
all devices (configured or unconfigured), or 'generate' if you will.
The problem however is 'what are all NIC's in the system'. And again, if
there are MAC's in the eeprom, which device gets what eeprom.
Normally, u-boot probes the hardware, based on that you get a fixed
order of eth devices and that fixed order decides which MAC to use.
Unless we make the FDT leading here, so that the FDT decides on the
probing order.
You may have noticed however, I am in somewhat unfamiliary territory
here as to what does u-boot do now, how does the fdt fit in here.
But to summarize, ignoring the ordering of devices for now (which runs
havoc in the 'unconfigured' case to speed up booting),
check eeprom for mac
if not, call device read rom hook
which may be overriden by the board
set to env
let env override hw generated code
check fdt and let the fdt override the env
if fdt has more ethernet devices, add those to the env as well.
Ideally this is where re-ording would take place to match what the fdt
says, but we don't know of course what eth0, eth1 and eth2 are bound
too... but again, i need to mull and look at the fdt_support bit.
olliver
Simon Glass
Nov 17, 2016, 8:14:17 PM11/17/16
to Olliver Schinagl, Ian Campbell, Hans De Goede, Albert Aribaud, Iain Paton, FUKAUMI Naoki, Joe Hershberger, Jeffy Chen, Michal Simek, Stefan Brüns, Nathan Rossi, Andreas Bießmann, Stefano Babic, Stefan Roese, Maxime Ripard, d...@linux-sunxi.org, U-Boot Mailing List
Hi Oliver,
a driver supplied by the board to read this value. It should be
possible to supply the 'hardware-address reading' device to any
Ethernet driver, not just dwmmc.
> +
> +static int designware_eth_read_rom_hwaddr(struct udevice *dev)
> +{
> + int retval;
> + struct eth_pdata *pdata = dev_get_platdata(dev);
> +
> + retval = dw_board_read_rom_hwaddr(pdata->enetaddr);
> + if (retval == -ENOSYS)
> + return 0;
> +
> + return retval;
> +}
> +
> static void dw_adjust_link(struct eth_mac_regs *mac_p,
> struct phy_device *phydev)
> {
> @@ -685,6 +702,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
>
Regards,
Simon
> 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 <oli...@schinagl.nl>
> ---
> drivers/net/designware.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> index 9e6d726..aa87f30 100644
> --- a/drivers/net/designware.c
> +++ b/drivers/net/designware.c
> @@ -230,6 +230,23 @@ 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)
> +{
> + return -ENOSYS;
> +}
Instead of a weak function I think this should use driver model, with
> to read the mac from a ROM chip.
>
> Signed-off-by: Olliver Schinagl <oli...@schinagl.nl>
> ---
> drivers/net/designware.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> index 9e6d726..aa87f30 100644
> --- a/drivers/net/designware.c
> +++ b/drivers/net/designware.c
> @@ -230,6 +230,23 @@ 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)
> +{
> + return -ENOSYS;
> +}
a driver supplied by the board to read this value. It should be
possible to supply the 'hardware-address reading' device to any
Ethernet driver, not just dwmmc.
> +
> +static int designware_eth_read_rom_hwaddr(struct udevice *dev)
> +{
> + int retval;
> + struct eth_pdata *pdata = dev_get_platdata(dev);
> +
> + retval = dw_board_read_rom_hwaddr(pdata->enetaddr);
> + if (retval == -ENOSYS)
> + return 0;
> +
> + return retval;
> +}
> +
> static void dw_adjust_link(struct eth_mac_regs *mac_p,
> struct phy_device *phydev)
> {
> @@ -685,6 +702,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
>
Simon
Simon Glass
Nov 29, 2016, 4:40:30 PM11/29/16
to Joe Hershberger, Olliver Schinagl, Albert Aribaud, U-Boot Mailing List, Hans De Goede, d...@linux-sunxi.org, Jeffy Chen, Michal Simek, Joe Hershberger, Ian Campbell, Stefan Roese
Hi Joe,
On 29 November 2016 at 14:24, Joe Hershberger <joe.her...@gmail.com> wrote:
>
> Hi Simon,
> device tree for boards using only Linux bindings, and sharing the
> device tree with Linux? Linux probably doesn't care about this and so
> won't have a binding for defining this relationship. This is a fairly
> generic question. Where have we landed on this?
So far I have not seen something that cannot be solved either as I
suggest above or with platform data.
Often you can pass platform data to the driver - e.g. see the end of
board_init() in gurnard.c which tells the video driver which LCD to
use.
Is there another case? I certainly have ideas but don't want to create
something without a solid case.
Regards,
Simon
On 29 November 2016 at 14:24, Joe Hershberger <joe.her...@gmail.com> wrote:
>
> Hi Simon,
>
> On Thu, Nov 17, 2016 at 7:13 PM, Simon Glass <s...@chromium.org> wrote:
> > Hi Oliver,
> >
> > On 8 November 2016 at 08:54, Olliver Schinagl <oli...@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 <oli...@schinagl.nl>
> >> ---
> >> drivers/net/designware.c | 18 ++++++++++++++++++
> >> 1 file changed, 18 insertions(+)
> >>
> >> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> >> index 9e6d726..aa87f30 100644
> >> --- a/drivers/net/designware.c
> >> +++ b/drivers/net/designware.c
> >> @@ -230,6 +230,23 @@ 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)
> >> +{
> >> + return -ENOSYS;
> >> +}
> >
> > Instead of a weak function I think this should use driver model, with
> > a driver supplied by the board to read this value. It should be
> > possible to supply the 'hardware-address reading' device to any
> > Ethernet driver, not just dwmmc.
>
> How do we reconcile something like that with the concern of using the
> On Thu, Nov 17, 2016 at 7:13 PM, Simon Glass <s...@chromium.org> wrote:
> > Hi Oliver,
> >
> > On 8 November 2016 at 08:54, Olliver Schinagl <oli...@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 <oli...@schinagl.nl>
> >> ---
> >> drivers/net/designware.c | 18 ++++++++++++++++++
> >> 1 file changed, 18 insertions(+)
> >>
> >> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> >> index 9e6d726..aa87f30 100644
> >> --- a/drivers/net/designware.c
> >> +++ b/drivers/net/designware.c
> >> @@ -230,6 +230,23 @@ 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)
> >> +{
> >> + return -ENOSYS;
> >> +}
> >
> > Instead of a weak function I think this should use driver model, with
> > a driver supplied by the board to read this value. It should be
> > possible to supply the 'hardware-address reading' device to any
> > Ethernet driver, not just dwmmc.
>
> device tree for boards using only Linux bindings, and sharing the
> device tree with Linux? Linux probably doesn't care about this and so
> won't have a binding for defining this relationship. This is a fairly
> generic question. Where have we landed on this?
So far I have not seen something that cannot be solved either as I
suggest above or with platform data.
Often you can pass platform data to the driver - e.g. see the end of
board_init() in gurnard.c which tells the video driver which LCD to
use.
Is there another case? I certainly have ideas but don't want to create
something without a solid case.
Regards,
Simon
Reply all
Reply to author
Forward
0 new messages