Hi Paul,
On Mon, 17 Feb 2025 at 04:47, Paul HENRYS
<paul.henrys_...@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_...@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_...@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_...@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
