Hi Heinrich,

On Mon, Jul 05, 2021 at 12:25:47PM +0200, Heinrich Schuchardt wrote:

On 7/5/21 10:49 AM, Ilias Apalodimas wrote:

...

SMBIOS entries for manufacturer and board can't be empty, as the spec explicitly requires a value. Instead of passing "Unknown" and "Unknown product" for the manufacturer and product name if the .dts options are missing, try to use CONFIG_SYS_VENDOR and CONFIG_SYS_BOARD respectively. If those are not found either warn the user at runtime and use "Unknown" for both entries.

Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org

These defaults don't make any sense to me:

Hardkernel Odroid C2 CONFIG_SYS_VENDOR="amlogic" CONFIG_SYS_BOARD="p200"

Orange Pi PC CONFIG_SYS_BOARD="sunxi" CONFIG_SYS_CONFIG_NAME="sun8i"

Solidrun MacchiatoBIN CONFIG_SYS_VENDOR="Marvell" CONFIG_SYS_BOARD="mvebu_armada-8k"

None of these CONFIG_SYS_VENDOR and CONFIG_SYS_CONFIG_NAME values match the manufacturer nor the board name.

The point here is to provide a viable fallback in case the .dts options are missing and define the semantics we expect from U-Boot. I do think Simon did the right thing by deleting the specific SMBIOS Kconfig options for these values. We should have a common place keeping those instead of scattering around config options. If the .config values don't make sense then the SMBIOS wont either. Perhaps we should just fix the .config options properly?

If you want a default for the board name, use the 'model' property of the devicetree.

That would be require a different patch. Right now the code *already* uses 'product=' from the device tree. If everyone is fine I can send a different patch, using 'model=' instead of that. I don't think parsing multiple .dts values makes sense here.

Cheers /Ilias

Hardkernel Odroid C2: model = "Hardkernel ODROID-C2"

Orange Pi PC model = "Xunlong Orange Pi PC"

Solidrun MacchiatoBIN: model = "MACCHIATOBin-8040"

Best regards

Heinrich

...

---

lib/smbios.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c index b52e125eeb14..d1997ce70d19 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -79,8 +79,10 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str) int i = 1; char *p = ctx->eos;

• if (!*str)

• if (!*str) { str = "Unknown";

• log_warning("Empty string found. Please fix your DTS and/or CONFIG_SYS_* options");
  

• }

  for (;;) { if (!*p) {

@@ -259,10 +261,10 @@ static int smbios_write_type1(ulong *current, int handle, smbios_set_eos(ctx, t->eos); t->manufacturer = smbios_add_prop(ctx, "manufacturer"); if (!t->manufacturer)

• t->manufacturer = smbios_add_string(ctx, "Unknown");
  

• t->manufacturer = smbios_add_string(ctx, CONFIG_SYS_VENDOR);
  

  t->product_name = smbios_add_prop(ctx, "product"); if (!t->product_name)

• t->product_name = smbios_add_string(ctx, "Unknown Product");
  

• t->product_name = smbios_add_string(ctx, CONFIG_SYS_BOARD);
  

  t->version = smbios_add_prop_si(ctx, "version", SYSINFO_ID_SMBIOS_SYSTEM_VERSION); if (serial_str) {

@@ -293,10 +295,10 @@ static int smbios_write_type2(ulong *current, int handle, smbios_set_eos(ctx, t->eos); t->manufacturer = smbios_add_prop(ctx, "manufacturer"); if (!t->manufacturer)

• t->manufacturer = smbios_add_string(ctx, "Unknown");
  

• t->manufacturer = smbios_add_string(ctx, CONFIG_SYS_VENDOR);
  

  t->product_name = smbios_add_prop(ctx, "product"); if (!t->product_name)

• t->product_name = smbios_add_string(ctx, "Unknown Product");
  

• t->product_name = smbios_add_string(ctx, CONFIG_SYS_BOARD);
  

  t->version = smbios_add_prop_si(ctx, "version", SYSINFO_ID_SMBIOS_BASEBOARD_VERSION); t->asset_tag_number = smbios_add_prop(ctx, "asset-tag");

@@ -322,7 +324,7 @@ static int smbios_write_type3(ulong *current, int handle, smbios_set_eos(ctx, t->eos); t->manufacturer = smbios_add_prop(ctx, "manufacturer"); if (!t->manufacturer)

• t->manufacturer = smbios_add_string(ctx, "Unknown");
  

• t->manufacturer = smbios_add_string(ctx, CONFIG_SYS_VENDOR);
  

  t->chassis_type = SMBIOS_ENCLOSURE_DESKTOP; t->bootup_state = SMBIOS_STATE_SAFE; t->power_supply_state = SMBIOS_STATE_SAFE;