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@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@quicinc.com Signed-off-by: Jae Hyun Yoo quic_jaehyoo@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-u...
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