[U-Boot] [PATCH 1/3] cmd: fru: Add support for FRU commands

[Michal Simek]

On 22. 10. 19 16:43, Michael Walle wrote:
> Am 2019-10-22 15:45, schrieb Michal Simek:
>> On 22. 10. 19 15:09, Michael Walle wrote:
>>> Am 2019-10-14 15:29, schrieb Michal Simek:
>>>> From: Siva Durga Prasad Paladugu <siva.durga.paladugu at xilinx.com>
>>>>
>>>> This patch adds support for fru commands "fru capture" and "fru
>>>> display".
>>>> The fru capture parses the FRU table present at an address and stores
>>>> in a
>>>> structure for later use. The fru display prints the content of captured
>>>> structured in a readable format.
>>>>
>>>> As of now, it supports only common header and board area of FRU.
>>>> Also, it
>>>> supports only English language code and ASCII8 format.
>>>>
>>>> fru_data variable is placed to data section because fru parser can be
>>>> called very early before bss is initialized. And also information
>>>> needs to
>>>> be shared that's why it is exported via header.
>>>
>>> Wouldn't it make more sense to have a level of indirection so other
>>> "fru" formats might be supported as well. As far as I can see, only
>>> "your" type of FRU data is supported by this command and there is now
>>> way to extend it.
>>
>> Current code is just showing board information which is what it is in
>> FRU spec and it is common for all. There is no any OEM specific field
>> now.
> 
> I didn't meant the (I guess it is the IPMI) FRU spec. But there are also
> other specifications like the PICMG Embedded EEPROM [1]. What I wanted
> to say is, that if there will be a new "fru" command, it might make
> sense to have it more generic. Otherwise, we have the fru command which
> can only display IPMI FRU spec compliant stuff and in the future, maybe
> a "picmg" command, which can only display PICMG EEPROM content.

Yes it is IPMI FRU. I am not aware about PICMG but will read for sure.

FRU have a lot of options for OEMs to add their fields and defined them.
And I expect in future this should be handled. For Xilinx case there is
a consideration to use additional custom Mfg fields and also OEM record
types in multirecord area.

And yes different command should be used for picmg based on my quick scan.
The reason for FRU is that FMC cards used on Xilinx devices are using
FRU for identification.

> 
>>> Also why do the user have to manually do a "fru capture"? The use case
>>> is to display any FRU data, correct? So from a users perspective a "fru
>>> display" be sufficient to display the data.
>>
>> The code was written for using fru board detection code where capture
>> part can be performed without displaying results. Simply because you
>> need to find where you are running to be able to know which device to
>> use for showing results.
> 
> What does a "fru capture" do besides maybe printing an error. Briefly
> looking at the code, I didn't find anything else. (It sets the fru_data
> stuff which is used in "fru display" of course).

ok. NP to put them together.

> 
>> It means internally it needs to be divided.
>> That's going to second point if make sense to have two separate commands
>> or just one.
>> And it should be ok to have only one command till there is a need to do
>> it differently.
> 
> There might be more commands, but IMHO it should be sufficient to run
> "fru display" to print the data. If I read the code correctly, you have
> to do a "fru capture; fru display" at the moment; btw in the "fru
> display" error it reads "please run fru parse", I guess that is a typo.

I have fixed this in v2.
Will wait for Simon to suggest a way for tests and then will send v2.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs