[RFC PATCH 1/2] cmd/fru: cmd/fru: move FRU handling support to common region
Jae Hyun Yoo
quic_jaehyoo at quicinc.com
Fri Jul 29 16:38:20 CEST 2022
Hello Michal,
On 7/29/2022 4:13 AM, Michal Simek wrote:
> You should fix subject.
Ah, I'll remove one of 'cmd/fru:' prefix in the title.
> On 7/27/22 01:50, Jae Hyun Yoo wrote:
>> From: Graeme Gregory <quic_ggregory at quicinc.com>
>>
>> The FRU handling was added as a Xilinx board dependent support but it
>> would be useful for other boards too, so this commit moves the FRU
>> handling support to the common region to be enabled by CONFIG_CMD_FRU.
>> Since the Multirecord parsing logic should be implemented on each OEM
>> board specifically, it defines 'fru_parse_multirec' as a weak function
>> so that it can be replaced with the board specific implementation.
>
> Not entirely. Some multirecords are common and described by spec. But
> parising multirecord based on IANA ID is vendor specific.
> It means boards shouldn't replicate code for hearder checking. Only
> handle that IANA specific part.
Right. I'll change the multirecords parsing logic to call the vendor
specific parsing function only when record type is 0xc0 - 0xff. All
other standard type parsing logic and header checking will be placed
in the generic location.
>>
>> Signed-off-by: Graeme Gregory <quic_ggregory at quicinc.com>
>> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo at quicinc.com>
>> ---
>> board/xilinx/Kconfig | 8 ---
>> board/xilinx/common/Makefile | 3 --
>> board/xilinx/common/board.c | 63 +++++++++++++++++++----
>> cmd/Kconfig | 8 +++
>> cmd/Makefile | 1 +
>> {board/xilinx/common => cmd}/fru.c | 3 +-
>> common/Makefile | 2 +
>> {board/xilinx/common => common}/fru_ops.c | 37 ++++++-------
>> {board/xilinx/common => include}/fru.h | 15 +-----
>> 9 files changed, 82 insertions(+), 58 deletions(-)
>> rename {board/xilinx/common => cmd}/fru.c (99%)
>> rename {board/xilinx/common => common}/fru_ops.c (93%)
>> rename {board/xilinx/common => include}/fru.h (85%)
>
> The main reason why I didn't added to generic location was that in board
> field there are xilinx specific custom fields.
> With other vendor this won't work.
> I think this should be solved before this code can be moved to generic
> location.
>
> Also maybe make sense to move and spec that variable creation.
Yes, I realized that the Xilinx specific customization was added into
the standard board info area so actually it breaks the spec.
struct fru_board_data {
[....]
/* 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];
};
I think, this type of customization should be added using multirecords
instead of expanding the common board info structure, and it's the
reason why the FRU spec provides OEM multirecord types (0xc0 - 0xff).
Do you have any plan to replace them with OEM multirecords?
Thanks,
Jae
More information about the U-Boot
mailing list