[PATCH v1 1/2] cmd: fru: move FRU handling support to common region
Jae Hyun Yoo
quic_jaehyoo at quicinc.com
Mon Aug 1 17:08:56 CEST 2022
On 8/1/2022 5:34 AM, Heinrich Schuchardt wrote:
> On 7/29/22 23:54, 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 so that it can be enabled by
>> CONFIG_CMD_FRU.
>>
>> To provide manufacturer specific custom board info field parsing,
>> it defines 'fru_parse_board_custom' as a weak function so that it can
>> be replaced with the board specific implementation. In the same way,
>> OEM Multirecord type (0xc0 - 0xff) parsing logic can be replaced with
>> a board specific 'fru_parse_multirec' implementation.
>>
>> Signed-off-by: Graeme Gregory <quic_ggregory at quicinc.com>
>> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo at quicinc.com>
>
> Your patchset lacks documentation.
>
> Please, add a man-page in doc/usage/cmd/fru.rst.
Okay. I'll add a man-page in v2.
>> ---
>> Changes from RFC:
>> * Made FRU typecode string table as a generic and sharable table.
>> (Michal)
>> * Made OEM multirecord parsing call happen only on customizable type
>> IDs.
>> (Michal)
>> * Added manufacturer custom board info fields parsing flow. (Michal)
>>
>> board/xilinx/Kconfig | 8 --
>> board/xilinx/common/Makefile | 3 -
>> board/xilinx/common/board.c | 108 +++++++++++++++++--
>> cmd/Kconfig | 8 ++
>> cmd/Makefile | 1 +
>> {board/xilinx/common => cmd}/fru.c | 5 +-
>> common/Makefile | 2 +
>> {board/xilinx/common => common}/fru_ops.c | 126 ++++++++++++++--------
>
> Why should this live in common/. lib/ would make more sense.
Right, lib/ would be a right place. Will fix it in v2.
[...]
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index a8260aa170d0..644c907bf83a 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -1053,6 +1053,14 @@ config CMD_FPGAD
>> fpga_get_reg() function. This functions similarly to the 'md'
>> command.
>>
>> +config CMD_FRU
>> + bool "FRU information for product"
>> + help
>> + This option enables FRU commands to capture and display FRU
>> + information present in the device. The FRU Information is used
>> + to primarily to provide "inventory" information about the boards
>> + that the FRU Information Device is located on.
>
> Don't assume that a user knows what that FRU abbreviation stands for.
> Should it relate to a Field Replaceable Unit, please, write once the
> full text.
Right, it relates to a Field Replaceable Unit. I'll add the full text in
the next spin.
> Which revision of the IPMI Storage FRU Layout does it support?
It supports v1.0.
https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/ipmi-platform-mgt-fru-info-storage-def-v1-0-rev-1-3-spec-update.pdf
Will add this link into commit message.
> Where is the unit test for the command?
I'll add a unit test in v2.
Best Regards,
Jae
More information about the U-Boot
mailing list