Hi Tom,

On Wed, 13 Dec 2023 at 13:19, Tom Rini <tr...@konsulko.com> wrote:
>
> On Wed, Dec 13, 2023 at 12:50:03PM -0700, Simon Glass wrote:
> > Hi Ilias,
> >
> > On Mon, 11 Dec 2023 at 00:49, Ilias Apalodimas
> > <ilias.apalodi...@linaro.org> wrote:
> > >
> > > Hi Tom,
> > >
> > > On Sat, 9 Dec 2023 at 20:49, Tom Rini <tr...@konsulko.com> wrote:
> > > >
> > > > 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 <s...@chromium.org> wrote:
> > > > > >
> > > > > > Hi Ilias,
> > > > > >
> > > > > > On Wed, 29 Nov 2023 at 23:50, Ilias Apalodimas
> > > > > > <ilias.apalodi...@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".
> > >
> > > Fair enough, we can defer the init and testing of those late, just
> > > before we are about to boot. But this is irrelevant to what this patch
> > > does, can we get the fallback mechanism in first, assuming everyone is
> > > ok with the patch now?
> > >
> >
> > I would like this behind a Kconfig. Making it the default means people
> > are going to start relying on (in user space) and then discover later
> > that it is wrong.
>
> What do you mean wrong, exactly?

"raspberrypi" instead of "Raspberry Pi", for example, or "friendlyarm"
instead of "FriendlyElec"

I just wonder what this information is actually used for. If it is
just for fun, that is fine, but I believe some tools do use dmidecode,
at which point it needs to be accurate and should not change later,
e.g. when someone decides to actually add the info.

Regards,
Simon

Reply via email to