[U-Boot] [PATCH 00/10] ARM: clearfog: add run-time board detect

[U-Boot] [PATCH 00/10] ARM: clearfog: add run-time board detect

[Baruch Siach]

Newer revisions of SolidRun Clearfog Base/Pro carriers and Armada 388 SOM add 
EEPROM storage for board detection. This patch series adds support for reading 
EEPROM stored board information, and using it to set RAM training parameters, 
serdes configuration, and kernel DT selection.

The information is stored in EEPROM in TLV format defined for the ONIE project.

  https://opencomputeproject.github.io/onie/design-spec/hw_requirements.html

This series add the sys_eeprom command from ONIE carried U-Boot patch, with a 
number for changes as described in the patch log. The TLV format is unchanged.

This series also adds support for the newly introduced single board, Aramda 385 
based Clearfog GTR system. RAM configuration on that system requires both 
Armada 38x DDR clocks to be enabled. The first patch in this series adds the 
necessary code to allow per-board selection of DDR clock.

Baruch Siach (10):
  ddr: marvell: a38x: allow board specific clock out setup
  arm: mvebu: clearfog: enable both DDR clocks
  cmd: add sys_eeprom command
  ARM: mvebu: clearfog: add EEPROM devices
  ARM: mvebu: clearfog: add support for EEPROM TLV info
  ARM: mvebu: clearfog: read basic TLV data
  ARM: mvebu: clearfog: print TLV stored product name
  ARM: mvebu: clearfog: run-time selection of DT file
  ARM: mvebu: clearfog: add Clearfog GTR support
  ARM: mvebu: clearfog: add Clearfog Base serdes configuration

 arch/arm/dts/armada-388-clearfog-u-boot.dtsi  |   12 +
 arch/arm/dts/armada-388-clearfog.dts          |    6 +
 .../arm/dts/armada-38x-solidrun-microsom.dtsi |    8 +
 arch/arm/mach-mvebu/Kconfig                   |    1 +
 board/solidrun/clearfog/clearfog.c            |  164 ++-
 cmd/Kconfig                                   |   12 +
 cmd/Makefile                                  |    2 +
 cmd/sys_eeprom.c                              | 1078 +++++++++++++++++
 configs/clearfog_defconfig                    |    5 +
 drivers/ddr/marvell/a38x/ddr3_training.c      |   10 +-
 drivers/ddr/marvell/a38x/ddr_topology_def.h   |    3 +
 include/sys_eeprom.h                          |  169 +++
 12 files changed, 1466 insertions(+), 4 deletions(-)
 create mode 100644 cmd/sys_eeprom.c
 create mode 100644 include/sys_eeprom.h

-- 
2.24.0

[U-Boot] [PATCH 01/10] ddr: marvell: a38x: allow board specific clock out setup

[Baruch Siach]

DDR clock out might be unrelated to the number of active chip-select.
For example, the board might have two DDR components, but only one
chip-select. The clk_enable mask allows the board to enable DDR clocks
regardless of active chip-selects.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/ddr/marvell/a38x/ddr3_training.c    | 10 ++++++++--
 drivers/ddr/marvell/a38x/ddr_topology_def.h |  3 +++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/ddr/marvell/a38x/ddr3_training.c b/drivers/ddr/marvell/a38x/ddr3_training.c
index c7be700d641f..9718f18c3024 100644
--- a/drivers/ddr/marvell/a38x/ddr3_training.c
+++ b/drivers/ddr/marvell/a38x/ddr3_training.c
@@ -280,8 +280,14 @@ int ddr3_tip_configure_cs(u32 dev_num, u32 if_id, u32 cs_num, u32 enable)
 {
 	u32 data, addr_hi, data_high;
 	u32 mem_index;
+	u32 clk_enable;
 	struct mv_ddr_topology_map *tm = mv_ddr_topology_map_get();
 
+	if (tm->clk_enable & (1 << cs_num))
+		clk_enable = 1;
+	else
+		clk_enable = enable;
+
 	if (enable == 1) {
 		data = (tm->interface_params[if_id].bus_width ==
 			MV_DDR_DEV_WIDTH_8BIT) ? 0 : 1;
@@ -316,13 +322,13 @@ int ddr3_tip_configure_cs(u32 dev_num, u32 if_id, u32 cs_num, u32 enable)
 	case 2:
 		CHECK_STATUS(ddr3_tip_if_write
 			     (dev_num, ACCESS_TYPE_UNICAST, if_id,
-			      DUNIT_CTRL_LOW_REG, (enable << (cs_num + 11)),
+			      DUNIT_CTRL_LOW_REG, (clk_enable << (cs_num + 11)),
 			      1 << (cs_num + 11)));
 		break;
 	case 3:
 		CHECK_STATUS(ddr3_tip_if_write
 			     (dev_num, ACCESS_TYPE_UNICAST, if_id,
-			      DUNIT_CTRL_LOW_REG, (enable << 15), 1 << 15));
+			      DUNIT_CTRL_LOW_REG, (clk_enable << 15), 1 << 15));
 		break;
 	}
 
diff --git a/drivers/ddr/marvell/a38x/ddr_topology_def.h b/drivers/ddr/marvell/a38x/ddr_topology_def.h
index e6fe8a04289b..950f296ff984 100644
--- a/drivers/ddr/marvell/a38x/ddr_topology_def.h
+++ b/drivers/ddr/marvell/a38x/ddr_topology_def.h
@@ -124,6 +124,9 @@ struct mv_ddr_topology_map {
 
 	/* electrical parameters */
 	unsigned int electrical_data[MV_DDR_EDATA_LAST];
+
+	/* Clock enable mask */
+	u32 clk_enable;
 };
 
 enum mv_ddr_iface_mode {
-- 
2.24.0

[U-Boot] [PATCH 02/10] arm: mvebu: clearfog: enable both DDR clocks

[Baruch Siach]

Enabled both DDR clock signals to support Clearfog variants (currently,
Clearfog GTR) that need both clocks.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 board/solidrun/clearfog/clearfog.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
index 03724fee10c1..8b6381194688 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -68,7 +68,10 @@ static struct mv_ddr_topology_map board_topology_map = {
 	BUS_MASK_32BIT,			/* Busses mask */
 	MV_DDR_CFG_DEFAULT,		/* ddr configuration data source */
 	{ {0} },			/* raw spd data */
-	{0}				/* timing parameters */
+	{0},				/* timing parameters */
+	{ {0} },			/* electrical configuration */
+	{0,},				/* electrical parameters */
+	0x3,				/* clock enable mask */
 };
 
 struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
-- 
2.24.0

[U-Boot] [PATCH 03/10] cmd: add sys_eeprom command

[Baruch Siach]

Based on U-Boot patch from the Open Compute project:

  https://github.com/opencomputeproject/onie/blob/ec87e872d46b9805565d2c6124b2f701ef1c07b1/patches/u-boot/common/feature-sys-eeprom-tlv-common.patch

Keep only I2C EEPROM support. Use the generic eeprom driver.

Add support for multiple EEPROM TLV stores on the same system. This is
useful in case of SOM and carrier that both provide ID and hardware
configuration information.

Add option to enable for SPL. This allows selection of RAM configuration
based on EEPROM stored board identification.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 cmd/Kconfig          |   12 +
 cmd/Makefile         |    2 +
 cmd/sys_eeprom.c     | 1078 ++++++++++++++++++++++++++++++++++++++++++
 include/sys_eeprom.h |  169 +++++++
 4 files changed, 1261 insertions(+)
 create mode 100644 cmd/sys_eeprom.c
 create mode 100644 include/sys_eeprom.h

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 99b8a0e21822..483663404da4 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -234,6 +234,18 @@ config CMD_REGINFO
 	help
 	  Register dump
 
+config CMD_SYS_EEPROM
+	bool "sys_eeprom"
+	depends on I2C_EEPROM
+	help
+	  Display and program the system EEPROM data block.
+
+config SPL_CMD_SYS_EEPROM
+	bool "sys_eeprom for SPL"
+	depends on SPL_I2C_EEPROM
+	help
+	  Read system EEPROM data block.
+
 endmenu
 
 menu "Boot commands"
diff --git a/cmd/Makefile b/cmd/Makefile
index 2d723ea0f07d..cbb1d46b8f50 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -180,6 +180,8 @@ obj-$(CONFIG_X86) += x86/
 obj-$(CONFIG_ARCH_MVEBU) += mvebu/
 endif # !CONFIG_SPL_BUILD
 
+obj-$(CONFIG_$(SPL_)CMD_SYS_EEPROM) += sys_eeprom.o
+
 # core command
 obj-y += nvedit.o
 
diff --git a/cmd/sys_eeprom.c b/cmd/sys_eeprom.c
new file mode 100644
index 000000000000..373673a5266c
--- /dev/null
+++ b/cmd/sys_eeprom.c
@@ -0,0 +1,1078 @@
+/*
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <command.h>
+#include <dm.h>
+#include <i2c.h>
+#include <i2c_eeprom.h>
+#include <env.h>
+#include <linux/ctype.h>
+
+#include "sys_eeprom.h"
+
+#define MAX_TLV_DEVICES	2
+
+/* File scope function prototypes */
+static bool is_checksum_valid(u8 *eeprom);
+static int read_eeprom(u8 *eeprom);
+static void show_eeprom(u8 *eeprom);
+static void decode_tlv(tlvinfo_tlv_t * tlv);
+static void update_crc(u8 *eeprom);
+static int prog_eeprom(u8 * eeprom);
+static bool tlvinfo_find_tlv(u8 * eeprom, u8 tcode, int * eeprom_index);
+static bool tlvinfo_delete_tlv(u8 * eeprom, u8 code);
+static bool tlvinfo_add_tlv(u8 * eeprom, int tcode, char * strval);
+static int set_mac(char *buf, const char *string);
+static int set_date(char *buf, const char *string);
+static int set_bytes(char *buf, const char *string, int * converted_accum);
+static void show_tlv_devices(void);
+
+/* Set to 1 if we've read EEPROM into memory */
+static int has_been_read = 0;
+/* The EERPOM contents after being read into memory */
+static u8 eeprom[TLV_INFO_MAX_LEN];
+
+static struct udevice *tlv_devices[MAX_TLV_DEVICES];
+static unsigned current_dev;
+
+/**
+ *  is_valid_tlv
+ *
+ *  Perform basic sanity checks on a TLV field. The TLV is pointed to
+ *  by the parameter provided.
+ *      1. The type code is not reserved (0x00 or 0xFF)
+ */
+static inline bool is_valid_tlv(tlvinfo_tlv_t *tlv)
+{
+	return( (tlv->type != 0x00) &&
+		(tlv->type != 0xFF) );
+}
+
+/**
+ *  is_hex
+ *
+ *  Tests if character is an ASCII hex digit
+ */
+static inline u8 is_hex(char p)
+{
+	return (((p >= '0') && (p <= '9')) ||
+		((p >= 'A') && (p <= 'F')) ||
+		((p >= 'a') && (p <= 'f')));
+}
+
+/**
+ *  is_checksum_valid
+ *
+ *  Validate the checksum in the provided TlvInfo EEPROM data. First,
+ *  verify that the TlvInfo header is valid, then make sure the last
+ *  TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
+ *  and compare it to the value stored in the EEPROM CRC-32 TLV.
+ */
+static bool is_checksum_valid(u8 *eeprom)
+{
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+	tlvinfo_tlv_t    * eeprom_crc;
+	unsigned int       calc_crc;
+	unsigned int       stored_crc;
+
+	// Is the eeprom header valid?
+	if (!is_valid_tlvinfo_header(eeprom_hdr)) {
+		return(FALSE);
+	}
+
+	// Is the last TLV a CRC?
+	eeprom_crc = (tlvinfo_tlv_t *) &eeprom[sizeof(tlvinfo_header_t) +
+					       be16_to_cpu(eeprom_hdr->totallen) - (sizeof(tlvinfo_tlv_t) + 4)];
+	if ((eeprom_crc->type != TLV_CODE_CRC_32) || (eeprom_crc->length != 4)) {
+		return(FALSE);
+	}
+
+	// Calculate the checksum
+	calc_crc = crc32(0, (void *)eeprom,
+			 sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen) - 4);
+	stored_crc = (eeprom_crc->value[0] << 24) |
+		(eeprom_crc->value[1] << 16) |
+		(eeprom_crc->value[2] <<  8) |
+		eeprom_crc->value[3];
+	return( calc_crc == stored_crc);
+}
+
+/**
+ *  read_eeprom
+ *
+ *  Read the EEPROM into memory, if it hasn't already been read.
+ */
+static int read_eeprom(u8 *eeprom)
+{
+	int ret;
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+	tlvinfo_tlv_t    * eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[sizeof(tlvinfo_header_t)];
+
+	if (has_been_read)
+		return 0;
+
+	/* Read the header */
+	ret = read_sys_eeprom((void *)eeprom_hdr, 0, sizeof(tlvinfo_header_t));
+	/* If the header was successfully read, read the TLVs */
+	if ((ret == 0) && is_valid_tlvinfo_header(eeprom_hdr)) {
+		ret = read_sys_eeprom((void *)eeprom_tlv, sizeof(tlvinfo_header_t),
+				  be16_to_cpu(eeprom_hdr->totallen));
+	}
+
+	// If the contents are invalid, start over with default contents
+	if ( !is_valid_tlvinfo_header(eeprom_hdr) || !is_checksum_valid(eeprom) ){
+		strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
+		eeprom_hdr->version = TLV_INFO_VERSION;
+		eeprom_hdr->totallen = cpu_to_be16(0);
+		update_crc(eeprom);
+	}
+
+	has_been_read = 1;
+
+#ifdef DEBUG
+	show_eeprom(eeprom);
+#endif
+
+	return ret;
+}
+
+/**
+ *  show_eeprom
+ *
+ *  Display the contents of the EEPROM
+ */
+static void show_eeprom(u8 *eeprom)
+{
+	int tlv_end;
+	int curr_tlv;
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+	tlvinfo_tlv_t    * eeprom_tlv;
+
+	if ( !is_valid_tlvinfo_header(eeprom_hdr) ) {
+		printf("EEPROM does not contain data in a valid TlvInfo format.\n");
+		return;
+	}
+
+	printf("TLV: %u\n", current_dev);
+	printf("TlvInfo Header:\n");
+	printf("   Id String:    %s\n", eeprom_hdr->signature);
+	printf("   Version:      %d\n", eeprom_hdr->version);
+	printf("   Total Length: %d\n", be16_to_cpu(eeprom_hdr->totallen));
+
+	printf("TLV Name             Code Len Value\n");
+	printf("-------------------- ---- --- -----\n");
+	curr_tlv = sizeof(tlvinfo_header_t);
+	tlv_end  = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
+	while (curr_tlv < tlv_end) {
+		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[curr_tlv];
+		if (!is_valid_tlv(eeprom_tlv)) {
+			printf("Invalid TLV field starting@EEPROM offset %d\n", curr_tlv);
+			return;
+		}
+		decode_tlv(eeprom_tlv);
+		curr_tlv += sizeof(tlvinfo_tlv_t) + eeprom_tlv->length;
+	}
+
+	printf("Checksum is %s.\n", is_checksum_valid(eeprom) ? "valid" : "invalid");
+
+#ifdef DEBUG
+	printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
+	for (i = 0; i < TLV_INFO_MAX_LEN; i++) {
+		if ((i % 16) == 0)
+			printf("\n%02X: ", i);
+		printf("%02X ", eeprom[i]);
+	}
+	printf("\n");
+#endif
+
+	return;
+}
+
+/**
+ *  Struct for displaying the TLV codes and names.
+ */
+struct tlv_code_desc {
+	u8    m_code;
+	char* m_name;
+};
+
+/**
+ *  List of TLV codes and names.
+ */
+static struct tlv_code_desc tlv_code_list[] = {
+	{ TLV_CODE_PRODUCT_NAME	 , "Product Name"},
+	{ TLV_CODE_PART_NUMBER	 , "Part Number"},
+	{ TLV_CODE_SERIAL_NUMBER , "Serial Number"},
+	{ TLV_CODE_MAC_BASE	 , "Base MAC Address"},
+	{ TLV_CODE_MANUF_DATE	 , "Manufacture Date"},
+	{ TLV_CODE_DEVICE_VERSION, "Device Version"},
+	{ TLV_CODE_LABEL_REVISION, "Label Revision"},
+	{ TLV_CODE_PLATFORM_NAME , "Platform Name"},
+	{ TLV_CODE_ONIE_VERSION	 , "ONIE Version"},
+	{ TLV_CODE_MAC_SIZE	 , "MAC Addresses"},
+	{ TLV_CODE_MANUF_NAME	 , "Manufacturer"},
+	{ TLV_CODE_MANUF_COUNTRY , "Country Code"},
+	{ TLV_CODE_VENDOR_NAME	 , "Vendor Name"},
+	{ TLV_CODE_DIAG_VERSION	 , "Diag Version"},
+	{ TLV_CODE_SERVICE_TAG   , "Service Tag"},
+	{ TLV_CODE_VENDOR_EXT	 , "Vendor Extension"},
+	{ TLV_CODE_CRC_32	 , "CRC-32"},
+};
+
+/**
+ *  Look up a TLV name by its type.
+ */
+static inline const char* tlv_type2name(u8 type)
+{
+	char* name = "Unknown";
+	int   i;
+
+	for (i = 0; i < sizeof(tlv_code_list)/sizeof(tlv_code_list[0]); i++) {
+		if (tlv_code_list[i].m_code == type) {
+			name = tlv_code_list[i].m_name;
+			break;
+		}
+	}
+
+	return name;
+}
+
+/*
+ *  decode_tlv
+ *
+ *  Print a string representing the contents of the TLV field. The format of
+ *  the string is:
+ *      1. The name of the field left justified in 20 characters
+ *      2. The type code in hex right justified in 5 characters
+ *      3. The length in decimal right justified in 4 characters
+ *      4. The value, left justified in however many characters it takes
+ *  The validity of EEPROM contents and the TLV field have been verified
+ *  prior to calling this function.
+ */
+#define DECODE_NAME_MAX     20
+
+/*
+ * The max decode value is currently for the 'raw' type or the 'vendor
+ * extension' type, both of which have the same decode format.  The
+ * max decode string size is computed as follows:
+ *
+ *   strlen(" 0xFF") * TLV_VALUE_MAX_LEN + 1
+ *
+ */
+#define DECODE_VALUE_MAX    ((5 * TLV_VALUE_MAX_LEN) + 1)
+
+static void decode_tlv(tlvinfo_tlv_t * tlv)
+{
+	char name[DECODE_NAME_MAX];
+	char value[DECODE_VALUE_MAX];
+	int i;
+
+	strncpy(name, tlv_type2name(tlv->type), DECODE_NAME_MAX);
+
+	switch (tlv->type) {
+	case TLV_CODE_PRODUCT_NAME:
+	case TLV_CODE_PART_NUMBER:
+	case TLV_CODE_SERIAL_NUMBER:
+	case TLV_CODE_MANUF_DATE:
+	case TLV_CODE_LABEL_REVISION:
+	case TLV_CODE_PLATFORM_NAME:
+	case TLV_CODE_ONIE_VERSION:
+	case TLV_CODE_MANUF_NAME:
+	case TLV_CODE_MANUF_COUNTRY:
+	case TLV_CODE_VENDOR_NAME:
+	case TLV_CODE_DIAG_VERSION:
+	case TLV_CODE_SERVICE_TAG:
+		memcpy(value, tlv->value, tlv->length);
+		value[tlv->length] = 0;
+		break;
+	case TLV_CODE_MAC_BASE:
+		sprintf(value, "%02X:%02X:%02X:%02X:%02X:%02X",
+			tlv->value[0], tlv->value[1], tlv->value[2],
+			tlv->value[3], tlv->value[4], tlv->value[5]);
+		break;
+	case TLV_CODE_DEVICE_VERSION:
+		sprintf(value, "%u", tlv->value[0]);
+		break;
+	case TLV_CODE_MAC_SIZE:
+		sprintf(value, "%u", (tlv->value[0] << 8) | tlv->value[1]);
+		break;
+	case TLV_CODE_VENDOR_EXT:
+		value[0] = 0;
+		for (i = 0; (i < (DECODE_VALUE_MAX/5)) && (i < tlv->length); i++) {
+			sprintf(value, "%s 0x%02X", value, tlv->value[i]);
+		}
+		break;
+	case TLV_CODE_CRC_32:
+		sprintf(value, "0x%02X%02X%02X%02X",
+			tlv->value[0], tlv->value[1], tlv->value[2], tlv->value[3]);
+		break;
+	default:
+		value[0] = 0;
+		for (i = 0; (i < (DECODE_VALUE_MAX/5)) && (i < tlv->length); i++) {
+			sprintf(value, "%s 0x%02X", value, tlv->value[i]);
+		}
+		break;
+	}
+
+	name[DECODE_NAME_MAX-1] = 0;
+	printf("%-20s 0x%02X %3d %s\n", name, tlv->type, tlv->length, value);
+	return;
+}
+
+/**
+ *  update_crc
+ *
+ *  This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
+ *  one is added. This function should be called after each update to the
+ *  EEPROM structure, to make sure the CRC is always correct.
+ */
+static void update_crc(u8 *eeprom)
+{
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+	tlvinfo_tlv_t    * eeprom_crc;
+	unsigned int       calc_crc;
+	int                eeprom_index;
+
+	// Discover the CRC TLV
+	if (!tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index)) {
+		if ((be16_to_cpu(eeprom_hdr->totallen) + sizeof(tlvinfo_tlv_t) + 4) > TLV_TOTAL_LEN_MAX) {
+			return;
+		}
+		eeprom_index = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
+		eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
+						   sizeof(tlvinfo_tlv_t) + 4);
+	}
+	eeprom_crc = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
+	eeprom_crc->type = TLV_CODE_CRC_32;
+	eeprom_crc->length = 4;
+
+	// Calculate the checksum
+	calc_crc = crc32(0, (void *)eeprom,
+			 sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen) - 4);
+	eeprom_crc->value[0] = (calc_crc >> 24) & 0xFF;
+	eeprom_crc->value[1] = (calc_crc >> 16) & 0xFF;
+	eeprom_crc->value[2] = (calc_crc >>  8) & 0xFF;
+	eeprom_crc->value[3] = (calc_crc >>  0) & 0xFF;
+
+	return;
+}
+
+/**
+ *  prog_eeprom
+ *
+ *  Write the EEPROM data from CPU memory to the hardware.
+ */
+static int prog_eeprom(u8 * eeprom)
+{
+	int ret = 0;
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+	int eeprom_len;
+
+	update_crc(eeprom);
+
+	eeprom_len = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
+	ret = write_sys_eeprom(eeprom, eeprom_len);
+	if (ret) {
+		printf("Programming failed.\n");
+		return -1;
+	}
+
+	printf("Programming passed.\n");
+	return 0;
+}
+
+/**
+ *  show_tlv_code_list - Display the list of TLV codes and names
+ */
+void show_tlv_code_list(void)
+{
+	int i;
+
+	printf("TLV Code    TLV Name\n");
+	printf("========    =================\n");
+	for (i = 0; i < sizeof(tlv_code_list)/sizeof(tlv_code_list[0]); i++) {
+		printf("0x%02X        %s\n",
+		       tlv_code_list[i].m_code,
+		       tlv_code_list[i].m_name);
+	}
+}
+
+/**
+ *  do_sys_eeprom
+ *
+ *  This function implements the sys_eeprom command.
+ */
+int do_sys_eeprom(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	char cmd;
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+
+	// If no arguments, read the EERPOM and display its contents
+	if (argc == 1) {
+		read_eeprom(eeprom);
+		show_eeprom(eeprom);
+		return 0;
+	}
+
+	// We only look@the first character to the command, so "read" and "reset"
+	// will both be treated as "read".
+	cmd = argv[1][0];
+
+	// Read the EEPROM contents
+	if (cmd == 'r') {
+		has_been_read = 0;
+		if (!read_eeprom(eeprom))
+			printf("EEPROM data loaded from device to memory.\n");
+		return 0;
+	}
+
+	// Subsequent commands require that the EEPROM has already been read.
+	if (!has_been_read) {
+		printf("Please read the EEPROM data first, using the 'sys_eeprom read' command.\n");
+		return 0;
+	}
+
+	// Handle the commands that don't take parameters
+	if (argc == 2) {
+		switch (cmd) {
+		case 'w':   /* write */
+			prog_eeprom(eeprom);
+			break;
+		case 'e':   /* erase */
+			strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
+			eeprom_hdr->version = TLV_INFO_VERSION;
+			eeprom_hdr->totallen = cpu_to_be16(0);
+			update_crc(eeprom);
+			printf("EEPROM data in memory reset.\n");
+			break;
+		case 'l':   /* list */
+			show_tlv_code_list();
+			break;
+		case 'd':   /* dev */
+			show_tlv_devices();
+			break;
+		default:
+			cmd_usage(cmdtp);
+			break;
+		}
+		return 0;
+	}
+
+	// The set command takes one or two args.
+	if (argc > 4) {
+		cmd_usage(cmdtp);
+		return 0;
+	}
+
+	// Set command. If the TLV exists in the EEPROM, delete it. Then if data was
+	// supplied for this TLV add the TLV with the new contents at the end.
+	if (cmd == 's') {
+		int tcode;
+		tcode = simple_strtoul(argv[2], NULL, 0);
+		tlvinfo_delete_tlv(eeprom, tcode);
+		if (argc == 4) {
+			tlvinfo_add_tlv(eeprom, tcode, argv[3]);
+		}
+	} else if (cmd == 'd') { /* 'dev' command */
+		unsigned devnum;
+		devnum = simple_strtoul(argv[2], NULL, 0);
+		if (devnum > MAX_TLV_DEVICES || tlv_devices[devnum] == NULL) {
+			printf("Invalid device number\n");
+			return 0;
+		}
+		current_dev = devnum;
+		has_been_read = 0;
+	} else {
+		cmd_usage(cmdtp);
+	}
+
+	return 0;
+}
+
+/**
+ *  This macro defines the sys_eeprom command line command.
+ */
+U_BOOT_CMD(
+	sys_eeprom, 4, 1,  do_sys_eeprom,
+	"Display and program the system EEPROM data block.",
+	"[read|write|set <type_code> <string_value>|erase|list]\n"
+	"sys_eeprom\n"
+	"    - With no arguments display the current contents.\n"
+	"sys_eeprom dev [dev]\n"
+	"    - List devices or set current EEPROM device.\n"
+	"sys_eeprom read\n"
+	"    - Load EEPROM data from device to memory.\n"
+	"sys_eeprom write\n"
+	"    - Write the EEPROM data to persistent storage.\n"
+	"sys_eeprom set <type_code> <string_value>\n"
+	"    - Set a field to a value.\n"
+	"    - If no string_value, field is deleted.\n"
+	"    - Use 'sys_eeprom write' to make changes permanent.\n"
+	"sys_eeprom erase\n"
+	"    - Reset the in memory EEPROM data.\n"
+	"    - Use 'sys_eeprom read' to refresh the in memory EEPROM data.\n"
+	"    - Use 'sys_eeprom write' to make changes permanent.\n"
+	"sys_eeprom list\n"
+	"    - List the understood TLV codes and names.\n"
+	);
+
+/**
+ *  tlvinfo_find_tlv
+ *
+ *  This function finds the TLV with the supplied code in the EERPOM.
+ *  An offset from the beginning of the EEPROM is returned in the
+ *  eeprom_index parameter if the TLV is found.
+ */
+static bool tlvinfo_find_tlv(u8 * eeprom, u8 tcode, int * eeprom_index)
+{
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+	tlvinfo_tlv_t    * eeprom_tlv;
+	int eeprom_end;
+
+	// Search through the TLVs, looking for the first one which matches the
+	// supplied type code.
+	*eeprom_index = sizeof(tlvinfo_header_t);
+	eeprom_end    = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
+	while (*eeprom_index < eeprom_end) {
+		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[*eeprom_index];
+		if (!is_valid_tlv(eeprom_tlv)) {
+			return(FALSE);
+		}
+		if (eeprom_tlv->type == tcode) {
+			return(TRUE);
+		}
+		*eeprom_index += sizeof(tlvinfo_tlv_t) + eeprom_tlv->length;
+	}
+	return(FALSE);
+}
+
+/**
+ *  tlvinfo_delete_tlv
+ *
+ *  This function deletes the TLV with the specified type code from the
+ *  EEPROM.
+ */
+static bool tlvinfo_delete_tlv(u8 * eeprom, u8 code)
+{
+	int eeprom_index;
+	int tlength;
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+	tlvinfo_tlv_t * eeprom_tlv;
+
+	// Find the TLV and then move all following TLVs "forward"
+	if (tlvinfo_find_tlv(eeprom, code, &eeprom_index)) {
+		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
+		tlength = sizeof(tlvinfo_tlv_t) + eeprom_tlv->length;
+		memcpy(&eeprom[eeprom_index], &eeprom[eeprom_index+tlength],
+		       sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen) - eeprom_index - tlength);
+		eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) - tlength);
+		update_crc(eeprom);
+		return(TRUE);
+	}
+	return(FALSE);
+}
+
+/**
+ *  tlvinfo_add_tlv
+ *
+ *  This function adds a TLV to the EEPROM, converting the value (a string) to
+ *  the format in which it will be stored in the EEPROM.
+ */
+#define MAX_TLV_VALUE_LEN   256
+static bool tlvinfo_add_tlv(u8 * eeprom, int tcode, char * strval)
+{
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+	tlvinfo_tlv_t * eeprom_tlv;
+	int new_tlv_len = 0;
+	u32 value;
+	char data[MAX_TLV_VALUE_LEN];
+	int eeprom_index;
+
+	// Encode each TLV type into the format to be stored in the EERPOM
+	switch (tcode) {
+	case TLV_CODE_PRODUCT_NAME:
+	case TLV_CODE_PART_NUMBER:
+	case TLV_CODE_SERIAL_NUMBER:
+	case TLV_CODE_LABEL_REVISION:
+	case TLV_CODE_PLATFORM_NAME:
+	case TLV_CODE_ONIE_VERSION:
+	case TLV_CODE_MANUF_NAME:
+	case TLV_CODE_MANUF_COUNTRY:
+	case TLV_CODE_VENDOR_NAME:
+	case TLV_CODE_DIAG_VERSION:
+	case TLV_CODE_SERVICE_TAG:
+		strncpy(data, strval, MAX_TLV_VALUE_LEN);
+		new_tlv_len = min_t(size_t, MAX_TLV_VALUE_LEN, strlen(strval));
+		break;
+	case TLV_CODE_DEVICE_VERSION:
+		value = simple_strtoul(strval, NULL, 0);
+		if (value >= 256) {
+			printf("ERROR: Device version must be 255 or less. Value supplied: %u", value);
+			return(FALSE);
+		}
+		data[0] = value & 0xFF;
+		new_tlv_len = 1;
+		break;
+	case TLV_CODE_MAC_SIZE:
+		value = simple_strtoul(strval, NULL, 0);
+		if (value >= 65536) {
+			printf("ERROR: MAC Size must be 65535 or less. Value supplied: %u", value);
+			return(FALSE);
+		}
+		data[0] = (value >> 8) & 0xFF;
+		data[1] = value & 0xFF;
+		new_tlv_len = 2;
+		break;
+	case TLV_CODE_MANUF_DATE:
+		if (set_date(data, strval) != 0) {
+			return(FALSE);
+		}
+		new_tlv_len = 19;
+		break;
+	case TLV_CODE_MAC_BASE:
+		if (set_mac(data, strval) != 0) {
+			return(FALSE);
+		}
+		new_tlv_len = 6;
+		break;
+	case TLV_CODE_CRC_32:
+		printf("WARNING: The CRC TLV is set automatically and cannot be set manually.\n");
+		return(FALSE);
+	case TLV_CODE_VENDOR_EXT:
+	default:
+		if (set_bytes(data, strval, &new_tlv_len) != 0 ) {
+			return(FALSE);
+		}
+		break;
+	}
+
+	// Is there room for this TLV?
+	if ((be16_to_cpu(eeprom_hdr->totallen) + sizeof(tlvinfo_tlv_t) + new_tlv_len)
+			> TLV_TOTAL_LEN_MAX) {
+		printf("ERROR: There is not enough room in the EERPOM to save data.\n");
+		return(FALSE);
+	}
+
+	// Add TLV at the end, overwriting CRC TLV if it exists
+	if (tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index)) {
+		eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) - sizeof(tlvinfo_tlv_t) - 4);
+	}
+	else {
+		eeprom_index = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
+	}
+	eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
+	eeprom_tlv->type = tcode;
+	eeprom_tlv->length = new_tlv_len;
+	memcpy(eeprom_tlv->value, data, new_tlv_len);
+
+	// Update the total length and calculate (add) a new CRC-32 TLV
+	eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) + sizeof(tlvinfo_tlv_t) + new_tlv_len);
+	update_crc(eeprom);
+
+	return(TRUE);
+}
+
+/**
+ *  set_mac
+ *
+ *  Converts a string MAC address into a binary buffer.
+ *
+ *  This function takes a pointer to a MAC address string
+ *  (i.e."XX:XX:XX:XX:XX:XX", where "XX" is a two-digit hex number).
+ *  The string format is verified and then converted to binary and
+ *  stored in a buffer.
+ */
+static int set_mac(char *buf, const char *string)
+{
+	char *p = (char *) string;
+	int   i;
+	int   err = 0;
+	char *end;
+
+	if (!p) {
+		printf("ERROR: NULL mac addr string passed in.\n");
+		return -1;
+	}
+
+	if (strlen(p) != 17) {
+		printf("ERROR: MAC address strlen() != 17 -- %d\n", strlen(p));
+		printf("ERROR: Bad MAC address format: %s\n", string);
+		return -1;
+	}
+
+	for (i = 0; i < 17; i++) {
+		if ((i % 3) == 2) {
+			if (p[i] != ':') {
+				err++;
+				printf("ERROR: mac: p[%i] != :, found: `%c'\n",
+				       i, p[i]);
+				break;
+			}
+			continue;
+		} else if (!is_hex(p[i])) {
+			err++;
+			printf("ERROR: mac: p[%i] != hex digit, found: `%c'\n",
+			       i, p[i]);
+			break;
+		}
+	}
+
+	if (err != 0) {
+		printf("ERROR: Bad MAC address format: %s\n", string);
+		return -1;
+	}
+
+	/* Convert string to binary */
+	for (i = 0, p = (char *)string; i < 6; i++) {
+		buf[i] = p ? simple_strtoul(p, &end, 16) : 0;
+		if (p) {
+			p = (*end) ? end + 1 : end;
+		}
+	}
+
+	if (!is_valid_ethaddr((u8 *)buf)) {
+		printf("ERROR: MAC address must not be 00:00:00:00:00:00, "
+		       "a multicast address or FF:FF:FF:FF:FF:FF.\n");
+		printf("ERROR: Bad MAC address format: %s\n", string);
+		return -1;
+	}
+
+	return 0;
+}
+
+/**
+ *  set_date
+ *
+ *  Validates the format of the data string
+ *
+ *  This function takes a pointer to a date string (i.e. MM/DD/YYYY hh:mm:ss)
+ *  and validates that the format is correct. If so the string is copied
+ *  to the supplied buffer.
+ */
+static int set_date(char *buf, const char *string)
+{
+	int i;
+
+	if (!string) {
+		printf("ERROR: NULL date string passed in.\n");
+		return -1;
+	}
+
+	if (strlen(string) != 19) {
+		printf("ERROR: Date strlen() != 19 -- %d\n", strlen(string));
+		printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
+		return -1;
+	}
+
+	for (i = 0; string[i] != 0; i++) {
+		switch (i) {
+		case 2:
+		case 5:
+			if (string[i] != '/') {
+				printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
+				return -1;
+			}
+			break;
+		case 10:
+			if (string[i] != ' ') {
+				printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
+				return -1;
+			}
+			break;
+		case 13:
+		case 16:
+			if (string[i] != ':') {
+				printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
+				return -1;
+			}
+			break;
+		default:
+			if (!is_digit(string[i])) {
+				printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
+				return -1;
+			}
+			break;
+		}
+	}
+
+	strcpy(buf, string);
+	return 0;
+}
+
+/**
+ *  set_bytes
+ *
+ *  Converts a space-separated string of decimal numbers into a
+ *  buffer of bytes.
+ *
+ *  This function takes a pointer to a space-separated string of decimal
+ *  numbers (i.e. "128 0x55 0321") with "C" standard radix specifiers
+ *  and converts them to an array of bytes.
+ */
+static int set_bytes(char *buf, const char *string, int * converted_accum)
+{
+	char *p = (char *) string;
+	int   i;
+	uint  byte;
+
+	if (!p) {
+		printf("ERROR: NULL string passed in.\n");
+		return -1;
+	}
+
+	/* Convert string to bytes */
+	for (i = 0, p = (char *)string; (i < TLV_VALUE_MAX_LEN) && (*p != 0); i++) {
+		while ((*p == ' ') || (*p == '\t') || (*p == ',') || (*p == ';')) {
+			p++;
+		}
+		if (*p != 0) {
+			if (!is_digit(*p)) {
+				printf("ERROR: Non-digit found in byte string: (%s)\n", string);
+				return -1;
+			}
+			byte = simple_strtoul(p, &p, 0);
+			if (byte >= 256) {
+				printf("ERROR: The value specified is greater than 255: (%u) in string: %s\n",
+				       byte, string);
+				return -1;
+			}
+			buf[i] = byte & 0xFF;
+		}
+	}
+
+	if ((i == TLV_VALUE_MAX_LEN) && (*p != 0)) {
+		printf("ERROR: Trying to assign too many bytes "
+		       "(max: %d) in string: %s\n", TLV_VALUE_MAX_LEN, string);
+		return -1;
+	}
+
+	*converted_accum = i;
+	return 0;
+}
+
+static void show_tlv_devices(void)
+{
+	unsigned dev;
+
+	for (dev = 0; dev < MAX_TLV_DEVICES; dev++)
+		if (tlv_devices[dev])
+			printf("TLV: %u%s\n", dev,
+					(dev == current_dev) ? " (*)" : "");
+}
+
+static int find_tlv_devices(void)
+{
+	int ret;
+	int count_dev = 0;
+	struct udevice *dev;
+
+	for (ret = uclass_first_device_check(UCLASS_I2C_EEPROM, &dev);
+			dev;
+			ret = uclass_next_device_check(&dev)) {
+		if (ret == 0)
+			tlv_devices[count_dev++] = dev;
+	}
+
+	return (count_dev == 0) ? -ENODEV : 0;
+}
+
+/**
+ * read_sys_eeprom - read the hwinfo from i2c EEPROM
+ */
+int read_sys_eeprom(void *eeprom, int offset, int len)
+{
+	int ret;
+
+	if (tlv_devices[current_dev] == NULL) {
+		ret = find_tlv_devices();
+		if (ret)
+			return ret;
+	}
+
+	if (tlv_devices[current_dev] == NULL)
+		return -ENODEV;
+
+	return i2c_eeprom_read(tlv_devices[current_dev], offset, eeprom, len);
+}
+
+/**
+ * write_sys_eeprom - write the hwinfo to i2c EEPROM
+ */
+int write_sys_eeprom(void *eeprom, int len)
+{
+	if (tlv_devices[current_dev] == NULL)
+		return -ENODEV;
+
+	return i2c_eeprom_write(tlv_devices[current_dev], 0, eeprom, len);
+}
+
+int read_tlvinfo_sys_eeprom(void *eeprom, tlvinfo_header_t **hdr,
+		tlvinfo_tlv_t **first_entry)
+{
+	int ret;
+	tlvinfo_header_t *tlv_hdr;
+	tlvinfo_tlv_t *tlv_ent;
+
+	/* Read TLV header */
+	ret = read_sys_eeprom(eeprom, 0, sizeof(tlvinfo_header_t));
+	if (ret < 0)
+		return ret;
+
+	tlv_hdr = eeprom;
+	if (!is_valid_tlvinfo_header(tlv_hdr))
+		return -EINVAL;
+
+	/* Read TLV entries */
+	tlv_ent = (tlvinfo_tlv_t *) &tlv_hdr[1];
+	ret = read_sys_eeprom(tlv_ent, sizeof(tlvinfo_header_t),
+			be16_to_cpu(tlv_hdr->totallen));
+	if (ret < 0)
+		return ret;
+	if (!is_checksum_valid(eeprom))
+		return -EINVAL;
+
+	*hdr = tlv_hdr;
+	*first_entry = tlv_ent;
+
+	return 0;
+}
+
+int read_tlvinfo_sys_eeprom_dev(void *eeprom, tlvinfo_header_t **hdr,
+		tlvinfo_tlv_t **first_entry, unsigned devnum)
+{
+	int ret;
+	unsigned prev_dev = MAX_TLV_DEVICES + 1;
+
+	if (devnum > MAX_TLV_DEVICES)
+		return -EINVAL;
+
+	if (devnum != current_dev) {
+		prev_dev = current_dev;
+		current_dev = devnum;
+		has_been_read = 0;
+	}
+
+	ret = read_tlvinfo_sys_eeprom(eeprom, hdr, first_entry);
+	if (ret < 0 && prev_dev <= MAX_TLV_DEVICES)
+		current_dev = prev_dev;
+
+	return ret;
+}
+
+/**
+ *  mac_read_from_eeprom
+ *
+ *  Read the MAC addresses from EEPROM
+ *
+ *  This function reads the MAC addresses from EEPROM and sets the
+ *  appropriate environment variables for each one read.
+ *
+ *  The environment variables are only set if they haven't been set already.
+ *  This ensures that any user-saved variables are never overwritten.
+ *
+ *  This function must be called after relocation.
+ */
+int mac_read_from_eeprom(void)
+{
+	unsigned int i;
+	int eeprom_index;
+	tlvinfo_tlv_t * eeprom_tlv;
+	int maccount;
+	u8 macbase[6];
+	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
+
+	puts("EEPROM: ");
+
+	if (read_eeprom(eeprom)) {
+		printf("Read failed.\n");
+		return -1;
+	}
+
+	maccount = 1;
+	if (tlvinfo_find_tlv(eeprom, TLV_CODE_MAC_SIZE, &eeprom_index)) {
+		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
+		maccount = (eeprom_tlv->value[0] << 8) | eeprom_tlv->value[1];
+	}
+
+	memcpy(macbase, "\0\0\0\0\0\0", 6);
+	if (tlvinfo_find_tlv(eeprom, TLV_CODE_MAC_BASE, &eeprom_index)) {
+		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
+		memcpy(macbase, eeprom_tlv->value, 6);
+	}
+
+	for (i = 0; i < maccount; i++) {
+		if (is_valid_ethaddr(macbase)) {
+			char ethaddr[18];
+			char enetvar[11];
+
+			sprintf(ethaddr, "%02X:%02X:%02X:%02X:%02X:%02X",
+				macbase[0], macbase[1], macbase[2],
+				macbase[3], macbase[4], macbase[5]);
+			sprintf(enetvar, i ? "eth%daddr" : "ethaddr", i);
+			/* Only initialize environment variables that are blank
+			 * (i.e. have not yet been set)
+			 */
+			if (!env_get(enetvar))
+				env_set(enetvar, ethaddr);
+
+			macbase[5]++;
+			if (macbase[5] == 0) {
+				macbase[4]++;
+				if (macbase[4] == 0) {
+					macbase[3]++;
+					if (macbase[3] == 0) {
+						macbase[0] = 0;
+						macbase[1] = 0;
+						macbase[2] = 0;
+					}
+				}
+			}
+		}
+	}
+
+	printf("%s v%u len=%u\n", eeprom_hdr->signature, eeprom_hdr->version,
+	       be16_to_cpu(eeprom_hdr->totallen));
+
+	return 0;
+}
+
+/**
+ *  populate_serial_number - read the serial number from EEPROM
+ *
+ *  This function reads the serial number from the EEPROM and sets the
+ *  appropriate environment variable.
+ *
+ *  The environment variable is only set if it has not been set
+ *  already.  This ensures that any user-saved variables are never
+ *  overwritten.
+ *
+ *  This function must be called after relocation.
+ */
+int populate_serial_number(void)
+{
+	char serialstr[257];
+	int eeprom_index;
+	tlvinfo_tlv_t * eeprom_tlv;
+
+	if (env_get("serial#"))
+		return 0;
+
+	if (read_eeprom(eeprom)) {
+		printf("Read failed.\n");
+		return -1;
+	}
+
+	if (tlvinfo_find_tlv(eeprom, TLV_CODE_SERIAL_NUMBER, &eeprom_index)) {
+		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
+		memcpy(serialstr, eeprom_tlv->value, eeprom_tlv->length);
+		serialstr[eeprom_tlv->length] = 0;
+		env_set("serial#", serialstr);
+	}
+
+	return 0;
+}
diff --git a/include/sys_eeprom.h b/include/sys_eeprom.h
new file mode 100644
index 000000000000..49a7ee87d9db
--- /dev/null
+++ b/include/sys_eeprom.h
@@ -0,0 +1,169 @@
+/*
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __SYS_EEPROM_H_
+#define __SYS_EEPROM_H_
+
+/*
+ *  Without getting too philosophical, define truth, falsehood, and the
+ *  boolean type, if they are not already defined.
+ */
+#ifndef FALSE
+#define FALSE   0
+#endif
+
+#ifndef TRUE
+#define TRUE    (!FALSE)
+#endif
+
+#ifndef bool
+typedef unsigned char bool;
+#endif
+
+#ifndef is_digit
+#define is_digit(c)             ((c) >= '0' && (c) <= '9')
+#endif
+
+/*
+ *  The Definition of the TlvInfo EEPROM format can be found at onie.org or
+ *  github.com/onie
+ */
+
+/*
+ * TlvInfo header: Layout of the header for the TlvInfo format
+ *
+ * See the end of this file for details of this eeprom format
+ */
+struct __attribute__ ((__packed__)) tlvinfo_header_s {
+    char    signature[8];       /* 0x00 - 0x07 EEPROM Tag "TlvInfo" */
+    u8      version;            /* 0x08        Structure version    */
+    u16     totallen;           /* 0x09 - 0x0A Length of all data which follows */
+};
+typedef struct tlvinfo_header_s tlvinfo_header_t;
+
+// Header Field Constants
+#define TLV_INFO_ID_STRING      "TlvInfo"
+#define TLV_INFO_VERSION        0x01
+#define TLV_INFO_MAX_LEN        2048
+#define TLV_TOTAL_LEN_MAX       (TLV_INFO_MAX_LEN - sizeof(tlvinfo_header_t))
+
+/*
+ * TlvInfo TLV: Layout of a TLV field
+ */
+struct __attribute__ ((__packed__)) tlvinfo_tlv_s {
+    u8  type;
+    u8  length;
+    u8  value[0];
+};
+typedef struct tlvinfo_tlv_s tlvinfo_tlv_t;
+
+/* Maximum length of a TLV value in bytes */
+#define TLV_VALUE_MAX_LEN        255
+
+/**
+ *  The TLV Types.
+ *
+ *  Keep these in sync with tlv_code_list in cmd_sys_eeprom.c
+ */
+#define TLV_CODE_PRODUCT_NAME   0x21
+#define TLV_CODE_PART_NUMBER    0x22
+#define TLV_CODE_SERIAL_NUMBER  0x23
+#define TLV_CODE_MAC_BASE       0x24
+#define TLV_CODE_MANUF_DATE     0x25
+#define TLV_CODE_DEVICE_VERSION 0x26
+#define TLV_CODE_LABEL_REVISION 0x27
+#define TLV_CODE_PLATFORM_NAME  0x28
+#define TLV_CODE_ONIE_VERSION   0x29
+#define TLV_CODE_MAC_SIZE       0x2A
+#define TLV_CODE_MANUF_NAME     0x2B
+#define TLV_CODE_MANUF_COUNTRY  0x2C
+#define TLV_CODE_VENDOR_NAME    0x2D
+#define TLV_CODE_DIAG_VERSION   0x2E
+#define TLV_CODE_SERVICE_TAG    0x2F
+#define TLV_CODE_VENDOR_EXT     0xFD
+#define TLV_CODE_CRC_32         0xFE
+
+/**
+ * read_sys_eeprom - Read the EEPROM binary data from the hardware
+ * @eeprom: Pointer to buffer to hold the binary data
+ * @offset: Offset within EEPROM block to read data from
+ * @len   : Maximum size of buffer
+ *
+ * Note: this routine does not validate the EEPROM data.
+ *
+ */
+
+extern int read_sys_eeprom(void *eeprom, int offset, int len);
+
+/**
+ * write_sys_eeprom - Write the entire EEPROM binary data to the hardware
+ * @eeprom: Pointer to buffer to hold the binary data
+ * @len   : Maximum size of buffer
+ *
+ * Note: this routine does not validate the EEPROM data.
+ *
+ */
+extern int write_sys_eeprom(void *eeprom, int len);
+
+/**
+ * read_tlvinfo_sys_eeprom - Read the TLV from EEPROM, and validate
+ * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
+ *          of size at least TLV_INFO_MAX_LEN.
+ * @hdr   : Points to pointer to TLV header (output)
+ * @first_entry : Points to pointer to first TLV entry (output)
+ *
+ * Store the raw EEPROM data in the @eeprom buffer. If TLV is valid set *@hdr
+ * and *@first_entry.
+ *
+ * Returns 0 when read from EEPROM is successful, and the data is valid.
+ * Returns <0 error value when EEPROM read fails. Return -EINVAL when TLV is
+ * invalid.
+ *
+ */
+
+extern int read_tlvinfo_sys_eeprom(void *eeprom, tlvinfo_header_t **hdr,
+		tlvinfo_tlv_t **first_entry);
+
+/**
+ * read_tlvinfo_sys_eeprom_dev - Read the TLV from specific EEPROM, and validate
+ * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
+ *          of size@least TLV_INFO_MAX_LEN.
+ * @hdr   : Points to pointer to TLV header (output)
+ * @first_entry : Points to pointer to first TLV entry (output)
+ * @dev   : EEPROM device to read
+ *
+ * Store the raw EEPROM data from EEPROM @dev in the @eeprom buffer. If TLV is
+ * valid set *@hdr and *@first_entry.
+ *
+ * Returns 0 when read from EEPROM is successful, and the data is valid.
+ * Returns <0 error value when EEPROM read fails. Return -EINVAL when TLV is
+ * invalid.
+ *
+ */
+
+extern int read_tlvinfo_sys_eeprom_dev(void *eeprom, tlvinfo_header_t **hdr,
+		tlvinfo_tlv_t **first_entry, unsigned devnum);
+
+/**
+ *  is_valid_tlvinfo_header
+ *
+ *  Perform sanity checks on the first 11 bytes of the TlvInfo EEPROM
+ *  data pointed to by the parameter:
+ *      1. First 8 bytes contain null-terminated ASCII string "TlvInfo"
+ *      2. Version byte is 1
+ *      3. Total length bytes contain value which is less than or equal
+ *         to the allowed maximum (2048-11)
+ *
+ */
+static inline bool is_valid_tlvinfo_header(tlvinfo_header_t *hdr)
+{
+	return( (strcmp(hdr->signature, TLV_INFO_ID_STRING) == 0) &&
+		(hdr->version == TLV_INFO_VERSION) &&
+		(be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX) );
+}
+
+#endif /* __SYS_EEPROM_H_ */
-- 
2.24.0

[U-Boot] [PATCH 04/10] ARM: mvebu: clearfog: add EEPROM devices

[Baruch Siach]

Add device-tree description for the EEPROM devices on Armada 388 SOM rev
2.1.

Another EEPROM is now on Clearfog Pro carrier rev 2.2, and Clearfog Base
rev 1.3.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 arch/arm/dts/armada-388-clearfog-u-boot.dtsi   | 12 ++++++++++++
 arch/arm/dts/armada-388-clearfog.dts           |  6 ++++++
 arch/arm/dts/armada-38x-solidrun-microsom.dtsi |  8 ++++++++
 3 files changed, 26 insertions(+)

diff --git a/arch/arm/dts/armada-388-clearfog-u-boot.dtsi b/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
index 27af14555001..71a073a225a4 100644
--- a/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
+++ b/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
@@ -20,3 +20,15 @@
 &ahci1 {
 	u-boot,dm-spl;
 };
+
+&i2c0 {
+	u-boot,dm-spl;
+
+	eeprom at 52 {
+		u-boot,dm-spl;
+	};
+
+	eeprom at 53 {
+		u-boot,dm-spl;
+	};
+};
diff --git a/arch/arm/dts/armada-388-clearfog.dts b/arch/arm/dts/armada-388-clearfog.dts
index 4ddeaa02f122..e4164f49b24d 100644
--- a/arch/arm/dts/armada-388-clearfog.dts
+++ b/arch/arm/dts/armada-388-clearfog.dts
@@ -259,6 +259,12 @@
 		compatible = "microchip,mcp3021";
 		reg = <0x4c>;
 	};
+
+	eeprom at 52 {
+		compatible = "atmel,24c02";
+		reg = <0x52>;
+		pagesize = <16>;
+	};
 };
 
 &i2c1 {
diff --git a/arch/arm/dts/armada-38x-solidrun-microsom.dtsi b/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
index a2627223ce3b..a322a28c215c 100644
--- a/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
+++ b/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
@@ -99,3 +99,11 @@
 	status = "okay";
 	u-boot,dm-pre-reloc;
 };
+
+&i2c0 {
+	eeprom at 53 {
+		compatible = "atmel,24c02";
+		reg = <0x53>;
+		pagesize = <16>;
+	};
+};
-- 
2.24.0

[U-Boot] [PATCH 05/10] ARM: mvebu: clearfog: add support for EEPROM TLV info

[Baruch Siach]

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 configs/clearfog_defconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index dd48b325c0bc..2bf80ee70cec 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -11,6 +11,7 @@ CONFIG_TARGET_CLEARFOG=y
 CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC=y
 CONFIG_SPL_MMC_SUPPORT=y
 CONFIG_SPL_SERIAL_SUPPORT=y
+CONFIG_SPL_DRIVERS_MISC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
 CONFIG_SPL=y
 CONFIG_DEBUG_UART_BASE=0xd0012000
@@ -26,6 +27,8 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET=0x1
 CONFIG_SPL_I2C_SUPPORT=y
+CONFIG_CMD_SYS_EEPROM=y
+CONFIG_SPL_CMD_SYS_EEPROM=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
@@ -47,6 +50,8 @@ CONFIG_DM_GPIO=y
 CONFIG_DM_PCA953X=y
 CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_MVTWSI=y
+CONFIG_I2C_EEPROM=y
+CONFIG_SPL_I2C_EEPROM=y
 CONFIG_DM_MMC=y
 CONFIG_SUPPORT_EMMC_BOOT=y
 CONFIG_MMC_SDHCI=y
-- 
2.24.0

[U-Boot] [PATCH 06/10] ARM: mvebu: clearfog: read basic TLV data

[Baruch Siach]

Read RAM die capacity from the EEPROM TLV.

Follow the ONIE standard that defines the Vendor Extension entry type
for vendor specific data. We have no Private Enterprise Number at the
moment as the standard requires. Use the dummy all 0xff value for now.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 board/solidrun/clearfog/clearfog.c | 118 +++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
index 8b6381194688..707afabb11a7 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -7,6 +7,7 @@
 #include <i2c.h>
 #include <miiphy.h>
 #include <netdev.h>
+#include <sys_eeprom.h>
 #include <asm/io.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/soc.h>
@@ -16,6 +17,14 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#define SR_TLV_CODE_RAM_DIE_SIZE	0x81
+
+/*
+ * Some Clearfog variants have TLV on SOM and on carrier, with separate
+ * product name entries.
+ */
+static char tlv_product_name[2][32];
+
 /*
  * Those values and defines are taken from the Marvell U-Boot version
  * "u-boot-2013.01-15t1-clearfog"
@@ -74,8 +83,117 @@ static struct mv_ddr_topology_map board_topology_map = {
 	0x3,				/* clock enable mask */
 };
 
+static void store_product_name(tlvinfo_tlv_t *tlv_entry)
+{
+	int len;
+	char *dest;
+
+	if (strlen(tlv_product_name[0]) == 0)
+		dest = tlv_product_name[0];
+	else if (strlen(tlv_product_name[1]) == 0)
+		dest = tlv_product_name[1];
+	else
+		return;
+
+	len = min_t(unsigned, tlv_entry->length,
+			sizeof(tlv_product_name[0])-1);
+	memcpy(dest, tlv_entry->value, len);
+}
+
+static bool sr_product_is(const char *product)
+{
+	/* Allow prefix sub-string match */
+	if (strncmp(tlv_product_name[0], product, strlen(product)) == 0)
+		return true;
+	if (strncmp(tlv_product_name[1], product, strlen(product)) == 0)
+		return true;
+
+	return false;
+}
+
+static void parse_tlv_vendor_ext(tlvinfo_tlv_t *tlv_entry)
+{
+	struct if_params *ifp = &board_topology_map.interface_params[0];
+	u8 *val = tlv_entry->value;
+	uint32_t pen; /* IANA Private Enterprise Numbers */
+
+	if (tlv_entry->length < 5) /* 4 bytes PEN +@least 1 byte type */
+		return;
+
+	/* PEN is big endian */
+	pen = (val[0] << 24) | (val[1] << 16) | (val[2] << 8) | val[3];
+	/* Not a real PEN */
+	if (pen != 0xffffffff)
+		return;
+
+	switch (val[4]) {
+	case SR_TLV_CODE_RAM_DIE_SIZE:
+		if (tlv_entry->length != 6)
+			break;
+		switch (val[5]) {
+		case 4:
+		default:
+			ifp->memory_size = MV_DDR_DIE_CAP_4GBIT;
+			break;
+		case 8:
+			ifp->memory_size = MV_DDR_DIE_CAP_8GBIT;
+			break;
+		}
+	default:
+		break;
+	}
+}
+
+static void parse_tlv_data(uint8_t *eeprom, tlvinfo_header_t *hdr,
+		tlvinfo_tlv_t *entry)
+{
+	unsigned tlv_offset, tlv_len;
+
+	tlv_offset = sizeof(tlvinfo_header_t);
+	tlv_len = sizeof(tlvinfo_header_t) + be16_to_cpu(hdr->totallen);
+	while (tlv_offset < tlv_len) {
+		entry = (tlvinfo_tlv_t *) &eeprom[tlv_offset];
+
+		switch (entry->type) {
+		case TLV_CODE_PRODUCT_NAME:
+			store_product_name(entry);
+			break;
+		case TLV_CODE_VENDOR_EXT:
+			parse_tlv_vendor_ext(entry);
+			break;
+		default:
+			break;
+		}
+
+		tlv_offset += sizeof(tlvinfo_tlv_t) + entry->length;
+	}
+}
+
+static void read_tlv_data(void)
+{
+	uint8_t eeprom_data[TLV_TOTAL_LEN_MAX];
+	tlvinfo_header_t *tlv_hdr;
+	tlvinfo_tlv_t *tlv_entry;
+	static bool ran_once;
+	int ret, i;
+
+	if (ran_once)
+		return;
+	ran_once = true;
+
+	for (i = 0; i < 2; i++) {
+		ret = read_tlvinfo_sys_eeprom_dev(eeprom_data, &tlv_hdr,
+				&tlv_entry, i);
+		if (ret < 0)
+			continue;
+		parse_tlv_data(eeprom_data, tlv_hdr, tlv_entry);
+	}
+}
+
 struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
 {
+	read_tlv_data();
+
 	/* Return the board topology as defined in the board code */
 	return &board_topology_map;
 }
-- 
2.24.0

[U-Boot] [PATCH 07/10] ARM: mvebu: clearfog: print TLV stored product name

[Baruch Siach]

Use the data from EEPROM TLV to display the board identity.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 board/solidrun/clearfog/clearfog.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
index 707afabb11a7..852b1ad71bb2 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -246,7 +246,16 @@ int board_init(void)
 
 int checkboard(void)
 {
-	puts("Board: SolidRun ClearFog\n");
+	char *board = "ClearFog";
+
+	read_tlv_data();
+	if (strlen(tlv_product_name[0]) > 0)
+		board = tlv_product_name[0];
+
+	printf("Board: SolidRun %s", board);
+	if (strlen(tlv_product_name[1]) > 0)
+		printf(", %s", tlv_product_name[1]);
+	puts("\n");
 
 	return 0;
 }
-- 
2.24.0

[U-Boot] [PATCH 08/10] ARM: mvebu: clearfog: run-time selection of DT file

[Baruch Siach]

Set the kernel device-tree file (fdtfile environment variable) based on
run-time detection of the platform.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 arch/arm/mach-mvebu/Kconfig        |  1 +
 board/solidrun/clearfog/clearfog.c | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index fdd39685b75d..bc5eaa5a7679 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -91,6 +91,7 @@ choice
 config TARGET_CLEARFOG
 	bool "Support ClearFog"
 	select 88F6820
+	select BOARD_LATE_INIT
 
 config TARGET_HELIOS4
 	bool "Support Helios4"
diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
index 852b1ad71bb2..00c1306e9c80 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -265,3 +265,17 @@ int board_eth_init(bd_t *bis)
 	cpu_eth_init(bis); /* Built in controller(s) come first */
 	return pci_eth_init(bis);
 }
+
+int board_late_init(void)
+{
+	read_tlv_data();
+
+	if (sr_product_is("Clearfog Base"))
+		 env_set("fdtfile", "armada-388-clearfog-base.dtb");
+	else if (sr_product_is("Clearfog GTR S4"))
+		 env_set("fdtfile", "armada-385-clearfog-gtr-s4.dtb");
+	else if (sr_product_is("Clearfog GTR L8"))
+		 env_set("fdtfile", "armada-385-clearfog-gtr-l8.dtb");
+
+	return 0;
+}
-- 
2.24.0

[U-Boot] [PATCH 09/10] ARM: mvebu: clearfog: add Clearfog GTR support

[Baruch Siach]

Select the serdes configuration table based on the platform identity
read from EEPROM TLV data. Clearfog GTR needs a slightly different
serdes configuration.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 board/solidrun/clearfog/clearfog.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
index 00c1306e9c80..70bea1e8dc65 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -25,6 +25,8 @@ DECLARE_GLOBAL_DATA_PTR;
  */
 static char tlv_product_name[2][32];
 
+static void read_tlv_data(void);
+
 /*
  * Those values and defines are taken from the Marvell U-Boot version
  * "u-boot-2013.01-15t1-clearfog"
@@ -48,6 +50,8 @@ static struct serdes_map board_serdes_map[] = {
 
 int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
 {
+	read_tlv_data();
+
 	*serdes_map_array = board_serdes_map;
 	*count = ARRAY_SIZE(board_serdes_map);
 	return 0;
@@ -188,6 +192,12 @@ static void read_tlv_data(void)
 			continue;
 		parse_tlv_data(eeprom_data, tlv_hdr, tlv_entry);
 	}
+
+	if (sr_product_is("Clearfog GTR")) {
+		board_serdes_map[0].serdes_type = PEX0;
+		board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
+		board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
+	}
 }
 
 struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
-- 
2.24.0

[U-Boot] [PATCH 10/10] ARM: mvebu: clearfog: add Clearfog Base serdes configuration

[Baruch Siach]

Clearfog Base carrier uses serdes lane #5 as USB host. Use EEPROM stored
device identification to configure the serdes accordingly when
available.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 board/solidrun/clearfog/clearfog.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
index 70bea1e8dc65..f407f744bf0a 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -198,6 +198,12 @@ static void read_tlv_data(void)
 		board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
 		board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
 	}
+
+	if (sr_product_is("Clearfog Base")) {
+		board_serdes_map[4].serdes_type = USB3_HOST0;
+		board_serdes_map[4].serdes_speed = SERDES_SPEED_5_GBPS;
+		board_serdes_map[4].serdes_mode = SERDES_DEFAULT_MODE;
+	}
 }
 
 struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
-- 
2.24.0

[PATCH 01/10] ddr: marvell: a38x: allow board specific clock out setup

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> DDR clock out might be unrelated to the number of active chip-select.
> For example, the board might have two DDR components, but only one
> chip-select. The clk_enable mask allows the board to enable DDR clocks
> regardless of active chip-selects.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   drivers/ddr/marvell/a38x/ddr3_training.c    | 10 ++++++++--
>   drivers/ddr/marvell/a38x/ddr_topology_def.h |  3 +++
>   2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ddr/marvell/a38x/ddr3_training.c b/drivers/ddr/marvell/a38x/ddr3_training.c
> index c7be700d641f..9718f18c3024 100644
> --- a/drivers/ddr/marvell/a38x/ddr3_training.c
> +++ b/drivers/ddr/marvell/a38x/ddr3_training.c
> @@ -280,8 +280,14 @@ int ddr3_tip_configure_cs(u32 dev_num, u32 if_id, u32 cs_num, u32 enable)
>   {
>   	u32 data, addr_hi, data_high;
>   	u32 mem_index;
> +	u32 clk_enable;
>   	struct mv_ddr_topology_map *tm = mv_ddr_topology_map_get();
>   
> +	if (tm->clk_enable & (1 << cs_num))
> +		clk_enable = 1;
> +	else
> +		clk_enable = enable;
> +
>   	if (enable == 1) {
>   		data = (tm->interface_params[if_id].bus_width ==
>   			MV_DDR_DEV_WIDTH_8BIT) ? 0 : 1;
> @@ -316,13 +322,13 @@ int ddr3_tip_configure_cs(u32 dev_num, u32 if_id, u32 cs_num, u32 enable)
>   	case 2:
>   		CHECK_STATUS(ddr3_tip_if_write
>   			     (dev_num, ACCESS_TYPE_UNICAST, if_id,
> -			      DUNIT_CTRL_LOW_REG, (enable << (cs_num + 11)),
> +			      DUNIT_CTRL_LOW_REG, (clk_enable << (cs_num + 11)),
>   			      1 << (cs_num + 11)));
>   		break;
>   	case 3:
>   		CHECK_STATUS(ddr3_tip_if_write
>   			     (dev_num, ACCESS_TYPE_UNICAST, if_id,
> -			      DUNIT_CTRL_LOW_REG, (enable << 15), 1 << 15));
> +			      DUNIT_CTRL_LOW_REG, (clk_enable << 15), 1 << 15));
>   		break;
>   	}
>   
> diff --git a/drivers/ddr/marvell/a38x/ddr_topology_def.h b/drivers/ddr/marvell/a38x/ddr_topology_def.h
> index e6fe8a04289b..950f296ff984 100644
> --- a/drivers/ddr/marvell/a38x/ddr_topology_def.h
> +++ b/drivers/ddr/marvell/a38x/ddr_topology_def.h
> @@ -124,6 +124,9 @@ struct mv_ddr_topology_map {
>   
>   	/* electrical parameters */
>   	unsigned int electrical_data[MV_DDR_EDATA_LAST];
> +
> +	/* Clock enable mask */
> +	u32 clk_enable;
>   };
>   
>   enum mv_ddr_iface_mode {
> 

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 02/10] arm: mvebu: clearfog: enable both DDR clocks

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Enabled both DDR clock signals to support Clearfog variants (currently,
> Clearfog GTR) that need both clocks.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   board/solidrun/clearfog/clearfog.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> index 03724fee10c1..8b6381194688 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -68,7 +68,10 @@ static struct mv_ddr_topology_map board_topology_map = {
>   	BUS_MASK_32BIT,			/* Busses mask */
>   	MV_DDR_CFG_DEFAULT,		/* ddr configuration data source */
>   	{ {0} },			/* raw spd data */
> -	{0}				/* timing parameters */
> +	{0},				/* timing parameters */
> +	{ {0} },			/* electrical configuration */
> +	{0,},				/* electrical parameters */
> +	0x3,				/* clock enable mask */
>   };
>   
>   struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
> 

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 03/10] cmd: add sys_eeprom command

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Based on U-Boot patch from the Open Compute project:
> 
>    https://github.com/opencomputeproject/onie/blob/ec87e872d46b9805565d2c6124b2f701ef1c07b1/patches/u-boot/common/feature-sys-eeprom-tlv-common.patch
> 
> Keep only I2C EEPROM support. Use the generic eeprom driver.
> 
> Add support for multiple EEPROM TLV stores on the same system.

Could you please explain, what TLV stands for? Is it "Type Length
Value"? Please add it to the description.

> This is
> useful in case of SOM and carrier that both provide ID and hardware
> configuration information.
> 
> Add option to enable for SPL. This allows selection of RAM configuration
> based on EEPROM stored board identification.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Shouldn't you add the copyrights from the original patch:

Add to support 'sys_eeprom' command (common part)

Copyright (C) 2013 Curt Brune <curt@cumulusnetworks.com>
Copyright (C) 2014 Srideep <srideep_devireddy@dell.com>
Copyright (C) 2013 Miles Tseng <miles_tseng@accton.com>
Copyright (C) 2014,2016 david_yang <david_yang@accton.com>

?

> ---
>   cmd/Kconfig          |   12 +
>   cmd/Makefile         |    2 +
>   cmd/sys_eeprom.c     | 1078 ++++++++++++++++++++++++++++++++++++++++++
>   include/sys_eeprom.h |  169 +++++++
>   4 files changed, 1261 insertions(+)
>   create mode 100644 cmd/sys_eeprom.c
>   create mode 100644 include/sys_eeprom.h
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 99b8a0e21822..483663404da4 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -234,6 +234,18 @@ config CMD_REGINFO
>   	help
>   	  Register dump
>   
> +config CMD_SYS_EEPROM
> +	bool "sys_eeprom"
> +	depends on I2C_EEPROM
> +	help
> +	  Display and program the system EEPROM data block.
> +
> +config SPL_CMD_SYS_EEPROM
> +	bool "sys_eeprom for SPL"
> +	depends on SPL_I2C_EEPROM
> +	help
> +	  Read system EEPROM data block.
> +

I'm not sure, if this description is enough. This sounds too generic
for my taste as I assume that there are multiple formats to store system
data in an EEPROM. It might make sense to mention TLV here. And perhaps
its also better to name the files differently by adding "tlv" as well.

What do you think?

>   endmenu
>   
>   menu "Boot commands"
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 2d723ea0f07d..cbb1d46b8f50 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -180,6 +180,8 @@ obj-$(CONFIG_X86) += x86/
>   obj-$(CONFIG_ARCH_MVEBU) += mvebu/
>   endif # !CONFIG_SPL_BUILD
>   
> +obj-$(CONFIG_$(SPL_)CMD_SYS_EEPROM) += sys_eeprom.o
> +
>   # core command
>   obj-y += nvedit.o
>   
> diff --git a/cmd/sys_eeprom.c b/cmd/sys_eeprom.c
> new file mode 100644
> index 000000000000..373673a5266c
> --- /dev/null
> +++ b/cmd/sys_eeprom.c
> @@ -0,0 +1,1078 @@
> +/*
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <dm.h>
> +#include <i2c.h>
> +#include <i2c_eeprom.h>
> +#include <env.h>
> +#include <linux/ctype.h>
> +
> +#include "sys_eeprom.h"
> +
> +#define MAX_TLV_DEVICES	2
> +
> +/* File scope function prototypes */
> +static bool is_checksum_valid(u8 *eeprom);
> +static int read_eeprom(u8 *eeprom);
> +static void show_eeprom(u8 *eeprom);
> +static void decode_tlv(tlvinfo_tlv_t * tlv);
> +static void update_crc(u8 *eeprom);
> +static int prog_eeprom(u8 * eeprom);
> +static bool tlvinfo_find_tlv(u8 * eeprom, u8 tcode, int * eeprom_index);
> +static bool tlvinfo_delete_tlv(u8 * eeprom, u8 code);
> +static bool tlvinfo_add_tlv(u8 * eeprom, int tcode, char * strval);
> +static int set_mac(char *buf, const char *string);
> +static int set_date(char *buf, const char *string);
> +static int set_bytes(char *buf, const char *string, int * converted_accum);
> +static void show_tlv_devices(void);
> +
> +/* Set to 1 if we've read EEPROM into memory */
> +static int has_been_read = 0;
> +/* The EERPOM contents after being read into memory */
> +static u8 eeprom[TLV_INFO_MAX_LEN];
> +
> +static struct udevice *tlv_devices[MAX_TLV_DEVICES];
> +static unsigned current_dev;
> +
> +/**
> + *  is_valid_tlv
> + *
> + *  Perform basic sanity checks on a TLV field. The TLV is pointed to
> + *  by the parameter provided.
> + *      1. The type code is not reserved (0x00 or 0xFF)
> + */
> +static inline bool is_valid_tlv(tlvinfo_tlv_t *tlv)
> +{
> +	return( (tlv->type != 0x00) &&
> +		(tlv->type != 0xFF) );

This will generate checkpatch errors. I know that you copied this code
but still we should not add code that is not checkpatch clean.

Please rework this file so that it at least matches the basic U-Boot
coding style rules.

I'm stopping my review here for now and will review the new version.

Thanks,
Stefan


> +}
> +
> +/**
> + *  is_hex
> + *
> + *  Tests if character is an ASCII hex digit
> + */
> +static inline u8 is_hex(char p)
> +{
> +	return (((p >= '0') && (p <= '9')) ||
> +		((p >= 'A') && (p <= 'F')) ||
> +		((p >= 'a') && (p <= 'f')));
> +}
> +
> +/**
> + *  is_checksum_valid
> + *
> + *  Validate the checksum in the provided TlvInfo EEPROM data. First,
> + *  verify that the TlvInfo header is valid, then make sure the last
> + *  TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
> + *  and compare it to the value stored in the EEPROM CRC-32 TLV.
> + */
> +static bool is_checksum_valid(u8 *eeprom)
> +{
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +	tlvinfo_tlv_t    * eeprom_crc;
> +	unsigned int       calc_crc;
> +	unsigned int       stored_crc;
> +
> +	// Is the eeprom header valid?
> +	if (!is_valid_tlvinfo_header(eeprom_hdr)) {
> +		return(FALSE);
> +	}
> +
> +	// Is the last TLV a CRC?
> +	eeprom_crc = (tlvinfo_tlv_t *) &eeprom[sizeof(tlvinfo_header_t) +
> +					       be16_to_cpu(eeprom_hdr->totallen) - (sizeof(tlvinfo_tlv_t) + 4)];
> +	if ((eeprom_crc->type != TLV_CODE_CRC_32) || (eeprom_crc->length != 4)) {
> +		return(FALSE);
> +	}
> +
> +	// Calculate the checksum
> +	calc_crc = crc32(0, (void *)eeprom,
> +			 sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen) - 4);
> +	stored_crc = (eeprom_crc->value[0] << 24) |
> +		(eeprom_crc->value[1] << 16) |
> +		(eeprom_crc->value[2] <<  8) |
> +		eeprom_crc->value[3];
> +	return( calc_crc == stored_crc);
> +}
> +
> +/**
> + *  read_eeprom
> + *
> + *  Read the EEPROM into memory, if it hasn't already been read.
> + */
> +static int read_eeprom(u8 *eeprom)
> +{
> +	int ret;
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +	tlvinfo_tlv_t    * eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[sizeof(tlvinfo_header_t)];
> +
> +	if (has_been_read)
> +		return 0;
> +
> +	/* Read the header */
> +	ret = read_sys_eeprom((void *)eeprom_hdr, 0, sizeof(tlvinfo_header_t));
> +	/* If the header was successfully read, read the TLVs */
> +	if ((ret == 0) && is_valid_tlvinfo_header(eeprom_hdr)) {
> +		ret = read_sys_eeprom((void *)eeprom_tlv, sizeof(tlvinfo_header_t),
> +				  be16_to_cpu(eeprom_hdr->totallen));
> +	}
> +
> +	// If the contents are invalid, start over with default contents
> +	if ( !is_valid_tlvinfo_header(eeprom_hdr) || !is_checksum_valid(eeprom) ){
> +		strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
> +		eeprom_hdr->version = TLV_INFO_VERSION;
> +		eeprom_hdr->totallen = cpu_to_be16(0);
> +		update_crc(eeprom);
> +	}
> +
> +	has_been_read = 1;
> +
> +#ifdef DEBUG
> +	show_eeprom(eeprom);
> +#endif
> +
> +	return ret;
> +}
> +
> +/**
> + *  show_eeprom
> + *
> + *  Display the contents of the EEPROM
> + */
> +static void show_eeprom(u8 *eeprom)
> +{
> +	int tlv_end;
> +	int curr_tlv;
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +	tlvinfo_tlv_t    * eeprom_tlv;
> +
> +	if ( !is_valid_tlvinfo_header(eeprom_hdr) ) {
> +		printf("EEPROM does not contain data in a valid TlvInfo format.\n");
> +		return;
> +	}
> +
> +	printf("TLV: %u\n", current_dev);
> +	printf("TlvInfo Header:\n");
> +	printf("   Id String:    %s\n", eeprom_hdr->signature);
> +	printf("   Version:      %d\n", eeprom_hdr->version);
> +	printf("   Total Length: %d\n", be16_to_cpu(eeprom_hdr->totallen));
> +
> +	printf("TLV Name             Code Len Value\n");
> +	printf("-------------------- ---- --- -----\n");
> +	curr_tlv = sizeof(tlvinfo_header_t);
> +	tlv_end  = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
> +	while (curr_tlv < tlv_end) {
> +		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[curr_tlv];
> +		if (!is_valid_tlv(eeprom_tlv)) {
> +			printf("Invalid TLV field starting at EEPROM offset %d\n", curr_tlv);
> +			return;
> +		}
> +		decode_tlv(eeprom_tlv);
> +		curr_tlv += sizeof(tlvinfo_tlv_t) + eeprom_tlv->length;
> +	}
> +
> +	printf("Checksum is %s.\n", is_checksum_valid(eeprom) ? "valid" : "invalid");
> +
> +#ifdef DEBUG
> +	printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
> +	for (i = 0; i < TLV_INFO_MAX_LEN; i++) {
> +		if ((i % 16) == 0)
> +			printf("\n%02X: ", i);
> +		printf("%02X ", eeprom[i]);
> +	}
> +	printf("\n");
> +#endif
> +
> +	return;
> +}
> +
> +/**
> + *  Struct for displaying the TLV codes and names.
> + */
> +struct tlv_code_desc {
> +	u8    m_code;
> +	char* m_name;
> +};
> +
> +/**
> + *  List of TLV codes and names.
> + */
> +static struct tlv_code_desc tlv_code_list[] = {
> +	{ TLV_CODE_PRODUCT_NAME	 , "Product Name"},
> +	{ TLV_CODE_PART_NUMBER	 , "Part Number"},
> +	{ TLV_CODE_SERIAL_NUMBER , "Serial Number"},
> +	{ TLV_CODE_MAC_BASE	 , "Base MAC Address"},
> +	{ TLV_CODE_MANUF_DATE	 , "Manufacture Date"},
> +	{ TLV_CODE_DEVICE_VERSION, "Device Version"},
> +	{ TLV_CODE_LABEL_REVISION, "Label Revision"},
> +	{ TLV_CODE_PLATFORM_NAME , "Platform Name"},
> +	{ TLV_CODE_ONIE_VERSION	 , "ONIE Version"},
> +	{ TLV_CODE_MAC_SIZE	 , "MAC Addresses"},
> +	{ TLV_CODE_MANUF_NAME	 , "Manufacturer"},
> +	{ TLV_CODE_MANUF_COUNTRY , "Country Code"},
> +	{ TLV_CODE_VENDOR_NAME	 , "Vendor Name"},
> +	{ TLV_CODE_DIAG_VERSION	 , "Diag Version"},
> +	{ TLV_CODE_SERVICE_TAG   , "Service Tag"},
> +	{ TLV_CODE_VENDOR_EXT	 , "Vendor Extension"},
> +	{ TLV_CODE_CRC_32	 , "CRC-32"},
> +};
> +
> +/**
> + *  Look up a TLV name by its type.
> + */
> +static inline const char* tlv_type2name(u8 type)
> +{
> +	char* name = "Unknown";
> +	int   i;
> +
> +	for (i = 0; i < sizeof(tlv_code_list)/sizeof(tlv_code_list[0]); i++) {
> +		if (tlv_code_list[i].m_code == type) {
> +			name = tlv_code_list[i].m_name;
> +			break;
> +		}
> +	}
> +
> +	return name;
> +}
> +
> +/*
> + *  decode_tlv
> + *
> + *  Print a string representing the contents of the TLV field. The format of
> + *  the string is:
> + *      1. The name of the field left justified in 20 characters
> + *      2. The type code in hex right justified in 5 characters
> + *      3. The length in decimal right justified in 4 characters
> + *      4. The value, left justified in however many characters it takes
> + *  The validity of EEPROM contents and the TLV field have been verified
> + *  prior to calling this function.
> + */
> +#define DECODE_NAME_MAX     20
> +
> +/*
> + * The max decode value is currently for the 'raw' type or the 'vendor
> + * extension' type, both of which have the same decode format.  The
> + * max decode string size is computed as follows:
> + *
> + *   strlen(" 0xFF") * TLV_VALUE_MAX_LEN + 1
> + *
> + */
> +#define DECODE_VALUE_MAX    ((5 * TLV_VALUE_MAX_LEN) + 1)
> +
> +static void decode_tlv(tlvinfo_tlv_t * tlv)
> +{
> +	char name[DECODE_NAME_MAX];
> +	char value[DECODE_VALUE_MAX];
> +	int i;
> +
> +	strncpy(name, tlv_type2name(tlv->type), DECODE_NAME_MAX);
> +
> +	switch (tlv->type) {
> +	case TLV_CODE_PRODUCT_NAME:
> +	case TLV_CODE_PART_NUMBER:
> +	case TLV_CODE_SERIAL_NUMBER:
> +	case TLV_CODE_MANUF_DATE:
> +	case TLV_CODE_LABEL_REVISION:
> +	case TLV_CODE_PLATFORM_NAME:
> +	case TLV_CODE_ONIE_VERSION:
> +	case TLV_CODE_MANUF_NAME:
> +	case TLV_CODE_MANUF_COUNTRY:
> +	case TLV_CODE_VENDOR_NAME:
> +	case TLV_CODE_DIAG_VERSION:
> +	case TLV_CODE_SERVICE_TAG:
> +		memcpy(value, tlv->value, tlv->length);
> +		value[tlv->length] = 0;
> +		break;
> +	case TLV_CODE_MAC_BASE:
> +		sprintf(value, "%02X:%02X:%02X:%02X:%02X:%02X",
> +			tlv->value[0], tlv->value[1], tlv->value[2],
> +			tlv->value[3], tlv->value[4], tlv->value[5]);
> +		break;
> +	case TLV_CODE_DEVICE_VERSION:
> +		sprintf(value, "%u", tlv->value[0]);
> +		break;
> +	case TLV_CODE_MAC_SIZE:
> +		sprintf(value, "%u", (tlv->value[0] << 8) | tlv->value[1]);
> +		break;
> +	case TLV_CODE_VENDOR_EXT:
> +		value[0] = 0;
> +		for (i = 0; (i < (DECODE_VALUE_MAX/5)) && (i < tlv->length); i++) {
> +			sprintf(value, "%s 0x%02X", value, tlv->value[i]);
> +		}
> +		break;
> +	case TLV_CODE_CRC_32:
> +		sprintf(value, "0x%02X%02X%02X%02X",
> +			tlv->value[0], tlv->value[1], tlv->value[2], tlv->value[3]);
> +		break;
> +	default:
> +		value[0] = 0;
> +		for (i = 0; (i < (DECODE_VALUE_MAX/5)) && (i < tlv->length); i++) {
> +			sprintf(value, "%s 0x%02X", value, tlv->value[i]);
> +		}
> +		break;
> +	}
> +
> +	name[DECODE_NAME_MAX-1] = 0;
> +	printf("%-20s 0x%02X %3d %s\n", name, tlv->type, tlv->length, value);
> +	return;
> +}
> +
> +/**
> + *  update_crc
> + *
> + *  This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
> + *  one is added. This function should be called after each update to the
> + *  EEPROM structure, to make sure the CRC is always correct.
> + */
> +static void update_crc(u8 *eeprom)
> +{
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +	tlvinfo_tlv_t    * eeprom_crc;
> +	unsigned int       calc_crc;
> +	int                eeprom_index;
> +
> +	// Discover the CRC TLV
> +	if (!tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index)) {
> +		if ((be16_to_cpu(eeprom_hdr->totallen) + sizeof(tlvinfo_tlv_t) + 4) > TLV_TOTAL_LEN_MAX) {
> +			return;
> +		}
> +		eeprom_index = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
> +		eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
> +						   sizeof(tlvinfo_tlv_t) + 4);
> +	}
> +	eeprom_crc = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
> +	eeprom_crc->type = TLV_CODE_CRC_32;
> +	eeprom_crc->length = 4;
> +
> +	// Calculate the checksum
> +	calc_crc = crc32(0, (void *)eeprom,
> +			 sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen) - 4);
> +	eeprom_crc->value[0] = (calc_crc >> 24) & 0xFF;
> +	eeprom_crc->value[1] = (calc_crc >> 16) & 0xFF;
> +	eeprom_crc->value[2] = (calc_crc >>  8) & 0xFF;
> +	eeprom_crc->value[3] = (calc_crc >>  0) & 0xFF;
> +
> +	return;
> +}
> +
> +/**
> + *  prog_eeprom
> + *
> + *  Write the EEPROM data from CPU memory to the hardware.
> + */
> +static int prog_eeprom(u8 * eeprom)
> +{
> +	int ret = 0;
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +	int eeprom_len;
> +
> +	update_crc(eeprom);
> +
> +	eeprom_len = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
> +	ret = write_sys_eeprom(eeprom, eeprom_len);
> +	if (ret) {
> +		printf("Programming failed.\n");
> +		return -1;
> +	}
> +
> +	printf("Programming passed.\n");
> +	return 0;
> +}
> +
> +/**
> + *  show_tlv_code_list - Display the list of TLV codes and names
> + */
> +void show_tlv_code_list(void)
> +{
> +	int i;
> +
> +	printf("TLV Code    TLV Name\n");
> +	printf("========    =================\n");
> +	for (i = 0; i < sizeof(tlv_code_list)/sizeof(tlv_code_list[0]); i++) {
> +		printf("0x%02X        %s\n",
> +		       tlv_code_list[i].m_code,
> +		       tlv_code_list[i].m_name);
> +	}
> +}
> +
> +/**
> + *  do_sys_eeprom
> + *
> + *  This function implements the sys_eeprom command.
> + */
> +int do_sys_eeprom(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	char cmd;
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +
> +	// If no arguments, read the EERPOM and display its contents
> +	if (argc == 1) {
> +		read_eeprom(eeprom);
> +		show_eeprom(eeprom);
> +		return 0;
> +	}
> +
> +	// We only look at the first character to the command, so "read" and "reset"
> +	// will both be treated as "read".
> +	cmd = argv[1][0];
> +
> +	// Read the EEPROM contents
> +	if (cmd == 'r') {
> +		has_been_read = 0;
> +		if (!read_eeprom(eeprom))
> +			printf("EEPROM data loaded from device to memory.\n");
> +		return 0;
> +	}
> +
> +	// Subsequent commands require that the EEPROM has already been read.
> +	if (!has_been_read) {
> +		printf("Please read the EEPROM data first, using the 'sys_eeprom read' command.\n");
> +		return 0;
> +	}
> +
> +	// Handle the commands that don't take parameters
> +	if (argc == 2) {
> +		switch (cmd) {
> +		case 'w':   /* write */
> +			prog_eeprom(eeprom);
> +			break;
> +		case 'e':   /* erase */
> +			strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
> +			eeprom_hdr->version = TLV_INFO_VERSION;
> +			eeprom_hdr->totallen = cpu_to_be16(0);
> +			update_crc(eeprom);
> +			printf("EEPROM data in memory reset.\n");
> +			break;
> +		case 'l':   /* list */
> +			show_tlv_code_list();
> +			break;
> +		case 'd':   /* dev */
> +			show_tlv_devices();
> +			break;
> +		default:
> +			cmd_usage(cmdtp);
> +			break;
> +		}
> +		return 0;
> +	}
> +
> +	// The set command takes one or two args.
> +	if (argc > 4) {
> +		cmd_usage(cmdtp);
> +		return 0;
> +	}
> +
> +	// Set command. If the TLV exists in the EEPROM, delete it. Then if data was
> +	// supplied for this TLV add the TLV with the new contents at the end.
> +	if (cmd == 's') {
> +		int tcode;
> +		tcode = simple_strtoul(argv[2], NULL, 0);
> +		tlvinfo_delete_tlv(eeprom, tcode);
> +		if (argc == 4) {
> +			tlvinfo_add_tlv(eeprom, tcode, argv[3]);
> +		}
> +	} else if (cmd == 'd') { /* 'dev' command */
> +		unsigned devnum;
> +		devnum = simple_strtoul(argv[2], NULL, 0);
> +		if (devnum > MAX_TLV_DEVICES || tlv_devices[devnum] == NULL) {
> +			printf("Invalid device number\n");
> +			return 0;
> +		}
> +		current_dev = devnum;
> +		has_been_read = 0;
> +	} else {
> +		cmd_usage(cmdtp);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + *  This macro defines the sys_eeprom command line command.
> + */
> +U_BOOT_CMD(
> +	sys_eeprom, 4, 1,  do_sys_eeprom,
> +	"Display and program the system EEPROM data block.",
> +	"[read|write|set <type_code> <string_value>|erase|list]\n"
> +	"sys_eeprom\n"
> +	"    - With no arguments display the current contents.\n"
> +	"sys_eeprom dev [dev]\n"
> +	"    - List devices or set current EEPROM device.\n"
> +	"sys_eeprom read\n"
> +	"    - Load EEPROM data from device to memory.\n"
> +	"sys_eeprom write\n"
> +	"    - Write the EEPROM data to persistent storage.\n"
> +	"sys_eeprom set <type_code> <string_value>\n"
> +	"    - Set a field to a value.\n"
> +	"    - If no string_value, field is deleted.\n"
> +	"    - Use 'sys_eeprom write' to make changes permanent.\n"
> +	"sys_eeprom erase\n"
> +	"    - Reset the in memory EEPROM data.\n"
> +	"    - Use 'sys_eeprom read' to refresh the in memory EEPROM data.\n"
> +	"    - Use 'sys_eeprom write' to make changes permanent.\n"
> +	"sys_eeprom list\n"
> +	"    - List the understood TLV codes and names.\n"
> +	);
> +
> +/**
> + *  tlvinfo_find_tlv
> + *
> + *  This function finds the TLV with the supplied code in the EERPOM.
> + *  An offset from the beginning of the EEPROM is returned in the
> + *  eeprom_index parameter if the TLV is found.
> + */
> +static bool tlvinfo_find_tlv(u8 * eeprom, u8 tcode, int * eeprom_index)
> +{
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +	tlvinfo_tlv_t    * eeprom_tlv;
> +	int eeprom_end;
> +
> +	// Search through the TLVs, looking for the first one which matches the
> +	// supplied type code.
> +	*eeprom_index = sizeof(tlvinfo_header_t);
> +	eeprom_end    = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
> +	while (*eeprom_index < eeprom_end) {
> +		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[*eeprom_index];
> +		if (!is_valid_tlv(eeprom_tlv)) {
> +			return(FALSE);
> +		}
> +		if (eeprom_tlv->type == tcode) {
> +			return(TRUE);
> +		}
> +		*eeprom_index += sizeof(tlvinfo_tlv_t) + eeprom_tlv->length;
> +	}
> +	return(FALSE);
> +}
> +
> +/**
> + *  tlvinfo_delete_tlv
> + *
> + *  This function deletes the TLV with the specified type code from the
> + *  EEPROM.
> + */
> +static bool tlvinfo_delete_tlv(u8 * eeprom, u8 code)
> +{
> +	int eeprom_index;
> +	int tlength;
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +	tlvinfo_tlv_t * eeprom_tlv;
> +
> +	// Find the TLV and then move all following TLVs "forward"
> +	if (tlvinfo_find_tlv(eeprom, code, &eeprom_index)) {
> +		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
> +		tlength = sizeof(tlvinfo_tlv_t) + eeprom_tlv->length;
> +		memcpy(&eeprom[eeprom_index], &eeprom[eeprom_index+tlength],
> +		       sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen) - eeprom_index - tlength);
> +		eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) - tlength);
> +		update_crc(eeprom);
> +		return(TRUE);
> +	}
> +	return(FALSE);
> +}
> +
> +/**
> + *  tlvinfo_add_tlv
> + *
> + *  This function adds a TLV to the EEPROM, converting the value (a string) to
> + *  the format in which it will be stored in the EEPROM.
> + */
> +#define MAX_TLV_VALUE_LEN   256
> +static bool tlvinfo_add_tlv(u8 * eeprom, int tcode, char * strval)
> +{
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +	tlvinfo_tlv_t * eeprom_tlv;
> +	int new_tlv_len = 0;
> +	u32 value;
> +	char data[MAX_TLV_VALUE_LEN];
> +	int eeprom_index;
> +
> +	// Encode each TLV type into the format to be stored in the EERPOM
> +	switch (tcode) {
> +	case TLV_CODE_PRODUCT_NAME:
> +	case TLV_CODE_PART_NUMBER:
> +	case TLV_CODE_SERIAL_NUMBER:
> +	case TLV_CODE_LABEL_REVISION:
> +	case TLV_CODE_PLATFORM_NAME:
> +	case TLV_CODE_ONIE_VERSION:
> +	case TLV_CODE_MANUF_NAME:
> +	case TLV_CODE_MANUF_COUNTRY:
> +	case TLV_CODE_VENDOR_NAME:
> +	case TLV_CODE_DIAG_VERSION:
> +	case TLV_CODE_SERVICE_TAG:
> +		strncpy(data, strval, MAX_TLV_VALUE_LEN);
> +		new_tlv_len = min_t(size_t, MAX_TLV_VALUE_LEN, strlen(strval));
> +		break;
> +	case TLV_CODE_DEVICE_VERSION:
> +		value = simple_strtoul(strval, NULL, 0);
> +		if (value >= 256) {
> +			printf("ERROR: Device version must be 255 or less. Value supplied: %u", value);
> +			return(FALSE);
> +		}
> +		data[0] = value & 0xFF;
> +		new_tlv_len = 1;
> +		break;
> +	case TLV_CODE_MAC_SIZE:
> +		value = simple_strtoul(strval, NULL, 0);
> +		if (value >= 65536) {
> +			printf("ERROR: MAC Size must be 65535 or less. Value supplied: %u", value);
> +			return(FALSE);
> +		}
> +		data[0] = (value >> 8) & 0xFF;
> +		data[1] = value & 0xFF;
> +		new_tlv_len = 2;
> +		break;
> +	case TLV_CODE_MANUF_DATE:
> +		if (set_date(data, strval) != 0) {
> +			return(FALSE);
> +		}
> +		new_tlv_len = 19;
> +		break;
> +	case TLV_CODE_MAC_BASE:
> +		if (set_mac(data, strval) != 0) {
> +			return(FALSE);
> +		}
> +		new_tlv_len = 6;
> +		break;
> +	case TLV_CODE_CRC_32:
> +		printf("WARNING: The CRC TLV is set automatically and cannot be set manually.\n");
> +		return(FALSE);
> +	case TLV_CODE_VENDOR_EXT:
> +	default:
> +		if (set_bytes(data, strval, &new_tlv_len) != 0 ) {
> +			return(FALSE);
> +		}
> +		break;
> +	}
> +
> +	// Is there room for this TLV?
> +	if ((be16_to_cpu(eeprom_hdr->totallen) + sizeof(tlvinfo_tlv_t) + new_tlv_len)
> +			> TLV_TOTAL_LEN_MAX) {
> +		printf("ERROR: There is not enough room in the EERPOM to save data.\n");
> +		return(FALSE);
> +	}
> +
> +	// Add TLV at the end, overwriting CRC TLV if it exists
> +	if (tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index)) {
> +		eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) - sizeof(tlvinfo_tlv_t) - 4);
> +	}
> +	else {
> +		eeprom_index = sizeof(tlvinfo_header_t) + be16_to_cpu(eeprom_hdr->totallen);
> +	}
> +	eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
> +	eeprom_tlv->type = tcode;
> +	eeprom_tlv->length = new_tlv_len;
> +	memcpy(eeprom_tlv->value, data, new_tlv_len);
> +
> +	// Update the total length and calculate (add) a new CRC-32 TLV
> +	eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) + sizeof(tlvinfo_tlv_t) + new_tlv_len);
> +	update_crc(eeprom);
> +
> +	return(TRUE);
> +}
> +
> +/**
> + *  set_mac
> + *
> + *  Converts a string MAC address into a binary buffer.
> + *
> + *  This function takes a pointer to a MAC address string
> + *  (i.e."XX:XX:XX:XX:XX:XX", where "XX" is a two-digit hex number).
> + *  The string format is verified and then converted to binary and
> + *  stored in a buffer.
> + */
> +static int set_mac(char *buf, const char *string)
> +{
> +	char *p = (char *) string;
> +	int   i;
> +	int   err = 0;
> +	char *end;
> +
> +	if (!p) {
> +		printf("ERROR: NULL mac addr string passed in.\n");
> +		return -1;
> +	}
> +
> +	if (strlen(p) != 17) {
> +		printf("ERROR: MAC address strlen() != 17 -- %d\n", strlen(p));
> +		printf("ERROR: Bad MAC address format: %s\n", string);
> +		return -1;
> +	}
> +
> +	for (i = 0; i < 17; i++) {
> +		if ((i % 3) == 2) {
> +			if (p[i] != ':') {
> +				err++;
> +				printf("ERROR: mac: p[%i] != :, found: `%c'\n",
> +				       i, p[i]);
> +				break;
> +			}
> +			continue;
> +		} else if (!is_hex(p[i])) {
> +			err++;
> +			printf("ERROR: mac: p[%i] != hex digit, found: `%c'\n",
> +			       i, p[i]);
> +			break;
> +		}
> +	}
> +
> +	if (err != 0) {
> +		printf("ERROR: Bad MAC address format: %s\n", string);
> +		return -1;
> +	}
> +
> +	/* Convert string to binary */
> +	for (i = 0, p = (char *)string; i < 6; i++) {
> +		buf[i] = p ? simple_strtoul(p, &end, 16) : 0;
> +		if (p) {
> +			p = (*end) ? end + 1 : end;
> +		}
> +	}
> +
> +	if (!is_valid_ethaddr((u8 *)buf)) {
> +		printf("ERROR: MAC address must not be 00:00:00:00:00:00, "
> +		       "a multicast address or FF:FF:FF:FF:FF:FF.\n");
> +		printf("ERROR: Bad MAC address format: %s\n", string);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + *  set_date
> + *
> + *  Validates the format of the data string
> + *
> + *  This function takes a pointer to a date string (i.e. MM/DD/YYYY hh:mm:ss)
> + *  and validates that the format is correct. If so the string is copied
> + *  to the supplied buffer.
> + */
> +static int set_date(char *buf, const char *string)
> +{
> +	int i;
> +
> +	if (!string) {
> +		printf("ERROR: NULL date string passed in.\n");
> +		return -1;
> +	}
> +
> +	if (strlen(string) != 19) {
> +		printf("ERROR: Date strlen() != 19 -- %d\n", strlen(string));
> +		printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
> +		return -1;
> +	}
> +
> +	for (i = 0; string[i] != 0; i++) {
> +		switch (i) {
> +		case 2:
> +		case 5:
> +			if (string[i] != '/') {
> +				printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
> +				return -1;
> +			}
> +			break;
> +		case 10:
> +			if (string[i] != ' ') {
> +				printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
> +				return -1;
> +			}
> +			break;
> +		case 13:
> +		case 16:
> +			if (string[i] != ':') {
> +				printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
> +				return -1;
> +			}
> +			break;
> +		default:
> +			if (!is_digit(string[i])) {
> +				printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n", string);
> +				return -1;
> +			}
> +			break;
> +		}
> +	}
> +
> +	strcpy(buf, string);
> +	return 0;
> +}
> +
> +/**
> + *  set_bytes
> + *
> + *  Converts a space-separated string of decimal numbers into a
> + *  buffer of bytes.
> + *
> + *  This function takes a pointer to a space-separated string of decimal
> + *  numbers (i.e. "128 0x55 0321") with "C" standard radix specifiers
> + *  and converts them to an array of bytes.
> + */
> +static int set_bytes(char *buf, const char *string, int * converted_accum)
> +{
> +	char *p = (char *) string;
> +	int   i;
> +	uint  byte;
> +
> +	if (!p) {
> +		printf("ERROR: NULL string passed in.\n");
> +		return -1;
> +	}
> +
> +	/* Convert string to bytes */
> +	for (i = 0, p = (char *)string; (i < TLV_VALUE_MAX_LEN) && (*p != 0); i++) {
> +		while ((*p == ' ') || (*p == '\t') || (*p == ',') || (*p == ';')) {
> +			p++;
> +		}
> +		if (*p != 0) {
> +			if (!is_digit(*p)) {
> +				printf("ERROR: Non-digit found in byte string: (%s)\n", string);
> +				return -1;
> +			}
> +			byte = simple_strtoul(p, &p, 0);
> +			if (byte >= 256) {
> +				printf("ERROR: The value specified is greater than 255: (%u) in string: %s\n",
> +				       byte, string);
> +				return -1;
> +			}
> +			buf[i] = byte & 0xFF;
> +		}
> +	}
> +
> +	if ((i == TLV_VALUE_MAX_LEN) && (*p != 0)) {
> +		printf("ERROR: Trying to assign too many bytes "
> +		       "(max: %d) in string: %s\n", TLV_VALUE_MAX_LEN, string);
> +		return -1;
> +	}
> +
> +	*converted_accum = i;
> +	return 0;
> +}
> +
> +static void show_tlv_devices(void)
> +{
> +	unsigned dev;
> +
> +	for (dev = 0; dev < MAX_TLV_DEVICES; dev++)
> +		if (tlv_devices[dev])
> +			printf("TLV: %u%s\n", dev,
> +					(dev == current_dev) ? " (*)" : "");
> +}
> +
> +static int find_tlv_devices(void)
> +{
> +	int ret;
> +	int count_dev = 0;
> +	struct udevice *dev;
> +
> +	for (ret = uclass_first_device_check(UCLASS_I2C_EEPROM, &dev);
> +			dev;
> +			ret = uclass_next_device_check(&dev)) {
> +		if (ret == 0)
> +			tlv_devices[count_dev++] = dev;
> +	}
> +
> +	return (count_dev == 0) ? -ENODEV : 0;
> +}
> +
> +/**
> + * read_sys_eeprom - read the hwinfo from i2c EEPROM
> + */
> +int read_sys_eeprom(void *eeprom, int offset, int len)
> +{
> +	int ret;
> +
> +	if (tlv_devices[current_dev] == NULL) {
> +		ret = find_tlv_devices();
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (tlv_devices[current_dev] == NULL)
> +		return -ENODEV;
> +
> +	return i2c_eeprom_read(tlv_devices[current_dev], offset, eeprom, len);
> +}
> +
> +/**
> + * write_sys_eeprom - write the hwinfo to i2c EEPROM
> + */
> +int write_sys_eeprom(void *eeprom, int len)
> +{
> +	if (tlv_devices[current_dev] == NULL)
> +		return -ENODEV;
> +
> +	return i2c_eeprom_write(tlv_devices[current_dev], 0, eeprom, len);
> +}
> +
> +int read_tlvinfo_sys_eeprom(void *eeprom, tlvinfo_header_t **hdr,
> +		tlvinfo_tlv_t **first_entry)
> +{
> +	int ret;
> +	tlvinfo_header_t *tlv_hdr;
> +	tlvinfo_tlv_t *tlv_ent;
> +
> +	/* Read TLV header */
> +	ret = read_sys_eeprom(eeprom, 0, sizeof(tlvinfo_header_t));
> +	if (ret < 0)
> +		return ret;
> +
> +	tlv_hdr = eeprom;
> +	if (!is_valid_tlvinfo_header(tlv_hdr))
> +		return -EINVAL;
> +
> +	/* Read TLV entries */
> +	tlv_ent = (tlvinfo_tlv_t *) &tlv_hdr[1];
> +	ret = read_sys_eeprom(tlv_ent, sizeof(tlvinfo_header_t),
> +			be16_to_cpu(tlv_hdr->totallen));
> +	if (ret < 0)
> +		return ret;
> +	if (!is_checksum_valid(eeprom))
> +		return -EINVAL;
> +
> +	*hdr = tlv_hdr;
> +	*first_entry = tlv_ent;
> +
> +	return 0;
> +}
> +
> +int read_tlvinfo_sys_eeprom_dev(void *eeprom, tlvinfo_header_t **hdr,
> +		tlvinfo_tlv_t **first_entry, unsigned devnum)
> +{
> +	int ret;
> +	unsigned prev_dev = MAX_TLV_DEVICES + 1;
> +
> +	if (devnum > MAX_TLV_DEVICES)
> +		return -EINVAL;
> +
> +	if (devnum != current_dev) {
> +		prev_dev = current_dev;
> +		current_dev = devnum;
> +		has_been_read = 0;
> +	}
> +
> +	ret = read_tlvinfo_sys_eeprom(eeprom, hdr, first_entry);
> +	if (ret < 0 && prev_dev <= MAX_TLV_DEVICES)
> +		current_dev = prev_dev;
> +
> +	return ret;
> +}
> +
> +/**
> + *  mac_read_from_eeprom
> + *
> + *  Read the MAC addresses from EEPROM
> + *
> + *  This function reads the MAC addresses from EEPROM and sets the
> + *  appropriate environment variables for each one read.
> + *
> + *  The environment variables are only set if they haven't been set already.
> + *  This ensures that any user-saved variables are never overwritten.
> + *
> + *  This function must be called after relocation.
> + */
> +int mac_read_from_eeprom(void)
> +{
> +	unsigned int i;
> +	int eeprom_index;
> +	tlvinfo_tlv_t * eeprom_tlv;
> +	int maccount;
> +	u8 macbase[6];
> +	tlvinfo_header_t * eeprom_hdr = (tlvinfo_header_t *) eeprom;
> +
> +	puts("EEPROM: ");
> +
> +	if (read_eeprom(eeprom)) {
> +		printf("Read failed.\n");
> +		return -1;
> +	}
> +
> +	maccount = 1;
> +	if (tlvinfo_find_tlv(eeprom, TLV_CODE_MAC_SIZE, &eeprom_index)) {
> +		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
> +		maccount = (eeprom_tlv->value[0] << 8) | eeprom_tlv->value[1];
> +	}
> +
> +	memcpy(macbase, "\0\0\0\0\0\0", 6);
> +	if (tlvinfo_find_tlv(eeprom, TLV_CODE_MAC_BASE, &eeprom_index)) {
> +		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
> +		memcpy(macbase, eeprom_tlv->value, 6);
> +	}
> +
> +	for (i = 0; i < maccount; i++) {
> +		if (is_valid_ethaddr(macbase)) {
> +			char ethaddr[18];
> +			char enetvar[11];
> +
> +			sprintf(ethaddr, "%02X:%02X:%02X:%02X:%02X:%02X",
> +				macbase[0], macbase[1], macbase[2],
> +				macbase[3], macbase[4], macbase[5]);
> +			sprintf(enetvar, i ? "eth%daddr" : "ethaddr", i);
> +			/* Only initialize environment variables that are blank
> +			 * (i.e. have not yet been set)
> +			 */
> +			if (!env_get(enetvar))
> +				env_set(enetvar, ethaddr);
> +
> +			macbase[5]++;
> +			if (macbase[5] == 0) {
> +				macbase[4]++;
> +				if (macbase[4] == 0) {
> +					macbase[3]++;
> +					if (macbase[3] == 0) {
> +						macbase[0] = 0;
> +						macbase[1] = 0;
> +						macbase[2] = 0;
> +					}
> +				}
> +			}
> +		}
> +	}
> +
> +	printf("%s v%u len=%u\n", eeprom_hdr->signature, eeprom_hdr->version,
> +	       be16_to_cpu(eeprom_hdr->totallen));
> +
> +	return 0;
> +}
> +
> +/**
> + *  populate_serial_number - read the serial number from EEPROM
> + *
> + *  This function reads the serial number from the EEPROM and sets the
> + *  appropriate environment variable.
> + *
> + *  The environment variable is only set if it has not been set
> + *  already.  This ensures that any user-saved variables are never
> + *  overwritten.
> + *
> + *  This function must be called after relocation.
> + */
> +int populate_serial_number(void)
> +{
> +	char serialstr[257];
> +	int eeprom_index;
> +	tlvinfo_tlv_t * eeprom_tlv;
> +
> +	if (env_get("serial#"))
> +		return 0;
> +
> +	if (read_eeprom(eeprom)) {
> +		printf("Read failed.\n");
> +		return -1;
> +	}
> +
> +	if (tlvinfo_find_tlv(eeprom, TLV_CODE_SERIAL_NUMBER, &eeprom_index)) {
> +		eeprom_tlv = (tlvinfo_tlv_t *) &eeprom[eeprom_index];
> +		memcpy(serialstr, eeprom_tlv->value, eeprom_tlv->length);
> +		serialstr[eeprom_tlv->length] = 0;
> +		env_set("serial#", serialstr);
> +	}
> +
> +	return 0;
> +}
> diff --git a/include/sys_eeprom.h b/include/sys_eeprom.h
> new file mode 100644
> index 000000000000..49a7ee87d9db
> --- /dev/null
> +++ b/include/sys_eeprom.h
> @@ -0,0 +1,169 @@
> +/*
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __SYS_EEPROM_H_
> +#define __SYS_EEPROM_H_
> +
> +/*
> + *  Without getting too philosophical, define truth, falsehood, and the
> + *  boolean type, if they are not already defined.
> + */
> +#ifndef FALSE
> +#define FALSE   0
> +#endif
> +
> +#ifndef TRUE
> +#define TRUE    (!FALSE)
> +#endif
> +
> +#ifndef bool
> +typedef unsigned char bool;
> +#endif
> +
> +#ifndef is_digit
> +#define is_digit(c)             ((c) >= '0' && (c) <= '9')
> +#endif
> +
> +/*
> + *  The Definition of the TlvInfo EEPROM format can be found at onie.org or
> + *  github.com/onie
> + */
> +
> +/*
> + * TlvInfo header: Layout of the header for the TlvInfo format
> + *
> + * See the end of this file for details of this eeprom format
> + */
> +struct __attribute__ ((__packed__)) tlvinfo_header_s {
> +    char    signature[8];       /* 0x00 - 0x07 EEPROM Tag "TlvInfo" */
> +    u8      version;            /* 0x08        Structure version    */
> +    u16     totallen;           /* 0x09 - 0x0A Length of all data which follows */
> +};
> +typedef struct tlvinfo_header_s tlvinfo_header_t;
> +
> +// Header Field Constants
> +#define TLV_INFO_ID_STRING      "TlvInfo"
> +#define TLV_INFO_VERSION        0x01
> +#define TLV_INFO_MAX_LEN        2048
> +#define TLV_TOTAL_LEN_MAX       (TLV_INFO_MAX_LEN - sizeof(tlvinfo_header_t))
> +
> +/*
> + * TlvInfo TLV: Layout of a TLV field
> + */
> +struct __attribute__ ((__packed__)) tlvinfo_tlv_s {
> +    u8  type;
> +    u8  length;
> +    u8  value[0];
> +};
> +typedef struct tlvinfo_tlv_s tlvinfo_tlv_t;
> +
> +/* Maximum length of a TLV value in bytes */
> +#define TLV_VALUE_MAX_LEN        255
> +
> +/**
> + *  The TLV Types.
> + *
> + *  Keep these in sync with tlv_code_list in cmd_sys_eeprom.c
> + */
> +#define TLV_CODE_PRODUCT_NAME   0x21
> +#define TLV_CODE_PART_NUMBER    0x22
> +#define TLV_CODE_SERIAL_NUMBER  0x23
> +#define TLV_CODE_MAC_BASE       0x24
> +#define TLV_CODE_MANUF_DATE     0x25
> +#define TLV_CODE_DEVICE_VERSION 0x26
> +#define TLV_CODE_LABEL_REVISION 0x27
> +#define TLV_CODE_PLATFORM_NAME  0x28
> +#define TLV_CODE_ONIE_VERSION   0x29
> +#define TLV_CODE_MAC_SIZE       0x2A
> +#define TLV_CODE_MANUF_NAME     0x2B
> +#define TLV_CODE_MANUF_COUNTRY  0x2C
> +#define TLV_CODE_VENDOR_NAME    0x2D
> +#define TLV_CODE_DIAG_VERSION   0x2E
> +#define TLV_CODE_SERVICE_TAG    0x2F
> +#define TLV_CODE_VENDOR_EXT     0xFD
> +#define TLV_CODE_CRC_32         0xFE
> +
> +/**
> + * read_sys_eeprom - Read the EEPROM binary data from the hardware
> + * @eeprom: Pointer to buffer to hold the binary data
> + * @offset: Offset within EEPROM block to read data from
> + * @len   : Maximum size of buffer
> + *
> + * Note: this routine does not validate the EEPROM data.
> + *
> + */
> +
> +extern int read_sys_eeprom(void *eeprom, int offset, int len);
> +
> +/**
> + * write_sys_eeprom - Write the entire EEPROM binary data to the hardware
> + * @eeprom: Pointer to buffer to hold the binary data
> + * @len   : Maximum size of buffer
> + *
> + * Note: this routine does not validate the EEPROM data.
> + *
> + */
> +extern int write_sys_eeprom(void *eeprom, int len);
> +
> +/**
> + * read_tlvinfo_sys_eeprom - Read the TLV from EEPROM, and validate
> + * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
> + *          of size at least TLV_INFO_MAX_LEN.
> + * @hdr   : Points to pointer to TLV header (output)
> + * @first_entry : Points to pointer to first TLV entry (output)
> + *
> + * Store the raw EEPROM data in the @eeprom buffer. If TLV is valid set *@hdr
> + * and *@first_entry.
> + *
> + * Returns 0 when read from EEPROM is successful, and the data is valid.
> + * Returns <0 error value when EEPROM read fails. Return -EINVAL when TLV is
> + * invalid.
> + *
> + */
> +
> +extern int read_tlvinfo_sys_eeprom(void *eeprom, tlvinfo_header_t **hdr,
> +		tlvinfo_tlv_t **first_entry);
> +
> +/**
> + * read_tlvinfo_sys_eeprom_dev - Read the TLV from specific EEPROM, and validate
> + * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
> + *          of size at least TLV_INFO_MAX_LEN.
> + * @hdr   : Points to pointer to TLV header (output)
> + * @first_entry : Points to pointer to first TLV entry (output)
> + * @dev   : EEPROM device to read
> + *
> + * Store the raw EEPROM data from EEPROM @dev in the @eeprom buffer. If TLV is
> + * valid set *@hdr and *@first_entry.
> + *
> + * Returns 0 when read from EEPROM is successful, and the data is valid.
> + * Returns <0 error value when EEPROM read fails. Return -EINVAL when TLV is
> + * invalid.
> + *
> + */
> +
> +extern int read_tlvinfo_sys_eeprom_dev(void *eeprom, tlvinfo_header_t **hdr,
> +		tlvinfo_tlv_t **first_entry, unsigned devnum);
> +
> +/**
> + *  is_valid_tlvinfo_header
> + *
> + *  Perform sanity checks on the first 11 bytes of the TlvInfo EEPROM
> + *  data pointed to by the parameter:
> + *      1. First 8 bytes contain null-terminated ASCII string "TlvInfo"
> + *      2. Version byte is 1
> + *      3. Total length bytes contain value which is less than or equal
> + *         to the allowed maximum (2048-11)
> + *
> + */
> +static inline bool is_valid_tlvinfo_header(tlvinfo_header_t *hdr)
> +{
> +	return( (strcmp(hdr->signature, TLV_INFO_ID_STRING) == 0) &&
> +		(hdr->version == TLV_INFO_VERSION) &&
> +		(be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX) );
> +}
> +
> +#endif /* __SYS_EEPROM_H_ */
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 04/10] ARM: mvebu: clearfog: add EEPROM devices

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Add device-tree description for the EEPROM devices on Armada 388 SOM rev
> 2.1.
> 
> Another EEPROM is now on Clearfog Pro carrier rev 2.2, and Clearfog Base
> rev 1.3.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   arch/arm/dts/armada-388-clearfog-u-boot.dtsi   | 12 ++++++++++++
>   arch/arm/dts/armada-388-clearfog.dts           |  6 ++++++
>   arch/arm/dts/armada-38x-solidrun-microsom.dtsi |  8 ++++++++
>   3 files changed, 26 insertions(+)
> 
> diff --git a/arch/arm/dts/armada-388-clearfog-u-boot.dtsi b/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
> index 27af14555001..71a073a225a4 100644
> --- a/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
> +++ b/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
> @@ -20,3 +20,15 @@
>   &ahci1 {
>   	u-boot,dm-spl;
>   };
> +
> +&i2c0 {
> +	u-boot,dm-spl;
> +
> +	eeprom at 52 {
> +		u-boot,dm-spl;
> +	};
> +
> +	eeprom at 53 {
> +		u-boot,dm-spl;
> +	};
> +};
> diff --git a/arch/arm/dts/armada-388-clearfog.dts b/arch/arm/dts/armada-388-clearfog.dts
> index 4ddeaa02f122..e4164f49b24d 100644
> --- a/arch/arm/dts/armada-388-clearfog.dts
> +++ b/arch/arm/dts/armada-388-clearfog.dts
> @@ -259,6 +259,12 @@
>   		compatible = "microchip,mcp3021";
>   		reg = <0x4c>;
>   	};
> +
> +	eeprom at 52 {
> +		compatible = "atmel,24c02";
> +		reg = <0x52>;
> +		pagesize = <16>;
> +	};
>   };
>   
>   &i2c1 {
> diff --git a/arch/arm/dts/armada-38x-solidrun-microsom.dtsi b/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
> index a2627223ce3b..a322a28c215c 100644
> --- a/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
> +++ b/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
> @@ -99,3 +99,11 @@
>   	status = "okay";
>   	u-boot,dm-pre-reloc;
>   };
> +
> +&i2c0 {
> +	eeprom at 53 {
> +		compatible = "atmel,24c02";
> +		reg = <0x53>;
> +		pagesize = <16>;
> +	};
> +};
> 

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 05/10] ARM: mvebu: clearfog: add support for EEPROM TLV info

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   configs/clearfog_defconfig | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> index dd48b325c0bc..2bf80ee70cec 100644
> --- a/configs/clearfog_defconfig
> +++ b/configs/clearfog_defconfig
> @@ -11,6 +11,7 @@ CONFIG_TARGET_CLEARFOG=y
>   CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC=y
>   CONFIG_SPL_MMC_SUPPORT=y
>   CONFIG_SPL_SERIAL_SUPPORT=y
> +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y
>   CONFIG_NR_DRAM_BANKS=2
>   CONFIG_SPL=y
>   CONFIG_DEBUG_UART_BASE=0xd0012000
> @@ -26,6 +27,8 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
>   CONFIG_DISPLAY_BOARDINFO_LATE=y
>   CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET=0x1
>   CONFIG_SPL_I2C_SUPPORT=y
> +CONFIG_CMD_SYS_EEPROM=y
> +CONFIG_SPL_CMD_SYS_EEPROM=y
>   # CONFIG_CMD_FLASH is not set
>   CONFIG_CMD_GPIO=y
>   CONFIG_CMD_I2C=y
> @@ -47,6 +50,8 @@ CONFIG_DM_GPIO=y
>   CONFIG_DM_PCA953X=y
>   CONFIG_DM_I2C=y
>   CONFIG_SYS_I2C_MVTWSI=y
> +CONFIG_I2C_EEPROM=y
> +CONFIG_SPL_I2C_EEPROM=y

This might change with the suggested Kconfig symbol renaming.

Thanks,
Stefan

[PATCH 06/10] ARM: mvebu: clearfog: read basic TLV data

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Read RAM die capacity from the EEPROM TLV.
> 
> Follow the ONIE standard that defines the Vendor Extension entry type
> for vendor specific data. We have no Private Enterprise Number at the
> moment as the standard requires. Use the dummy all 0xff value for now.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   board/solidrun/clearfog/clearfog.c | 118 +++++++++++++++++++++++++++++
>   1 file changed, 118 insertions(+)
> 
> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> index 8b6381194688..707afabb11a7 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -7,6 +7,7 @@
>   #include <i2c.h>
>   #include <miiphy.h>
>   #include <netdev.h>
> +#include <sys_eeprom.h>
>   #include <asm/io.h>
>   #include <asm/arch/cpu.h>
>   #include <asm/arch/soc.h>
> @@ -16,6 +17,14 @@
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> +#define SR_TLV_CODE_RAM_DIE_SIZE	0x81
> +
> +/*
> + * Some Clearfog variants have TLV on SOM and on carrier, with separate
> + * product name entries.
> + */
> +static char tlv_product_name[2][32];
> +
>   /*
>    * Those values and defines are taken from the Marvell U-Boot version
>    * "u-boot-2013.01-15t1-clearfog"
> @@ -74,8 +83,117 @@ static struct mv_ddr_topology_map board_topology_map = {
>   	0x3,				/* clock enable mask */
>   };
>   
> +static void store_product_name(tlvinfo_tlv_t *tlv_entry)
> +{
> +	int len;
> +	char *dest;
> +
> +	if (strlen(tlv_product_name[0]) == 0)
> +		dest = tlv_product_name[0];
> +	else if (strlen(tlv_product_name[1]) == 0)
> +		dest = tlv_product_name[1];
> +	else
> +		return;
> +
> +	len = min_t(unsigned, tlv_entry->length,
> +			sizeof(tlv_product_name[0])-1);

Nitpicking: Space missing around "-". Please run checkpatch.

> +	memcpy(dest, tlv_entry->value, len);
> +}
> +
> +static bool sr_product_is(const char *product)
> +{
> +	/* Allow prefix sub-string match */
> +	if (strncmp(tlv_product_name[0], product, strlen(product)) == 0)
> +		return true;
> +	if (strncmp(tlv_product_name[1], product, strlen(product)) == 0)
> +		return true;
> +
> +	return false;
> +}
> +
> +static void parse_tlv_vendor_ext(tlvinfo_tlv_t *tlv_entry)
> +{
> +	struct if_params *ifp = &board_topology_map.interface_params[0];
> +	u8 *val = tlv_entry->value;
> +	uint32_t pen; /* IANA Private Enterprise Numbers */
> +
> +	if (tlv_entry->length < 5) /* 4 bytes PEN +@least 1 byte type */
> +		return;
> +
> +	/* PEN is big endian */
> +	pen = (val[0] << 24) | (val[1] << 16) | (val[2] << 8) | val[3];
> +	/* Not a real PEN */
> +	if (pen != 0xffffffff)
> +		return;
> +
> +	switch (val[4]) {
> +	case SR_TLV_CODE_RAM_DIE_SIZE:
> +		if (tlv_entry->length != 6)
> +			break;
> +		switch (val[5]) {
> +		case 4:
> +		default:
> +			ifp->memory_size = MV_DDR_DIE_CAP_4GBIT;
> +			break;
> +		case 8:
> +			ifp->memory_size = MV_DDR_DIE_CAP_8GBIT;
> +			break;
> +		}
> +	default:
> +		break;
> +	}
> +}
> +
> +static void parse_tlv_data(uint8_t *eeprom, tlvinfo_header_t *hdr,
> +		tlvinfo_tlv_t *entry)
> +{
> +	unsigned tlv_offset, tlv_len;
> +
> +	tlv_offset = sizeof(tlvinfo_header_t);
> +	tlv_len = sizeof(tlvinfo_header_t) + be16_to_cpu(hdr->totallen);
> +	while (tlv_offset < tlv_len) {
> +		entry = (tlvinfo_tlv_t *) &eeprom[tlv_offset];
> +
> +		switch (entry->type) {
> +		case TLV_CODE_PRODUCT_NAME:
> +			store_product_name(entry);
> +			break;
> +		case TLV_CODE_VENDOR_EXT:
> +			parse_tlv_vendor_ext(entry);
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		tlv_offset += sizeof(tlvinfo_tlv_t) + entry->length;
> +	}
> +}
> +
> +static void read_tlv_data(void)
> +{
> +	uint8_t eeprom_data[TLV_TOTAL_LEN_MAX];
> +	tlvinfo_header_t *tlv_hdr;

AFAIK, using typedefs (in the sys_eeprom header most likely) is frowned
upon nowadays. Not sure if it makes sense to remove these typedefs as
this might make the code incompatible with other potential users.

> +	tlvinfo_tlv_t *tlv_entry;
> +	static bool ran_once;
> +	int ret, i;
> +
> +	if (ran_once)
> +		return;
> +	ran_once = true;
> +
> +	for (i = 0; i < 2; i++) {
> +		ret = read_tlvinfo_sys_eeprom_dev(eeprom_data, &tlv_hdr,
> +				&tlv_entry, i);
> +		if (ret < 0)
> +			continue;
> +		parse_tlv_data(eeprom_data, tlv_hdr, tlv_entry);
> +	}
> +}
> +
>   struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
>   {
> +	read_tlv_data();
> +
>   	/* Return the board topology as defined in the board code */
>   	return &board_topology_map;
>   }
> 

Other than my comments above:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 07/10] ARM: mvebu: clearfog: print TLV stored product name

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Use the data from EEPROM TLV to display the board identity.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   board/solidrun/clearfog/clearfog.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> index 707afabb11a7..852b1ad71bb2 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -246,7 +246,16 @@ int board_init(void)
>   
>   int checkboard(void)
>   {
> -	puts("Board: SolidRun ClearFog\n");
> +	char *board = "ClearFog";
> +
> +	read_tlv_data();
> +	if (strlen(tlv_product_name[0]) > 0)
> +		board = tlv_product_name[0];
> +
> +	printf("Board: SolidRun %s", board);
> +	if (strlen(tlv_product_name[1]) > 0)
> +		printf(", %s", tlv_product_name[1]);
> +	puts("\n");
>   
>   	return 0;
>   }
> 

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 08/10] ARM: mvebu: clearfog: run-time selection of DT file

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Set the kernel device-tree file (fdtfile environment variable) based on
> run-time detection of the platform.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   arch/arm/mach-mvebu/Kconfig        |  1 +
>   board/solidrun/clearfog/clearfog.c | 14 ++++++++++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index fdd39685b75d..bc5eaa5a7679 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -91,6 +91,7 @@ choice
>   config TARGET_CLEARFOG
>   	bool "Support ClearFog"
>   	select 88F6820
> +	select BOARD_LATE_INIT
>   
>   config TARGET_HELIOS4
>   	bool "Support Helios4"
> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> index 852b1ad71bb2..00c1306e9c80 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -265,3 +265,17 @@ int board_eth_init(bd_t *bis)
>   	cpu_eth_init(bis); /* Built in controller(s) come first */
>   	return pci_eth_init(bis);
>   }
> +
> +int board_late_init(void)
> +{
> +	read_tlv_data();
> +
> +	if (sr_product_is("Clearfog Base"))
> +		 env_set("fdtfile", "armada-388-clearfog-base.dtb");
> +	else if (sr_product_is("Clearfog GTR S4"))
> +		 env_set("fdtfile", "armada-385-clearfog-gtr-s4.dtb");
> +	else if (sr_product_is("Clearfog GTR L8"))
> +		 env_set("fdtfile", "armada-385-clearfog-gtr-l8.dtb");
> +
> +	return 0;
> +}
> 

Where does this sr_product_is() function come from? I might have missed
it in a previous patch - please point me to it, thanks.

Does it make sense to match string values here or wouldn't it be better
to match board ID's (enum)?

Thanks,
Stefan

[PATCH 09/10] ARM: mvebu: clearfog: add Clearfog GTR support

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Select the serdes configuration table based on the platform identity
> read from EEPROM TLV data. Clearfog GTR needs a slightly different
> serdes configuration.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   board/solidrun/clearfog/clearfog.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> index 00c1306e9c80..70bea1e8dc65 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -25,6 +25,8 @@ DECLARE_GLOBAL_DATA_PTR;
>    */
>   static char tlv_product_name[2][32];
>   
> +static void read_tlv_data(void);
> +
>   /*
>    * Those values and defines are taken from the Marvell U-Boot version
>    * "u-boot-2013.01-15t1-clearfog"
> @@ -48,6 +50,8 @@ static struct serdes_map board_serdes_map[] = {
>   
>   int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
>   {
> +	read_tlv_data();
> +
>   	*serdes_map_array = board_serdes_map;
>   	*count = ARRAY_SIZE(board_serdes_map);
>   	return 0;
> @@ -188,6 +192,12 @@ static void read_tlv_data(void)
>   			continue;
>   		parse_tlv_data(eeprom_data, tlv_hdr, tlv_entry);
>   	}
> +
> +	if (sr_product_is("Clearfog GTR")) {
> +		board_serdes_map[0].serdes_type = PEX0;
> +		board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
> +		board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
> +	}
>   }
>   
>   struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
> 

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 10/10] ARM: mvebu: clearfog: add Clearfog Base serdes configuration

[Stefan Roese]

On 25.11.19 11:30, Baruch Siach wrote:
> Clearfog Base carrier uses serdes lane #5 as USB host. Use EEPROM stored
> device identification to configure the serdes accordingly when
> available.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>   board/solidrun/clearfog/clearfog.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> index 70bea1e8dc65..f407f744bf0a 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -198,6 +198,12 @@ static void read_tlv_data(void)
>   		board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
>   		board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
>   	}
> +
> +	if (sr_product_is("Clearfog Base")) {
> +		board_serdes_map[4].serdes_type = USB3_HOST0;
> +		board_serdes_map[4].serdes_speed = SERDES_SPEED_5_GBPS;
> +		board_serdes_map[4].serdes_mode = SERDES_DEFAULT_MODE;
> +	}
>   }
>   
>   struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
> 

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 00/10] ARM: clearfog: add run-time board detect

[Stefan Roese]

Hi Baruch,

On 25.11.19 11:30, Baruch Siach wrote:
> Newer revisions of SolidRun Clearfog Base/Pro carriers and Armada 388 SOM add
> EEPROM storage for board detection. This patch series adds support for reading
> EEPROM stored board information, and using it to set RAM training parameters,
> serdes configuration, and kernel DT selection.
> 
> The information is stored in EEPROM in TLV format defined for the ONIE project.
> 
>    https://opencomputeproject.github.io/onie/design-spec/hw_requirements.html
> 
> This series add the sys_eeprom command from ONIE carried U-Boot patch, with a
> number for changes as described in the patch log. The TLV format is unchanged.
> 
> This series also adds support for the newly introduced single board, Aramda 385
> based Clearfog GTR system. RAM configuration on that system requires both
> Armada 38x DDR clocks to be enabled. The first patch in this series adds the
> necessary code to allow per-board selection of DDR clock.

Apart from my comments in the patches, please rebase the patchset on
top of current mainline before resubmitting.

Thanks,
Stefan
  
> Baruch Siach (10):
>    ddr: marvell: a38x: allow board specific clock out setup
>    arm: mvebu: clearfog: enable both DDR clocks
>    cmd: add sys_eeprom command
>    ARM: mvebu: clearfog: add EEPROM devices
>    ARM: mvebu: clearfog: add support for EEPROM TLV info
>    ARM: mvebu: clearfog: read basic TLV data
>    ARM: mvebu: clearfog: print TLV stored product name
>    ARM: mvebu: clearfog: run-time selection of DT file
>    ARM: mvebu: clearfog: add Clearfog GTR support
>    ARM: mvebu: clearfog: add Clearfog Base serdes configuration
> 
>   arch/arm/dts/armada-388-clearfog-u-boot.dtsi  |   12 +
>   arch/arm/dts/armada-388-clearfog.dts          |    6 +
>   .../arm/dts/armada-38x-solidrun-microsom.dtsi |    8 +
>   arch/arm/mach-mvebu/Kconfig                   |    1 +
>   board/solidrun/clearfog/clearfog.c            |  164 ++-
>   cmd/Kconfig                                   |   12 +
>   cmd/Makefile                                  |    2 +
>   cmd/sys_eeprom.c                              | 1078 +++++++++++++++++
>   configs/clearfog_defconfig                    |    5 +
>   drivers/ddr/marvell/a38x/ddr3_training.c      |   10 +-
>   drivers/ddr/marvell/a38x/ddr_topology_def.h   |    3 +
>   include/sys_eeprom.h                          |  169 +++
>   12 files changed, 1466 insertions(+), 4 deletions(-)
>   create mode 100644 cmd/sys_eeprom.c
>   create mode 100644 include/sys_eeprom.h
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 03/10] cmd: add sys_eeprom command

[Baruch Siach]

Hi Stefan,

On Mon, Jan 13, 2020 at 08:11:05AM +0100, Stefan Roese wrote:
> On 25.11.19 11:30, Baruch Siach wrote:
> > Based on U-Boot patch from the Open Compute project:
> > 
> >    https://github.com/opencomputeproject/onie/blob/ec87e872d46b9805565d2c6124b2f701ef1c07b1/patches/u-boot/common/feature-sys-eeprom-tlv-common.patch
> > 
> > Keep only I2C EEPROM support. Use the generic eeprom driver.
> > 
> > Add support for multiple EEPROM TLV stores on the same system.
> 
> Could you please explain, what TLV stands for? Is it "Type Length
> Value"? Please add it to the description.

Will do.

> > This is
> > useful in case of SOM and carrier that both provide ID and hardware
> > configuration information.
> > 
> > Add option to enable for SPL. This allows selection of RAM configuration
> > based on EEPROM stored board identification.
> > 
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> Shouldn't you add the copyrights from the original patch:
> 
> Add to support 'sys_eeprom' command (common part)
> 
> Copyright (C) 2013 Curt Brune <curt@cumulusnetworks.com>
> Copyright (C) 2014 Srideep <srideep_devireddy@dell.com>
> Copyright (C) 2013 Miles Tseng <miles_tseng@accton.com>
> Copyright (C) 2014,2016 david_yang <david_yang@accton.com>
> 
> ?

Copyright information is not usually listed in commit log messages. Is there 
any reason to do that in this patch?

> > ---
> >   cmd/Kconfig          |   12 +
> >   cmd/Makefile         |    2 +
> >   cmd/sys_eeprom.c     | 1078 ++++++++++++++++++++++++++++++++++++++++++
> >   include/sys_eeprom.h |  169 +++++++
> >   4 files changed, 1261 insertions(+)
> >   create mode 100644 cmd/sys_eeprom.c
> >   create mode 100644 include/sys_eeprom.h
> > 
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 99b8a0e21822..483663404da4 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -234,6 +234,18 @@ config CMD_REGINFO
> >   	help
> >   	  Register dump
> > +config CMD_SYS_EEPROM
> > +	bool "sys_eeprom"
> > +	depends on I2C_EEPROM
> > +	help
> > +	  Display and program the system EEPROM data block.
> > +
> > +config SPL_CMD_SYS_EEPROM
> > +	bool "sys_eeprom for SPL"
> > +	depends on SPL_I2C_EEPROM
> > +	help
> > +	  Read system EEPROM data block.
> > +
> 
> I'm not sure, if this description is enough. This sounds too generic
> for my taste as I assume that there are multiple formats to store system
> data in an EEPROM. It might make sense to mention TLV here. And perhaps
> its also better to name the files differently by adding "tlv" as well.
> 
> What do you think?

Sounds reasonable. I'll rename to something less generic.

> >   endmenu
> >   menu "Boot commands"
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index 2d723ea0f07d..cbb1d46b8f50 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -180,6 +180,8 @@ obj-$(CONFIG_X86) += x86/
> >   obj-$(CONFIG_ARCH_MVEBU) += mvebu/
> >   endif # !CONFIG_SPL_BUILD
> > +obj-$(CONFIG_$(SPL_)CMD_SYS_EEPROM) += sys_eeprom.o
> > +
> >   # core command
> >   obj-y += nvedit.o
> > diff --git a/cmd/sys_eeprom.c b/cmd/sys_eeprom.c
> > new file mode 100644
> > index 000000000000..373673a5266c
> > --- /dev/null
> > +++ b/cmd/sys_eeprom.c
> > @@ -0,0 +1,1078 @@
> > +/*
> > + * See file CREDITS for list of people who contributed to this
> > + * project.
> > + *
> > + * SPDX-License-Identifier:	GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <command.h>
> > +#include <dm.h>
> > +#include <i2c.h>
> > +#include <i2c_eeprom.h>
> > +#include <env.h>
> > +#include <linux/ctype.h>
> > +
> > +#include "sys_eeprom.h"
> > +
> > +#define MAX_TLV_DEVICES	2
> > +
> > +/* File scope function prototypes */
> > +static bool is_checksum_valid(u8 *eeprom);
> > +static int read_eeprom(u8 *eeprom);
> > +static void show_eeprom(u8 *eeprom);
> > +static void decode_tlv(tlvinfo_tlv_t * tlv);
> > +static void update_crc(u8 *eeprom);
> > +static int prog_eeprom(u8 * eeprom);
> > +static bool tlvinfo_find_tlv(u8 * eeprom, u8 tcode, int * eeprom_index);
> > +static bool tlvinfo_delete_tlv(u8 * eeprom, u8 code);
> > +static bool tlvinfo_add_tlv(u8 * eeprom, int tcode, char * strval);
> > +static int set_mac(char *buf, const char *string);
> > +static int set_date(char *buf, const char *string);
> > +static int set_bytes(char *buf, const char *string, int * converted_accum);
> > +static void show_tlv_devices(void);
> > +
> > +/* Set to 1 if we've read EEPROM into memory */
> > +static int has_been_read = 0;
> > +/* The EERPOM contents after being read into memory */
> > +static u8 eeprom[TLV_INFO_MAX_LEN];
> > +
> > +static struct udevice *tlv_devices[MAX_TLV_DEVICES];
> > +static unsigned current_dev;
> > +
> > +/**
> > + *  is_valid_tlv
> > + *
> > + *  Perform basic sanity checks on a TLV field. The TLV is pointed to
> > + *  by the parameter provided.
> > + *      1. The type code is not reserved (0x00 or 0xFF)
> > + */
> > +static inline bool is_valid_tlv(tlvinfo_tlv_t *tlv)
> > +{
> > +	return( (tlv->type != 0x00) &&
> > +		(tlv->type != 0xFF) );
> 
> This will generate checkpatch errors. I know that you copied this code
> but still we should not add code that is not checkpatch clean.
> 
> Please rework this file so that it at least matches the basic U-Boot
> coding style rules.
> 
> I'm stopping my review here for now and will review the new version.

Thanks for your review so far. I'll update and resend.

baruch


-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[PATCH 08/10] ARM: mvebu: clearfog: run-time selection of DT file

[Baruch Siach]

Hi Stefan,

On Mon, Jan 13, 2020 at 08:27:31AM +0100, Stefan Roese wrote:
> On 25.11.19 11:30, Baruch Siach wrote:
> > Set the kernel device-tree file (fdtfile environment variable) based on
> > run-time detection of the platform.
> > 
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> >   arch/arm/mach-mvebu/Kconfig        |  1 +
> >   board/solidrun/clearfog/clearfog.c | 14 ++++++++++++++
> >   2 files changed, 15 insertions(+)
> > 
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index fdd39685b75d..bc5eaa5a7679 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -91,6 +91,7 @@ choice
> >   config TARGET_CLEARFOG
> >   	bool "Support ClearFog"
> >   	select 88F6820
> > +	select BOARD_LATE_INIT
> >   config TARGET_HELIOS4
> >   	bool "Support Helios4"
> > diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> > index 852b1ad71bb2..00c1306e9c80 100644
> > --- a/board/solidrun/clearfog/clearfog.c
> > +++ b/board/solidrun/clearfog/clearfog.c
> > @@ -265,3 +265,17 @@ int board_eth_init(bd_t *bis)
> >   	cpu_eth_init(bis); /* Built in controller(s) come first */
> >   	return pci_eth_init(bis);
> >   }
> > +
> > +int board_late_init(void)
> > +{
> > +	read_tlv_data();
> > +
> > +	if (sr_product_is("Clearfog Base"))
> > +		 env_set("fdtfile", "armada-388-clearfog-base.dtb");
> > +	else if (sr_product_is("Clearfog GTR S4"))
> > +		 env_set("fdtfile", "armada-385-clearfog-gtr-s4.dtb");
> > +	else if (sr_product_is("Clearfog GTR L8"))
> > +		 env_set("fdtfile", "armada-385-clearfog-gtr-l8.dtb");
> > +
> > +	return 0;
> > +}
> > 
> 
> Where does this sr_product_is() function come from? I might have missed
> it in a previous patch - please point me to it, thanks.

sr_product_is() is added in patch #6 of this series. Match is tested against 
TLV stored product name string.

> Does it make sense to match string values here or wouldn't it be better
> to match board ID's (enum)?

sr_product_is() matches also string prefixes. I use this feature in patch #9 
to set Clearfog GTR S4/L8 serdes configuration.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[PATCH 03/10] cmd: add sys_eeprom command

[Stefan Roese]

Hi Baruch,

On 14.01.20 11:18, Baruch Siach wrote:
> Hi Stefan,
> 
> On Mon, Jan 13, 2020 at 08:11:05AM +0100, Stefan Roese wrote:
>> On 25.11.19 11:30, Baruch Siach wrote:
>>> Based on U-Boot patch from the Open Compute project:
>>>
>>>     https://github.com/opencomputeproject/onie/blob/ec87e872d46b9805565d2c6124b2f701ef1c07b1/patches/u-boot/common/feature-sys-eeprom-tlv-common.patch
>>>
>>> Keep only I2C EEPROM support. Use the generic eeprom driver.
>>>
>>> Add support for multiple EEPROM TLV stores on the same system.
>>
>> Could you please explain, what TLV stands for? Is it "Type Length
>> Value"? Please add it to the description.
> 
> Will do.
> 
>>> This is
>>> useful in case of SOM and carrier that both provide ID and hardware
>>> configuration information.
>>>
>>> Add option to enable for SPL. This allows selection of RAM configuration
>>> based on EEPROM stored board identification.
>>>
>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>>
>> Shouldn't you add the copyrights from the original patch:
>>
>> Add to support 'sys_eeprom' command (common part)
>>
>> Copyright (C) 2013 Curt Brune <curt@cumulusnetworks.com>
>> Copyright (C) 2014 Srideep <srideep_devireddy@dell.com>
>> Copyright (C) 2013 Miles Tseng <miles_tseng@accton.com>
>> Copyright (C) 2014,2016 david_yang <david_yang@accton.com>
>>
>> ?
> 
> Copyright information is not usually listed in commit log messages. Is there
> any reason to do that in this patch?

No. I meant putting it into the source code file instead.
  
>>> ---
>>>    cmd/Kconfig          |   12 +
>>>    cmd/Makefile         |    2 +
>>>    cmd/sys_eeprom.c     | 1078 ++++++++++++++++++++++++++++++++++++++++++
>>>    include/sys_eeprom.h |  169 +++++++
>>>    4 files changed, 1261 insertions(+)
>>>    create mode 100644 cmd/sys_eeprom.c
>>>    create mode 100644 include/sys_eeprom.h
>>>
>>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>>> index 99b8a0e21822..483663404da4 100644
>>> --- a/cmd/Kconfig
>>> +++ b/cmd/Kconfig
>>> @@ -234,6 +234,18 @@ config CMD_REGINFO
>>>    	help
>>>    	  Register dump
>>> +config CMD_SYS_EEPROM
>>> +	bool "sys_eeprom"
>>> +	depends on I2C_EEPROM
>>> +	help
>>> +	  Display and program the system EEPROM data block.
>>> +
>>> +config SPL_CMD_SYS_EEPROM
>>> +	bool "sys_eeprom for SPL"
>>> +	depends on SPL_I2C_EEPROM
>>> +	help
>>> +	  Read system EEPROM data block.
>>> +
>>
>> I'm not sure, if this description is enough. This sounds too generic
>> for my taste as I assume that there are multiple formats to store system
>> data in an EEPROM. It might make sense to mention TLV here. And perhaps
>> its also better to name the files differently by adding "tlv" as well.
>>
>> What do you think?
> 
> Sounds reasonable. I'll rename to something less generic.

Thanks.
  
>>>    endmenu
>>>    menu "Boot commands"
>>> diff --git a/cmd/Makefile b/cmd/Makefile
>>> index 2d723ea0f07d..cbb1d46b8f50 100644
>>> --- a/cmd/Makefile
>>> +++ b/cmd/Makefile
>>> @@ -180,6 +180,8 @@ obj-$(CONFIG_X86) += x86/
>>>    obj-$(CONFIG_ARCH_MVEBU) += mvebu/
>>>    endif # !CONFIG_SPL_BUILD
>>> +obj-$(CONFIG_$(SPL_)CMD_SYS_EEPROM) += sys_eeprom.o
>>> +
>>>    # core command
>>>    obj-y += nvedit.o
>>> diff --git a/cmd/sys_eeprom.c b/cmd/sys_eeprom.c
>>> new file mode 100644
>>> index 000000000000..373673a5266c
>>> --- /dev/null
>>> +++ b/cmd/sys_eeprom.c
>>> @@ -0,0 +1,1078 @@
>>> +/*
>>> + * See file CREDITS for list of people who contributed to this
>>> + * project.
>>> + *
>>> + * SPDX-License-Identifier:	GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <command.h>
>>> +#include <dm.h>
>>> +#include <i2c.h>
>>> +#include <i2c_eeprom.h>
>>> +#include <env.h>
>>> +#include <linux/ctype.h>
>>> +
>>> +#include "sys_eeprom.h"
>>> +
>>> +#define MAX_TLV_DEVICES	2
>>> +
>>> +/* File scope function prototypes */
>>> +static bool is_checksum_valid(u8 *eeprom);
>>> +static int read_eeprom(u8 *eeprom);
>>> +static void show_eeprom(u8 *eeprom);
>>> +static void decode_tlv(tlvinfo_tlv_t * tlv);
>>> +static void update_crc(u8 *eeprom);
>>> +static int prog_eeprom(u8 * eeprom);
>>> +static bool tlvinfo_find_tlv(u8 * eeprom, u8 tcode, int * eeprom_index);
>>> +static bool tlvinfo_delete_tlv(u8 * eeprom, u8 code);
>>> +static bool tlvinfo_add_tlv(u8 * eeprom, int tcode, char * strval);
>>> +static int set_mac(char *buf, const char *string);
>>> +static int set_date(char *buf, const char *string);
>>> +static int set_bytes(char *buf, const char *string, int * converted_accum);
>>> +static void show_tlv_devices(void);
>>> +
>>> +/* Set to 1 if we've read EEPROM into memory */
>>> +static int has_been_read = 0;
>>> +/* The EERPOM contents after being read into memory */
>>> +static u8 eeprom[TLV_INFO_MAX_LEN];
>>> +
>>> +static struct udevice *tlv_devices[MAX_TLV_DEVICES];
>>> +static unsigned current_dev;
>>> +
>>> +/**
>>> + *  is_valid_tlv
>>> + *
>>> + *  Perform basic sanity checks on a TLV field. The TLV is pointed to
>>> + *  by the parameter provided.
>>> + *      1. The type code is not reserved (0x00 or 0xFF)
>>> + */
>>> +static inline bool is_valid_tlv(tlvinfo_tlv_t *tlv)
>>> +{
>>> +	return( (tlv->type != 0x00) &&
>>> +		(tlv->type != 0xFF) );
>>
>> This will generate checkpatch errors. I know that you copied this code
>> but still we should not add code that is not checkpatch clean.
>>
>> Please rework this file so that it at least matches the basic U-Boot
>> coding style rules.
>>
>> I'm stopping my review here for now and will review the new version.
> 
> Thanks for your review so far. I'll update and resend.

Good.

Thanks,
Stefan

[PATCH 08/10] ARM: mvebu: clearfog: run-time selection of DT file

[Stefan Roese]

Hi Baruch,

On 14.01.20 11:26, Baruch Siach wrote:
> Hi Stefan,
> 
> On Mon, Jan 13, 2020 at 08:27:31AM +0100, Stefan Roese wrote:
>> On 25.11.19 11:30, Baruch Siach wrote:
>>> Set the kernel device-tree file (fdtfile environment variable) based on
>>> run-time detection of the platform.
>>>
>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>>> ---
>>>    arch/arm/mach-mvebu/Kconfig        |  1 +
>>>    board/solidrun/clearfog/clearfog.c | 14 ++++++++++++++
>>>    2 files changed, 15 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
>>> index fdd39685b75d..bc5eaa5a7679 100644
>>> --- a/arch/arm/mach-mvebu/Kconfig
>>> +++ b/arch/arm/mach-mvebu/Kconfig
>>> @@ -91,6 +91,7 @@ choice
>>>    config TARGET_CLEARFOG
>>>    	bool "Support ClearFog"
>>>    	select 88F6820
>>> +	select BOARD_LATE_INIT
>>>    config TARGET_HELIOS4
>>>    	bool "Support Helios4"
>>> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
>>> index 852b1ad71bb2..00c1306e9c80 100644
>>> --- a/board/solidrun/clearfog/clearfog.c
>>> +++ b/board/solidrun/clearfog/clearfog.c
>>> @@ -265,3 +265,17 @@ int board_eth_init(bd_t *bis)
>>>    	cpu_eth_init(bis); /* Built in controller(s) come first */
>>>    	return pci_eth_init(bis);
>>>    }
>>> +
>>> +int board_late_init(void)
>>> +{
>>> +	read_tlv_data();
>>> +
>>> +	if (sr_product_is("Clearfog Base"))
>>> +		 env_set("fdtfile", "armada-388-clearfog-base.dtb");
>>> +	else if (sr_product_is("Clearfog GTR S4"))
>>> +		 env_set("fdtfile", "armada-385-clearfog-gtr-s4.dtb");
>>> +	else if (sr_product_is("Clearfog GTR L8"))
>>> +		 env_set("fdtfile", "armada-385-clearfog-gtr-l8.dtb");
>>> +
>>> +	return 0;
>>> +}
>>>
>>
>> Where does this sr_product_is() function come from? I might have missed
>> it in a previous patch - please point me to it, thanks.
> 
> sr_product_is() is added in patch #6 of this series. Match is tested against
> TLV stored product name string.

Ah, I missed it. Thanks.
  
>> Does it make sense to match string values here or wouldn't it be better
>> to match board ID's (enum)?
> 
> sr_product_is() matches also string prefixes. I use this feature in patch #9
> to set Clearfog GTR S4/L8 serdes configuration.

If you prefer to continue this was with string comparison, then feel free
to add my:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan