Re: [PATCH v2 6/8] armv8: Add generic smbios information into the device tree

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

Am 29. Oktober 2024 16:45:44 MEZ schrieb Simon Glass <sjg@chromium.org>:
>On Mon, 28 Oct 2024 at 21:00, Raymond Mao <raymond.mao@linaro.org> wrote:
>>
>> Hi Simon,
>>
>> On Mon, 28 Oct 2024 at 13:05, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Raymond,
>>>
>>> On Tue, 22 Oct 2024 at 22:07, Raymond Mao <raymond.mao@linaro.org> wrote:
>>> >
>>> > Add common smbios information that can be used by all armv8
>>> > platforms and set it as default for qemu-arm64.
>>> > From now smbios library can load values from here for those fields
>>> > doesn't exist in the sysinfo driver.
>>> >
>>> > To run this with QEMU arm64, please dump the generated DTB
>>> > from QEMU first, merge it with the one we build and then re-run
>>> > QEMU with the merged DTB.
>>> > ```
>>> > qemu-system-arm -machine virt -machine dumpdtb=qemu.dtb
>>> > cat  <(dtc -I dtb qemu.dtb) <(dtc -I dtb ./dts/dt.dtb | \
>>> >   grep -v /dts-v1/) | dtc - -o merged.dtb
>>> > qemu-system-arm -machine virt -nographic -bios u-boot.bin \
>>> >   -dtb merged.dtb
>>>
>>> and please talk to Peter about accepting my patch[1]
>>>
>>>
>>> > ```
>>> > For details please take reference on dt_qemu.rst
>>> >
>>> > Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
>>> > ---
>>> > Changes in v2
>>> > - Initial patch.
>>> >
>>> >  arch/arm/dts/qemu-arm64.dts      |  4 ++
>>> >  arch/arm/dts/smbios_generic.dtsi | 82 ++++++++++++++++++++++++++++++++
>>> >  2 files changed, 86 insertions(+)
>>> >  create mode 100644 arch/arm/dts/smbios_generic.dtsi
>>> >
>>> > diff --git a/arch/arm/dts/qemu-arm64.dts b/arch/arm/dts/qemu-arm64.dts
>>> > index 096b3910728..95fcf53ed74 100644
>>> > --- a/arch/arm/dts/qemu-arm64.dts
>>> > +++ b/arch/arm/dts/qemu-arm64.dts
>>> > @@ -7,5 +7,9 @@
>>> >
>>> >  /dts-v1/;
>>> >
>>> > +#if defined(CONFIG_SYSINFO_SMBIOS) && !defined(QFW_SMBIOS)
>>> > +#include "smbios_generic.dtsi"
>>> > +#endif
>>> > +
>>> >  / {
>>> >  };
>>> > diff --git a/arch/arm/dts/smbios_generic.dtsi b/arch/arm/dts/smbios_generic.dtsi
>>> > new file mode 100644
>>> > index 00000000000..c9f07283403
>>> > --- /dev/null
>>> > +++ b/arch/arm/dts/smbios_generic.dtsi
>>> > @@ -0,0 +1,82 @@
>>> > +// SPDX-License-Identifier: GPL-2.0+
>>> > +/*
>>> > + * Default SMBIOS information for Arm64 platforms
>>> > + *
>>> > + * Copyright (c) 2024 Linaro Limited
>>> > + * Author: Raymond Mao <raymond.mao@linaro.org>
>>> > + */
>>> > +#include <config.h>
>>> > +#include <smbios_def.h>
>>> > +
>>> > +/ {
>>> > +       smbios {
>>> > +               compatible = "u-boot,sysinfo-smbios";
>>> > +
>>> > +               smbios {
>>> > +                       system {
>>> > +                               manufacturer = CONFIG_SYS_VENDOR;
>>> > +                               product = CONFIG_SYS_BOARD;
>>> > +                               version = "Not Specified";
>>> > +                               serial = "Not Specified";
>>>
>>> Does it make sense to add these strings? Perhaps if the property is missing in the DT, it should be missing in the SMBIOS table? Is 'Not Specified' a special string in the SMBIO spec?
>>

The spec says:

"If a string field references no string, a null (0) is placed in  that string field."

Please, remove the 'Not specified' values from the patch and do not provide any property value in the device-tree instead.

Best regards

Heinrich


>>
>> This is not defined in the spec and actually those strings can be anything.
>> As currently this is for qemu_arm64 only and we don't have real vendor-defined values,
>> "Not Specified" is used as a placeholder to show an example for other vendors if they
>> want to add similar properties.
>
>OK I see.
>
>Reviewed-by: Simon Glass <sjg@chromium.org>