On 07.07.23 14:42, Simon Glass wrote:

Hi Jan,

On Fri, 7 Jul 2023 at 11:05, Jan Kiszka jan.kiszka@siemens.com wrote:

...

On 05.07.23 00:14, Simon Glass wrote:

...

Hi Jan,

On Mon, 3 Jul 2023 at 20:34, Jan Kiszka jan.kiszka@siemens.com wrote:

...

Hi Simon,

On 28.06.23 13:41, Simon Glass wrote:

...

Collections can used to collect the contents of other entries into a single entry, but they result in a single entry, with the original entries 'left behind' in their old place.

It is useful to be able to specific a set of entries ones and have it used in multiple images, or parts of an image.

Implement this mechanism.

Signed-off-by: Simon Glass sjg@chromium.org

tools/binman/binman.rst | 80 ++++++++++++++++++++++++ tools/binman/control.py | 9 +++ tools/binman/etype/section.py | 3 +- tools/binman/ftest.py | 8 +++ tools/binman/test/286_entry_template.dts | 42 +++++++++++++ 5 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/286_entry_template.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index a4b31fe5397b..9be979ae1c5b 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -727,6 +727,13 @@ optional: Note that missing, optional blobs do not produce a non-zero exit code from binman, although it does show a warning about the missing external blob.

+insert-template:

• This is not strictly speaking an entry property, since it is processed early
• in Binman before the entries are read. It is a list of phandles of nodes to
• include in the current (target) node. For each node, its subnodes and their
• properties are brought into the target node. See Templates_ below for
• more information.

The attributes supported for images and sections are described below. Several are similar to those for entries.

@@ -1172,6 +1179,79 @@ If you are having trouble figuring out what is going on, you can use arch/arm/dts/u-boot.dtsi ... found: "arch/arm/dts/juno-r2-u-boot.dtsi"

+Templates +=========

+Sometimes multiple images need to be created which have all have a common +part. For example, a board may generate SPI and eMMC images which both include +a FIT. Since the FIT includes many entries, it is tedious to repeat them twice +in the image description.

+Templates provide a simple way to handle this::

• binman {

•    multiple-images;
  

•    common_part: template-1 {
  

•        fit {
  

•            ... lots of entries in here
  

•        };
  

•        text {
  

•            text = "base image";
  

•        };
  

•    };
  

•    spi-image {
  

•        filename = "image-spi.bin";
  

•        insert-template = <&fit>;
  

•        /* things specific to SPI follow */
  

•        header {
  

•        ];
  

•        text {
  

•            text = "SPI image";
  

•        };
  

•    };
  

•    mmc-image {
  

•        filename = "image-mmc.bin";
  

•        insert-template = <&fit>;
  

•        /* things specific to MMC follow */
  

•        header {
  

•        ];
  

•        text {
  

•            text = "MMC image";
  

•        };
  

•    };
  

• };

+The template node name must start with 'template', so it is not considered to be +an image itself.

+The mechanism is very simple. For each phandle in the 'insert-templates' +property, the source node is looked up. Then the subnodes of that source node +are copied into the target node, i.e. the one containing the `insert-template` +property.

+If the target node has a node with the same name as a template, its properties +override corresponding properties in the template. This allows the template to +be uses as a base, with the node providing updates to the properties as needed. +The overriding happens recursively.

+At present there is an unpleasant limitation on this feature: it works by +appending the template nodes after any existing subnodes to the target node. +This means that if the target node includes any subnodes, these appear in order +before the template node. In the above example, 'header' becomes the first +subnode of each image, followed by `fit` and `text`. If this is not what is +desired, there is no way to adjust it.

+Note: The above limitation will likely be removed in future, so that the +template subnodes appear before the target subnodes.

Updating an ELF file

diff --git a/tools/binman/control.py b/tools/binman/control.py index 68597c4e7792..e7faca78e9aa 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -22,6 +22,7 @@ from binman import bintool from binman import cbfs_util from binman import elf from binman import entry +from dtoc import fdt_util from u_boot_pylib import command from u_boot_pylib import tools from u_boot_pylib import tout @@ -478,6 +479,12 @@ def SignEntries(image_fname, input_fname, privatekey_fname, algo, entry_paths,

 AfterReplace(image, allow_resize=True, write_map=write_map)

+def _ProcessTemplates(parent):

• for node in parent.subnodes:

•    tmpl = fdt_util.GetPhandleList(node, 'insert-template')
  

•    if tmpl:
  

•        node.copy_subnodes_from_phandles(tmpl)
  

def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): """Prepare the images to be processed and select the device tree

@@ -520,6 +527,8 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): raise ValueError("Device tree '%s' does not have a 'binman' " "node" % dtb_fname)

• _ProcessTemplates(node)

• images = _ReadImageDesc(node, use_expanded)

  if select_images:

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index d56cc11d1023..adac2ff7fa87 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -179,7 +179,8 @@ class Entry_section(Entry): Returns: bool: True if the node is a special one, else False """

•    return node.name.startswith('hash') or node.name.startswith('signature')
  

•    start_list = ('hash', 'signature', 'template')
  

•    return any(node.name.startswith(name) for name in start_list)
  
  

  def ReadNode(self): """Read properties from the section node"""

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 4db54c69682c..9d9e47ce26b0 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6763,6 +6763,14 @@ fdt fdtmap Extract the devicetree blob from the fdtmap data = self._DoReadFileDtb('285_spl_expand.dts', use_expanded=True, entry_args=entry_args)[0]

• def testEntryTemplate(self):

•    """Test using a template"""
  

•    TestFunctional._MakeInputFile('vga2.bin', b'#' + VGA_DATA)
  

•    data = self._DoReadFile('286_entry_template.dts')
  

•    first = U_BOOT_DTB_DATA + U_BOOT_DATA + VGA_DATA
  

•    second = U_BOOT_DTB_DATA + b'#' + VGA_DATA + U_BOOT_DATA
  

•    self.assertEqual(U_BOOT_IMG_DATA + first + second, data)
  

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/286_entry_template.dts b/tools/binman/test/286_entry_template.dts new file mode 100644 index 000000000000..6980dbfafcc6 --- /dev/null +++ b/tools/binman/test/286_entry_template.dts @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0+

+/dts-v1/;

+/ {

• #address-cells = <1>;
  

• #size-cells = <1>;
  

• binman {
  

•         u-boot-img {
  

•         };
  

•         common_part: template {
  

•                 u-boot {
  

•                 };
  

•                 intel-vga {
  

•                         filename = "vga.bin";
  

•                 };
  

•         };
  

•         first {
  

•                 type = "section";
  

•                 insert-template = <&common_part>;
  

•                 u-boot-dtb {
  

•                 };
  

•         };
  

•         second {
  

•                 type = "section";
  

•                 insert-template = <&common_part>;
  

•                 u-boot-dtb {
  

•                 };
  

•                 intel-vga {
  

•                         filename = "vga2.bin";
  

•                 };
  

•         };
  

• };
  

+};

I tried to apply this on the IOT2050 use case, but it failed in two cases:

1. We need a template where nodes can be manipulated afterwards, thus we

need anchors into those nodes. Naturally, those anchors need to get unique names so that something like this is possible:

common: template { anchor: some-node { value-1 = <1>; }; };

user-a { insert-template = <&common>; };

user-b { insert-template = <&common>; };

&anchor-a { value-2 = <2>; };

&anchor-b { value-2 = <3>; };

Rather than using a phandle, can you not use the node name in the template? For example:

user-a { insert-template = <&common>; some-node { value2 = <2>; }

user-b { insert-template = <&common>; some-node { value2 = <3>; }

I don't think this is working already. This is what I tried:

/dts-v1/; / { binman: binman { multiple-images;

            my_template: template {
                    blob-ext@0 {
                            filename = "my-blob.bin";
                            offset = <0>;
                    };
                    blob-ext@100 {
                            filename = "/dev/null";
                            offset = <0x100>;
                    };
            };

            my-image {
                    filename = "my-image.bin";
                    insert-template = <&my_template>;
                    blob-ext@100 {
                            filename = "my-blob2.bin";
                    };
            };
    };

};

First, I had to apply that null filename workaround, and the overridden filename is taken for the final image (if I leave out blob-ext@0), but this is at least ugly.

However, the file above as-is does not build:

binman: Node '/binman/my-image/blob-ext@0': Offset 0x0 (0) overlaps with previous entry '/binman/my-image/blob-ext@100' ending at 0x107 (263)

Probably for the same reason, leaving fit,fdt-list-val out in a template also generates an error during build. Putting an empty string there and overriding it later does not work.

I think it is just that it doesn't support templating of multiple-images nodes property. I sent a patch for this [1]

There may be other cases too, but if you have a failure, please see if you can adjust that test (287) to fail for you.

Hmm, already the test pattern from [1] still gives me

binman: Node '/binman/image/blob-ext@0': Offset 0x0 (0) overlaps with previous entry '/binman/image/blob-ext@8' ending at 0xf (15)

I've applied this series and then [1] on top.

Jan

Regards, Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20230707124024.1668724-1-sj...