[PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing

[Simon Glass]

Hi Ilias,

On Wed, 13 Dec 2023 at 14:43, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
> [...]
>
> >
> > Can we add a Kconfig to enable this?
>
> I don't see the point, and I am not the only one.

The information is not curated and isn't accurate. E.g. it can end up
putting the vendor, device and version all into a single field,
instead of using the fields correctly.

>
> > > + * @size: str buffer length
>
> [...]
>
> > > +       int cnt = 0;
> > > +       char *token;
> > > +
> > > +       memset(str, 0, size);
> >
> > *str = '\0' should be enough, assuming size is not 0
>
> I generally prefer initializing all memory, it's not that big of a burden here.

OK, not a big deal, just odd. Also it is a character, so '\0' is better.

>
> [...]
>
> > > -               str = ofnode_read_string(ctx->node, prop);
> > > +               const char *str = NULL;
> > > +               char str_dt[128] = { 0 };
> > > +               /*
> > > +                * If the node is not valid fallback and try the entire DT
> >
> > s/and/then/ ?
> >
>
> Sure
>
> > > +                * so we can at least fill in manufacturer and board type
> > > +                */
> > > +               if (ofnode_valid(ctx->node)) {
> > > +                       str = ofnode_read_string(ctx->node, prop);
> > > +               } else {
> > > +                       const struct map_sysinfo *nprop;
> > > +
> > > +                       nprop = convert_sysinfo_to_dt(prop);
> > > +                       get_str_from_dt(nprop, str_dt, sizeof(str_dt));
> > > +                       str = (const char *)str_dt;
> >
> > can't you drop the case?
>
> What case?

Sorry, I meant 'cast'.

Regards,
Simon