[PATCH v4 1/6] xilinx: common: refactor FRU handling support

Michal Simek michal.simek at amd.com
Wed Sep 21 15:40:23 CEST 2022



On 8/25/22 18:42, Jae Hyun Yoo wrote:
> Refactor FRU handling support to remove Xilinx customization dependency.
> With this change, single or multiple custom board fields and
> multi-records can be added dynamically using linked list so that each
> board support can utilize them in their own custom way.
> 
> It's a preparation change for moving the FRU command support to common
> region.
> 
> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo at quicinc.com>
> ---
> Changes from v3:
>   * None.
> 
> Changes from v2:
>   * None.
> 
> Changes from v1:
>   * Newly added in v2.
> 
>   board/xilinx/common/board.c   |  63 ++++++++--
>   board/xilinx/common/fru.c     |  12 +-
>   board/xilinx/common/fru.h     |  76 +++++++-----
>   board/xilinx/common/fru_ops.c | 227 ++++++++++++++++++++++++----------
>   4 files changed, 263 insertions(+), 115 deletions(-)

When this patch is applied fru board_gen is not working properly.
Before:
ZynqMP> fru board_gen 100000 Xilinx test 112545 12551 A
ZynqMP> md 100000
00100000: 01000001 fe000000 00000701 58c60000  ...............X
00100010: 6e696c69 6574c478 31c67473 34353231  ilinx.test.11254
00100020: 3231c535 d0313535 6f422d55 6720746f  5.12551.U-Boot g
00100030: 72656e65 726f7461 00c141c1 e7000000  enerator.A......
00100040: 03000000 04000000 38000000 ad0f2b63  ...........8c+..
00100050: 03000000 11000000 00000000 42205444  ............DT B
00100060: 20626f6c 61657243 6e6f6974 00000000  lob Creation....
00100070: 01000000 67616d69 00007365 01000000  ....images......
00100080: 2d746466 74737973 742d6d65 0000706f  fdt-system-top..
00100090: 03000000 04000000 54000000 d07f0000  ...........T....
001000a0: 03000000 04000000 48000000 00000000  ...........H....
001000b0: 03000000 04000000 00000000 004b4d53  ............SMK.
001000c0: 03000000 08000000 11000000 74616c66  ............flat
001000d0: 0074645f 03000000 06000000 16000000  _dt.............
001000e0: 366d7261 00000034 03000000 05000000  arm64...........
001000f0: 1b000000 656e6f6e 00000000 01000000  ....none........
ZynqMP> fru capture 100000
ZynqMP> fru display
*****COMMON HEADER*****
Version:1
*** No Internal Area ***
*** No Chassis Info Area ***
Board Area Offset:8
*** No Product Info Area ***
*** No MultiRecord Area ***
*****BOARD INFO*****
Version:1
Board Area Length:56
Time in Minutes from 0:00hrs 1/1/96: 0
  Manufacturer Name: Xilinx
  Product Name: test
  Serial No: 112545
  Part Number: 12551
  File ID: U-Boot generator
ZynqMP>

when patch is applied.

ZynqMP> fru board_gen 100000 Xilinx test 112545 12551 A
ZynqMP> md 100000
00100000: 01000001 fe000000 00000401 74c40000  ...............t
00100010: c6747365 35323131 31c53534 31353532  est.112545.12551
00100020: 00c141c1 f9000000 00000000 00000000  .A..............
00100030: 00000000 00000000 01000000 00000000  ................
00100040: 03000000 04000000 38000000 e3102b63  ...........8c+..
00100050: 03000000 11000000 00000000 42205444  ............DT B
00100060: 20626f6c 61657243 6e6f6974 00000000  lob Creation....
00100070: 01000000 67616d69 00007365 01000000  ....images......
00100080: 2d746466 74737973 742d6d65 0000706f  fdt-system-top..
00100090: 03000000 04000000 54000000 d07f0000  ...........T....
001000a0: 03000000 04000000 48000000 00000000  ...........H....
001000b0: 03000000 04000000 00000000 004b4d53  ............SMK.
001000c0: 03000000 08000000 11000000 74616c66  ............flat
001000d0: 0074645f 03000000 06000000 16000000  _dt.............
001000e0: 366d7261 00000034 03000000 05000000  arm64...........
001000f0: 1b000000 656e6f6e 00000000 01000000  ....none........
ZynqMP> fru capture 100000
Board area require minimum 5 fields
ZynqMP>

it is clear that manufacturer parameter is ignored.




> 
> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
> index 9b4aded466ab..e979f0176273 100644
> --- a/board/xilinx/common/board.c
> +++ b/board/xilinx/common/board.c
> @@ -88,6 +88,9 @@ int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
>   #define EEPROM_HDR_NO_OF_MAC_ADDR	4
>   #define EEPROM_HDR_ETH_ALEN		ETH_ALEN
>   #define EEPROM_HDR_UUID_LEN		16
> +#define EEPROM_MULTIREC_TYPE_XILINX_OEM	0xD2
> +#define EEPROM_MULTIREC_MAC_OFFSET	4
> +#define EEPROM_MULTIREC_DUT_MACID	0x31
>   
>   struct xilinx_board_description {
>   	u32 header;
> @@ -116,6 +119,19 @@ struct xilinx_legacy_format {
>   	char unused3[29]; /* 0xe3 */
>   };
>   
> +struct xilinx_multirec_mac {
> +	u8 xlnx_iana_id[3];
> +	u8 ver;
> +	u8 macid[EEPROM_HDR_NO_OF_MAC_ADDR][ETH_ALEN];
> +};
> +
> +enum xilinx_board_custom_field {
> +	brd_custom_field_rev = 0,
> +	brd_custom_field_pcie,
> +	brd_custom_field_uuid,
> +	brd_custom_field_max
> +};
> +
>   static void xilinx_eeprom_legacy_cleanup(char *eeprom, int size)
>   {
>   	int i;
> @@ -200,9 +216,14 @@ static bool xilinx_detect_legacy(u8 *buffer)
>   static int xilinx_read_eeprom_fru(struct udevice *dev, char *name,
>   				  struct xilinx_board_description *desc)
>   {
> +	u8 parsed_macid[EEPROM_HDR_NO_OF_MAC_ADDR][ETH_ALEN] = { 0 };
> +	struct fru_custom_info custom_info[brd_custom_field_max] = { 0 };
> +	struct fru_custom_field_node *ci_node;
> +	struct fru_multirec_node *mr_node;
> +	const struct fru_table *fru_data;
>   	int i, ret, eeprom_size;
>   	u8 *fru_content;
> -	u8 id = 0;
> +	u8 id = 0, field = 0;
>   
>   	/* FIXME this is shortcut - if eeprom type is wrong it will fail */
>   	eeprom_size = i2c_eeprom_size(dev);
> @@ -237,30 +258,56 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name,
>   		goto end;
>   	}
>   
> +	fru_data = fru_get_fru_data();
> +
> +	list_for_each_entry(ci_node, &fru_data->brd.custom_fields, list) {
> +		custom_info[field].type_len = ci_node->info.type_len;
> +		memcpy(custom_info[field].data, ci_node->info.data,
> +		       ci_node->info.type_len & FRU_TYPELEN_LEN_MASK);
> +		if (++field < brd_custom_field_max)
> +			break;
> +	}
> +
> +	list_for_each_entry(mr_node, &fru_data->multi_recs, list) {
> +		struct fru_multirec_hdr *hdr = &mr_node->info.hdr;
> +
> +		if (hdr->rec_type == EEPROM_MULTIREC_TYPE_XILINX_OEM) {
> +			struct xilinx_multirec_mac *mac;
> +
> +			mac = (struct xilinx_multirec_mac *)mr_node->info.data;
> +			if (mac->ver == EEPROM_MULTIREC_DUT_MACID) {
> +				int mac_len = hdr->len -
> +					      EEPROM_MULTIREC_MAC_OFFSET;
> +
> +				memcpy(parsed_macid, mac->macid, mac_len);
> +			}
> +		}
> +	}
> +
>   	/* It is clear that FRU was captured and structures were filled */
> -	strlcpy(desc->manufacturer, (char *)fru_data.brd.manufacturer_name,
> +	strlcpy(desc->manufacturer, (char *)fru_data->brd.manufacturer_name,
>   		sizeof(desc->manufacturer));
> -	strlcpy(desc->uuid, (char *)fru_data.brd.uuid,
> +	strlcpy(desc->uuid, (char *)custom_info[brd_custom_field_uuid].data,
>   		sizeof(desc->uuid));
> -	strlcpy(desc->name, (char *)fru_data.brd.product_name,
> +	strlcpy(desc->name, (char *)fru_data->brd.product_name,
>   		sizeof(desc->name));
>   	for (i = 0; i < sizeof(desc->name); i++) {
>   		if (desc->name[i] == ' ')
>   			desc->name[i] = '\0';
>   	}
> -	strlcpy(desc->revision, (char *)fru_data.brd.rev,
> +	strlcpy(desc->revision, (char *)custom_info[brd_custom_field_rev].data,
>   		sizeof(desc->revision));
>   	for (i = 0; i < sizeof(desc->revision); i++) {
>   		if (desc->revision[i] == ' ')
>   			desc->revision[i] = '\0';
>   	}
> -	strlcpy(desc->serial, (char *)fru_data.brd.serial_number,
> +	strlcpy(desc->serial, (char *)fru_data->brd.serial_number,
>   		sizeof(desc->serial));
>   
>   	while (id < EEPROM_HDR_NO_OF_MAC_ADDR) {
> -		if (is_valid_ethaddr((const u8 *)fru_data.mac.macid[id]))
> +		if (is_valid_ethaddr((const u8 *)parsed_macid[id]))
>   			memcpy(&desc->mac_addr[id],
> -			       (char *)fru_data.mac.macid[id], ETH_ALEN);
> +			       (char *)parsed_macid[id], ETH_ALEN);
>   		id++;
>   	}
>   
> diff --git a/board/xilinx/common/fru.c b/board/xilinx/common/fru.c
> index f6ca46c3cecc..0d72911df04d 100644
> --- a/board/xilinx/common/fru.c
> +++ b/board/xilinx/common/fru.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /*
>    * (C) Copyright 2019 - 2020 Xilinx, Inc.
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
>    */
>   
>   #include <common.h>
> @@ -43,7 +44,7 @@ static int do_fru_generate(struct cmd_tbl *cmdtp, int flag, int argc,
>   
>   	addr = hextoul(argv[2], NULL);
>   
> -	return fru_generate(addr, argv[3], argv[4], argv[5], argv[6], argv[7]);
> +	return fru_generate(addr, argc - 3, argv + 3);
>   }
>   
>   static struct cmd_tbl cmd_fru_sub[] = {
> @@ -78,14 +79,15 @@ static char fru_help_text[] =
>   	"fru display - Displays content of FRU table that was captured using\n"
>   	"              fru capture command\n"
>   	"fru board_gen <addr> <manufacturer> <board name> <serial number>\n"
> -	"              <part number> <revision> - Generate FRU format with\n"
> -	"              board info area filled based on parameters. <addr> is\n"
> -	"              pointing to place where FRU is generated.\n"
> +	"              <part number> <file id> [custom ...] - Generate FRU\n"

That command was created for generating fragment with board revision. Here you 
are changing it to file id without any reason why.


> +	"              format with board info area filled based on\n"
> +	"                parameters. <addr> is pointing to place where FRU is\n"
> +	"                generated.\n"
>   	;
>   #endif
>   
>   U_BOOT_CMD(
> -	fru, 8, 1, do_fru,
> +	fru, CONFIG_SYS_MAXARGS, 1, do_fru,

What's the intention to change this? We have this macro setup to 64. Do you want 
to support to write all fields?




>   	"FRU table info",
>   	fru_help_text
>   )
> diff --git a/board/xilinx/common/fru.h b/board/xilinx/common/fru.h
> index 59f6b722cf12..41655864dbf5 100644
> --- a/board/xilinx/common/fru.h
> +++ b/board/xilinx/common/fru.h
> @@ -2,11 +2,13 @@
>   /*
>    * (C) Copyright 2019 Xilinx, Inc.
>    * Siva Durga Prasad Paladugu <siva.durga.paladugu at xilinx.com>
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
>    */
>   
>   #ifndef __FRU_H
>   #define __FRU_H
> -#include <net.h>
> +
> +#include <linux/list.h>
>   
>   struct fru_common_hdr {
>   	u8 version;
> @@ -17,21 +19,31 @@ struct fru_common_hdr {
>   	u8 off_multirec;
>   	u8 pad;
>   	u8 crc;
> -};
> +} __packed;
>   
> -#define FRU_BOARD_MAX_LEN	32
> -#define FRU_MAX_NO_OF_MAC_ADDR	4
> +#define FRU_INFO_FIELD_LEN_MAX		32

This is pretty much just macro name change in a lot of places. It should be done 
in separate patch.


> +#define FRU_MULTIREC_DATA_LEN_MAX	255
>   
> -struct __packed fru_board_info_header {
> +struct fru_board_info_header {
>   	u8 ver;
>   	u8 len;
>   	u8 lang_code;
>   	u8 time[3];
> -};
> +} __packed;
>   
> -struct __packed fru_board_info_member {
> +struct fru_board_info_member {
>   	u8 type_len;
>   	u8 *name;
> +} __packed;

Also pretty much self contained change without proper description.

> +
> +struct fru_custom_info {
> +	u8 type_len;
> +	u8 data[FRU_INFO_FIELD_LEN_MAX];
> +} __packed;
> +
> +struct fru_custom_field_node {
> +	struct list_head list;
> +	struct fru_custom_info info;
>   };
>   
>   struct fru_board_data {
> @@ -40,22 +52,16 @@ struct fru_board_data {
>   	u8 lang_code;
>   	u8 time[3];
>   	u8 manufacturer_type_len;
> -	u8 manufacturer_name[FRU_BOARD_MAX_LEN];
> +	u8 manufacturer_name[FRU_INFO_FIELD_LEN_MAX];
>   	u8 product_name_type_len;
> -	u8 product_name[FRU_BOARD_MAX_LEN];
> +	u8 product_name[FRU_INFO_FIELD_LEN_MAX];
>   	u8 serial_number_type_len;
> -	u8 serial_number[FRU_BOARD_MAX_LEN];
> +	u8 serial_number[FRU_INFO_FIELD_LEN_MAX];
>   	u8 part_number_type_len;
> -	u8 part_number[FRU_BOARD_MAX_LEN];
> +	u8 part_number[FRU_INFO_FIELD_LEN_MAX];
>   	u8 file_id_type_len;
> -	u8 file_id[FRU_BOARD_MAX_LEN];
> -	/* Xilinx custom fields */
> -	u8 rev_type_len;
> -	u8 rev[FRU_BOARD_MAX_LEN];
> -	u8 pcie_type_len;
> -	u8 pcie[FRU_BOARD_MAX_LEN];
> -	u8 uuid_type_len;
> -	u8 uuid[FRU_BOARD_MAX_LEN];
> +	u8 file_id[FRU_INFO_FIELD_LEN_MAX];
> +	struct list_head custom_fields;
>   };
>   
>   struct fru_multirec_hdr {
> @@ -64,32 +70,35 @@ struct fru_multirec_hdr {
>   	u8 len;
>   	u8 csum;
>   	u8 hdr_csum;
> -};
> +} __packed;
>   
> -struct fru_multirec_mac {
> -	u8 xlnx_iana_id[3];
> -	u8 ver;
> -	u8 macid[FRU_MAX_NO_OF_MAC_ADDR][ETH_ALEN];
> +struct fru_multirec_info {
> +	struct fru_multirec_hdr hdr;
> +	u8 data[FRU_MULTIREC_DATA_LEN_MAX];
> +} __packed;
> +
> +struct fru_multirec_node {
> +	struct list_head list;
> +	struct fru_multirec_info info;
>   };
>   
>   struct fru_table {
>   	struct fru_common_hdr hdr;
>   	struct fru_board_data brd;
> -	struct fru_multirec_mac mac;
> +	struct list_head multi_recs;
>   	bool captured;
>   };
>   
> -#define FRU_TYPELEN_CODE_MASK	0xC0
> -#define FRU_TYPELEN_LEN_MASK	0x3F
> +#define FRU_TYPELEN_CODE_MASK		0xC0
> +#define FRU_TYPELEN_LEN_MASK		0x3F
>   #define FRU_COMMON_HDR_VER_MASK		0xF
>   #define FRU_COMMON_HDR_LEN_MULTIPLIER	8
>   #define FRU_LANG_CODE_ENGLISH		0
>   #define FRU_LANG_CODE_ENGLISH_1		25
>   #define FRU_TYPELEN_EOF			0xC1
> -#define FRU_MULTIREC_TYPE_OEM		0xD2
> -#define FRU_MULTIREC_MAC_OFFSET		4
>   #define FRU_LAST_REC			BIT(7)
> -#define FRU_DUT_MACID			0x31
> +#define FRU_MULTIREC_TYPE_OEM_START	0xC0
> +#define FRU_MULTIREC_TYPE_OEM_END	0xFF
>   
>   /* This should be minimum of fields */
>   #define FRU_BOARD_AREA_TOTAL_FIELDS	5
> @@ -99,10 +108,9 @@ struct fru_table {
>   
>   int fru_display(int verbose);
>   int fru_capture(unsigned long addr);
> -int fru_generate(unsigned long addr, char *manufacturer, char *board_name,
> -		 char *serial_no, char *part_no, char *revision);
> +int fru_generate(unsigned long addr, int argc, char *const argv[]);
>   u8 fru_checksum(u8 *addr, u8 len);
> -
> -extern struct fru_table fru_data;
> +int fru_check_type_len(u8 type_len, u8 language, u8 *type);

This is used just inside fru_ops.c that means that change in this patch is 
unrelated to change you need to do here. Please move it to patch which really 
needs this.

> +const struct fru_table *fru_get_fru_data(void);
>   
>   #endif /* FRU_H */
> diff --git a/board/xilinx/common/fru_ops.c b/board/xilinx/common/fru_ops.c
> index 49846ae3d660..8291e347716b 100644
> --- a/board/xilinx/common/fru_ops.c
> +++ b/board/xilinx/common/fru_ops.c
> @@ -1,21 +1,24 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /*
>    * (C) Copyright 2019 - 2020 Xilinx, Inc.
> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
>    */
>   
>   #include <common.h>
> -#include <cpu_func.h>
>   #include <env.h>
>   #include <fdtdec.h>
> +#include <hexdump.h>
>   #include <log.h>
>   #include <malloc.h>
> -#include <net.h>
>   #include <asm/io.h>
> -#include <asm/arch/hardware.h>
> +#include <linux/compat.h>
>   
>   #include "fru.h"
>   
> -struct fru_table fru_data __section(".data");
> +struct fru_table fru_data __section(".data") = {
> +	.brd.custom_fields = LIST_HEAD_INIT(fru_data.brd.custom_fields),
> +	.multi_recs = LIST_HEAD_INIT(fru_data.multi_recs),
> +};
>   
>   static u16 fru_cal_area_len(u8 len)
>   {
> @@ -57,7 +60,7 @@ u8 fru_checksum(u8 *addr, u8 len)
>   	return checksum;
>   }
>   
> -static int fru_check_type_len(u8 type_len, u8 language, u8 *type)
> +int fru_check_type_len(u8 type_len, u8 language, u8 *type)
>   {
>   	int len;
>   
> @@ -89,8 +92,7 @@ static u8 fru_gen_type_len(u8 *addr, char *name)
>   	return 1 + len;
>   }
>   
> -int fru_generate(unsigned long addr, char *manufacturer, char *board_name,
> -		 char *serial_no, char *part_no, char *revision)
> +int fru_generate(unsigned long addr, int argc, char *const argv[])

This is again the self contained change where you want to support generation for 
more fields in the patch which is doing a lot of other things.

I am fine with changing it but in steps. It means improve fru generation. 
Renaming macros, extraction OEM fields from board are, change multi record 
handling.



>   {
>   	struct fru_common_hdr *header = (struct fru_common_hdr *)addr;
>   	struct fru_board_info_header *board_info;
> @@ -126,18 +128,10 @@ int fru_generate(unsigned long addr, char *manufacturer, char *board_name,
>   	/* Member fields are just after board_info header */
>   	member = (u8 *)board_info + sizeof(*board_info);
>   
> -	len = fru_gen_type_len(member, manufacturer); /* Board Manufacturer */
> -	member += len;
> -	len = fru_gen_type_len(member, board_name); /* Board Product name */
> -	member += len;
> -	len = fru_gen_type_len(member, serial_no); /* Board Serial number */
> -	member += len;
> -	len = fru_gen_type_len(member, part_no); /* Board part number */
> -	member += len;
> -	len = fru_gen_type_len(member, "U-Boot generator"); /* File ID */
> -	member += len;
> -	len = fru_gen_type_len(member, revision); /* Revision */
> -	member += len;
> +	while (--argc > 0 && ++argv) {
> +		len = fru_gen_type_len(member, *argv);
> +		member += len;
> +	}
>   
>   	*member++ = 0xc1; /* Indication of no more fields */
>   
> @@ -168,6 +162,35 @@ int fru_generate(unsigned long addr, char *manufacturer, char *board_name,
>   	return 0;
>   }
>   
> +static void fru_delete_custom_fields(struct list_head *custom_fields)
> +{
> +	struct fru_custom_field_node *node, *next;
> +
> +	list_for_each_entry_safe(node, next, custom_fields, list) {
> +		list_del(&node->list);
> +		kfree(node);
> +	}
> +}
> +
> +static int fru_append_custom_info(unsigned long addr,
> +				  struct list_head *custom_fields)
> +{
> +	struct fru_custom_info *info = (struct fru_custom_info *)addr;
> +	struct fru_custom_field_node *ci_new;
> +
> +	ci_new = kzalloc(sizeof(*ci_new), GFP_KERNEL);
> +	if (!ci_new)
> +		return -ENOMEM;
> +
> +	ci_new->info.type_len = info->type_len;
> +	memcpy(&ci_new->info.data, (void *)info->data,
> +	       ci_new->info.type_len & FRU_TYPELEN_LEN_MASK);
> +
> +	list_add_tail(&ci_new->list, custom_fields);
> +
> +	return 0;
> +}
> +
>   static int fru_parse_board(unsigned long addr)
>   {
>   	u8 i, type;
> @@ -179,9 +202,10 @@ static int fru_parse_board(unsigned long addr)
>   	data = (u8 *)&fru_data.brd.manufacturer_type_len;
>   
>   	/* Record max structure limit not to write data over allocated space */
> -	limit = (u8 *)&fru_data.brd + sizeof(struct fru_board_data);
> +	limit = (u8 *)&fru_data.brd + sizeof(struct fru_board_data) -
> +		sizeof(struct fru_custom_field_node *);
>   
> -	for (i = 0; ; i++, data += FRU_BOARD_MAX_LEN) {
> +	for (i = 0; ; i++, data += FRU_INFO_FIELD_LEN_MAX) {
>   		len = fru_check_type_len(*(u8 *)addr, fru_data.brd.lang_code,
>   					 &type);
>   		/*
> @@ -190,11 +214,14 @@ static int fru_parse_board(unsigned long addr)
>   		if (len == -EINVAL)
>   			break;
>   
> -		/* Stop when amount of chars is more then fields to record */
> -		if (data + len > limit)
> -			break;
> -		/* This record type/len field */
> -		*data++ = *(u8 *)addr;
> +		if (i < FRU_BOARD_AREA_TOTAL_FIELDS) {
> +			/* Stop when length is more then fields to record */
> +			if (data + len > limit)
> +				break;
> +
> +			/* This record type/len field */
> +			*data++ = *(u8 *)addr;
> +		}
>   
>   		/* Add offset to match data */
>   		addr += 1;
> @@ -203,10 +230,23 @@ static int fru_parse_board(unsigned long addr)
>   		if (!len)
>   			continue;
>   
> -		/* Record data field */
> -		memcpy(data, (u8 *)addr, len);
> -		term = data + (u8)len;
> -		*term = 0;
> +		if (i < FRU_BOARD_AREA_TOTAL_FIELDS) {
> +			/* Record data field */
> +			memcpy(data, (u8 *)addr, len);
> +			term = data + (u8)len;
> +			*term = 0;
> +		} else {
> +			struct list_head *custom_fields;
> +
> +			custom_fields = &fru_data.brd.custom_fields;
> +
> +			/* Add a custom info field */
> +			if (fru_append_custom_info(addr - 1, custom_fields)) {
> +				fru_delete_custom_fields(custom_fields);
> +				return -EINVAL;
> +			}
> +		}
> +
>   		addr += len;
>   	}
>   
> @@ -219,34 +259,52 @@ static int fru_parse_board(unsigned long addr)
>   	return 0;
>   }
>   
> +static void fru_delete_multirecs(struct list_head *multi_recs)
> +{
> +	struct fru_multirec_node *node, *next;
> +
> +	list_for_each_entry_safe(node, next, multi_recs, list) {
> +		list_del(&node->list);
> +		kfree(node);
> +	}
> +}
> +
> +static int fru_append_multirec(unsigned long addr,
> +			       struct list_head *multi_recs)
> +{
> +	struct fru_multirec_info *mr_src = (struct fru_multirec_info *)addr;
> +	struct fru_multirec_node *mr_new;
> +
> +	mr_new = kzalloc(sizeof(*mr_new), GFP_KERNEL);
> +	if (!mr_new)
> +		return -ENOMEM;
> +
> +	memcpy(&mr_new->info.hdr, (void *)&mr_src->hdr, sizeof(mr_src->hdr));
> +	memcpy(&mr_new->info.data, (void *)mr_src->data, mr_src->hdr.len);
> +
> +	list_add_tail(&mr_new->list, multi_recs);
> +
> +	return 0;
> +}
> +
>   static int fru_parse_multirec(unsigned long addr)
>   {
> -	struct fru_multirec_hdr mrc;
> -	u8 checksum = 0;
>   	u8 hdr_len = sizeof(struct fru_multirec_hdr);
> -	int mac_len = 0;
> +	struct fru_multirec_hdr *mr_hdr;
>   
> -	debug("%s: multirec addr %lx\n", __func__, addr);
> +	debug("%s: multirec addr %lx\n", __func__, (ulong)addr);
>   
>   	do {
> -		memcpy(&mrc.rec_type, (void *)addr, hdr_len);
> -
> -		checksum = fru_checksum((u8 *)addr, hdr_len);
> -		if (checksum) {
> +		if (fru_checksum((u8 *)addr, hdr_len)) {
>   			debug("%s header CRC error\n", __func__);
>   			return -EINVAL;
>   		}
>   
> -		if (mrc.rec_type == FRU_MULTIREC_TYPE_OEM) {
> -			struct fru_multirec_mac *mac = (void *)addr + hdr_len;
> +		fru_append_multirec(addr, &fru_data.multi_recs);
>   
> -			if (mac->ver == FRU_DUT_MACID) {
> -				mac_len = mrc.len - FRU_MULTIREC_MAC_OFFSET;
> -				memcpy(&fru_data.mac.macid, mac->macid, mac_len);
> -			}
> -		}
> -		addr += mrc.len + hdr_len;
> -	} while (!(mrc.type & FRU_LAST_REC));
> +		mr_hdr = (struct fru_multirec_hdr *)addr;
> +		addr += mr_hdr->len + hdr_len;
> +	} while (!(mr_hdr->type & FRU_LAST_REC));
>   
>   	return 0;
>   }
> @@ -255,7 +313,6 @@ int fru_capture(unsigned long addr)
>   {
>   	struct fru_common_hdr *hdr;
>   	u8 checksum = 0;
> -	unsigned long multirec_addr = addr;
>   
>   	checksum = fru_checksum((u8 *)addr, sizeof(struct fru_common_hdr));
>   	if (checksum) {
> @@ -264,33 +321,28 @@ int fru_capture(unsigned long addr)
>   	}
>   
>   	hdr = (struct fru_common_hdr *)addr;
> +	fru_delete_custom_fields(&fru_data.brd.custom_fields);
> +	fru_delete_multirecs(&fru_data.multi_recs);
>   	memset((void *)&fru_data, 0, sizeof(fru_data));
> -	memcpy((void *)&fru_data, (void *)hdr,
> -	       sizeof(struct fru_common_hdr));
> +	INIT_LIST_HEAD(&fru_data.brd.custom_fields);
> +	INIT_LIST_HEAD(&fru_data.multi_recs);
> +	memcpy((void *)&fru_data, (void *)hdr, sizeof(struct fru_common_hdr));
>   
>   	fru_data.captured = true;
>   
> -	if (hdr->off_board) {
> -		addr += fru_cal_area_len(hdr->off_board);
> -		fru_parse_board(addr);
> -	}
> +	if (hdr->off_board)
> +		fru_parse_board(addr + fru_cal_area_len(hdr->off_board));
>   
> -	env_set_hex("fru_addr", addr);
> +	if (hdr->off_multirec)
> +		fru_parse_multirec(addr + fru_cal_area_len(hdr->off_multirec));
>   
> -	if (hdr->off_multirec) {
> -		multirec_addr += fru_cal_area_len(hdr->off_multirec);
> -		fru_parse_multirec(multirec_addr);
> -	}
> +	env_set_hex("fru_addr", (ulong)addr);
>   
>   	return 0;
>   }
>   
>   static int fru_display_board(struct fru_board_data *brd, int verbose)
>   {
> -	u32 time = 0;
> -	u8 type;
> -	int len;
> -	u8 *data;
>   	static const char * const typecode[] = {
>   		"Binary/Unspecified",
>   		"BCD plus",
> @@ -301,12 +353,15 @@ static int fru_display_board(struct fru_board_data *brd, int verbose)
>   	static const char * const boardinfo[] = {
>   		"Manufacturer Name",
>   		"Product Name",
> -		"Serial No",
> +		"Serial Number",
>   		"Part Number",
>   		"File ID",
> -		/* Xilinx spec */
> -		"Revision Number",
>   	};
> +	struct fru_custom_field_node *node;
> +	u32 time = 0;
> +	u8 *data;
> +	u8 type;
> +	int len;
>   
>   	if (verbose) {
>   		printf("*****BOARD INFO*****\n");
> @@ -358,7 +413,32 @@ static int fru_display_board(struct fru_board_data *brd, int verbose)
>   			debug("Unsupported type %x\n", type);
>   		}
>   
> -		data += FRU_BOARD_MAX_LEN;
> +		data += FRU_INFO_FIELD_LEN_MAX;
> +	}
> +
> +	list_for_each_entry(node, &brd->custom_fields, list) {
> +		printf(" Custom Type/Length: 0x%x\n", node->info.type_len);
> +		print_hex_dump_bytes("  ", DUMP_PREFIX_OFFSET, node->info.data,
> +				     node->info.type_len &
> +				     FRU_TYPELEN_LEN_MASK);


this is pretty much debug information. Bootlog contains a lot of data and won't 
look good. I think this should be moved to verbose or debug.

  File ID: 0x0
  Custom Type/Length: 0xc8
   00000000: 41 00 00 00 00 00 00 00                          A.......
  Custom Type/Length: 0x8
   00000000: 10 ee 00 00 00 00 00 00                          ........
  Custom Type/Length: 0x10
   00000000: 52 64 da 0f 73 44 46 03 ac 1b 1e f6 35 92 3c f1  Rd..sDF.....5.<.
Type ID: 0x2, Type: 0x2 Length: 13
  00000000: 01 f4 01 c2 01 26 02 64 00 00 00 a0 0f           .....&.d.....
Type ID: 0xd2, Type: 0x2 Length: 10
  00000000: da 10 00 31 00 0a 35 10 39 cd                    ...1..5.9.
Type ID: 0xd3, Type: 0x82 Length: 87
  00000000: da 10 00 4d 65 6d 6f 72 79 3a 20 51 53 50 49 3a  ...Memory: QSPI:
  00000010: 35 31 32 4d 62 20 20 00 4d 65 6d 6f 72 79 3a 20  512Mb  .Memory:
  00000020: 65 4d 4d 43 3a 4e 6f 6e 65 20 20 20 00 4d 65 6d  eMMC:None   .Mem
  00000030: 6f 72 79 3a 20 50 53 44 44 52 34 3a 34 47 42 20  ory: PSDDR4:4GB
  00000040: 20 00 4d 65 6d 6f 72 79 3a 20 50 4c 44 44 52 34   .Memory: PLDDR4
  00000050: 3a 4e 6f 6e 65 20 00                             :None .




> +	}
> +
> +	return 0;
> +}
> +
> +static int fru_display_multirec(struct list_head *multi_recs, int verbose)
> +{
> +	struct fru_multirec_node *node;
> +
> +	if (verbose)
> +		printf("*****MULTIRECORDS*****\n");
> +
> +	list_for_each_entry(node, multi_recs, list) {
> +		printf("Type ID: 0x%x, Type: 0x%x Length: %d\n",
> +		       node->info.hdr.rec_type, node->info.hdr.type,
> +		       node->info.hdr.len);
> +		print_hex_dump_bytes(" ", DUMP_PREFIX_OFFSET, node->info.data,
> +				     node->info.hdr.len);

And here again the same.



>   	}
>   
>   	return 0;
> @@ -404,6 +484,8 @@ static void fru_display_common_hdr(struct fru_common_hdr *hdr, int verbose)
>   
>   int fru_display(int verbose)
>   {
> +	int ret;
> +
>   	if (!fru_data.captured) {
>   		printf("FRU data not available please run fru parse\n");
>   		return -EINVAL;
> @@ -411,5 +493,14 @@ int fru_display(int verbose)
>   
>   	fru_display_common_hdr(&fru_data.hdr, verbose);
>   
> -	return fru_display_board(&fru_data.brd, verbose);
> +	ret = fru_display_board(&fru_data.brd, verbose);
> +	if (ret)
> +		return ret;
> +
> +	return fru_display_multirec(&fru_data.multi_recs, verbose);
> +}
> +
> +const struct fru_table *fru_get_fru_data(void)
> +{
> +	return &fru_data;
>   }

Definitely patch is going in the right direction to remove Xilinx custom parts 
but I would like to see this to be split to couple of patches. I don't think 
this is going to be problem and it will be much easier to review.

Thanks,
Michal


More information about the U-Boot mailing list