binman: Support generating FITs with multiple dtbs

In some cases it is useful to generate a FIT which has a number of DTB
images, selectable by configuration. Add support for this in binman,
using a simple iterator and string substitution.

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

tools/binman/README.entries

@@ -339,6 +339,7 @@ For example, this creates an image containing a FIT with U-Boot SPL:
     binman {
         fit {
             description = "Test FIT";
+            fit,fdt-list = "of-list";
 
             images {
                 kernel@1 {
@@ -357,7 +358,52 @@ For example, this creates an image containing a FIT with U-Boot SPL:
         };
     };
 
-Properties:
+U-Boot supports creating fdt and config nodes automatically. To do this,
+pass an of-list property (e.g. -a of-list=file1 file2). This tells binman
+that you want to generates nodes for two files: file1.dtb and file2.dtb
+The fit,fdt-list property (see above) indicates that of-list should be used.
+If the property is missing you will get an error.
+
+Then add a 'generator node', a node with a name starting with '@':
+
+    images {
+        @fdt-SEQ {
+            description = "fdt-NAME";
+            type = "flat_dt";
+            compression = "none";
+        };
+    };
+
+This tells binman to create nodes fdt-1 and fdt-2 for each of your two
+files. All the properties you specify will be included in the node. This
+node acts like a template to generate the nodes. The generator node itself
+does not appear in the output - it is replaced with what binman generates.
+
+You can create config nodes in a similar way:
+
+    configurations {
+        default = "@config-DEFAULT-SEQ";
+        @config-SEQ {
+            description = "NAME";
+            firmware = "uboot";
+            loadables = "atf";
+            fdt = "fdt-SEQ";
+        };
+    };
+
+This tells binman to create nodes config-1 and config-2, i.e. a config for
+each of your two files.
+
+Available substitutions for '@' nodes are:
+
+    SEQ    Sequence number of the generated fdt (1, 2, ...)
+    NAME   Name of the dtb as provided (i.e. without adding '.dtb')
+
+Note that if no devicetree files are provided (with '-a of-list' as above)
+then no nodes will be generated.
+
+
+Properties (in the 'fit' node itself):
     fit,external-offset: Indicates that the contents of the FIT are external
         and provides the external offset. This is passsed to mkimage via
         the -E and -p flags.

tools/binman/etype/fit.py

@@ -8,7 +8,7 @@
 from collections import defaultdict, OrderedDict
 import libfdt
 
-from binman.entry import Entry
+from binman.entry import Entry, EntryArg
 from dtoc import fdt_util
 from dtoc.fdt import Fdt
 from patman import tools
@@ -27,6 +27,7 @@ class Entry_fit(Entry):
         binman {
             fit {
                 description = "Test FIT";
+                fit,fdt-list = "of-list";
 
                 images {
                     kernel@1 {
@@ -45,7 +46,52 @@ class Entry_fit(Entry):
             };
         };
 
-    Properties:
+    U-Boot supports creating fdt and config nodes automatically. To do this,
+    pass an of-list property (e.g. -a of-list=file1 file2). This tells binman
+    that you want to generates nodes for two files: file1.dtb and file2.dtb
+    The fit,fdt-list property (see above) indicates that of-list should be used.
+    If the property is missing you will get an error.
+
+    Then add a 'generator node', a node with a name starting with '@':
+
+        images {
+            @fdt-SEQ {
+                description = "fdt-NAME";
+                type = "flat_dt";
+                compression = "none";
+            };
+        };
+
+    This tells binman to create nodes fdt-1 and fdt-2 for each of your two
+    files. All the properties you specify will be included in the node. This
+    node acts like a template to generate the nodes. The generator node itself
+    does not appear in the output - it is replaced with what binman generates.
+
+    You can create config nodes in a similar way:
+
+        configurations {
+            default = "@config-DEFAULT-SEQ";
+            @config-SEQ {
+                description = "NAME";
+                firmware = "uboot";
+                loadables = "atf";
+                fdt = "fdt-SEQ";
+            };
+        };
+
+    This tells binman to create nodes config-1 and config-2, i.e. a config for
+    each of your two files.
+
+    Available substitutions for '@' nodes are:
+
+        SEQ    Sequence number of the generated fdt (1, 2, ...)
+        NAME   Name of the dtb as provided (i.e. without adding '.dtb')
+
+    Note that if no devicetree files are provided (with '-a of-list' as above)
+    then no nodes will be generated.
+
+
+    Properties (in the 'fit' node itself):
         fit,external-offset: Indicates that the contents of the FIT are external
             and provides the external offset. This is passsed to mkimage via
             the -E and -p flags.
@@ -64,6 +110,17 @@ class Entry_fit(Entry):
         self._fit = None
         self._fit_sections = {}
         self._fit_props = {}
+        for pname, prop in self._node.props.items():
+            if pname.startswith('fit,'):
+                self._fit_props[pname] = prop
+
+        self._fdts = None
+        self._fit_list_prop = self._fit_props.get('fit,fdt-list')
+        if self._fit_list_prop:
+            fdts, = self.GetEntryArgsOrProps(
+                [EntryArg(self._fit_list_prop.value, str)])
+            if fdts is not None:
+                self._fdts = fdts.split()
 
     def ReadNode(self):
         self._ReadSubnodes()
@@ -84,13 +141,12 @@ class Entry_fit(Entry):
                   image
             """
             for pname, prop in node.props.items():
-                if pname.startswith('fit,'):
+                if not pname.startswith('fit,'):
-                    self._fit_props[pname] = prop
-                else:
                     fsw.property(pname, prop.bytes)
 
             rel_path = node.path[len(base_node.path):]
-            has_images = depth == 2 and rel_path.startswith('/images/')
+            in_images = rel_path.startswith('/images')
+            has_images = depth == 2 and in_images
             if has_images:
                 # This node is a FIT subimage node (e.g. "/images/kernel")
                 # containing content nodes. We collect the subimage nodes and
@@ -108,6 +164,32 @@ class Entry_fit(Entry):
                     # the FIT (e.g. "/images/kernel/u-boot"), so don't call
                     # fsw.add_node() or _AddNode() for it.
                     pass
+                elif subnode.name.startswith('@'):
+                    if self._fdts:
+                        # Generate notes for each FDT
+                        for seq, fdt_fname in enumerate(self._fdts):
+                            node_name = subnode.name[1:].replace('SEQ',
+                                                                 str(seq + 1))
+                            fname = tools.GetInputFilename(fdt_fname + '.dtb')
+                            with fsw.add_node(node_name):
+                                for pname, prop in subnode.props.items():
+                                    val = prop.bytes.replace(
+                                        b'NAME', tools.ToBytes(fdt_fname))
+                                    val = val.replace(
+                                        b'SEQ', tools.ToBytes(str(seq + 1)))
+                                    fsw.property(pname, val)
+
+                                    # Add data for 'fdt' nodes (but not 'config')
+                                    if depth == 1 and in_images:
+                                        fsw.property('data',
+                                                     tools.ReadFile(fname))
+                    else:
+                        if self._fdts is None:
+                            if self._fit_list_prop:
+                                self.Raise("Generator node requires '%s' entry argument" %
+                                           self._fit_list_prop.value)
+                            else:
+                                self.Raise("Generator node requires 'fit,fdt-list' property")
                 else:
                     with fsw.add_node(subnode.name):
                         _AddNode(base_node, depth + 1, subnode)

tools/binman/ftest.py

@@ -75,6 +75,11 @@ FSP_M_DATA            = b'fsp_m'
 FSP_S_DATA            = b'fsp_s'
 FSP_T_DATA            = b'fsp_t'
 ATF_BL31_DATA         = b'bl31'
+TEST_FDT1_DATA        = b'fdt1'
+TEST_FDT2_DATA        = b'test-fdt2'
+
+# Subdirectory of the input dir to use to put test FDTs
+TEST_FDT_SUBDIR       = 'fdts'
 
 # The expected size for the device tree in some tests
 EXTRACT_DTB_SIZE = 0x3c9
@@ -170,6 +175,12 @@ class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('compress', COMPRESS_DATA)
         TestFunctional._MakeInputFile('bl31.bin', ATF_BL31_DATA)
 
+        # Add a few .dtb files for testing
+        TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
+                                      TEST_FDT1_DATA)
+        TestFunctional._MakeInputFile('%s/test-fdt2.dtb' % TEST_FDT_SUBDIR,
+                                      TEST_FDT2_DATA)
+
         # Travis-CI may have an old lz4
         cls.have_lz4 = True
         try:
@@ -287,7 +298,7 @@ class TestFunctional(unittest.TestCase):
 
     def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False,
                     entry_args=None, images=None, use_real_dtb=False,
-                    verbosity=None, allow_missing=False):
+                    verbosity=None, allow_missing=False, extra_indirs=None):
         """Run binman with a given test file
 
         Args:
@@ -307,6 +318,7 @@ class TestFunctional(unittest.TestCase):
             verbosity: Verbosity level to use (0-3, None=don't set it)
             allow_missing: Set the '--allow-missing' flag so that missing
                 external binaries just produce a warning instead of an error
+            extra_indirs: Extra input directories to add using -I
         """
         args = []
         if debug:
@@ -333,6 +345,9 @@ class TestFunctional(unittest.TestCase):
         if images:
             for image in images:
                 args += ['-i', image]
+        if extra_indirs:
+            for indir in extra_indirs:
+                args += ['-I', indir]
         return self._DoBinman(*args)
 
     def _SetupDtb(self, fname, outfile='u-boot.dtb'):
@@ -382,7 +397,8 @@ class TestFunctional(unittest.TestCase):
         return dtb.GetContents()
 
     def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False,
-                       update_dtb=False, entry_args=None, reset_dtbs=True):
+                       update_dtb=False, entry_args=None, reset_dtbs=True,
+                       extra_indirs=None):
         """Run binman and return the resulting image
 
         This runs binman with a given test file and then reads the resulting
@@ -406,6 +422,7 @@ class TestFunctional(unittest.TestCase):
             reset_dtbs: With use_real_dtb the test dtb is overwritten by this
                 function. If reset_dtbs is True, then the original test dtb
                 is written back before this function finishes
+            extra_indirs: Extra input directories to add using -I
 
         Returns:
             Tuple:
@@ -429,7 +446,8 @@ class TestFunctional(unittest.TestCase):
 
         try:
             retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb,
-                    entry_args=entry_args, use_real_dtb=use_real_dtb)
+                    entry_args=entry_args, use_real_dtb=use_real_dtb,
+                    extra_indirs=extra_indirs)
             self.assertEqual(0, retcode)
             out_dtb_fname = tools.GetOutputFilename('u-boot.dtb.out')
 
@@ -3557,7 +3575,87 @@ class TestFunctional(unittest.TestCase):
         data = self._DoReadFile('169_atf_bl31.dts')
         self.assertEqual(ATF_BL31_DATA, data[:len(ATF_BL31_DATA)])
 
-ATF_BL31_DATA
+    def testFitFdt(self):
+        """Test an image with an FIT with multiple FDT images"""
+        def _CheckFdt(seq, expected_data):
+            """Check the FDT nodes
+
+            Args:
+                seq: Sequence number to check (0 or 1)
+                expected_data: Expected contents of 'data' property
+            """
+            name = 'fdt-%d' % seq
+            fnode = dtb.GetNode('/images/%s' % name)
+            self.assertIsNotNone(fnode)
+            self.assertEqual({'description','type', 'compression', 'data'},
+                             set(fnode.props.keys()))
+            self.assertEqual(expected_data, fnode.props['data'].bytes)
+            self.assertEqual('fdt-test-fdt%d.dtb' % seq,
+                             fnode.props['description'].value)
+
+        def _CheckConfig(seq, expected_data):
+            """Check the configuration nodes
+
+            Args:
+                seq: Sequence number to check (0 or 1)
+                expected_data: Expected contents of 'data' property
+            """
+            cnode = dtb.GetNode('/configurations')
+            self.assertIn('default', cnode.props)
+            self.assertEqual('config-1', cnode.props['default'].value)
+
+            name = 'config-%d' % seq
+            fnode = dtb.GetNode('/configurations/%s' % name)
+            self.assertIsNotNone(fnode)
+            self.assertEqual({'description','firmware', 'loadables', 'fdt'},
+                             set(fnode.props.keys()))
+            self.assertEqual('conf-test-fdt%d.dtb' % seq,
+                             fnode.props['description'].value)
+            self.assertEqual('fdt-%d' % seq, fnode.props['fdt'].value)
+
+        entry_args = {
+            'of-list': 'test-fdt1 test-fdt2',
+        }
+        data = self._DoReadFileDtb(
+            '170_fit_fdt.dts',
+            entry_args=entry_args,
+            extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0]
+        self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
+        fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)]
+
+        dtb = fdt.Fdt.FromData(fit_data)
+        dtb.Scan()
+        fnode = dtb.GetNode('/images/kernel')
+        self.assertIn('data', fnode.props)
+
+        # Check all the properties in fdt-1 and fdt-2
+        _CheckFdt(1, TEST_FDT1_DATA)
+        _CheckFdt(2, TEST_FDT2_DATA)
+
+        # Check configurations
+        _CheckConfig(1, TEST_FDT1_DATA)
+        _CheckConfig(2, TEST_FDT2_DATA)
+
+    def testFitFdtMissingList(self):
+        """Test handling of a missing 'of-list' entry arg"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFile('170_fit_fdt.dts')
+        self.assertIn("Generator node requires 'of-list' entry argument",
+                      str(e.exception))
+
+    def testFitFdtEmptyList(self):
+        """Test handling of an empty 'of-list' entry arg"""
+        entry_args = {
+            'of-list': '',
+        }
+        data = self._DoReadFileDtb('170_fit_fdt.dts', entry_args=entry_args)[0]
+
+    def testFitFdtMissingProp(self):
+        """Test handling of a missing 'fit,fdt-list' property"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFile('171_fit_fdt_missing_prop.dts')
+        self.assertIn("Generator node requires 'fit,fdt-list' property",
+                      str(e.exception))
 
 if __name__ == "__main__":
     unittest.main()

tools/binman/test/170_fit_fdt.dts

@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		fit {
+			description = "test-desc";
+			#address-cells = <1>;
+			fit,fdt-list = "of-list";
+
+			images {
+				kernel {
+					description = "Vanilla Linux kernel";
+					type = "kernel";
+					arch = "ppc";
+					os = "linux";
+					compression = "gzip";
+					load = <00000000>;
+					entry = <00000000>;
+					hash-1 {
+						algo = "crc32";
+					};
+					hash-2 {
+						algo = "sha1";
+					};
+					u-boot {
+					};
+				};
+				@fdt-SEQ {
+					description = "fdt-NAME.dtb";
+					type = "flat_dt";
+					compression = "none";
+				};
+			};
+
+			configurations {
+				default = "config-1";
+				@config-SEQ {
+					description = "conf-NAME.dtb";
+					firmware = "uboot";
+					loadables = "atf";
+					fdt = "fdt-SEQ";
+				};
+			};
+		};
+		u-boot-nodtb {
+		};
+	};
+};

tools/binman/test/171_fit_fdt_missing_prop.dts

@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		fit {
+			description = "test-desc";
+			#address-cells = <1>;
+
+			images {
+				kernel {
+					description = "Vanilla Linux kernel";
+					type = "kernel";
+					arch = "ppc";
+					os = "linux";
+					compression = "gzip";
+					load = <00000000>;
+					entry = <00000000>;
+					hash-1 {
+						algo = "crc32";
+					};
+					hash-2 {
+						algo = "sha1";
+					};
+					u-boot {
+					};
+				};
+				@fdt-SEQ {
+					description = "fdt-NAME.dtb";
+					type = "flat_dt";
+					compression = "none";
+				};
+			};
+
+			configurations {
+				default = "config-1";
+				@config-SEQ {
+					description = "conf-NAME.dtb";
+					firmware = "uboot";
+					loadables = "atf";
+					fdt = "fdt-SEQ";
+				};
+			};
+		};
+		u-boot-nodtb {
+		};
+	};
+};