[PATCH 1/1] tools: binman: etype: Allow to replace 'NAME' in node names

[Paul HENRYS]

Hi Simon,

On 15/02/2025 12:59, Simon Glass wrote:
> Hi Paul,
>
> On Thu, 21 Nov 2024 at 03:53, Paul HENRYS
> <paul.henrys_ext at softathome.com> wrote:
>> Hi Simon,
>>
>> On 20/11/2024 14:35, Simon Glass wrote:
>>> This Mail comes from Outside of SoftAtHome: Do not answer, click links or open attachments unless you recognize the sender and know the content is safe.
>>>
>>> Hi Paul,
>>>
>>> On Wed, 20 Nov 2024 at 03:40, Paul HENRYS
>>> <paul.henrys_ext at softathome.com> wrote:
>>>> This change allows to replace both 'SEQ' and 'NAME' keywords by respectively a
>>>> sequence number and the name of the FDT to provide more flexibility in the node
>>>> name for the device trees included in the FIT.
>>> This seems OK to me, but it would help to understand the motivation
>>> better. Can you expand this a bit?
>> The rational behind this change is because some projects uses some kind
>> of board data to look for the right config in a FIT image in U-Boot. The
>> default sequence number (config-1, config-2...) does not make it easy to
>> retrieve the right config in such a case. For instance, OpenWrt uses the
>> script "scripts/mkits.sh" to create an ITS passed to mkimage, where the
>> config node names can be customized when supporting multiple configs and
>> typically uses the device tree name appended to "config-".
>> The idea is thus to provide the flexibility in binman to use the
>> traditional scheme with the sequence numbers (config-1, config-2...) or
>> use NAME instead (e.g. config-myboard1, config-myboard2...) or a
>> combination of both.
> Sorry, I dropped this for ages.
>
> You should use the compatible string, not the node name. What kind of
> board is this?

No worries but you actually reviewed this change and it was integrated 
in the master branch in the commit a4345b193479.

I agree that the compatibility property can be used and that might be a 
better practice but U-Boot current implementation also uses the 
configuration node names to select the configuration. Hereafter is my 
understanding of the implementation.

The configuration name (spec) in the FIT image can be passed to the 
bootm command, typically with such a command: bootm 0x1000000#config-1.

do_bootm() -> bootm_run_states() -> bootm_find_os() -> boot_get_kernel() 
-> genimg_get_kernel_addr_fit() -> fit_parse_conf()

The above sequence retrieves the configuration name passed through the 
bootm arguments and stored in fit_uname_config.

Later on, fit_image_load() is called in boot_get_kernel() and the 
previously configuration name stored in fit_uname_config can be used to 
retrieve the images to load. There are actually different solutions to 
retrieve the configuration and the images to be loaded:
   * Using the compatibility information if provided and when 
CONFIG_FIT_BEST_MATCH is enabled. This happens when calling 
fit_conf_find_compat() if fit_uname_config is NULL
   * Using the configuration node name when calling fit_conf_get_node() 
if no configuration could be previously found

So basically, U-Boot current implementation allows to use both 
compatibility property and the config node name from the FIT but so far 
only a sequence number could be set (config-1, config-2...). The 
proposed changes allow to also set a name based on the device tree names.

>
>>>> Signed-off-by: Paul HENRYS <paul.henrys_ext at softathome.com>
>>>> ---
>>>>    tools/binman/etype/fit.py | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
>>>> index e0c1ac08d8..b9ebc3afd0 100644
>>>> --- a/tools/binman/etype/fit.py
>>>> +++ b/tools/binman/etype/fit.py
>>>> @@ -732,6 +732,7 @@ class Entry_fit(Entry_section):
>>>>                    # Generate nodes for each FDT
>>>>                    for seq, fdt_fname in enumerate(self._fdts):
>>>>                        node_name = node.name[1:].replace('SEQ', str(seq + 1))
>>>> +                    node_name = node_name.replace('NAME', fdt_fname)
>>>>                        if self._fdt_dir:
>>>>                            fname = os.path.join(self._fdt_dir, fdt_fname + '.dtb')
>>>>                        else:
>>>> --
>>>> 2.43.0
>>> Please update the documentation (near the top of this file) and
>>> regenerate entries.rst
>>>
>>> It also needs a test so that coverage passes (binman test -T)...
>> Ok. I am going to update the documentation and add a test.
>>> [..]
>>>
>>> Regards,
>>> Simon
>> Regards,
>> Paul
> Regards,
> Simon
Regards,
Paul