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

[Simon Glass]

Hi Paul,

On Mon, 17 Feb 2025 at 04:47, Paul HENRYS
<paul.henrys_ext at softathome.com> wrote:
>
> 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.

As I think you have said here, the idea with 'bootm' is that it can
automatically select the right configuration, if CONFIG_FIT_BEST_MATCH
is enabled.

Thanks for the details above, it is a helpful summary.


>
> >
> >>>> 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