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 <[email protected]>
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 <[email protected]>
Signed-off-by: Jae Hyun Yoo <[email protected]>
---
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