[PATCH v2 12/12] smbios: Allow a few values to come from sysinfo

[Simon Glass]

Hi Bin,

On Wed, 20 Jan 2021 at 23:55, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > While static configuration is useful it cannot cover every case. Sometimes
> > board revisions are encoded in resistor straps and must be read at
> > runtime.
> >
> > The easiest way to provide this information is via sysinfo, since the
> > board can then provide a driver to read whatever is needed.
> >
> > Add some standard sysinfo options for this, and use them to obtain the
> > required information.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  include/sysinfo.h | 11 +++++++++++
> >  lib/smbios.c      | 32 +++++++++++++++++++++++++++++---
> >  2 files changed, 40 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/sysinfo.h b/include/sysinfo.h
> > index 6e021253524..743f3554659 100644
> > --- a/include/sysinfo.h
> > +++ b/include/sysinfo.h
> > @@ -31,6 +31,17 @@
> >   * to read the serial number.
> >   */
> >
> > +/** enum sysinfo_id - Standard IDs defined by U-Boot */
> > +enum sysinfo_id {
> > +       SYSINFO_ID_NONE,
> > +
> > +       SYSINFO_ID_SMBIOS_SYSTEM_VERSION,
> > +       SYSINFO_ID_SMBIOS_BASEBOARD_VERSION,
> > +
> > +       /* First value available for downstream/board used */
> > +       SYSINFO_ID_USER = 0x1000,
> > +};
> > +
> >  struct sysinfo_ops {
> >         /**
> >          * detect() - Run the hardware info detection procedure for this
> > diff --git a/lib/smbios.c b/lib/smbios.c
> > index d46569b09f4..9bdde0b953f 100644
> > --- a/lib/smbios.c
> > +++ b/lib/smbios.c
> > @@ -10,6 +10,7 @@
> >  #include <env.h>
> >  #include <mapmem.h>
> >  #include <smbios.h>
> > +#include <sysinfo.h>
> >  #include <tables_csum.h>
> >  #include <version.h>
> >  #ifdef CONFIG_CPU
> > @@ -106,15 +107,26 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
> >  }
> >
> >  /**
> > - * smbios_add_prop() - Add a property from the device tree
> > + * smbios_add_prop_si() - Add a property from the devicetree or sysinfo
> > + *
> > + * Sysinfo is used if available, with a fallback to devicetree
> >   *
> >   * @start:     string area start address
> >   * @node:      node containing the information to write (ofnode_null() if none)
> >   * @prop:      property to write
> >   * @return 0 if not found, else SMBIOS string number (1 or more)
> >   */
> > -static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
> > +static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
> > +                             int sysinfo_id)
> >  {
> > +       if (sysinfo_id && ctx->dev) {
> > +               char val[80];
>
> Is 80 the limitation defined by sysinfo drivers? Can this be a macro?

I found this limitation in the SMBIOS spec.

Yes I will make it a macro.

>
> > +               int ret;
> > +
> > +               ret = sysinfo_get_str(ctx->dev, sysinfo_id, sizeof(val), val);
> > +               if (!ret)
> > +                       return smbios_add_string(ctx, val);
> > +       }
> >         if (IS_ENABLED(CONFIG_OF_CONTROL)) {
> >                 const char *str;
> >
> > @@ -126,6 +138,17 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
> >         return 0;
> >  }
> >
> > +/**
> > + * smbios_add_prop() - Add a property from the devicetree
> > + *
> > + * @prop:      property to write
> > + * @return 0 if not found, else SMBIOS string number (1 or more)
> > + */
> > +static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
> > +{
> > +       return smbios_add_prop_si(ctx, prop, SYSINFO_ID_NONE);
> > +}
> > +
> >  static void set_eos(struct smbios_ctx *ctx, char *eos)
> >  {
> >         ctx->eos = eos;
> > @@ -239,7 +262,8 @@ static int smbios_write_type1(ulong *current, int handle,
> >         set_eos(ctx, t->eos);
> >         t->manufacturer = smbios_add_prop(ctx, "manufacturer");
> >         t->product_name = smbios_add_prop(ctx, "product");
> > -       t->version = smbios_add_prop(ctx, "version");
> > +       t->version = smbios_add_prop_si(ctx, "version",
> > +                                       SYSINFO_ID_SMBIOS_SYSTEM_VERSION);
> >         if (serial_str) {
> >                 t->serial_number = smbios_add_string(ctx, serial_str);
> >                 strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
> > @@ -268,6 +292,8 @@ static int smbios_write_type2(ulong *current, int handle,
> >         set_eos(ctx, t->eos);
> >         t->manufacturer = smbios_add_prop(ctx, "manufacturer");
> >         t->product_name = smbios_add_prop(ctx, "product");
> > +       t->version = smbios_add_prop_si(ctx, "version",
> > +                                       SYSINFO_ID_SMBIOS_BASEBOARD_VERSION);
> >         t->asset_tag_number = smbios_add_prop(ctx, "asset-tag");
> >         t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
> >         t->board_type = SMBIOS_BOARD_MOTHERBOARD;
> > --
>
> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>

Regards,
Simon