From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ABAA9C07E99 for ; Mon, 5 Jul 2021 10:54:35 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A7DBE61222 for ; Mon, 5 Jul 2021 10:54:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A7DBE61222 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id AC7AE82BC7; Mon, 5 Jul 2021 12:54:32 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="mhRa9jEv"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 72E0182BD5; Mon, 5 Jul 2021 12:54:30 +0200 (CEST) Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 6DB2780F47 for ; Mon, 5 Jul 2021 12:54:27 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-wr1-x431.google.com with SMTP id a13so21520056wrf.10 for ; Mon, 05 Jul 2021 03:54:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=9icixZ2UhXFLdeGPnLjXBSph17D+0eF/gMtHl1F1etA=; b=mhRa9jEvkl5WLncCKmV0yVoP7eBXVFQrLPg+iQS2mxwRiZmGW7caublToex69/+t4x O7WYZmegwXTl0IXZ5DPotq/CG7isnduRpm/NDf6XGjiRDmOCdStUu+ITlaUKRI5whI9Y p+y3KiCYoCRoAguZTHXnaQTBp/sy3tNq/XkETjE7bGdn/VLIAPSEYaC0RBz8aRwDpApE GzHIv+i1ADDknjuTGfje4W4Ep3uc5iNZvMEfI2Fs4AH4aHq5zoEwtkXquNA4508TTAYJ DB8yQkznjXtDTL8G1Ab6g12D9XDG0EOMXfYnHJsVqbloMj3RmxRek6mG8xn0PzDSIX15 fJbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=9icixZ2UhXFLdeGPnLjXBSph17D+0eF/gMtHl1F1etA=; b=MjOR077vz7S83pM7pesZ4w46byCsQmvhkF+x2RtBG21yU+TuxdFJ70qCUwT/ulY1dX 7vsWMfrHRc63gGRhHCVfB/Tnf0Frg4/P1XEgmxmQTjVheWQnXUn4g4P2IEw7Fm/W4LC7 fK5qDEwntXiloq9l24sDVvhzEO4nT2sJNkoggXqriyy28bsl5/AgEwIVx2/n+Hobnbye EyiPYQEiOZaSNMrDfJZBQ41VJ+HN4fSFHkVH4upW97fIdkGk07gPRO0hsstYLkJDz+Pr cwW/0DhcG01wzzO1cAY31Bh6mtS4yp4B5bGQjKKBZVEKeoXhaj6kJTETtdSKhwZwROIy qDFg== X-Gm-Message-State: AOAM532At1t7hmcUXGnhF4n1daIIQ1sBeCt958N1Th7gkKW5DuJWmovf sJi8nmNhzQLE9/9rneefx9cxIg== X-Google-Smtp-Source: ABdhPJwrCZfLUyy6QV8YkqjaLxIraM81Uc/tMu9u3pO/cUi+XKq/6G6iO5cXE7LeNgXTci1vwFnd2Q== X-Received: by 2002:adf:e983:: with SMTP id h3mr14891886wrm.366.1625482466972; Mon, 05 Jul 2021 03:54:26 -0700 (PDT) Received: from enceladus (ppp-94-66-242-227.home.otenet.gr. [94.66.242.227]) by smtp.gmail.com with ESMTPSA id e23sm7882505wme.31.2021.07.05.03.54.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Jul 2021 03:54:26 -0700 (PDT) Date: Mon, 5 Jul 2021 13:54:24 +0300 From: Ilias Apalodimas To: Heinrich Schuchardt Cc: da@libre.computer, Simon Glass , Bin Meng , Christian Gmeiner , u-boot@lists.denx.de Subject: Re: [PATCH] smbios: Try CONFIG_SYS_ options before using "Unknow" as a value Message-ID: References: <20210705084915.32008-1-ilias.apalodimas@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean 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 > > 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; > > >