Re: [PATCH v5 1/4] efi_loader: get version information from device tree

[Heinrich Schuchardt]

Am 28. April 2023 01:35:04 MESZ schrieb Simon Glass <sjg at chromium.org>:
>Hi Masahisa,
>
>On Mon, 10 Apr 2023 at 03:07, Masahisa Kojima
><masahisa.kojima at linaro.org> wrote:
>>
>> Current FMP->GetImageInfo() always return 0 for the firmware
>> version, user can not identify which firmware version is currently
>> running through the EFI interface.
>>
>> This commit gets the version information from device tree,
>> then fills the firmware version, lowest supported version
>> in FMP->GetImageInfo().
>>
>> Now FMP->GetImageInfo() and ESRT have the meaningful version number.
>>
>> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
>> ---
>> Changes in v5:
>> - newly implement a device tree based versioning
>>
>>  .../firmware/firmware-version.txt             | 25 ++++++++
>>  lib/efi_loader/efi_firmware.c                 | 63 +++++++++++++++++--
>>  2 files changed, 84 insertions(+), 4 deletions(-)
>>  create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt
>>
>> diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt b/doc/device-tree-bindings/firmware/firmware-version.txt
>> new file mode 100644
>> index 0000000000..6112de4a1d
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/firmware/firmware-version.txt
>> @@ -0,0 +1,25 @@
>> +firmware-version bindings
>> +-------------------------------
>> +
>> +Required properties:
>> +- image-type-id                        : guid for image blob type
>> +- image-index                  : image index
>> +- fw-version                   : firmware version
>> +- lowest-supported-version     : lowest supported version
>> +
>> +Example:
>> +
>> +       firmware-version {
>> +               image1 {
>> +                       image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
>
>Nit: please use lower-case hex and add a decoder to uuid.c so we can
>look it up when debugging.

The GUIDs are board specific. No, we should not clutter uuid.c with strings for dozens of boards. Our development aim is to keep U-Boot small and these GUIDs are not printed anywhere.

Instead we should define a string per firmware image. We already have a field for it:

fw_array[i].fw_name

Best regards

Heinrich

>
>> +                       image-index = <1>;
>> +                       fw-version = <5>;
>> +                       lowest-supported-version = <3>;
>> +               };
>> +               image2 {
>> +                       image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0";
>> +                       image-index = <2>;
>> +                       fw-version = <10>;
>> +                       lowest-supported-version = <7>;
>> +               };
>> +       };
>[..]
>
>Regards,
>Simon