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

Jae Hyun Yoo quic_jaehyoo at quicinc.com
Thu Sep 22 18:22:10 CEST 2022


Hi,

On 9/22/2022 12:29 AM, Michal Simek wrote:
> Hi,
> 
> On 9/22/22 08:39, Jae Hyun Yoo wrote:
>> 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.
> 
> I get it what you do. I am just saying that it is so many things in the 
> single patch to review and focus.
> It means one thing is to extract that vendor specific bits.
> And second to extend it to make that generation generic for other fields.
> 
> Again no problem with doing it. I just have a problem that it is in all 
> in one patch. If there is any problem finds in future and you will 
> bisect it you will end up in this patch and you will have hard time to 
> find which exact change does it.

Okay. I'll split it to a couple of patches in the next version.

> 
>>
>>>> +    "              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'.
> 
> no problem - was commenting that 
> s/FRU_BOARD_MAX_LEN/FRU_INFO_FIELD_LEN_MAX/
> 
>>
>> 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.
> 
> All good expect for last sentence. It shouldn't be the part of this 
> patch. It should be done in separate patch and you have also commit 
> message written in this paragraph. And it is easy to review and 
> understand in single patch.

I'll split the macro renaming part as a 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.
>>
>> 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'.
> 
> As above. Separate patch and you have commit message here too.

Will do it too.

> 
>>
>>>> +
>>>> +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.
> 
> But keep it under verbose.
> 
> We are using this code for dtb reselection at boot and don't really want 
> to see this is all the time when I boot our SOC. Take a look how it 
> looks like now when your patch is applied. As I said use verbose 
> parameter and I think we both will be fine with it.
> 
> 
> U-Boot SPL 2022.10-rc5-00262-gd471037d6f89 (Sep 21 2022 - 15:11:03 +0200)
> PMUFW:  v1.1
> Loading new PMUFW cfg obj (32 bytes)
> PMUFW no permission to change config object
> Loading new PMUFW cfg obj (2032 bytes)
> Silicon version:        3
> EL Level:       EL3
> Secure Boot:    not authenticated, not encrypted
> Multiboot:      64
> Trying to boot from SPI
> NOTICE:  BL31: v2.7(release):v2.7.0-415-g8edd190e6416
> NOTICE:  BL31: Built : 10:45:09, Sep 16 2022
> 
> 
> U-Boot 2022.10-rc5-00256-gc829283b8093 (Sep 21 2022 - 15:18:37 +0200)
> 
> CPU:   ZynqMP
> Silicon: v3
> Chip:  xck26
> Detected name: zynqmp-smk-k26-xcl2g-revA-sck-kv-g-revB
> Model: ZynqMP SMK-K26 Rev1/B/A
> Board: Xilinx ZynqMP
> DRAM:  4 GiB
> PMUFW:  v1.1
> Cannot load PMUFW configuration object (1)
> PMUFW returned 0x00000001 status!
> Xilinx I2C FRU format at nvmem0:
>   Manufacturer Name: XILINX
>   Product Name: SMK-K26-XCL2G
>   Serial Number: 50571A21CT4G
>   Part Number: 5057-04
>   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 .
> Xilinx I2C FRU format at nvmem1:
>   Manufacturer Name: XILINX
>   Product Name: SCK-KV-G
>   Serial Number: 50582B112M07
>   Part Number: 5058-02
>   File ID: 0x0
>   Custom Type/Length: 0xc8
>    00000000: 42 00 00 00 00 00 00 00                          B.......
>   Custom Type/Length: 0x8
>    00000000: 10 ee 00 00 00 00 00 00                          ........
>   Custom Type/Length: 0x10
>    00000000: 7f f2 1d 3e 25 3f 4f 72 9d cb 48 8b e1 3c 6a 52  
> ...>%?Or..H..<jR
> Type ID: 0x2, Type: 0x2 Length: 13
>   00000000: 01 b0 04 7e 04 e2 04 64 00 00 00 b8 0b           ...~...d.....
> EL Level:       EL2
> Secure Boot:    not authenticated, not encrypted
> Core:  124 devices, 33 uclasses, devicetree: fit
> NAND:  0 MiB
> MMC:   mmc at ff170000: 1
> Loading Environment from nowhere... OK
> In:    serial
> Out:   serial
> Err:   serial
> Bootmode: QSPI_MODE
> Reset reason:   SOFT
> Net:   PHY reset timed out
> 
> ZYNQ GEM: ff0e0000, mdio bus ff0e0000, phyaddr 1, interface rgmii-id
> eth0: ethernet at ff0e0000

Okay. I'll move this printing under verbose.

>>>>       }
>>>>       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.
> 
> Good.

Thanks for your review. I'll submit a new version soon.

Regards,
Jae


More information about the U-Boot mailing list