[U-Boot] [PATCH v2 0/5] Retrieve MAC address from EEPROM

[U-Boot] [PATCH v2 0/5] Retrieve MAC address from EEPROM

[Olliver Schinagl]

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 (5):
  net: Add ability to set MAC address via EEPROM to Kconfig
  sunxi: net: Allow the sunxi to set the MAC from an EEPROM
  sunxi: net: Enable eeprom on OLinuXino Lime2 boards
  tools: Allow crc8 to be used
  tools: Add tool to add crc8 to a mac address

 board/sunxi/Kconfig                   |  4 +++
 board/sunxi/board.c                   | 36 +++++++++++++++++++++
 configs/A20-OLinuXino-Lime2_defconfig |  3 ++
 doc/README.enetaddr                   | 34 ++++++++++++++++++++
 include/u-boot/crc.h                  |  3 ++
 net/Kconfig                           | 59 +++++++++++++++++++++++++++++++++++
 tools/Makefile                        |  5 +++
 tools/gen_mac_addr.c                  | 51 ++++++++++++++++++++++++++++++
 8 files changed, 195 insertions(+)
 create mode 100644 tools/gen_mac_addr.c

-- 
2.6.2

[U-Boot] [PATCH v2 1/5] net: Add ability to set MAC address via EEPROM to Kconfig

[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.schinagl@ultimaker.com>
---
 doc/README.enetaddr | 34 +++++++++++++++++++++++++++++++
 net/Kconfig         | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/doc/README.enetaddr b/doc/README.enetaddr
index 50e4899..1dc4119 100644
--- a/doc/README.enetaddr
+++ b/doc/README.enetaddr
@@ -47,6 +47,40 @@ 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.
+CONFIG_NET_ETHADDR_EEPROM_I2C_BUS I2C bus on which the eeprom is present and
+CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR set 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 header magic.
+
+Appending the 6 MAC bytes a CRC8 byte over the previous 6 bytes. Whether to check
+this CRC8 or not is dependent on CONFIG_NET_ETHADDR_EEPROM_CRC8.
+
+After the 6 bytes used for the MAC address, the 8th byte is used to indicate
+the ID of the network interface this MAC address is for. 0xff here means 'for
+the first available interface' and 0x00 means the first network interface, 0x01
+the second, etc. It is up to the platform however to enforce this.
+
+Layout example:
+
+00000000  21 4d 61 67 69 63 2e 21  01 23 45 67 89 ab be ff |!Magic.!.#Eg....|
+
+where the Header magic (!Magic.!) is spread over the first 8 bytes, and the MAC
+address '01-23-45-67-89-ab' with the CRC8 checksum 0xbe for any interface (0xff)
+
 -------
  Usage
 -------
diff --git a/net/Kconfig b/net/Kconfig
index a44a783..aced51e 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.6.2

[U-Boot] [PATCH v2 2/5] sunxi: net: Allow the sunxi to set the MAC from an EEPROM

[Olliver Schinagl]

This patch uses the newly introduced Kconfig options to set the MAC
address from an EEPROM, this will be especially useful for the Olimex
OLinuXino series of sunxi boards as they all have an 2k i2c eeprom chip.

The MAC address is in the eeprom is ignored if there is already a MAC
address in the environment or (if enabled) the CRC8 check fails.

Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
---
 board/sunxi/Kconfig |  4 ++++
 board/sunxi/board.c | 36 ++++++++++++++++++++++++++++++++++++
 net/Kconfig         |  1 +
 3 files changed, 41 insertions(+)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 2dd9d3b..a2da3a6 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -339,18 +339,21 @@ config I2C0_ENABLE
 
 config I2C1_ENABLE
 	bool "Enable I2C/TWI controller 1"
+	default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
 	default n
 	---help---
 	See I2C0_ENABLE help text.
 
 config I2C2_ENABLE
 	bool "Enable I2C/TWI controller 2"
+	default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
 	default n
 	---help---
 	See I2C0_ENABLE help text.
 
 if MACH_SUN6I || MACH_SUN7I
 config I2C3_ENABLE
+	default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
 	bool "Enable I2C/TWI controller 3"
 	default n
 	---help---
@@ -359,6 +362,7 @@ endif
 
 if MACH_SUN7I
 config I2C4_ENABLE
+	default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
 	bool "Enable I2C/TWI controller 4"
 	default n
 	---help---
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 6ac398c..28310a2 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -12,6 +12,7 @@
  */
 
 #include <common.h>
+#include <i2c.h>
 #include <mmc.h>
 #include <axp_pmic.h>
 #include <asm/arch/clock.h>
@@ -23,6 +24,7 @@
 #include <asm/arch/usb_phy.h>
 #include <asm/gpio.h>
 #include <asm/io.h>
+#include <linux/crc8.h>
 #include <nand.h>
 #include <net.h>
 
@@ -510,6 +512,37 @@ void get_board_serial(struct tag_serialnr *serialnr)
 }
 #endif
 
+#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
+static int read_mac_from_eeprom(uint8_t *mac_addr)
+{
+	uint8_t eeprom[7];
+	int old_i2c_bus;
+
+	old_i2c_bus = i2c_get_bus_num();
+	i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
+	if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
+		     CONFIG_NET_ETHADDR_EEPROM_OFFSET,
+		     CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
+		     eeprom, 7)) {
+		i2c_set_bus_num(old_i2c_bus);
+		puts("Could not read the EEPROM; EEPROM missing?\n");
+		return -1;
+	}
+	i2c_set_bus_num(old_i2c_bus);
+#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
+	if (crc8(eeprom, 6) != eeprom[6]) {
+		puts("CRC error on MAC address from EEPROM.\n");
+		return -1;
+	}
+#endif
+	memcpy(mac_addr, eeprom, 6);
+
+	return 0;
+}
+#else
+static int read_mac_from_eeprom(uint8_t *mac_addr) { return -1; }
+#endif
+
 #if !defined(CONFIG_SPL_BUILD)
 #include <asm/arch/spl.h>
 
@@ -553,6 +586,9 @@ int misc_init_r(void)
 	}
 #endif
 
+	if ((!getenv("ethaddr")) && (!read_mac_from_eeprom(mac_addr)))
+		eth_setenv_enetaddr("ethaddr", mac_addr);
+
 	ret = sunxi_get_sid(sid);
 	if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
 		if (!getenv("ethaddr")) {
diff --git a/net/Kconfig b/net/Kconfig
index aced51e..0f4cc5a 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -8,6 +8,7 @@ menuconfig NET
 if NET
 
 config NET_ETHADDR_EEPROM
+	depends on ARCH_SUNXI
 	bool "Get ethaddr from eeprom"
 	help
 	  Selecting this will try to get the Ethernet address from an onboard
-- 
2.6.2

[U-Boot] [PATCH v2 3/5] sunxi: net: Enable eeprom on OLinuXino Lime2 boards

[Olliver Schinagl]

This patch enables the I2C EEPROM to be probed for a MAC address on the
OLinuXino board.

Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
---
 configs/A20-OLinuXino-Lime2_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/A20-OLinuXino-Lime2_defconfig b/configs/A20-OLinuXino-Lime2_defconfig
index b5181c6..5e38618 100644
--- a/configs/A20-OLinuXino-Lime2_defconfig
+++ b/configs/A20-OLinuXino-Lime2_defconfig
@@ -14,4 +14,7 @@ CONFIG_SYS_EXTRA_OPTIONS="SUNXI_GMAC,RGMII,AHCI,SATAPWR=SUNXI_GPC(3)"
 # CONFIG_CMD_FPGA is not set
 CONFIG_CMD_GPIO=y
 CONFIG_ETH_DESIGNWARE=y
+CONFIG_NET_ETHADDR_EEPROM=y
+CONFIG_NET_ETHADDR_EEPROM_I2C=y
+CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
 CONFIG_USB_EHCI_HCD=y
-- 
2.6.2

[U-Boot] [PATCH v2 4/5] tools: Allow crc8 to be used

[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.schinagl@ultimaker.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..cbcf175 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(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 9cfd80b..4a50744 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -167,6 +167,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.6.2

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[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.

Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
---
 tools/Makefile       |  4 ++++
 tools/gen_mac_addr.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 tools/gen_mac_addr.c

diff --git a/tools/Makefile b/tools/Makefile
index 4a50744..6191c26 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_mac_addr
+gen_mac_addr-objs := gen_mac_addr.o lib/crc8.o
+HOSTCFLAGS_gen_mac_addr.o := -pedantic
+
 hostprogs-$(CONFIG_CMD_LOADS) += img2srec
 HOSTCFLAGS_img2srec.o := -pedantic
 
diff --git a/tools/gen_mac_addr.c b/tools/gen_mac_addr.c
new file mode 100644
index 0000000..bd8688f
--- /dev/null
+++ b/tools/gen_mac_addr.c
@@ -0,0 +1,51 @@
+/*
+ * (C) Copyright 2016
+ * Olliver Schinagl <o.schinagl@ultimaker.com>
+ *
+ * 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>
+
+
+int main(int argc, char *argv[])
+{
+	uint_fast8_t i;
+	uint8_t mac_addr[7] = { 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[6] = crc8(mac_addr, 6);
+
+	for (i = 0; i < 7; i++)
+		printf("%.2x", mac_addr[i]);
+	putchar('\n');
+
+	return 0;
+}
-- 
2.6.2

[U-Boot] [PATCH v2 1/5] net: Add ability to set MAC address via EEPROM to Kconfig

[Joe Hershberger]

On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> 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.schinagl@ultimaker.com>

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

[U-Boot] [PATCH v2 2/5] sunxi: net: Allow the sunxi to set the MAC from an EEPROM

[Joe Hershberger]

On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> This patch uses the newly introduced Kconfig options to set the MAC
> address from an EEPROM, this will be especially useful for the Olimex
> OLinuXino series of sunxi boards as they all have an 2k i2c eeprom chip.
>
> The MAC address is in the eeprom is ignored if there is already a MAC

"The MAC address in the eeprom is ignored..."

> address in the environment or (if enabled) the CRC8 check fails.
>
> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
> ---
>  board/sunxi/Kconfig |  4 ++++
>  board/sunxi/board.c | 36 ++++++++++++++++++++++++++++++++++++
>  net/Kconfig         |  1 +
>  3 files changed, 41 insertions(+)
>
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index 2dd9d3b..a2da3a6 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -339,18 +339,21 @@ config I2C0_ENABLE
>
>  config I2C1_ENABLE
>         bool "Enable I2C/TWI controller 1"
> +       default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
>         default n
>         ---help---
>         See I2C0_ENABLE help text.
>
>  config I2C2_ENABLE
>         bool "Enable I2C/TWI controller 2"
> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
>         default n
>         ---help---
>         See I2C0_ENABLE help text.
>
>  if MACH_SUN6I || MACH_SUN7I
>  config I2C3_ENABLE
> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
>         bool "Enable I2C/TWI controller 3"
>         default n
>         ---help---
> @@ -359,6 +362,7 @@ endif
>
>  if MACH_SUN7I
>  config I2C4_ENABLE
> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
>         bool "Enable I2C/TWI controller 4"
>         default n
>         ---help---
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 6ac398c..28310a2 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -12,6 +12,7 @@
>   */
>
>  #include <common.h>
> +#include <i2c.h>
>  #include <mmc.h>
>  #include <axp_pmic.h>
>  #include <asm/arch/clock.h>
> @@ -23,6 +24,7 @@
>  #include <asm/arch/usb_phy.h>
>  #include <asm/gpio.h>
>  #include <asm/io.h>
> +#include <linux/crc8.h>
>  #include <nand.h>
>  #include <net.h>
>
> @@ -510,6 +512,37 @@ void get_board_serial(struct tag_serialnr *serialnr)
>  }
>  #endif
>
> +#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
> +static int read_mac_from_eeprom(uint8_t *mac_addr)
> +{
> +       uint8_t eeprom[7];
> +       int old_i2c_bus;
> +
> +       old_i2c_bus = i2c_get_bus_num();
> +       i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
> +       if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
> +                    CONFIG_NET_ETHADDR_EEPROM_OFFSET,
> +                    CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
> +                    eeprom, 7)) {
> +               i2c_set_bus_num(old_i2c_bus);
> +               puts("Could not read the EEPROM; EEPROM missing?\n");
> +               return -1;
> +       }
> +       i2c_set_bus_num(old_i2c_bus);
> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
> +       if (crc8(eeprom, 6) != eeprom[6]) {
> +               puts("CRC error on MAC address from EEPROM.\n");
> +               return -1;
> +       }
> +#endif
> +       memcpy(mac_addr, eeprom, 6);
> +
> +       return 0;
> +}
> +#else
> +static int read_mac_from_eeprom(uint8_t *mac_addr) { return -1; }
> +#endif
> +
>  #if !defined(CONFIG_SPL_BUILD)
>  #include <asm/arch/spl.h>
>
> @@ -553,6 +586,9 @@ int misc_init_r(void)
>         }
>  #endif
>
> +       if ((!getenv("ethaddr")) && (!read_mac_from_eeprom(mac_addr)))
> +               eth_setenv_enetaddr("ethaddr", mac_addr);

It seems a bit odd to actually write this to the environment instead
of it being a read_rom_hwaddr operation for the Ethernet driver that
this board uses.

Do we need a different board-level ROM callback? Maybe the drivers
used in such situations can be the only ones to support it.

> +
>         ret = sunxi_get_sid(sid);
>         if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
>                 if (!getenv("ethaddr")) {
> diff --git a/net/Kconfig b/net/Kconfig
> index aced51e..0f4cc5a 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -8,6 +8,7 @@ menuconfig NET
>  if NET
>
>  config NET_ETHADDR_EEPROM
> +       depends on ARCH_SUNXI
>         bool "Get ethaddr from eeprom"
>         help
>           Selecting this will try to get the Ethernet address from an onboard
> --
> 2.6.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/5] sunxi: net: Enable eeprom on OLinuXino Lime2 boards

[Joe Hershberger]

Hi Olliver,

On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> This patch enables the I2C EEPROM to be probed for a MAC address on the
> OLinuXino board.
>
> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
> ---

You should use patman so that you can keep track of changes in
revisions of your patches.

>  configs/A20-OLinuXino-Lime2_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/configs/A20-OLinuXino-Lime2_defconfig b/configs/A20-OLinuXino-Lime2_defconfig
> index b5181c6..5e38618 100644
> --- a/configs/A20-OLinuXino-Lime2_defconfig
> +++ b/configs/A20-OLinuXino-Lime2_defconfig
> @@ -14,4 +14,7 @@ CONFIG_SYS_EXTRA_OPTIONS="SUNXI_GMAC,RGMII,AHCI,SATAPWR=SUNXI_GPC(3)"
>  # CONFIG_CMD_FPGA is not set
>  CONFIG_CMD_GPIO=y
>  CONFIG_ETH_DESIGNWARE=y
> +CONFIG_NET_ETHADDR_EEPROM=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C=y
> +CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
>  CONFIG_USB_EHCI_HCD=y
> --
> 2.6.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 4/5] tools: Allow crc8 to be used

[Joe Hershberger]

On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> This patch enables crc8 to be used from within the tools directory using
> u-boot/crc.h.
>
> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>

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

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Joe Hershberger]

On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> 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.
>
> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
> ---
>  tools/Makefile       |  4 ++++
>  tools/gen_mac_addr.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
>  create mode 100644 tools/gen_mac_addr.c
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 4a50744..6191c26 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_mac_addr
> +gen_mac_addr-objs := gen_mac_addr.o lib/crc8.o
> +HOSTCFLAGS_gen_mac_addr.o := -pedantic
> +
>  hostprogs-$(CONFIG_CMD_LOADS) += img2srec
>  HOSTCFLAGS_img2srec.o := -pedantic
>
> diff --git a/tools/gen_mac_addr.c b/tools/gen_mac_addr.c
> new file mode 100644
> index 0000000..bd8688f
> --- /dev/null
> +++ b/tools/gen_mac_addr.c

This is not "generating a mac address", right? Its point is to create
a CRC for the user-supplied MAC address.

Perhaps a better name would be "gen_ethaddr_rom_crc".

> @@ -0,0 +1,51 @@
> +/*
> + * (C) Copyright 2016
> + * Olliver Schinagl <o.schinagl@ultimaker.com>
> + *
> + * 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>
> +
> +
> +int main(int argc, char *argv[])
> +{
> +       uint_fast8_t i;
> +       uint8_t mac_addr[7] = { 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;
> +       }

You could use the eth_validate_ethaddr_str() function here instead of this code.

> +
> +       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++;
> +               }
> +       }

Instead of all this you could just compile in (maybe it already is?)
the eth_parse_enetaddr() function that U-Boot uses for this very
purpose.

> +       mac_addr[6] = crc8(mac_addr, 6);
> +
> +       for (i = 0; i < 7; i++)
> +               printf("%.2x", mac_addr[i]);
> +       putchar('\n');
> +
> +       return 0;
> +}
> --
> 2.6.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Albert ARIBAUD]

Hello Joe,

On Mon, 25 Jan 2016 18:45:52 -0600, Joe Hershberger
<joe.hershberger@gmail.com> wrote:
> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:

> > +++ b/tools/gen_mac_addr.c
> 
> This is not "generating a mac address", right? Its point is to create
> a CRC for the user-supplied MAC address.
> 
> Perhaps a better name would be "gen_ethaddr_rom_crc".

Hmm, why the "_rom" part?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Joe Hershberger]

Hi Albert,

On Tue, Jan 26, 2016 at 7:15 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hello Joe,
>
> On Mon, 25 Jan 2016 18:45:52 -0600, Joe Hershberger
> <joe.hershberger@gmail.com> wrote:
>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>> <o.schinagl@ultimaker.com> wrote:
>
>> > +++ b/tools/gen_mac_addr.c
>>
>> This is not "generating a mac address", right? Its point is to create
>> a CRC for the user-supplied MAC address.
>>
>> Perhaps a better name would be "gen_ethaddr_rom_crc".
>
> Hmm, why the "_rom" part?

Just to give more hints to the observer that this is for creating a
rom or eeprom image to be read by the associated new code in U-Boot.

Cheers,
-Joe

>
> Amicalement,
> --
> Albert.

[U-Boot] [PATCH v2 2/5] sunxi: net: Allow the sunxi to set the MAC from an EEPROM

[Olliver Schinagl]

Hey Joe

On 26-01-16 01:32, Joe Hershberger wrote:
> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>> This patch uses the newly introduced Kconfig options to set the MAC
>> address from an EEPROM, this will be especially useful for the Olimex
>> OLinuXino series of sunxi boards as they all have an 2k i2c eeprom chip.
>>
>> The MAC address is in the eeprom is ignored if there is already a MAC
> "The MAC address in the eeprom is ignored..."
oops :) fixed for v3
>
>> address in the environment or (if enabled) the CRC8 check fails.
>>
>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>> ---
>>   board/sunxi/Kconfig |  4 ++++
>>   board/sunxi/board.c | 36 ++++++++++++++++++++++++++++++++++++
>>   net/Kconfig         |  1 +
>>   3 files changed, 41 insertions(+)
>>
>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>> index 2dd9d3b..a2da3a6 100644
>> --- a/board/sunxi/Kconfig
>> +++ b/board/sunxi/Kconfig
>> @@ -339,18 +339,21 @@ config I2C0_ENABLE
>>
>>   config I2C1_ENABLE
>>          bool "Enable I2C/TWI controller 1"
>> +       default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
>>          default n
>>          ---help---
>>          See I2C0_ENABLE help text.
>>
>>   config I2C2_ENABLE
>>          bool "Enable I2C/TWI controller 2"
>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
>>          default n
>>          ---help---
>>          See I2C0_ENABLE help text.
>>
>>   if MACH_SUN6I || MACH_SUN7I
>>   config I2C3_ENABLE
>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
>>          bool "Enable I2C/TWI controller 3"
>>          default n
>>          ---help---
>> @@ -359,6 +362,7 @@ endif
>>
>>   if MACH_SUN7I
>>   config I2C4_ENABLE
>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
>>          bool "Enable I2C/TWI controller 4"
>>          default n
>>          ---help---
>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> index 6ac398c..28310a2 100644
>> --- a/board/sunxi/board.c
>> +++ b/board/sunxi/board.c
>> @@ -12,6 +12,7 @@
>>    */
>>
>>   #include <common.h>
>> +#include <i2c.h>
>>   #include <mmc.h>
>>   #include <axp_pmic.h>
>>   #include <asm/arch/clock.h>
>> @@ -23,6 +24,7 @@
>>   #include <asm/arch/usb_phy.h>
>>   #include <asm/gpio.h>
>>   #include <asm/io.h>
>> +#include <linux/crc8.h>
>>   #include <nand.h>
>>   #include <net.h>
>>
>> @@ -510,6 +512,37 @@ void get_board_serial(struct tag_serialnr *serialnr)
>>   }
>>   #endif
>>
>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) && defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
>> +static int read_mac_from_eeprom(uint8_t *mac_addr)
>> +{
>> +       uint8_t eeprom[7];
>> +       int old_i2c_bus;
>> +
>> +       old_i2c_bus = i2c_get_bus_num();
>> +       i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
>> +       if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
>> +                    CONFIG_NET_ETHADDR_EEPROM_OFFSET,
>> +                    CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
>> +                    eeprom, 7)) {
>> +               i2c_set_bus_num(old_i2c_bus);
>> +               puts("Could not read the EEPROM; EEPROM missing?\n");
>> +               return -1;
>> +       }
>> +       i2c_set_bus_num(old_i2c_bus);
>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
>> +       if (crc8(eeprom, 6) != eeprom[6]) {
>> +               puts("CRC error on MAC address from EEPROM.\n");
>> +               return -1;
>> +       }
>> +#endif
>> +       memcpy(mac_addr, eeprom, 6);
>> +
>> +       return 0;
>> +}
>> +#else
>> +static int read_mac_from_eeprom(uint8_t *mac_addr) { return -1; }
>> +#endif
>> +
>>   #if !defined(CONFIG_SPL_BUILD)
>>   #include <asm/arch/spl.h>
>>
>> @@ -553,6 +586,9 @@ int misc_init_r(void)
>>          }
>>   #endif
>>
>> +       if ((!getenv("ethaddr")) && (!read_mac_from_eeprom(mac_addr)))
>> +               eth_setenv_enetaddr("ethaddr", mac_addr);
> It seems a bit odd to actually write this to the environment instead
> of it being a read_rom_hwaddr operation for the Ethernet driver that
> this board uses.
>
> Do we need a different board-level ROM callback? Maybe the drivers
> used in such situations can be the only ones to support it.
I'm hoping your thinking out-loud, as I'm not too familiar with all the 
backing code. But if I understand you correctly, you suggest to 
implement a new callback for ethernet drivers to get mac's from eeprom? 
Anyhow, I just followed suit with how the address gets set in case of 
when it is not in the environment and we need to generate a random one.

I was reading the u-boot documentation and though the assumed method was 
to use the environment and to only use other methods (such as the eeprom 
method) where it is absolutely a must. In this case the eeprom is not 
directly related to the ethernet, it's a generic eeprom.

But then again, I am new to this stuff so I can be gladly very wrong here :)


>
>> +
>>          ret = sunxi_get_sid(sid);
>>          if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
>>                  if (!getenv("ethaddr")) {
>> diff --git a/net/Kconfig b/net/Kconfig
>> index aced51e..0f4cc5a 100644
>> --- a/net/Kconfig
>> +++ b/net/Kconfig
>> @@ -8,6 +8,7 @@ menuconfig NET
>>   if NET
>>
>>   config NET_ETHADDR_EEPROM
>> +       depends on ARCH_SUNXI
>>          bool "Get ethaddr from eeprom"
>>          help
>>            Selecting this will try to get the Ethernet address from an onboard
>> --
>> 2.6.2
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot

-- 
Met vriendelijke groeten, Kind regards, ??????

Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.

[U-Boot] [PATCH v2 3/5] sunxi: net: Enable eeprom on OLinuXino Lime2 boards

[Olliver Schinagl]

Hey Joe,

On 26-01-16 01:34, Joe Hershberger wrote:
> Hi Olliver,
>
> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>> This patch enables the I2C EEPROM to be probed for a MAC address on the
>> OLinuXino board.
>>
>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>> ---
> You should use patman so that you can keep track of changes in
> revisions of your patches.
I just looked up on it and it looks like a good tool! So I will try to 
get familiar with it, thanks for the tip!
>
>>   configs/A20-OLinuXino-Lime2_defconfig | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/configs/A20-OLinuXino-Lime2_defconfig b/configs/A20-OLinuXino-Lime2_defconfig
>> index b5181c6..5e38618 100644
>> --- a/configs/A20-OLinuXino-Lime2_defconfig
>> +++ b/configs/A20-OLinuXino-Lime2_defconfig
>> @@ -14,4 +14,7 @@ CONFIG_SYS_EXTRA_OPTIONS="SUNXI_GMAC,RGMII,AHCI,SATAPWR=SUNXI_GPC(3)"
>>   # CONFIG_CMD_FPGA is not set
>>   CONFIG_CMD_GPIO=y
>>   CONFIG_ETH_DESIGNWARE=y
>> +CONFIG_NET_ETHADDR_EEPROM=y
>> +CONFIG_NET_ETHADDR_EEPROM_I2C=y
>> +CONFIG_NET_ETHADDR_EEPROM_I2C_BUS=1
>>   CONFIG_USB_EHCI_HCD=y
>> --
>> 2.6.2
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot

-- 
Met vriendelijke groeten, Kind regards, ??????

Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.

[U-Boot] [PATCH v2 2/5] sunxi: net: Allow the sunxi to set the MAC from an EEPROM

[Joe Hershberger]

On Tue, Jan 26, 2016 at 10:10 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> Hey Joe
>
> On 26-01-16 01:32, Joe Hershberger wrote:
>>
>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>> <o.schinagl@ultimaker.com> wrote:
>>>
>>> This patch uses the newly introduced Kconfig options to set the MAC
>>> address from an EEPROM, this will be especially useful for the Olimex
>>> OLinuXino series of sunxi boards as they all have an 2k i2c eeprom chip.
>>>
>>> The MAC address is in the eeprom is ignored if there is already a MAC
>>
>> "The MAC address in the eeprom is ignored..."
>
> oops :) fixed for v3
>
>>
>>> address in the environment or (if enabled) the CRC8 check fails.
>>>
>>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>>> ---
>>>   board/sunxi/Kconfig |  4 ++++
>>>   board/sunxi/board.c | 36 ++++++++++++++++++++++++++++++++++++
>>>   net/Kconfig         |  1 +
>>>   3 files changed, 41 insertions(+)
>>>
>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>> index 2dd9d3b..a2da3a6 100644
>>> --- a/board/sunxi/Kconfig
>>> +++ b/board/sunxi/Kconfig
>>> @@ -339,18 +339,21 @@ config I2C0_ENABLE
>>>
>>>   config I2C1_ENABLE
>>>          bool "Enable I2C/TWI controller 1"
>>> +       default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
>>>          default n
>>>          ---help---
>>>          See I2C0_ENABLE help text.
>>>
>>>   config I2C2_ENABLE
>>>          bool "Enable I2C/TWI controller 2"
>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
>>>          default n
>>>          ---help---
>>>          See I2C0_ENABLE help text.
>>>
>>>   if MACH_SUN6I || MACH_SUN7I
>>>   config I2C3_ENABLE
>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
>>>          bool "Enable I2C/TWI controller 3"
>>>          default n
>>>          ---help---
>>> @@ -359,6 +362,7 @@ endif
>>>
>>>   if MACH_SUN7I
>>>   config I2C4_ENABLE
>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
>>>          bool "Enable I2C/TWI controller 4"
>>>          default n
>>>          ---help---
>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>> index 6ac398c..28310a2 100644
>>> --- a/board/sunxi/board.c
>>> +++ b/board/sunxi/board.c
>>> @@ -12,6 +12,7 @@
>>>    */
>>>
>>>   #include <common.h>
>>> +#include <i2c.h>
>>>   #include <mmc.h>
>>>   #include <axp_pmic.h>
>>>   #include <asm/arch/clock.h>
>>> @@ -23,6 +24,7 @@
>>>   #include <asm/arch/usb_phy.h>
>>>   #include <asm/gpio.h>
>>>   #include <asm/io.h>
>>> +#include <linux/crc8.h>
>>>   #include <nand.h>
>>>   #include <net.h>
>>>
>>> @@ -510,6 +512,37 @@ void get_board_serial(struct tag_serialnr *serialnr)
>>>   }
>>>   #endif
>>>
>>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) &&
>>> defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
>>> +static int read_mac_from_eeprom(uint8_t *mac_addr)
>>> +{
>>> +       uint8_t eeprom[7];
>>> +       int old_i2c_bus;
>>> +
>>> +       old_i2c_bus = i2c_get_bus_num();
>>> +       i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
>>> +       if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
>>> +                    CONFIG_NET_ETHADDR_EEPROM_OFFSET,
>>> +                    CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
>>> +                    eeprom, 7)) {
>>> +               i2c_set_bus_num(old_i2c_bus);
>>> +               puts("Could not read the EEPROM; EEPROM missing?\n");
>>> +               return -1;
>>> +       }
>>> +       i2c_set_bus_num(old_i2c_bus);
>>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
>>> +       if (crc8(eeprom, 6) != eeprom[6]) {
>>> +               puts("CRC error on MAC address from EEPROM.\n");
>>> +               return -1;
>>> +       }
>>> +#endif
>>> +       memcpy(mac_addr, eeprom, 6);
>>> +
>>> +       return 0;
>>> +}
>>> +#else
>>> +static int read_mac_from_eeprom(uint8_t *mac_addr) { return -1; }
>>> +#endif
>>> +
>>>   #if !defined(CONFIG_SPL_BUILD)
>>>   #include <asm/arch/spl.h>
>>>
>>> @@ -553,6 +586,9 @@ int misc_init_r(void)
>>>          }
>>>   #endif
>>>
>>> +       if ((!getenv("ethaddr")) && (!read_mac_from_eeprom(mac_addr)))
>>> +               eth_setenv_enetaddr("ethaddr", mac_addr);
>>
>> It seems a bit odd to actually write this to the environment instead
>> of it being a read_rom_hwaddr operation for the Ethernet driver that
>> this board uses.
>>
>> Do we need a different board-level ROM callback? Maybe the drivers
>> used in such situations can be the only ones to support it.
>
> I'm hoping your thinking out-loud, as I'm not too familiar with all the

Indeed. I was thinking out loud.

> backing code. But if I understand you correctly, you suggest to implement a
> new callback for ethernet drivers to get mac's from eeprom? Anyhow, I just

Not for now... at least not in the core code.

> followed suit with how the address gets set in case of when it is not in the
> environment and we need to generate a random one.

Sure. I don't think it's the same case though. We already have the
entry for the driver to read the rom ethaddr. I think you should use
that.

> I was reading the u-boot documentation and though the assumed method was to
> use the environment and to only use other methods (such as the eeprom
> method) where it is absolutely a must. In this case the eeprom is not
> directly related to the ethernet, it's a generic eeprom.

Sure. I think you should add support to whatever driver your board
uses (it's a driver model driver, right?) for a weak board function.
That way that one driver is responsible for that behavior because of
your board's need. Any other boards that use that driver just don't
supply that function and it is a no-op. I'd rather be able to easily
follow where that ethaddr is getting configured instead of using
misc_init_r() and having the env populate magically. This also means
you'll be in a context to write the ethaddr to the pdata struct
instead of the env.

>
> But then again, I am new to this stuff so I can be gladly very wrong here :)

Seems it's not a case that comes up, so it's fairly new territory.

>
>
>>
>>> +
>>>          ret = sunxi_get_sid(sid);
>>>          if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
>>>                  if (!getenv("ethaddr")) {
>>> diff --git a/net/Kconfig b/net/Kconfig
>>> index aced51e..0f4cc5a 100644
>>> --- a/net/Kconfig
>>> +++ b/net/Kconfig
>>> @@ -8,6 +8,7 @@ menuconfig NET
>>>   if NET
>>>
>>>   config NET_ETHADDR_EEPROM
>>> +       depends on ARCH_SUNXI
>>>          bool "Get ethaddr from eeprom"
>>>          help
>>>            Selecting this will try to get the Ethernet address from an
>>> onboard

Thanks,
-Joe

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Olliver Schinagl]

Hey Joe,

On 26-01-16 01:45, Joe Hershberger wrote:
> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>> 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.
>>
>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>> ---
>>   tools/Makefile       |  4 ++++
>>   tools/gen_mac_addr.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 55 insertions(+)
>>   create mode 100644 tools/gen_mac_addr.c
>>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 4a50744..6191c26 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_mac_addr
>> +gen_mac_addr-objs := gen_mac_addr.o lib/crc8.o
>> +HOSTCFLAGS_gen_mac_addr.o := -pedantic
>> +
>>   hostprogs-$(CONFIG_CMD_LOADS) += img2srec
>>   HOSTCFLAGS_img2srec.o := -pedantic
>>
>> diff --git a/tools/gen_mac_addr.c b/tools/gen_mac_addr.c
>> new file mode 100644
>> index 0000000..bd8688f
>> --- /dev/null
>> +++ b/tools/gen_mac_addr.c
> This is not "generating a mac address", right? Its point is to create
> a CRC for the user-supplied MAC address.
>
> Perhaps a better name would be "gen_ethaddr_rom_crc".
Yes, it takes a mac address as input and generates a macaddress + crc as 
output.

e.g. 11:22:33:44:55:66 -> 0x11 0x22 0x33 0x44 0x55 0x66 0x77 0xcc

But maybe it should get some flags to output either both (as described 
here) or just the crc based on the input, and then a few flags to either 
output it in hex format or as an int?
>
>> @@ -0,0 +1,51 @@
>> +/*
>> + * (C) Copyright 2016
>> + * Olliver Schinagl <o.schinagl@ultimaker.com>
>> + *
>> + * 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>
>> +
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +       uint_fast8_t i;
>> +       uint8_t mac_addr[7] = { 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;
>> +       }
> You could use the eth_validate_ethaddr_str() function here instead of this code.
A right, well when I first wrote this, I think this didn't exist there 
yet. I'll replace it and use it instead.
>
>> +
>> +       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++;
>> +               }
>> +       }
> Instead of all this you could just compile in (maybe it already is?)
> the eth_parse_enetaddr() function that U-Boot uses for this very
> purpose.
I'll see if I can have access to this function and then rewrite this to 
use that.
>
>> +       mac_addr[6] = crc8(mac_addr, 6);
>> +
>> +       for (i = 0; i < 7; i++)
>> +               printf("%.2x", mac_addr[i]);
>> +       putchar('\n');
>> +
>> +       return 0;
>> +}
>> --
>> 2.6.2
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot

-- 
Met vriendelijke groeten, Kind regards, ??????

Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Joe Hershberger]

On Tue, Jan 26, 2016 at 10:27 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> Hey Joe,
>
>
> On 26-01-16 01:45, Joe Hershberger wrote:
>>
>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>> <o.schinagl@ultimaker.com> wrote:
>>>
>>> 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.
>>>
>>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>>> ---
>>>   tools/Makefile       |  4 ++++
>>>   tools/gen_mac_addr.c | 51
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 55 insertions(+)
>>>   create mode 100644 tools/gen_mac_addr.c
>>>
>>> diff --git a/tools/Makefile b/tools/Makefile
>>> index 4a50744..6191c26 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_mac_addr
>>> +gen_mac_addr-objs := gen_mac_addr.o lib/crc8.o
>>> +HOSTCFLAGS_gen_mac_addr.o := -pedantic
>>> +
>>>   hostprogs-$(CONFIG_CMD_LOADS) += img2srec
>>>   HOSTCFLAGS_img2srec.o := -pedantic
>>>
>>> diff --git a/tools/gen_mac_addr.c b/tools/gen_mac_addr.c
>>> new file mode 100644
>>> index 0000000..bd8688f
>>> --- /dev/null
>>> +++ b/tools/gen_mac_addr.c
>>
>> This is not "generating a mac address", right? Its point is to create
>> a CRC for the user-supplied MAC address.
>>
>> Perhaps a better name would be "gen_ethaddr_rom_crc".
>
> Yes, it takes a mac address as input and generates a macaddress + crc as
> output.
>
> e.g. 11:22:33:44:55:66 -> 0x11 0x22 0x33 0x44 0x55 0x66 0x77 0xcc
>
> But maybe it should get some flags to output either both (as described here)
> or just the crc based on the input, and then a few flags to either output it
> in hex format or as an int?

What ever features you think will be useful for the users of this dev
board I guess. I'm not sure if this is a platform (the included
eeprom) that would carry over into other products.

>>
>>> @@ -0,0 +1,51 @@
>>> +/*
>>> + * (C) Copyright 2016
>>> + * Olliver Schinagl <o.schinagl@ultimaker.com>
>>> + *
>>> + * 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>
>>> +
>>> +
>>> +int main(int argc, char *argv[])
>>> +{
>>> +       uint_fast8_t i;
>>> +       uint8_t mac_addr[7] = { 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;
>>> +       }
>>
>> You could use the eth_validate_ethaddr_str() function here instead of this
>> code.
>
> A right, well when I first wrote this, I think this didn't exist there yet.
> I'll replace it and use it instead.
>>
>>
>>> +
>>> +       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++;
>>> +               }
>>> +       }
>>
>> Instead of all this you could just compile in (maybe it already is?)
>> the eth_parse_enetaddr() function that U-Boot uses for this very
>> purpose.
>
> I'll see if I can have access to this function and then rewrite this to use
> that.
>
>>
>>> +       mac_addr[6] = crc8(mac_addr, 6);
>>> +
>>> +       for (i = 0; i < 7; i++)
>>> +               printf("%.2x", mac_addr[i]);
>>> +       putchar('\n');
>>> +
>>> +       return 0;
>>> +}
>>> --
>>> 2.6.2
>>>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>
>
> --
> Met vriendelijke groeten, Kind regards, ??????
>
> Olliver Schinagl
> Software Engineer
> Research & Development
> Ultimaker B.V.
>

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Olliver Schinagl]

Hey all,

On 26-01-16 15:56, Joe Hershberger wrote:
> Hi Albert,
>
> On Tue, Jan 26, 2016 at 7:15 AM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Hello Joe,
>>
>> On Mon, 25 Jan 2016 18:45:52 -0600, Joe Hershberger
>> <joe.hershberger@gmail.com> wrote:
>>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>>> <o.schinagl@ultimaker.com> wrote:
>>>> +++ b/tools/gen_mac_addr.c
>>> This is not "generating a mac address", right? Its point is to create
>>> a CRC for the user-supplied MAC address.
>>>
>>> Perhaps a better name would be "gen_ethaddr_rom_crc".
>> Hmm, why the "_rom" part?
> Just to give more hints to the observer that this is for creating a
> rom or eeprom image to be read by the associated new code in U-Boot.
Well depending on the output, it could even be used to verify what is 
stored in an ROM. So i understand Albert's reasoning that it may not be 
related to ROM at all
>
> Cheers,
> -Joe
>
>> Amicalement,
>> --
>> Albert.

-- 
Met vriendelijke groeten, Kind regards, ??????

Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.

[U-Boot] [PATCH v2 2/5] sunxi: net: Allow the sunxi to set the MAC from an EEPROM

[Olliver Schinagl]

Hey Joe,

On 26-01-16 17:23, Joe Hershberger wrote:
> On Tue, Jan 26, 2016 at 10:10 AM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>> Hey Joe
>>
>> On 26-01-16 01:32, Joe Hershberger wrote:
>>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>>> <o.schinagl@ultimaker.com> wrote:
>>>> This patch uses the newly introduced Kconfig options to set the MAC
>>>> address from an EEPROM, this will be especially useful for the Olimex
>>>> OLinuXino series of sunxi boards as they all have an 2k i2c eeprom chip.
>>>>
>>>> The MAC address is in the eeprom is ignored if there is already a MAC
>>> "The MAC address in the eeprom is ignored..."
>> oops :) fixed for v3
>>
>>>> address in the environment or (if enabled) the CRC8 check fails.
>>>>
>>>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>>>> ---
>>>>    board/sunxi/Kconfig |  4 ++++
>>>>    board/sunxi/board.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>    net/Kconfig         |  1 +
>>>>    3 files changed, 41 insertions(+)
>>>>
>>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>>> index 2dd9d3b..a2da3a6 100644
>>>> --- a/board/sunxi/Kconfig
>>>> +++ b/board/sunxi/Kconfig
>>>> @@ -339,18 +339,21 @@ config I2C0_ENABLE
>>>>
>>>>    config I2C1_ENABLE
>>>>           bool "Enable I2C/TWI controller 1"
>>>> +       default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
>>>>           default n
>>>>           ---help---
>>>>           See I2C0_ENABLE help text.
>>>>
>>>>    config I2C2_ENABLE
>>>>           bool "Enable I2C/TWI controller 2"
>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
>>>>           default n
>>>>           ---help---
>>>>           See I2C0_ENABLE help text.
>>>>
>>>>    if MACH_SUN6I || MACH_SUN7I
>>>>    config I2C3_ENABLE
>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
>>>>           bool "Enable I2C/TWI controller 3"
>>>>           default n
>>>>           ---help---
>>>> @@ -359,6 +362,7 @@ endif
>>>>
>>>>    if MACH_SUN7I
>>>>    config I2C4_ENABLE
>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
>>>>           bool "Enable I2C/TWI controller 4"
>>>>           default n
>>>>           ---help---
>>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>>> index 6ac398c..28310a2 100644
>>>> --- a/board/sunxi/board.c
>>>> +++ b/board/sunxi/board.c
>>>> @@ -12,6 +12,7 @@
>>>>     */
>>>>
>>>>    #include <common.h>
>>>> +#include <i2c.h>
>>>>    #include <mmc.h>
>>>>    #include <axp_pmic.h>
>>>>    #include <asm/arch/clock.h>
>>>> @@ -23,6 +24,7 @@
>>>>    #include <asm/arch/usb_phy.h>
>>>>    #include <asm/gpio.h>
>>>>    #include <asm/io.h>
>>>> +#include <linux/crc8.h>
>>>>    #include <nand.h>
>>>>    #include <net.h>
>>>>
>>>> @@ -510,6 +512,37 @@ void get_board_serial(struct tag_serialnr *serialnr)
>>>>    }
>>>>    #endif
>>>>
>>>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) &&
>>>> defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
>>>> +static int read_mac_from_eeprom(uint8_t *mac_addr)
>>>> +{
>>>> +       uint8_t eeprom[7];
>>>> +       int old_i2c_bus;
>>>> +
>>>> +       old_i2c_bus = i2c_get_bus_num();
>>>> +       i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
>>>> +       if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
>>>> +                    CONFIG_NET_ETHADDR_EEPROM_OFFSET,
>>>> +                    CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
>>>> +                    eeprom, 7)) {
>>>> +               i2c_set_bus_num(old_i2c_bus);
>>>> +               puts("Could not read the EEPROM; EEPROM missing?\n");
>>>> +               return -1;
>>>> +       }
>>>> +       i2c_set_bus_num(old_i2c_bus);
>>>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
>>>> +       if (crc8(eeprom, 6) != eeprom[6]) {
>>>> +               puts("CRC error on MAC address from EEPROM.\n");
>>>> +               return -1;
>>>> +       }
>>>> +#endif
>>>> +       memcpy(mac_addr, eeprom, 6);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +#else
>>>> +static int read_mac_from_eeprom(uint8_t *mac_addr) { return -1; }
>>>> +#endif
>>>> +
>>>>    #if !defined(CONFIG_SPL_BUILD)
>>>>    #include <asm/arch/spl.h>
>>>>
>>>> @@ -553,6 +586,9 @@ int misc_init_r(void)
>>>>           }
>>>>    #endif
>>>>
>>>> +       if ((!getenv("ethaddr")) && (!read_mac_from_eeprom(mac_addr)))
>>>> +               eth_setenv_enetaddr("ethaddr", mac_addr);
>>> It seems a bit odd to actually write this to the environment instead
>>> of it being a read_rom_hwaddr operation for the Ethernet driver that
>>> this board uses.
>>>
>>> Do we need a different board-level ROM callback? Maybe the drivers
>>> used in such situations can be the only ones to support it.
>> I'm hoping your thinking out-loud, as I'm not too familiar with all the
> Indeed. I was thinking out loud.
>
>> backing code. But if I understand you correctly, you suggest to implement a
>> new callback for ethernet drivers to get mac's from eeprom? Anyhow, I just
> Not for now... at least not in the core code.
>
>> followed suit with how the address gets set in case of when it is not in the
>> environment and we need to generate a random one.
> Sure. I don't think it's the same case though. We already have the
> entry for the driver to read the rom ethaddr. I think you should use
> that.
I think I lost you here, what function/example can you point me to that 
uses this already so I can spy/learn from?

olliver
>
>> I was reading the u-boot documentation and though the assumed method was to
>> use the environment and to only use other methods (such as the eeprom
>> method) where it is absolutely a must. In this case the eeprom is not
>> directly related to the ethernet, it's a generic eeprom.
> Sure. I think you should add support to whatever driver your board
> uses (it's a driver model driver, right?) for a weak board function.
> That way that one driver is responsible for that behavior because of
> your board's need. Any other boards that use that driver just don't
> supply that function and it is a no-op. I'd rather be able to easily
> follow where that ethaddr is getting configured instead of using
> misc_init_r() and having the env populate magically. This also means
> you'll be in a context to write the ethaddr to the pdata struct
> instead of the env.
>
>> But then again, I am new to this stuff so I can be gladly very wrong here :)
> Seems it's not a case that comes up, so it's fairly new territory.
>
>>
>>>> +
>>>>           ret = sunxi_get_sid(sid);
>>>>           if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
>>>>                   if (!getenv("ethaddr")) {
>>>> diff --git a/net/Kconfig b/net/Kconfig
>>>> index aced51e..0f4cc5a 100644
>>>> --- a/net/Kconfig
>>>> +++ b/net/Kconfig
>>>> @@ -8,6 +8,7 @@ menuconfig NET
>>>>    if NET
>>>>
>>>>    config NET_ETHADDR_EEPROM
>>>> +       depends on ARCH_SUNXI
>>>>           bool "Get ethaddr from eeprom"
>>>>           help
>>>>             Selecting this will try to get the Ethernet address from an
>>>> onboard
> Thanks,
> -Joe

-- 
Met vriendelijke groeten, Kind regards, ??????

Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Olliver Schinagl]

Hey Joe,

On 26-01-16 17:31, Joe Hershberger wrote:
> On Tue, Jan 26, 2016 at 10:27 AM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>> Hey Joe,
>>
>>
>> On 26-01-16 01:45, Joe Hershberger wrote:
>>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>>> <o.schinagl@ultimaker.com> wrote:
>>>> 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.
>>>>
>>>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>>>> ---
>>>>    tools/Makefile       |  4 ++++
>>>>    tools/gen_mac_addr.c | 51
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 55 insertions(+)
>>>>    create mode 100644 tools/gen_mac_addr.c
>>>>
>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>> index 4a50744..6191c26 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_mac_addr
>>>> +gen_mac_addr-objs := gen_mac_addr.o lib/crc8.o
>>>> +HOSTCFLAGS_gen_mac_addr.o := -pedantic
>>>> +
>>>>    hostprogs-$(CONFIG_CMD_LOADS) += img2srec
>>>>    HOSTCFLAGS_img2srec.o := -pedantic
>>>>
>>>> diff --git a/tools/gen_mac_addr.c b/tools/gen_mac_addr.c
>>>> new file mode 100644
>>>> index 0000000..bd8688f
>>>> --- /dev/null
>>>> +++ b/tools/gen_mac_addr.c
>>> This is not "generating a mac address", right? Its point is to create
>>> a CRC for the user-supplied MAC address.
>>>
>>> Perhaps a better name would be "gen_ethaddr_rom_crc".
>> Yes, it takes a mac address as input and generates a macaddress + crc as
>> output.
>>
>> e.g. 11:22:33:44:55:66 -> 0x11 0x22 0x33 0x44 0x55 0x66 0x77 0xcc
>>
>> But maybe it should get some flags to output either both (as described here)
>> or just the crc based on the input, and then a few flags to either output it
>> in hex format or as an int?
> What ever features you think will be useful for the users of this dev
> board I guess. I'm not sure if this is a platform (the included
> eeprom) that would carry over into other products.
Why not? It just reads a mac address and generates a crc8-appended mac 
address (or just the crc8) everybody that uses a mac address to be 
stored somewhere (even if the env is pre-generated) could use this. I 
don't think it is limited to our platform since it does something very 
generic.
>
>>>> @@ -0,0 +1,51 @@
>>>> +/*
>>>> + * (C) Copyright 2016
>>>> + * Olliver Schinagl <o.schinagl@ultimaker.com>
>>>> + *
>>>> + * 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>
>>>> +
>>>> +
>>>> +int main(int argc, char *argv[])
>>>> +{
>>>> +       uint_fast8_t i;
>>>> +       uint8_t mac_addr[7] = { 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;
>>>> +       }
>>> You could use the eth_validate_ethaddr_str() function here instead of this
>>> code.
>> A right, well when I first wrote this, I think this didn't exist there yet.
>> I'll replace it and use it instead.
>>>
>>>> +
>>>> +       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++;
>>>> +               }
>>>> +       }
>>> Instead of all this you could just compile in (maybe it already is?)
>>> the eth_parse_enetaddr() function that U-Boot uses for this very
>>> purpose.
>> I'll see if I can have access to this function and then rewrite this to use
>> that.
>>
>>>> +       mac_addr[6] = crc8(mac_addr, 6);
>>>> +
>>>> +       for (i = 0; i < 7; i++)
>>>> +               printf("%.2x", mac_addr[i]);
>>>> +       putchar('\n');
>>>> +
>>>> +       return 0;
>>>> +}
>>>> --
>>>> 2.6.2
>>>>
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot at lists.denx.de
>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>> --
>> Met vriendelijke groeten, Kind regards, ??????
>>
>> Olliver Schinagl
>> Software Engineer
>> Research & Development
>> Ultimaker B.V.
>>

-- 
Met vriendelijke groeten, Kind regards, ??????

Olliver Schinagl
Software Engineer
Research & Development
Ultimaker B.V.

[U-Boot] [PATCH v2 2/5] sunxi: net: Allow the sunxi to set the MAC from an EEPROM

[Joe Hershberger]

On Tue, Jan 26, 2016 at 10:35 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> Hey Joe,
>
>
> On 26-01-16 17:23, Joe Hershberger wrote:
>>
>> On Tue, Jan 26, 2016 at 10:10 AM, Olliver Schinagl
>> <o.schinagl@ultimaker.com> wrote:
>>>
>>> Hey Joe
>>>
>>> On 26-01-16 01:32, Joe Hershberger wrote:
>>>>
>>>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>>>> <o.schinagl@ultimaker.com> wrote:
>>>>>
>>>>> This patch uses the newly introduced Kconfig options to set the MAC
>>>>> address from an EEPROM, this will be especially useful for the Olimex
>>>>> OLinuXino series of sunxi boards as they all have an 2k i2c eeprom
>>>>> chip.
>>>>>
>>>>> The MAC address is in the eeprom is ignored if there is already a MAC
>>>>
>>>> "The MAC address in the eeprom is ignored..."
>>>
>>> oops :) fixed for v3
>>>
>>>>> address in the environment or (if enabled) the CRC8 check fails.
>>>>>
>>>>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>>>>> ---
>>>>>    board/sunxi/Kconfig |  4 ++++
>>>>>    board/sunxi/board.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>>    net/Kconfig         |  1 +
>>>>>    3 files changed, 41 insertions(+)
>>>>>
>>>>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>>>>> index 2dd9d3b..a2da3a6 100644
>>>>> --- a/board/sunxi/Kconfig
>>>>> +++ b/board/sunxi/Kconfig
>>>>> @@ -339,18 +339,21 @@ config I2C0_ENABLE
>>>>>
>>>>>    config I2C1_ENABLE
>>>>>           bool "Enable I2C/TWI controller 1"
>>>>> +       default y if (NET_ETHADDR_EEPROM_I2C_BUS = 1)
>>>>>           default n
>>>>>           ---help---
>>>>>           See I2C0_ENABLE help text.
>>>>>
>>>>>    config I2C2_ENABLE
>>>>>           bool "Enable I2C/TWI controller 2"
>>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 2
>>>>>           default n
>>>>>           ---help---
>>>>>           See I2C0_ENABLE help text.
>>>>>
>>>>>    if MACH_SUN6I || MACH_SUN7I
>>>>>    config I2C3_ENABLE
>>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 3
>>>>>           bool "Enable I2C/TWI controller 3"
>>>>>           default n
>>>>>           ---help---
>>>>> @@ -359,6 +362,7 @@ endif
>>>>>
>>>>>    if MACH_SUN7I
>>>>>    config I2C4_ENABLE
>>>>> +       default y if NET_ETHADDR_EEPROM_I2C_BUS = 4
>>>>>           bool "Enable I2C/TWI controller 4"
>>>>>           default n
>>>>>           ---help---
>>>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>>>> index 6ac398c..28310a2 100644
>>>>> --- a/board/sunxi/board.c
>>>>> +++ b/board/sunxi/board.c
>>>>> @@ -12,6 +12,7 @@
>>>>>     */
>>>>>
>>>>>    #include <common.h>
>>>>> +#include <i2c.h>
>>>>>    #include <mmc.h>
>>>>>    #include <axp_pmic.h>
>>>>>    #include <asm/arch/clock.h>
>>>>> @@ -23,6 +24,7 @@
>>>>>    #include <asm/arch/usb_phy.h>
>>>>>    #include <asm/gpio.h>
>>>>>    #include <asm/io.h>
>>>>> +#include <linux/crc8.h>
>>>>>    #include <nand.h>
>>>>>    #include <net.h>
>>>>>
>>>>> @@ -510,6 +512,37 @@ void get_board_serial(struct tag_serialnr
>>>>> *serialnr)
>>>>>    }
>>>>>    #endif
>>>>>
>>>>> +#if defined(CONFIG_NET_ETHADDR_EEPROM) &&
>>>>> defined(CONFIG_NET_ETHADDR_EEPROM_I2C)
>>>>> +static int read_mac_from_eeprom(uint8_t *mac_addr)
>>>>> +{
>>>>> +       uint8_t eeprom[7];
>>>>> +       int old_i2c_bus;
>>>>> +
>>>>> +       old_i2c_bus = i2c_get_bus_num();
>>>>> +       i2c_set_bus_num(CONFIG_NET_ETHADDR_EEPROM_I2C_BUS);
>>>>> +       if (i2c_read(CONFIG_NET_ETHADDR_EEPROM_I2C_ADDR,
>>>>> +                    CONFIG_NET_ETHADDR_EEPROM_OFFSET,
>>>>> +                    CONFIG_NET_ETHADDR_EEPROM_I2C_ADDRLEN,
>>>>> +                    eeprom, 7)) {
>>>>> +               i2c_set_bus_num(old_i2c_bus);
>>>>> +               puts("Could not read the EEPROM; EEPROM missing?\n");
>>>>> +               return -1;
>>>>> +       }
>>>>> +       i2c_set_bus_num(old_i2c_bus);
>>>>> +#ifdef CONFIG_NET_ETHADDR_EEPROM_CRC8
>>>>> +       if (crc8(eeprom, 6) != eeprom[6]) {
>>>>> +               puts("CRC error on MAC address from EEPROM.\n");
>>>>> +               return -1;
>>>>> +       }
>>>>> +#endif
>>>>> +       memcpy(mac_addr, eeprom, 6);
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +#else
>>>>> +static int read_mac_from_eeprom(uint8_t *mac_addr) { return -1; }
>>>>> +#endif
>>>>> +
>>>>>    #if !defined(CONFIG_SPL_BUILD)
>>>>>    #include <asm/arch/spl.h>
>>>>>
>>>>> @@ -553,6 +586,9 @@ int misc_init_r(void)
>>>>>           }
>>>>>    #endif
>>>>>
>>>>> +       if ((!getenv("ethaddr")) && (!read_mac_from_eeprom(mac_addr)))
>>>>> +               eth_setenv_enetaddr("ethaddr", mac_addr);
>>>>
>>>> It seems a bit odd to actually write this to the environment instead
>>>> of it being a read_rom_hwaddr operation for the Ethernet driver that
>>>> this board uses.
>>>>
>>>> Do we need a different board-level ROM callback? Maybe the drivers
>>>> used in such situations can be the only ones to support it.
>>>
>>> I'm hoping your thinking out-loud, as I'm not too familiar with all the
>>
>> Indeed. I was thinking out loud.
>>
>>> backing code. But if I understand you correctly, you suggest to implement
>>> a
>>> new callback for ethernet drivers to get mac's from eeprom? Anyhow, I
>>> just
>>
>> Not for now... at least not in the core code.
>>
>>> followed suit with how the address gets set in case of when it is not in
>>> the
>>> environment and we need to generate a random one.
>>
>> Sure. I don't think it's the same case though. We already have the
>> entry for the driver to read the rom ethaddr. I think you should use
>> that.
>
> I think I lost you here, what function/example can you point me to that uses
> this already so I can spy/learn from?

I think you'll be the first driver ported to use this (it's driver model only).

It should be fairly straightforward. I've mocked up a simple version
in the Zynq GEM driver as an example.




diff --git a/arch/arm/mach-zynq/include/mach/sys_proto.h
b/arch/arm/mach-zynq/include/mach/sys_proto.h
index 882beab..44c9b50 100644
--- a/arch/arm/mach-zynq/include/mach/sys_proto.h
+++ b/arch/arm/mach-zynq/include/mach/sys_proto.h
@@ -19,6 +19,8 @@ extern int zynq_slcr_get_mio_pin_status(const char *periph);
 extern void zynq_ddrc_init(void);
 extern unsigned int zynq_get_silicon_version(void);

+int zynq_board_read_rom_ethaddr(unsigned char *ethaddr);
+
 /* Driver extern functions */
 extern void ps7_init(void);

diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
index 414f530..ca617a5 100644
--- a/board/xilinx/zynq/board.c
+++ b/board/xilinx/zynq/board.c
@@ -122,6 +122,16 @@ int board_eth_init(bd_t *bis)
        return ret;
 }

+#ifdef CONFIG_TARGET_ZYNQ_ZC770
+int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
+{
+       // TODO: Read from eeprom
+       memset(ethaddr, 0, ARP_HLEN);
+
+       return 0;
+}
+#endif
+
 int dram_init(void)
 {
 #if CONFIG_IS_ENABLED(OF_CONTROL)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
index 7059c84..068f3d0 100644
--- a/drivers/net/zynq_gem.c
+++ b/drivers/net/zynq_gem.c
@@ -588,6 +588,23 @@ static void zynq_gem_halt(struct udevice *dev)
                                                ZYNQ_GEM_NWCTRL_TXEN_MASK, 0);
 }

+__weak int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
+{
+       return -ENOSYS;
+}
+
+static int zynq_gem_read_rom_mac(struct udevice *dev)
+{
+       int retval;
+       struct eth_pdata *pdata = dev_get_platdata(dev);
+
+       retval = zynq_board_read_rom_ethaddr(pdata->enetaddr);
+       if (retval == -ENOSYS)
+               retval = 0;
+
+       return retval;
+}
+
 static int zynq_gem_miiphy_read(struct mii_dev *bus, int addr,
                                int devad, int reg)
 {
@@ -661,6 +678,7 @@ static const struct eth_ops zynq_gem_ops = {
        .free_pkt               = zynq_gem_free_pkt,
        .stop                   = zynq_gem_halt,
        .write_hwaddr           = zynq_gem_setup_mac,
+       .read_rom_hwaddr        = zynq_gem_read_rom_mac,
 };

 static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
--


Hopefully this makes it more clear what I have in mind.

Thanks,
-Joe

> olliver
>
>>
>>> I was reading the u-boot documentation and though the assumed method was
>>> to
>>> use the environment and to only use other methods (such as the eeprom
>>> method) where it is absolutely a must. In this case the eeprom is not
>>> directly related to the ethernet, it's a generic eeprom.
>>
>> Sure. I think you should add support to whatever driver your board
>> uses (it's a driver model driver, right?) for a weak board function.
>> That way that one driver is responsible for that behavior because of
>> your board's need. Any other boards that use that driver just don't
>> supply that function and it is a no-op. I'd rather be able to easily
>> follow where that ethaddr is getting configured instead of using
>> misc_init_r() and having the env populate magically. This also means
>> you'll be in a context to write the ethaddr to the pdata struct
>> instead of the env.
>>
>>> But then again, I am new to this stuff so I can be gladly very wrong here
>>> :)
>>
>> Seems it's not a case that comes up, so it's fairly new territory.
>>
>>>
>>>>> +
>>>>>           ret = sunxi_get_sid(sid);
>>>>>           if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
>>>>>                   if (!getenv("ethaddr")) {
>>>>> diff --git a/net/Kconfig b/net/Kconfig
>>>>> index aced51e..0f4cc5a 100644
>>>>> --- a/net/Kconfig
>>>>> +++ b/net/Kconfig
>>>>> @@ -8,6 +8,7 @@ menuconfig NET
>>>>>    if NET
>>>>>
>>>>>    config NET_ETHADDR_EEPROM
>>>>> +       depends on ARCH_SUNXI
>>>>>           bool "Get ethaddr from eeprom"
>>>>>           help
>>>>>             Selecting this will try to get the Ethernet address from an
>>>>> onboard
>>
>> Thanks,
>> -Joe
>
>
> --
> Met vriendelijke groeten, Kind regards, ??????
>
> Olliver Schinagl
> Software Engineer
> Research & Development
> Ultimaker B.V.
>

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Joe Hershberger]

On Tue, Jan 26, 2016 at 10:36 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> Hey Joe,
>
>
> On 26-01-16 17:31, Joe Hershberger wrote:
>>
>> On Tue, Jan 26, 2016 at 10:27 AM, Olliver Schinagl
>> <o.schinagl@ultimaker.com> wrote:
>>>
>>> Hey Joe,
>>>
>>>
>>> On 26-01-16 01:45, Joe Hershberger wrote:
>>>>
>>>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>>>> <o.schinagl@ultimaker.com> wrote:
>>>>>
>>>>> 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.
>>>>>
>>>>> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
>>>>> ---
>>>>>    tools/Makefile       |  4 ++++
>>>>>    tools/gen_mac_addr.c | 51
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>    2 files changed, 55 insertions(+)
>>>>>    create mode 100644 tools/gen_mac_addr.c
>>>>>
>>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>>> index 4a50744..6191c26 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_mac_addr
>>>>> +gen_mac_addr-objs := gen_mac_addr.o lib/crc8.o
>>>>> +HOSTCFLAGS_gen_mac_addr.o := -pedantic
>>>>> +
>>>>>    hostprogs-$(CONFIG_CMD_LOADS) += img2srec
>>>>>    HOSTCFLAGS_img2srec.o := -pedantic
>>>>>
>>>>> diff --git a/tools/gen_mac_addr.c b/tools/gen_mac_addr.c
>>>>> new file mode 100644
>>>>> index 0000000..bd8688f
>>>>> --- /dev/null
>>>>> +++ b/tools/gen_mac_addr.c
>>>>
>>>> This is not "generating a mac address", right? Its point is to create
>>>> a CRC for the user-supplied MAC address.
>>>>
>>>> Perhaps a better name would be "gen_ethaddr_rom_crc".
>>>
>>> Yes, it takes a mac address as input and generates a macaddress + crc as
>>> output.
>>>
>>> e.g. 11:22:33:44:55:66 -> 0x11 0x22 0x33 0x44 0x55 0x66 0x77 0xcc
>>>
>>> But maybe it should get some flags to output either both (as described
>>> here)
>>> or just the crc based on the input, and then a few flags to either output
>>> it
>>> in hex format or as an int?
>>
>> What ever features you think will be useful for the users of this dev
>> board I guess. I'm not sure if this is a platform (the included
>> eeprom) that would carry over into other products.
>
> Why not? It just reads a mac address and generates a crc8-appended mac
> address (or just the crc8) everybody that uses a mac address to be stored
> somewhere (even if the env is pre-generated) could use this. I don't think
> it is limited to our platform since it does something very generic.

OK.

>
>>
>>>>> @@ -0,0 +1,51 @@
>>>>> +/*
>>>>> + * (C) Copyright 2016
>>>>> + * Olliver Schinagl <o.schinagl@ultimaker.com>
>>>>> + *
>>>>> + * 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>
>>>>> +
>>>>> +
>>>>> +int main(int argc, char *argv[])
>>>>> +{
>>>>> +       uint_fast8_t i;
>>>>> +       uint8_t mac_addr[7] = { 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;
>>>>> +       }
>>>>
>>>> You could use the eth_validate_ethaddr_str() function here instead of
>>>> this
>>>> code.
>>>
>>> A right, well when I first wrote this, I think this didn't exist there
>>> yet.
>>> I'll replace it and use it instead.
>>>>
>>>>
>>>>> +
>>>>> +       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++;
>>>>> +               }
>>>>> +       }
>>>>
>>>> Instead of all this you could just compile in (maybe it already is?)
>>>> the eth_parse_enetaddr() function that U-Boot uses for this very
>>>> purpose.
>>>
>>> I'll see if I can have access to this function and then rewrite this to
>>> use
>>> that.
>>>
>>>>> +       mac_addr[6] = crc8(mac_addr, 6);
>>>>> +
>>>>> +       for (i = 0; i < 7; i++)
>>>>> +               printf("%.2x", mac_addr[i]);
>>>>> +       putchar('\n');
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> --
>>>>> 2.6.2
>>>>>
>>>>> _______________________________________________
>>>>> U-Boot mailing list
>>>>> U-Boot at lists.denx.de
>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>>>
>>> --
>>> Met vriendelijke groeten, Kind regards, ??????
>>>
>>> Olliver Schinagl
>>> Software Engineer
>>> Research & Development
>>> Ultimaker B.V.
>>>
>
> --
> Met vriendelijke groeten, Kind regards, ??????
>
> Olliver Schinagl
> Software Engineer
> Research & Development
> Ultimaker B.V.
>

[U-Boot] [PATCH v2 5/5] tools: Add tool to add crc8 to a mac address

[Joe Hershberger]

On Tue, Jan 26, 2016 at 10:31 AM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> Hey all,
>
> On 26-01-16 15:56, Joe Hershberger wrote:
>>
>> Hi Albert,
>>
>> On Tue, Jan 26, 2016 at 7:15 AM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> wrote:
>>>
>>> Hello Joe,
>>>
>>> On Mon, 25 Jan 2016 18:45:52 -0600, Joe Hershberger
>>> <joe.hershberger@gmail.com> wrote:
>>>>
>>>> On Mon, Dec 14, 2015 at 6:41 AM, Olliver Schinagl
>>>> <o.schinagl@ultimaker.com> wrote:
>>>>>
>>>>> +++ b/tools/gen_mac_addr.c
>>>>
>>>> This is not "generating a mac address", right? Its point is to create
>>>> a CRC for the user-supplied MAC address.
>>>>
>>>> Perhaps a better name would be "gen_ethaddr_rom_crc".
>>>
>>> Hmm, why the "_rom" part?
>>
>> Just to give more hints to the observer that this is for creating a
>> rom or eeprom image to be read by the associated new code in U-Boot.
>
> Well depending on the output, it could even be used to verify what is stored
> in an ROM. So i understand Albert's reasoning that it may not be related to
> ROM at all

You could make it even more generic and just call it gen_crc8... make
it take an arbitrary byte stream. Anyway. I think there's no need for
speculative generality. Make the tool be named for what its purpose
is. Include the features you need it to have. If someone wants to
expand it, it can be renamed at that point.

Thanks,
-Joe