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

[Tom Rini]

On Wed, Dec 06, 2023 at 06:03:14PM +0200, Ilias Apalodimas wrote:
> Hi Simon,
> 
> On Sat, 2 Dec 2023 at 20:28, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Wed, 29 Nov 2023 at 23:50, Ilias Apalodimas
> > <ilias.apalodimas at linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > [...]
> > >
> > >> > Changes since v1:
> > >> > - Tokenize the DT node entry and use the appropriate value instead of
> > >> >   the entire string
> > >> > - Removed Peters tested/reviewed-by tags due to the above
> > >> >  lib/smbios.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++---
> > >> >  1 file changed, 90 insertions(+), 4 deletions(-)
> > >> >
> > >>
> > >> Can this be put behind a Kconfig? It adds quite a bit of code which
> > >> punishes those boards which do the right thing.
> > >
> > >
> > > Sure but OTOH the code increase should be really minimal. But I don't mind I can add a Kconfig
> > >
> > >>
> > >> > +
> > >> > +       dt_str = ofnode_read_string(ofnode_root(), nprop->dt_str);
> > >>
> > >> Could this use ofnode_read_string_index() ?
> > >
> > >
> > > Maybe, I'll have a look and change it if that works
> 
> Unless I am missing something this doesn't work.
> This is designed to return a string index from a DT property that's defined as
> foo_property = "value1", "value2" isn't it?
> 
> The code above is trying to read an existing property (e.g compatible)
> and get the string after the comma delimiter.
> Perhaps I should add this in drivers/core/ofnode.c instead?
> 
> > >
> > > [...]
> > >
> > >>
> > >> Any chance of a test for this code?
> > >
> > >
> > > Sure, but any suggestions on where to add the test?
> > > SMBIOS tables are populated on OS booting, do we have a test somewhere that boots an OS?
> >
> > They are written on startup, right? They should certainly be in place
> > before U-Boot enters the command line.
> 
> Not always.  I am not sure if x86 does that, but on the rest of the
> architectures, they are only initialized when the efi smbios code
> runs. Wasn't this something you were trying to change?

One of those things I keep repeating is that we don't know for sure what
the right values here are until we load the DT the OS _will_ use. It's
quite valid to start U-Boot and pass it a generic "good enough" DT at
run time until U-Boot can see (GPIOs, EEPROM, whatever) what it's on and
what the real DT to load before passing to the OS is. Since U-Boot
doesn't need SMBIOS tables itself, we should make these "late" not
"early".

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231209/c400a9c7/attachment.sig>