Re: [U-Boot] [PATCH V3 5/6] board: ge: Move VPD reading to the vpd_reader

On 17/10/18 10:33, Fabien Lahoudere wrote:

From: Denis Zalevskiy denis.zalevskiy@ge.com

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.

Signed-off-by: Denis Zalevskiy denis.zalevskiy@ge.com Signed-off-by: Fabien Lahoudere fabien.lahoudere@collabora.com

---

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(-)

diff --git a/board/ge/bx50v3/bx50v3.c b/board/ge/bx50v3/bx50v3.c index ca500f5..78e7ee6 100644 --- a/board/ge/bx50v3/bx50v3.c +++ b/board/ge/bx50v3/bx50v3.c @@ -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);

diff --git a/board/ge/common/vpd_reader.c b/board/ge/common/vpd_reader.c index c471583..12410d9 100644 --- a/board/ge/common/vpd_reader.c +++ b/board/ge/common/vpd_reader.c @@ -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;

+} diff --git a/board/ge/common/vpd_reader.h b/board/ge/common/vpd_reader.h index e60acf3..3045b7e 100644 --- a/board/ge/common/vpd_reader.h +++ b/board/ge/common/vpd_reader.h @@ -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));
  

diff --git a/board/ge/mx53ppd/mx53ppd.c b/board/ge/mx53ppd/mx53ppd.c index 3fa53ff..f0c8abf 100644 --- a/board/ge/mx53ppd/mx53ppd.c +++ b/board/ge/mx53ppd/mx53ppd.c @@ -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);

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