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

[Heinrich Schuchardt]

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.

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