[PATCH 1/2] boot: continue in fit_find_config_node()

[Simon Glass]

Hi Heinrich,

On Sun, 9 Feb 2025 at 10:33, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 09.02.25 17:24, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sun, 9 Feb 2025 at 09:10, Heinrich Schuchardt
> > <heinrich.schuchardt at canonical.com> wrote:
> >>
> >> On 09.02.25 17:02, Simon Glass wrote:
> >>> Hi Heinrich,
> >>>
> >>> On Sun, 9 Feb 2025 at 08:53, Heinrich Schuchardt
> >>> <heinrich.schuchardt at canonical.com> wrote:
> >>>>
> >>>> On 09.02.25 15:29, Simon Glass wrote:
> >>>>> Hi Heinrich,
> >>>>>
> >>>>> On Sun, 9 Feb 2025 at 04:52, Heinrich Schuchardt
> >>>>> <heinrich.schuchardt at canonical.com> wrote:
> >>>>>>
> >>>>>> If a single configuration node lacks a description, this does not rule out
> >>>>>> that another node with a description matches. Anyway we have the default
> >>>>>> configuration as a fallback.
> >>>>>>
> >>>>>> So continue if a description is missing.
> >>>>>>
> >>>>>
> >>>>> That field is mandatory, so we should really fail if it is missing.
> >>>>>
> >>>>> What problem are you trying to solve here?
> >>>>
> >>>> Sure the field is needed to make a configuration selectable.
> >>>
> >>> It's actually normally informational. Have you looked a
> >>
> >> If the field is informational, we should not worry if it is missing.
> >>
> >>> CONFIG_FIT_BEST_MATCH? That is how this is supposed to be done. But
> >>> with starfive_visionfive2 I see:
> >>>
> >>> # CONFIG_FIT_BEST_MATCH is not set
> >>
> >> How would CONFIG_FIT_BEST_MATCH match strings in the EEPROM to a FIT
> >> configuration?
> >
> > It uses the compatible strings from the running devicetree. If you
> > know that it is wrong, or too generic, you could enhance
> > fit_conf_find_compat() to take a compatible string-list instead of an
> > FDT blob, then add a SYSID for reading that from your board's sysinfo
> > driver.
> >
> > How does it work at present?
>
> board_fit_config_name_match() in board/starfive/visionfive2/spl.c checks
> the configuration name against the EEPROM strings. This is invoked via
> fit_conf_get_node(). Several other boards override the same weak function.

So we have fit_find_config_node() and fit_conf_get_node() which calls
it. There is similar logic in fit_image_load() and the 'best match'
logic is not present in SPL.

It looks like we should try to unify things a bit more.

Regards,
Simon


>
> Best regards
>
> Heinrich
>
> >
> >>
> >>>
> >>>>
> >>>> But as long as we can find a good configuration we should boot instead
> >>>> of leaving the user with a bricked device.
> >>>
> >>> The more likely impact is that people won't bother adding the field on RISC-V.
> >>
> >> The current runtime check is inconsistent: You will get an error if the
> >> first config lacks a description but may not get an error if the last
> >> one lacks it. And you are potentially bricking bootable devices.
> >>
> >> The right place to check is mkimage. Here you can bail out if a string
> >> is missing without bricking devices.
> >
> > OK, I can see that the current thing is inconsistent, so I don't mind.
> >
> > Regards,
> > Simon
>