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

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

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

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

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

Regards,
Simon