[PATCH v4 1/6] xilinx: common: refactor FRU handling support
Jae Hyun Yoo
quic_jaehyoo at quicinc.com
Thu Sep 22 08:39:31 CEST 2022
Hello Michal,
On 9/21/2022 6:40 AM, Michal Simek wrote:
>
>
> 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.
The 'fru board_gen' command will be replaced with 'fru generate -b'
command if you apply all patches in this series.
fru generate -b <addr> <manufacturer> <board name> <serial number>
<part number> <file id> [custom ...] - Generate FRU
format with board info area filled based on
parameters. <addr> is pointing to place where FRU is
generated.
It works well using the new command like below. (I changed the last
argument to AA fro your test example because field info length should be
longer than 2)
=> fru generate -b 100000 Xilinx test 112545 12551 AA
=> md 100000
00100000: 01000001 fe000000 00000501 58c60000 ...............X
00100010: 6e696c69 6574c478 31c67473 34353231 ilinx.test.11254
00100020: 3231c535 c2313535 00c14141 74000000 5.12551.AA.....t
00100030: 00000000 00000000 00000000 00000000 ................
00100040: 00000000 00000000 00000000 00000000 ................
00100050: 00000000 00000000 00000000 00000000 ................
00100060: 00000000 00000000 00000000 00000000 ................
00100070: 00000000 00000000 00000000 00000000 ................
00100080: 00000000 00000000 00000000 00000000 ................
00100090: 00000000 00000000 00000000 00000000 ................
001000a0: 00000000 00000000 00000000 00000000 ................
001000b0: 00000000 00000000 00000000 00000000 ................
001000c0: 00000000 00000000 00000000 00000000 ................
001000d0: 00000000 00000000 00000000 00000000 ................
001000e0: 00000000 00000000 00000000 00000000 ................
001000f0: 00000000 00000000 00000000 00000000 ................
=> fru capture 100000
=> 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:40
Time in Minutes from 0:00hrs 1/1/96: 0
Manufacturer Name: Xilinx
Product Name: test
Serial Number: 112545
Part Number: 12551
File ID: AA
*****PRODUCT INFO*****
Version:0
Product Area Length:0
*****MULTIRECORDS*****
=>
I changed the previous hardcoded File ID setting as configurable so the
last argument 'AA' is applied to the File ID field.
I agree with your comment on the 4th patch in this series that we need
to keep the old command for a while before deprecating the command to
support all previous user custom scripts. I'll add the 'fru board_gen'
command back in the next version.
>>
>> 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.
Intention of this series is to move the 'fru' command into common place
so I removed the Xilinx dependent custom field, 'revision'. The revision
field can be added as one of generic custom info fields flexibly using
the new command. I also modified 'board/xilinx/common/board.c' file to
make it parse the custom revision field in a generic way so please
check the code.
>> + " 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?
As I described above, the new command can add multiple custom fields
flexibly by taking multiple arguments so it's the reason of this change.
fru generate -b <addr> <manufacturer> <board name> <serial number>
<part number> <file id> [custom ...] - Generate FRU
format with board info area filled based on
parameters. <addr> is pointing to place where FRU is
generated.
>> "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.
FRU_MAX_NO_OF_MAC_ADDR is Xilinx dependent customization so I moved it
to 'board/xilinx/common/board.c'.
FRU_BOARD_MAX_LEN means maximum length of info field in FRU so it should
be used for the product info so I changed it to FRU_INFO_FIELD_LEN_MAX
which is more generic macro name. This change is also a part of this
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.
I just moved __packed to the end of struct definition so it doesn't
make any functional change but it improves browsability of this code
since it's easier to search 'struct fru_board_info_header' instead of
'struct __packed fru_board_info_header'.
>> +
>> +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.
Oh, you are right. I'll remove this prototype definition from this
header and will make the function as a static in fru_ops.c.
>> +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.
Okay. I'll split this patch as you suggested.
>> {
>> struct fru_common_hdr *header = (structfru_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 charsis 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 .
No, it's not a debug printing but a printing of custom field data in
hex dump because the field can be any types other than string type.
>> + }
>> +
>> + 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.
No, it's also an intentional printing out in hex dump format to support
it in generic way. For an example, if we add a MAX address as a multi
record, it should be printed out in hex dump instead of a string format.
>> }
>> 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.
Okay. I'll split out this patch to couple of patches in the next
version.
Thank you,
Jae
More information about the U-Boot
mailing list