Re: [PATCH v2 06/10] binman: Add support for pre-patching FDTs in a FIT with a /chosen node

Hi Simon,

On Tue, Aug 4, 2026 at 5:03 PM Simon Glass <[email protected]> wrote:
>
> Hi Alexey,
>
> On 2026-07-31T17:10:44, Alexey Charkov <[email protected]> wrote:
> > binman: Add support for pre-patching FDTs in a FIT with a /chosen node
> >
> > When a generated FIT is used to boot Linux directly, bypassing U-Boot
> > proper (Falcon mode), there is no runtime code to discover the kernel
> > command line and (optional) initrd location and include them in a FDT.
> >
> > To facilitate easier preparation of a ready-to-boot FIT, add support for
> > pre-patching the FDTs in a FIT with a preconfigured /chosen node including
> > the bootargs and initrd location.
> >
> > Signed-off-by: Alexey Charkov <[email protected]>
> >
> > tools/binman/entry.py     |   8 ++++
> >  tools/binman/etype/fit.py | 103 ++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 111 insertions(+)
>
> > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> > @@ -745,6 +779,66 @@ class Entry_fit(Entry_section):
> > +        if initrd_name:
> > +            entry = self._priv_entries.get(initrd_name)
> > +            initrd = entry.GetData(required=False) if entry else None
> > +            size = len(initrd) if initrd else 0
> > +            if size:
>
> If fit,initrd names an image that does not exist in the FIT (e.g. a
> typo - 'randisk' instead of 'ramdisk'), entry is None and the initrd
> fixup is silently skipped. The user ends up with a Falcon-mode image
> that has no initrd, with no hint from binman as to why. Please raise
> here when the referenced image is not present at all, and only fall
> back to the silent skip when the entry exists but has no data (the
> "optional initramfs was not supplied" case the docs mention).

Indeed, let me add an explicit check for that, thanks.

> > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> > @@ -745,6 +779,66 @@ class Entry_fit(Entry_section):
> > +                # Use the device tree's root #address-cells for the width, as
> > +                # fdt_initrd() does (4 bytes per cell)
> > +                cells = 4 * libfdt.check_err(
> > +                    libfdt.fdt_address_cells(fdt._fdt, 0))
> > +                fdt.setprop(chosen, 'linux,initrd-start',
> > +                            start.to_bytes(cells, 'big'))
> > +                fdt.setprop(chosen, 'linux,initrd-end',
> > +                            end.to_bytes(cells, 'big'))
> > +                # Reserve the initramfs region so the kernel does not reuse it
> > +                libfdt.check_err(
> > +                    libfdt.fdt_add_mem_rsv(fdt._fdt, start, size))
>
> Hmm should we add address_cells() and add_mem_rsv() to the fdt
> library? We should not really be going into the private attribute.

Yep. Let me expose it publicly, thanks.

> > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> > @@ -302,6 +302,40 @@ class Entry_fit(Entry_section):
> > +    Patching /chosen into the generated device trees
> > +    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The underline is longer than the heading title - please trim it to match.
>
> > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> > @@ -302,6 +302,40 @@ class Entry_fit(Entry_section):
> > +    fit,bootargs
> > +        A string written to /chosen/bootargs of every generated device tree
> > +        (any existing value is overwritten). Typically set to `CONFIG_BOOTARGS`
> > +        via the preprocessed devicetree source.
>
> The docs say 'any existing value is overwritten', but the
> implementation guards with 'if bootargs:', so an empty fit,bootargs =
> '' leaves any pre-existing /chosen/bootargs in place. Either the docs
> or the code should change - I'd expect the empty case to be treated
> the same as absent (leave untouched), and the docs updated to say so.

Makes sense, will adjust.

> > diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> > @@ -636,6 +636,14 @@ class Entry(object):
> > +    def GetNode(self):
> > +        """Get the devicetree node which describes this entry
> > +
> > +        Returns:
> > +            Node: Node for this entry
> > +        """
> > +        return self._node
>
> Nice - good addition. There are a few existing entry._node accessors
> in control.py that could be converted in a follow-up, but no need in
> this series.

Thanks a lot for looking into this!

Best regards,
Alexey