Skip to contentSkip to conversation

board: ge: Move bootcount to EEPROM

PATCH New Fabien Lahoudere 2 participants 13 messages Oct 17, 2018 – Dec 8, 2018

lore
Share & tools
lore
#0 lore raw

v3 cover board: ge: Move bootcount to EEPROM

GE boards need bootcount set in EEPROM instead of file to avoid some issues.

This series replace v2 sent by Sebastian Reichel on 2018-07-20.

Changes since v2:
- Patches have been rebased to apply correctly on master.
- Remove a build time warning
- Remove checkpatch.pl warning

Denis Zalevskiy (6):
board: ge: Remove EEPROM bus param from read_vpd()
board: ge: Move VPD EEPROM configuration to the defconfig
bootcount: i2c: Add bus switching to the I2C bootcount driver
bootcount: Configure length limit for I2C bootcount
board: ge: Move VPD reading to the vpd_reader
board: ge: Store bootcount in EEPROM on PPD and Bx50v3

 board/ge/bx50v3/Kconfig           |  2 +
 board/ge/bx50v3/bx50v3.c          | 57 ++++++----------------------
 board/ge/common/Kconfig           | 14 +++++++
 board/ge/common/vpd_reader.c      | 57 ++++++++++++++++++++++++++--
 board/ge/common/vpd_reader.h      | 16 +++++---
 board/ge/mx53ppd/Kconfig          |  2 +
 board/ge/mx53ppd/mx53ppd.c        | 44 ++++------------------
 configs/ge_bx50v3_defconfig       | 27 +++++++++++--
 configs/mx53ppd_defconfig         | 29 ++++++++++++--
 drivers/bootcount/Kconfig         | 21 ++++++++++-
 drivers/bootcount/bootcount_i2c.c | 79 +++++++++++++++++++++++++++++++++------
 include/configs/ge_bx50v3.h       |  6 +--
 include/configs/mx53ppd.h         |  6 +--
 13 files changed, 243 insertions(+), 117 deletions(-)
 create mode 100644 board/ge/common/Kconfig
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH v3 1/6 board: ge: Remove EEPROM bus param from read_vpd()

From: Denis Zalevskiy <[email protected]>

The bus is statically defined, so remove redundant parameters from read_vpd() for PPD and Bx50v3.

---
 board/ge/bx50v3/bx50v3.c   | 6 +++---
 board/ge/mx53ppd/mx53ppd.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)
board/ge/bx50v3/bx50v3.c +3 -3
index b2d065c..c3c6ded 100644
@@ -623,14 +623,14 @@ static void process_vpd(struct vpd_cache *vpd)
 		eth_env_set_enetaddr_by_index("eth", i210_index, vpd->mac2);
 }
 
-static int read_vpd(uint eeprom_bus)
+static int read_vpd(void)
 {
 	int res;
 	int size = 1024;
 	uint8_t *data;
 	unsigned int current_i2c_bus = i2c_get_bus_num();
 
-	res = i2c_set_bus_num(eeprom_bus);
+	res = i2c_set_bus_num(CONFIG_SYS_I2C_EEPROM_BUS);
 	if (res < 0)
 		return res;
 
@@ -714,7 +714,7 @@ int board_init(void)
 	setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info2);
 	setup_i2c(3, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info3);
 
-	read_vpd(CONFIG_SYS_I2C_EEPROM_BUS);
+	read_vpd();
 
 	set_confidx(&vpd);
 
board/ge/mx53ppd/mx53ppd.c +3 -3
index cf278e8..62137f6 100644
@@ -328,7 +328,7 @@ static void process_vpd(struct vpd_cache *vpd)
 		eth_env_set_enetaddr("ethaddr", vpd->mac1);
 }
 
-static int read_vpd(uint eeprom_bus)
+static int read_vpd(void)
 {
 	struct vpd_cache vpd;
 	int res;
@@ -336,7 +336,7 @@ static int read_vpd(uint eeprom_bus)
 	u8 *data;
 	unsigned int current_i2c_bus = i2c_get_bus_num();
 
-	res = i2c_set_bus_num(eeprom_bus);
+	res = i2c_set_bus_num(VPD_EEPROM_BUS);
 	if (res < 0)
 		return res;
 
@@ -390,7 +390,7 @@ int board_late_init(void)
 {
 	int res;
 
-	read_vpd(VPD_EEPROM_BUS);
+	read_vpd();
 
 	res = clock_1GHz();
 	if (res != 0)
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH v3 1/6 board: ge: Remove EEPROM bus param from read_vpd()

Hi Fabien,

Quoted 8 lines · Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

The bus is statically defined, so remove redundant parameters from read_vpd() for PPD and Bx50v3.

---

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#3 lore raw

PATCH v3 2/6 board: ge: Move VPD EEPROM configuration to the defconfig

In reply to Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

Use standard configuration logic to define EEPROM constants. Names are based on VPD_EEPROM_ prefix because EEPROM_ is already used by i2c_eeprom driver.

---
 board/ge/bx50v3/Kconfig     |  2 ++
 board/ge/bx50v3/bx50v3.c    | 17 ++++-------------
 board/ge/common/Kconfig     | 14 ++++++++++++++
 board/ge/mx53ppd/Kconfig    |  2 ++
 board/ge/mx53ppd/mx53ppd.c  | 14 ++++----------
 configs/ge_bx50v3_defconfig |  9 +++++++++
 configs/mx53ppd_defconfig   | 10 ++++++++++
 7 files changed, 45 insertions(+), 23 deletions(-)
 create mode 100644 board/ge/common/Kconfig
board/ge/bx50v3/Kconfig +2 -0
index 993b055..0593856 100644
@@ -15,4 +15,6 @@ config SYS_SOC
 config SYS_CONFIG_NAME
 	default "ge_bx50v3"
 
+source "board/ge/common/Kconfig"
+
 endif
board/ge/bx50v3/bx50v3.c +4 -13
index c3c6ded..ca500f5 100644
@@ -38,15 +38,6 @@ struct vpd_cache;
 static int confidx = 3;  /* Default to b850v3. */
 static struct vpd_cache vpd;
 
-#ifndef CONFIG_SYS_I2C_EEPROM_ADDR
-# define CONFIG_SYS_I2C_EEPROM_ADDR     0x50
-# define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 1
-#endif
-
-#ifndef CONFIG_SYS_I2C_EEPROM_BUS
-#define CONFIG_SYS_I2C_EEPROM_BUS       4
-#endif
-
 #define NC_PAD_CTRL (PAD_CTL_PUS_100K_UP |	\
 	PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm |	\
 	PAD_CTL_HYS)
@@ -626,11 +617,11 @@ static void process_vpd(struct vpd_cache *vpd)
 static int read_vpd(void)
 {
 	int res;
-	int size = 1024;
+	static const int size = CONFIG_SYS_VPD_EEPROM_SIZE;
 	uint8_t *data;
 	unsigned int current_i2c_bus = i2c_get_bus_num();
 
-	res = i2c_set_bus_num(CONFIG_SYS_I2C_EEPROM_BUS);
+	res = i2c_set_bus_num(CONFIG_SYS_VPD_EEPROM_I2C_BUS);
 	if (res < 0)
 		return res;
 
@@ -638,8 +629,8 @@ static int read_vpd(void)
 	if (!data)
 		return -ENOMEM;
 
-	res = i2c_read(CONFIG_SYS_I2C_EEPROM_ADDR, 0,
-			CONFIG_SYS_I2C_EEPROM_ADDR_LEN, data, size);
+	res = i2c_read(CONFIG_SYS_VPD_EEPROM_I2C_ADDR, 0,
+		       CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN, data, size);
 
 	if (res == 0) {
 		memset(&vpd, 0, sizeof(vpd));
board/ge/common/Kconfig new file +14 -0
new file mode 100644
index 0000000..637b264
@@ -0,0 +1,14 @@
+config SYS_VPD_EEPROM_I2C_ADDR
+	hex "I2C address of the EEPROM device used for VPD"
+	help
+	  VPD = Vital Product Data
+
+config SYS_VPD_EEPROM_I2C_BUS
+	int "I2C bus of the EEPROM device used for VPD."
+
+config SYS_VPD_EEPROM_SIZE
+	int "Size in bytes of the EEPROM device used for VPD"
+
+config SYS_VPD_EEPROM_I2C_ADDR_LEN
+	int "Number of bytes to use for VPD EEPROM address"
+	default 1
board/ge/mx53ppd/Kconfig +2 -0
index 6dc3818..bebb2fa 100644
@@ -13,4 +13,6 @@ config SYS_SOC
 config SYS_CONFIG_NAME
 	default "mx53ppd"
 
+source "board/ge/common/Kconfig"
+
 endif
board/ge/mx53ppd/mx53ppd.c +4 -10
index 62137f6..3fa53ff 100644
@@ -40,13 +40,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-/* Index of I2C1, SEGMENT 1 (see CONFIG_SYS_I2C_BUSES). */
-#define VPD_EEPROM_BUS 2
-
-/* Address of 24C08 EEPROM. */
-#define VPD_EEPROM_ADDR		0x50
-#define VPD_EEPROM_ADDR_LEN	1
-
 static u32 mx53_dram_size[2];
 
 phys_size_t get_effective_memsize(void)
@@ -332,11 +325,11 @@ static int read_vpd(void)
 {
 	struct vpd_cache vpd;
 	int res;
-	int size = 1024;
+	static const int size = CONFIG_SYS_VPD_EEPROM_SIZE;
 	u8 *data;
 	unsigned int current_i2c_bus = i2c_get_bus_num();
 
-	res = i2c_set_bus_num(VPD_EEPROM_BUS);
+	res = i2c_set_bus_num(CONFIG_SYS_VPD_EEPROM_I2C_BUS);
 	if (res < 0)
 		return res;
 
@@ -344,7 +337,8 @@ static int read_vpd(void)
 	if (!data)
 		return -ENOMEM;
 
-	res = i2c_read(VPD_EEPROM_ADDR, 0, VPD_EEPROM_ADDR_LEN, data, size);
+	res = i2c_read(CONFIG_SYS_VPD_EEPROM_I2C_ADDR, 0,
+		       CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN, data, size);
 	if (res == 0) {
 		memset(&vpd, 0, sizeof(vpd));
 		vpd_reader(size, data, &vpd, vpd_callback);
configs/ge_bx50v3_defconfig +9 -0
index 729377d..8426279 100644
@@ -8,6 +8,15 @@ CONFIG_FIT=y
 CONFIG_BOOTDELAY=1
 CONFIG_SYS_CONSOLE_IS_IN_ENV=y
 CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y
+
+# monitor's EEPROM is connected to the bus through the mux channel 1
+# (the number is the offset in CONFIG_SYS_I2C_BUSES)
+CONFIG_SYS_VPD_EEPROM_I2C_BUS=4
+# Address of Atmel 24C08 EEPROM
+CONFIG_SYS_VPD_EEPROM_I2C_ADDR=0x50
+CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN=1
+CONFIG_SYS_VPD_EEPROM_SIZE=1024
+
 CONFIG_SUPPORT_RAW_INITRD=y
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_LAST_STAGE_INIT=y
configs/mx53ppd_defconfig +10 -0
index 7be5c04..2953e01 100644
@@ -9,6 +9,16 @@ CONFIG_BOOTDELAY=1
 # CONFIG_CONSOLE_MUX is not set
 CONFIG_SYS_CONSOLE_IS_IN_ENV=y
 CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y
+
+# monitor's EEPROM is connected to the bus through the mux channel 1
+# (the number is the offset in CONFIG_SYS_I2C_BUSES)
+# (there is also Frame EEPROM connected to the channel 4 (bus 4))
+CONFIG_SYS_VPD_EEPROM_I2C_BUS=2
+# Address of Atmel 24C08 EEPROM
+CONFIG_SYS_VPD_EEPROM_I2C_ADDR=0x50
+CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN=1
+CONFIG_SYS_VPD_EEPROM_SIZE=1024
+
 CONFIG_SUPPORT_RAW_INITRD=y
 CONFIG_MISC_INIT_R=y
 CONFIG_HUSH_PARSER=y
Signature · 1 line
1.8.3.1
#4 lore raw

PATCH v3 2/6 board: ge: Move VPD EEPROM configuration to the defconfig

Quoted 9 lines · Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

Use standard configuration logic to define EEPROM constants. Names are based on VPD_EEPROM_ prefix because EEPROM_ is already used by i2c_eeprom driver.

---

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#5 lore raw

PATCH v3 3/6 bootcount: i2c: Add bus switching to the I2C bootcount driver

In reply to Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

If there is an I2C mux, current bus should be switched before manipulating with I2C.

---
 drivers/bootcount/Kconfig         | 15 ++++++++-
 drivers/bootcount/bootcount_i2c.c | 71 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 77 insertions(+), 9 deletions(-)
drivers/bootcount/Kconfig +14 -1
index 9a0bd51..f67f518 100644
@@ -62,7 +62,9 @@ config BOOTCOUNT_I2C
 	bool "Boot counter on I2C device"
 	help
 	  Enable support for the bootcounter on an i2c (like RTC) device.
-	  CONFIG_SYS_I2C_RTC_ADDR = i2c chip address
+	  CONFIG_SYS_BOOTCOUNT_I2C_BUS = bus of the target I2C device,
+	                                 CONFIG_SYS_I2C_RTC_ADDR is used as fallback
+	  CONFIG_SYS_BOOTCOUNT_I2C_ADDR = target I2C device address
 	  CONFIG_SYS_BOOTCOUNT_ADDR = i2c addr which is used for
 	                              the bootcounter.
 
@@ -127,4 +129,15 @@ config SYS_BOOTCOUNT_ADDR
 	help
 	  Set the address used for reading and writing the boot counter.
 
+config SYS_BOOTCOUNT_I2C_BUS
+	int "I2C bootcounter device bus"
+	depends on BOOTCOUNT_I2C
+	help
+	  I2C bus of the device used to store bootcounter
+
+config SYS_BOOTCOUNT_I2C_ADDR
+	hex "I2C bootcounter device address"
+	depends on BOOTCOUNT_I2C
+	help
+	  I2C address of the device used to store bootcounter
 endif
drivers/bootcount/bootcount_i2c.c +63 -8

99 lines not on this page · raw message

Signature · 1 line
1.8.3.1
#6 lore raw

PATCH v3 3/6 bootcount: i2c: Add bus switching to the I2C bootcount driver

Quoted 8 lines · Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

If there is an I2C mux, current bus should be switched before manipulating with I2C.

---

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#7 lore raw

PATCH v3 4/6 bootcount: Configure length limit for I2C bootcount

In reply to Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

Bootcount driver should verify size against the maximum available space. New configuration parameter adds this capability and keeps backward compatibility by providing default value.

---
 drivers/bootcount/Kconfig         |  6 ++++++
 drivers/bootcount/bootcount_i2c.c | 10 ++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)
drivers/bootcount/Kconfig +6 -0
index f67f518..3eccd5b 100644
@@ -140,4 +140,10 @@ config SYS_BOOTCOUNT_I2C_ADDR
 	depends on BOOTCOUNT_I2C
 	help
 	  I2C address of the device used to store bootcounter
+
+config BOOTCOUNT_I2C_LEN
+	int "Maximum length of bootcounter in bytes"
+	default 2
+	depends on BOOTCOUNT_I2C
+
 endif
drivers/bootcount/bootcount_i2c.c +6 -4
index a1fc219..73c920a 100644
@@ -54,14 +54,15 @@ void bootcount_store(ulong a)
 	if (prev_i2c_bus < 0)
 		return;
 
-	unsigned char buf[3];
+	unsigned char buf[2];
 	int ret;
 
+	BUILD_BUG_ON(sizeof(buf) > CONFIG_BOOTCOUNT_I2C_LEN);
 	buf[0] = BC_MAGIC;
 	buf[1] = (a & 0xff);
 	ret = i2c_write(CONFIG_SYS_BOOTCOUNT_I2C_ADDR,
 			CONFIG_SYS_BOOTCOUNT_ADDR,
-			CONFIG_BOOTCOUNT_ALEN, buf, 2);
+			CONFIG_BOOTCOUNT_ALEN, buf, sizeof(buf));
 	if (ret != 0)
 		puts("Error writing bootcount\n");
 
@@ -77,12 +78,13 @@ ulong bootcount_load(void)
 	if (prev_i2c_bus < 0)
 		return count;
 
-	unsigned char buf[3];
+	unsigned char buf[2];
 	int ret;
 
+	BUILD_BUG_ON(sizeof(buf) > CONFIG_BOOTCOUNT_I2C_LEN);
 	ret = i2c_read(CONFIG_SYS_BOOTCOUNT_I2C_ADDR,
 		       CONFIG_SYS_BOOTCOUNT_ADDR,
-		       CONFIG_BOOTCOUNT_ALEN, buf, 2);
+		       CONFIG_BOOTCOUNT_ALEN, buf, sizeof(buf));
 	if (ret != 0) {
 		puts("Error loading bootcount\n");
 		goto out;
Signature · 1 line
1.8.3.1
#8 lore raw

PATCH v3 4/6 bootcount: Configure length limit for I2C bootcount

Quoted 9 lines · Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

Bootcount driver should verify size against the maximum available space. New configuration parameter adds this capability and keeps backward compatibility by providing default value.

---

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#9 lore raw

PATCH v3 5/6 board: ge: Move VPD reading to the vpd_reader

In reply to Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

Merge functionality duplicated in bx50v3 and mx53ppd: the logic is the same except that process_vpd is called at different phases. Also read_vpd could end up in error, so there is no VPD data in this case - it shouldn't be processed.

---
 board/ge/bx50v3/bx50v3.c     | 48 ++++++++++----------------------------------
 board/ge/common/vpd_reader.c | 37 +++++++++++++++++++++++++++++++---
 board/ge/common/vpd_reader.h | 16 ++++++++++-----
 board/ge/mx53ppd/mx53ppd.c   | 38 +++++++----------------------------
 4 files changed, 63 insertions(+), 76 deletions(-)
board/ge/bx50v3/bx50v3.c +11 -37
index ca500f5..78e7ee6 100644
@@ -33,8 +33,6 @@
 #include "../../../drivers/net/e1000.h"
 DECLARE_GLOBAL_DATA_PTR;
 
-struct vpd_cache;
-
 static int confidx = 3;  /* Default to b850v3. */
 static struct vpd_cache vpd;
 
@@ -552,6 +550,7 @@ int overwrite_console(void)
 #define VPD_MAC_ADDRESS_LENGTH 6
 
 struct vpd_cache {
+	bool is_read;
 	u8 product_id;
 	u8 has;
 	unsigned char mac1[VPD_MAC_ADDRESS_LENGTH];
@@ -561,11 +560,9 @@ struct vpd_cache {
 /*
  * Extracts MAC and product information from the VPD.
  */
-static int vpd_callback(void *userdata, u8 id, u8 version, u8 type,
+static int vpd_callback(struct vpd_cache *vpd, u8 id, u8 version, u8 type,
 			size_t size, u8 const *data)
 {
-	struct vpd_cache *vpd = (struct vpd_cache *)userdata;
-
 	if (id == VPD_BLOCK_HWID && version == 1 && type != VPD_TYPE_INVALID &&
 	    size >= 1) {
 		vpd->product_id = data[0];
@@ -589,6 +586,11 @@ static void process_vpd(struct vpd_cache *vpd)
 	int fec_index = -1;
 	int i210_index = -1;
 
+	if (!vpd->is_read) {
+		printf("VPD wasn't read");
+		return;
+	}
+
 	switch (vpd->product_id) {
 	case VPD_PRODUCT_B450:
 		env_set("confidx", "1");
@@ -614,35 +616,6 @@ static void process_vpd(struct vpd_cache *vpd)
 		eth_env_set_enetaddr_by_index("eth", i210_index, vpd->mac2);
 }
 
-static int read_vpd(void)
-{
-	int res;
-	static const int size = CONFIG_SYS_VPD_EEPROM_SIZE;
-	uint8_t *data;
-	unsigned int current_i2c_bus = i2c_get_bus_num();
-
-	res = i2c_set_bus_num(CONFIG_SYS_VPD_EEPROM_I2C_BUS);
-	if (res < 0)
-		return res;
-
-	data = (uint8_t *)malloc(size);
-	if (!data)
-		return -ENOMEM;
-
-	res = i2c_read(CONFIG_SYS_VPD_EEPROM_I2C_ADDR, 0,
-		       CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN, data, size);
-
-	if (res == 0) {
-		memset(&vpd, 0, sizeof(vpd));
-		vpd_reader(size, data, &vpd, vpd_callback);
-	}
-
-	free(data);
-
-	i2c_set_bus_num(current_i2c_bus);
-	return res;
-}
-
 int board_eth_init(bd_t *bis)
 {
 	setup_iomux_enet();
@@ -705,9 +678,10 @@ int board_init(void)
 	setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info2);
 	setup_i2c(3, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info3);
 
-	read_vpd();
-
-	set_confidx(&vpd);
+	if (!read_vpd(&vpd, vpd_callback)) {
+		vpd.is_read = true;
+		set_confidx(&vpd);
+	}
 
 	gpio_direction_output(SUS_S3_OUT, 1);
 	gpio_direction_output(WIFI_EN, 1);
board/ge/common/vpd_reader.c +34 -3
index c471583..12410d9 100644
@@ -5,6 +5,7 @@
 
 #include "vpd_reader.h"
 
+#include <i2c.h>
 #include <linux/bch.h>
 #include <stdlib.h>
 
@@ -105,9 +106,9 @@ static const size_t HEADER_BLOCK_ECC_LEN = 4;
 
 static const u8 ECC_BLOCK_ID = 0xFF;
 
-int vpd_reader(size_t size, u8 *data, void *userdata,
-	       int (*fn)(void *userdata, u8 id, u8 version, u8 type,
-			 size_t size, u8 const *data))
+static int vpd_reader(size_t size, u8 *data, struct vpd_cache *userdata,
+		      int (*fn)(struct vpd_cache *, u8 id, u8 version, u8 type,
+				size_t size, u8 const *data))
 {
 	if (size < HEADER_BLOCK_LEN || !data || !fn)
 		return -EINVAL;
@@ -194,3 +195,33 @@ int vpd_reader(size_t size, u8 *data, void *userdata,
 			return ret;
 	}
 }
+
+int read_vpd(struct vpd_cache *cache,
+	     int (*process_block)(struct vpd_cache *, u8 id, u8 version,
+				  u8 type, size_t size, u8 const *data))
+{
+	static const size_t size = CONFIG_SYS_VPD_EEPROM_SIZE;
+
+	int res;
+	u8 *data;
+	unsigned int current_i2c_bus = i2c_get_bus_num();
+
+	res = i2c_set_bus_num(CONFIG_SYS_VPD_EEPROM_I2C_BUS);
+	if (res < 0)
+		return res;
+
+	data = malloc(size);
+	if (!data)
+		return -ENOMEM;
+
+	res = i2c_read(CONFIG_SYS_VPD_EEPROM_I2C_ADDR, 0,
+		       CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN,
+		       data, size);
+	if (res == 0)
+		res = vpd_reader(size, data, cache, process_block);
+
+	free(data);
+
+	i2c_set_bus_num(current_i2c_bus);
+	return res;
+}
board/ge/common/vpd_reader.h +11 -5
index e60acf3..3045b7e 100644
@@ -5,12 +5,18 @@
 
 #include "common.h"
 
+struct vpd_cache;
+
 /*
- * Read VPD from given data, verify content, and call callback
- * for each vital product data block.
+ * Read VPD from given data, verify content, call callback for each vital
+ * product data block.
+ *
+ * cache: structure used by process block to store VPD information
+ * process_block: callback called for each VPD data block
  *
  * Returns Non-zero on error.  Negative numbers encode errno.
  */
-int vpd_reader(size_t size, u8 *data, void *userdata,
-	       int (*fn)(void *userdata, u8 id, u8 version, u8 type,
-			 size_t size, u8 const *data));
+int read_vpd(struct vpd_cache *cache,
+	     int (*process_block)(struct vpd_cache *,
+				  u8 id, u8 version, u8 type,
+				  size_t size, u8 const *data));
board/ge/mx53ppd/mx53ppd.c +7 -31
index 3fa53ff..f0c8abf 100644
@@ -321,36 +321,6 @@ static void process_vpd(struct vpd_cache *vpd)
 		eth_env_set_enetaddr("ethaddr", vpd->mac1);
 }
 
-static int read_vpd(void)
-{
-	struct vpd_cache vpd;
-	int res;
-	static const int size = CONFIG_SYS_VPD_EEPROM_SIZE;
-	u8 *data;
-	unsigned int current_i2c_bus = i2c_get_bus_num();
-
-	res = i2c_set_bus_num(CONFIG_SYS_VPD_EEPROM_I2C_BUS);
-	if (res < 0)
-		return res;
-
-	data = malloc(size);
-	if (!data)
-		return -ENOMEM;
-
-	res = i2c_read(CONFIG_SYS_VPD_EEPROM_I2C_ADDR, 0,
-		       CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN, data, size);
-	if (res == 0) {
-		memset(&vpd, 0, sizeof(vpd));
-		vpd_reader(size, data, &vpd, vpd_callback);
-		process_vpd(&vpd);
-	}
-
-	free(data);
-
-	i2c_set_bus_num(current_i2c_bus);
-	return res;
-}
-
 int board_init(void)
 {
 	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
@@ -383,8 +353,14 @@ int misc_init_r(void)
 int board_late_init(void)
 {
 	int res;
+	struct vpd_cache vpd;
 
-	read_vpd();
+	memset(&vpd, 0, sizeof(vpd));
+	res = read_vpd(&vpd, vpd_callback);
+	if (!res)
+		process_vpd(&vpd);
+	else
+		printf("Can't read VPD");
 
 	res = clock_1GHz();
 	if (res != 0)
Signature · 1 line
1.8.3.1
#10 lore raw

PATCH v3 5/6 board: ge: Move VPD reading to the vpd_reader

Quoted 241 lines · Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

Merge functionality duplicated in bx50v3 and mx53ppd: the logic is the same except that process_vpd is called at different phases. Also read_vpd could end up in error, so there is no VPD data in this case - it shouldn't be processed.

---
 board/ge/bx50v3/bx50v3.c     | 48 ++++++++++----------------------------------
 board/ge/common/vpd_reader.c | 37 +++++++++++++++++++++++++++++++---
 board/ge/common/vpd_reader.h | 16 ++++++++++-----
 board/ge/mx53ppd/mx53ppd.c   | 38 +++++++----------------------------
 4 files changed, 63 insertions(+), 76 deletions(-)
board/ge/bx50v3/bx50v3.c +11 -37
index ca500f5..78e7ee6 100644
@@ -33,8 +33,6 @@
 #include "../../../drivers/net/e1000.h"
 DECLARE_GLOBAL_DATA_PTR;
 
-struct vpd_cache;
-
 static int confidx = 3;  /* Default to b850v3. */
 static struct vpd_cache vpd;
 
@@ -552,6 +550,7 @@ int overwrite_console(void)
 #define VPD_MAC_ADDRESS_LENGTH 6
 
 struct vpd_cache {
+	bool is_read;
 	u8 product_id;
 	u8 has;
 	unsigned char mac1[VPD_MAC_ADDRESS_LENGTH];
@@ -561,11 +560,9 @@ struct vpd_cache {
 /*
  * Extracts MAC and product information from the VPD.
  */
-static int vpd_callback(void *userdata, u8 id, u8 version, u8 type,
+static int vpd_callback(struct vpd_cache *vpd, u8 id, u8 version, u8 type,
 			size_t size, u8 const *data)
 {
-	struct vpd_cache *vpd = (struct vpd_cache *)userdata;
-
 	if (id == VPD_BLOCK_HWID && version == 1 && type != VPD_TYPE_INVALID &&
 	    size >= 1) {
 		vpd->product_id = data[0];
@@ -589,6 +586,11 @@ static void process_vpd(struct vpd_cache *vpd)
 	int fec_index = -1;
 	int i210_index = -1;
 
+	if (!vpd->is_read) {
+		printf("VPD wasn't read");
+		return;
+	}
+
 	switch (vpd->product_id) {
 	case VPD_PRODUCT_B450:
 		env_set("confidx", "1");
@@ -614,35 +616,6 @@ static void process_vpd(struct vpd_cache *vpd)
 		eth_env_set_enetaddr_by_index("eth", i210_index, vpd->mac2);
 }
 
-static int read_vpd(void)
-{
-	int res;
-	static const int size = CONFIG_SYS_VPD_EEPROM_SIZE;
-	uint8_t *data;
-	unsigned int current_i2c_bus = i2c_get_bus_num();
-
-	res = i2c_set_bus_num(CONFIG_SYS_VPD_EEPROM_I2C_BUS);
-	if (res < 0)
-		return res;
-
-	data = (uint8_t *)malloc(size);
-	if (!data)
-		return -ENOMEM;
-
-	res = i2c_read(CONFIG_SYS_VPD_EEPROM_I2C_ADDR, 0,
-		       CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN, data, size);
-
-	if (res == 0) {
-		memset(&vpd, 0, sizeof(vpd));
-		vpd_reader(size, data, &vpd, vpd_callback);
-	}
-
-	free(data);
-
-	i2c_set_bus_num(current_i2c_bus);
-	return res;
-}
-
 int board_eth_init(bd_t *bis)
 {
 	setup_iomux_enet();
@@ -705,9 +678,10 @@ int board_init(void)
 	setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info2);
 	setup_i2c(3, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info3);
 
-	read_vpd();
-
-	set_confidx(&vpd);
+	if (!read_vpd(&vpd, vpd_callback)) {
+		vpd.is_read = true;
+		set_confidx(&vpd);
+	}
 
 	gpio_direction_output(SUS_S3_OUT, 1);
 	gpio_direction_output(WIFI_EN, 1);
board/ge/common/vpd_reader.c +34 -3
index c471583..12410d9 100644
@@ -5,6 +5,7 @@
 
 #include "vpd_reader.h"
 
+#include <i2c.h>
 #include <linux/bch.h>
 #include <stdlib.h>
 
@@ -105,9 +106,9 @@ static const size_t HEADER_BLOCK_ECC_LEN = 4;
 
 static const u8 ECC_BLOCK_ID = 0xFF;
 
-int vpd_reader(size_t size, u8 *data, void *userdata,
-	       int (*fn)(void *userdata, u8 id, u8 version, u8 type,
-			 size_t size, u8 const *data))
+static int vpd_reader(size_t size, u8 *data, struct vpd_cache *userdata,
+		      int (*fn)(struct vpd_cache *, u8 id, u8 version, u8 type,
+				size_t size, u8 const *data))
 {
 	if (size < HEADER_BLOCK_LEN || !data || !fn)
 		return -EINVAL;
@@ -194,3 +195,33 @@ int vpd_reader(size_t size, u8 *data, void *userdata,
 			return ret;
 	}
 }
+
+int read_vpd(struct vpd_cache *cache,
+	     int (*process_block)(struct vpd_cache *, u8 id, u8 version,
+				  u8 type, size_t size, u8 const *data))
+{
+	static const size_t size = CONFIG_SYS_VPD_EEPROM_SIZE;
+
+	int res;
+	u8 *data;
+	unsigned int current_i2c_bus = i2c_get_bus_num();
+
+	res = i2c_set_bus_num(CONFIG_SYS_VPD_EEPROM_I2C_BUS);
+	if (res < 0)
+		return res;
+
+	data = malloc(size);
+	if (!data)
+		return -ENOMEM;
+
+	res = i2c_read(CONFIG_SYS_VPD_EEPROM_I2C_ADDR, 0,
+		       CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN,
+		       data, size);
+	if (res == 0)
+		res = vpd_reader(size, data, cache, process_block);
+
+	free(data);
+
+	i2c_set_bus_num(current_i2c_bus);
+	return res;
+}
board/ge/common/vpd_reader.h +11 -5
index e60acf3..3045b7e 100644
@@ -5,12 +5,18 @@
 
 #include "common.h"
 
+struct vpd_cache;
+
 /*
- * Read VPD from given data, verify content, and call callback
- * for each vital product data block.
+ * Read VPD from given data, verify content, call callback for each vital
+ * product data block.
+ *
+ * cache: structure used by process block to store VPD information
+ * process_block: callback called for each VPD data block
  *
  * Returns Non-zero on error.  Negative numbers encode errno.
  */
-int vpd_reader(size_t size, u8 *data, void *userdata,
-	       int (*fn)(void *userdata, u8 id, u8 version, u8 type,
-			 size_t size, u8 const *data));
+int read_vpd(struct vpd_cache *cache,
+	     int (*process_block)(struct vpd_cache *,
+				  u8 id, u8 version, u8 type,
+				  size_t size, u8 const *data));
board/ge/mx53ppd/mx53ppd.c +0 -30
index 3fa53ff..f0c8abf 100644
@@ -321,36 +321,6 @@ static void process_vpd(struct vpd_cache *vpd)
 		eth_env_set_enetaddr("ethaddr", vpd->mac1);
 }
 
-static int read_vpd(void)
-{
-	struct vpd_cache vpd;
-	int res;
-	static const int size = CONFIG_SYS_VPD_EEPROM_SIZE;
-	u8 *data;
-	unsigned int current_i2c_bus = i2c_get_bus_num();
-
-	res = i2c_set_bus_num(CONFIG_SYS_VPD_EEPROM_I2C_BUS);
-	if (res < 0)
-		return res;
-
-	data = malloc(size);
-	if (!data)
-		return -ENOMEM;
-
-	res = i2c_read(CONFIG_SYS_VPD_EEPROM_I2C_ADDR, 0,
-		       CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN, data, size);
-	if (res == 0) {
-		memset(&vpd, 0, sizeof(vpd));
-		vpd_reader(size, data, &vpd, vpd_callback);
-		process_vpd(&vpd);
-	}
-
-	free(data);
-
-	i2c_set_bus_num(current_i2c_bus);
-	return res;
-}
-
 int board_init(void)
 {
 	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;

board/ge/mx53ppd/mx53ppd.c int misc_init_r(void) · line 359

Quoted 8 lines
board/ge/mx53ppd/mx53ppd.c +3 -1
@@ -383,8 +353,14 @@ int misc_init_r(void)
 int board_late_init(void)
 {
 	int res;
+	struct vpd_cache vpd;
 
-	read_vpd();
+	memset(&vpd, 0, sizeof(vpd));
+	res = read_vpd(&vpd, vpd_callback);

I applied your patch, but this generates a warning. Please send a follow up patch to fix it.

+ res = read_vpd(&vpd, vpd_callback);
+ ^~~~~~~~~~~~
+In file included from board/ge/mx53ppd/mx53ppd.c:37:0:
+board/ge/mx53ppd/../../ge/common/vpd_reader.h:19:5: note: expected ?int (*)(struct vpd_cache *, u8, u8, u8, size_t, const u8 *) {aka int (*)(struct vpd_cache *, unsigned char, unsigned char, unsigned char, unsigned int, const unsigned char *)}? but argument is of type ?int (*)(void *, u8, u8, u8, size_t, const u8 *) {aka int (*)(void *, unsigned char, unsigned char, unsigned char, unsigned int, const unsigned char *)}?
+ int read_vpd(struct vpd_cache *cache,

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#11 lore raw

PATCH v3 6/6 board: ge: Store bootcount in EEPROM on PPD and Bx50v3

In reply to Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

u-boot's ext3/4 write/modify functionality sometimes corrupts filesystem in the case if it requires recovery (e.g. after unexpected shutdown) and we want to avoid the only filesystem modification we have - bootcounter writing. So, bootcounter will be stored in the EEPROM where VPD is stored.

---
 board/ge/common/vpd_reader.c | 20 ++++++++++++++++++++
 configs/ge_bx50v3_defconfig  | 18 ++++++++++++++----
 configs/mx53ppd_defconfig    | 19 +++++++++++++++----
 include/configs/ge_bx50v3.h  |  6 +++---
 include/configs/mx53ppd.h    |  6 +++---
 5 files changed, 55 insertions(+), 14 deletions(-)
board/ge/common/vpd_reader.c +20 -0
index 12410d9..14507b4 100644
@@ -5,6 +5,7 @@
 
 #include "vpd_reader.h"
 
+#include <linux/bug.h>
 #include <i2c.h>
 #include <linux/bch.h>
 #include <stdlib.h>
@@ -200,7 +201,26 @@ int read_vpd(struct vpd_cache *cache,
 	     int (*process_block)(struct vpd_cache *, u8 id, u8 version,
 				  u8 type, size_t size, u8 const *data))
 {
+#if defined(CONFIG_BOOTCOUNT_I2C) &&					\
+	(CONFIG_SYS_BOOTCOUNT_I2C_BUS == CONFIG_SYS_VPD_EEPROM_I2C_BUS) && \
+	((CONFIG_SYS_BOOTCOUNT_I2C_ADDR & 0xf0) == CONFIG_SYS_VPD_EEPROM_I2C_ADDR)
+	/*
+	 * bootcount is expected to reside at the end of EEPROM
+	 *
+	 * check is hard-wired to the logic of the 24C08 EEPROM 256-bytes
+	 * page-wise (4 pages) i2c_address/bootcount_address structure combined
+	 * with raw I2C access, so the I2C address and offset are combined into:
+	 *
+	 *     i2c_addr = (device_i2c_addr | page),
+	 *     offset = (offset - (page * 256)))
+	 */
+	BUILD_BUG_ON((CONFIG_SYS_BOOTCOUNT_I2C_ADDR & 0x0f) * 256 +
+		     CONFIG_SYS_BOOTCOUNT_ADDR + CONFIG_BOOTCOUNT_I2C_LEN !=
+		     CONFIG_SYS_VPD_EEPROM_SIZE);
+	static const size_t size = CONFIG_SYS_BOOTCOUNT_ADDR;
+#else
 	static const size_t size = CONFIG_SYS_VPD_EEPROM_SIZE;
+#endif
 
 	int res;
 	u8 *data;
configs/ge_bx50v3_defconfig +14 -4
index 8426279..6a052e0 100644
@@ -17,6 +17,20 @@ CONFIG_SYS_VPD_EEPROM_I2C_ADDR=0x50
 CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN=1
 CONFIG_SYS_VPD_EEPROM_SIZE=1024
 
+CONFIG_BOOTCOUNT_I2C=y
+CONFIG_BOOTCOUNT_LIMIT=y
+CONFIG_SYS_BOOTCOUNT_I2C_BUS=4
+# bootcount is stored in VPD EEPROM
+# we are using generic driver instead of EEPROM one (while should use it)
+# at the end of VPD EEPROM: (SYS_VPD_EEPROM_SIZE - CONFIG_BOOTCOUNT_I2C_LEN)
+# so, the address is 0x3FE:
+#     ("VPD EEPROM chip address" | "3rd 256-byte page"):
+CONFIG_SYS_BOOTCOUNT_I2C_ADDR=0x53
+#     (+ "offset in the page"):
+CONFIG_SYS_BOOTCOUNT_ADDR=0xfe
+CONFIG_BOOTCOUNT_I2C_LEN=2
+CONFIG_BOOTCOUNT_ALEN=1
+
 CONFIG_SUPPORT_RAW_INITRD=y
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_LAST_STAGE_INIT=y
@@ -36,11 +50,7 @@ CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_FS_GENERIC=y
 CONFIG_DOS_PARTITION=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
-CONFIG_BOOTCOUNT_LIMIT=y
-CONFIG_BOOTCOUNT_EXT=y
 CONFIG_BOOTCOUNT_BOOTLIMIT=10
-CONFIG_SYS_BOOTCOUNT_EXT_DEVPART="1:5"
-CONFIG_SYS_BOOTCOUNT_ADDR=0x7000A000
 CONFIG_FSL_ESDHC=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_STMICRO=y
configs/mx53ppd_defconfig +15 -4
index 2953e01..3470faa 100644
@@ -19,6 +19,21 @@ CONFIG_SYS_VPD_EEPROM_I2C_ADDR=0x50
 CONFIG_SYS_VPD_EEPROM_I2C_ADDR_LEN=1
 CONFIG_SYS_VPD_EEPROM_SIZE=1024
 
+CONFIG_BOOTCOUNT_LIMIT=y
+CONFIG_BOOTCOUNT_I2C=y
+# using bus where monitor's EEPROM is connected to
+CONFIG_SYS_BOOTCOUNT_I2C_BUS=2
+# bootcount is stored in Monitor's VPD EEPROM
+# we are using generic driver instead of EEPROM one (while should use it)
+# at the end of VPD EEPROM: (SYS_VPD_EEPROM_SIZE - CONFIG_BOOTCOUNT_I2C_LEN)
+# so, the address is 0x3FE:
+#     ("VPD EEPROM chip address" | "3rd 256-byte page"):
+CONFIG_SYS_BOOTCOUNT_I2C_ADDR=0x53
+#     (+ "offset in the page"):
+CONFIG_SYS_BOOTCOUNT_ADDR=0xfe
+CONFIG_BOOTCOUNT_I2C_LEN=2
+CONFIG_BOOTCOUNT_ALEN=1
+
 CONFIG_SUPPORT_RAW_INITRD=y
 CONFIG_MISC_INIT_R=y
 CONFIG_HUSH_PARSER=y
@@ -35,11 +50,7 @@ CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_FAT=y
 CONFIG_CMD_FS_GENERIC=y
 CONFIG_ENV_IS_IN_MMC=y
-CONFIG_BOOTCOUNT_LIMIT=y
-CONFIG_BOOTCOUNT_EXT=y
 CONFIG_BOOTCOUNT_BOOTLIMIT=10
-CONFIG_SYS_BOOTCOUNT_EXT_DEVPART="0:5"
-CONFIG_SYS_BOOTCOUNT_ADDR=0x7000A000
 CONFIG_FSL_ESDHC=y
 CONFIG_MII=y
 CONFIG_RTC_S35392A=y
include/configs/ge_bx50v3.h +3 -3
index fad840b..7212e7c 100644
@@ -114,9 +114,9 @@
 		"setenv stdout vga; " \
 		"echo \"\n\n\n\n    \" $msg; " \
 		"setenv stdout serial; " \
-		"mw.b 0x7000A000 0xbc; " \
-		"mw.b 0x7000A001 0x00; " \
-		"ext4write ${dev} ${devnum}:5 0x7000A000 /boot/failures 2\0" \
+		"i2c dev 4; " \
+		"i2c mw.b 53 fe bc; " \
+		"i2c mw.b 53 ff 00; \0" \
 	"altbootcmd=" \
 		"run doquiet; " \
 		"setenv partnum 1; run hasfirstboot || setenv partnum 2; " \
include/configs/mx53ppd.h +3 -3
index bbd4cd7..1ecf604 100644
@@ -126,9 +126,9 @@
 		"setenv stdout vga; " \
 		"echo \"\n\n\n\n    \" $msg; " \
 		"setenv stdout serial; " \
-		"mw.b 0x7000A000 0xbc; " \
-		"mw.b 0x7000A001 0x00; " \
-		"ext4write ${dev} ${devnum}:5 0x7000A000 /boot/failures 2\0" \
+		"i2c dev 2; " \
+		"i2c mw.b 53 fe bc; " \
+		"i2c mw.b 53 ff 00; \0" \
 	"altbootcmd=" \
 		"run doquiet; " \
 		"setenv partnum 1; run hasfirstboot || setenv partnum 2; " \
Signature · 1 line
1.8.3.1
#12 lore raw

PATCH v3 6/6 board: ge: Store bootcount in EEPROM on PPD and Bx50v3

Quoted 8 lines · Fabien Lahoudere

From: Denis Zalevskiy <[email protected]>

u-boot's ext3/4 write/modify functionality sometimes corrupts filesystem in the case if it requires recovery (e.g. after unexpected shutdown) and we want to avoid the only filesystem modification we have - bootcounter writing. So, bootcounter will be stored in the EEPROM where VPD is stored.

Quoted 4 lines

---

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================