[PATCH v2 00/11] binman: Add support for generating more complex FITs

This series allows binman to generate FITs that include multiple DTB images and the configuration to use them.
It is then possible to remove the sunxi FIT generator script, so this series handles that also.
With this, sunxi is fully converted to use binman.
Changes in v2: - Add new patch to move 'external' support into base class - Update docs to mention both bl31.bin and bl31.elf - Add the URL of ARM Trusted Firmware and mention of U-Boot docs - Fix copyright year - Update docs to indicate that BL31 is loaded from SPL - Add a check for a missing fit,fdt-list property - Add a check for a missing of-list property - Add a check for an empty of-list - Add new patch to support missing external blobs always - Add new patch to allow FITs to be missing binaries - Add a 'fit-fdt-list' property - Fix 'board' typo in commit message - Add new patch to allow selecting default FIT configuration
Simon Glass (11): binman: Allow entry args to be required binman: Fix up a few missing comments libfdt: Detected out-of-space with fdt_finish() binman: Move 'external' support into base class binman: Add support for ATF BL31 binman: Support generating FITs with multiple dtbs Makefile: Support missing external blobs always binman: Allow FITs to be missing binaries sunxi: Convert 64-bit boards to use binman sunxi: Drop the FIT-generator script binman: Allow selecting default FIT configuration
Kconfig | 3 +- Makefile | 23 +-- arch/arm/dts/sunxi-u-boot.dtsi | 61 +++++- board/sunxi/mksunxi_fit_atf.sh | 87 -------- scripts/dtc/pylibfdt/libfdt.i_shipped | 3 +- tools/binman/README.entries | 73 ++++++- tools/binman/control.py | 5 + tools/binman/entry.py | 14 ++ tools/binman/etype/atf_bl31.py | 24 +++ tools/binman/etype/blob.py | 8 +- tools/binman/etype/blob_ext.py | 11 -- tools/binman/etype/blob_named_by_arg.py | 10 +- tools/binman/etype/cros_ec_rw.py | 3 +- tools/binman/etype/fit.py | 128 +++++++++++- tools/binman/etype/section.py | 14 +- tools/binman/ftest.py | 185 +++++++++++++++++- tools/binman/test/165_atf_bl31.dts | 16 ++ tools/binman/test/166_fit_fdt.dts | 55 ++++++ .../binman/test/167_fit_fdt_missing_prop.dts | 54 +++++ 19 files changed, 622 insertions(+), 155 deletions(-) delete mode 100755 board/sunxi/mksunxi_fit_atf.sh create mode 100644 tools/binman/etype/atf_bl31.py create mode 100644 tools/binman/test/165_atf_bl31.dts create mode 100644 tools/binman/test/166_fit_fdt.dts create mode 100644 tools/binman/test/167_fit_fdt_missing_prop.dts

If an entry argument is needed by an entry but the entry argument is not present, then a strange error can occur when trying to read the file.
Fix this by allowing arguments to be required. Select this option for the cros-ec-rw entry. If a filename is provided in the node, allow that to be used.
Also tidy up a few related tests to make the error string easier to find, and fully ignore unused return values.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/README.entries | 7 ++++++- tools/binman/etype/blob_named_by_arg.py | 10 ++++++---- tools/binman/etype/cros_ec_rw.py | 3 +-- tools/binman/ftest.py | 15 +++++++++++---- 4 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index bf8edce02b4..97bfae16116 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -60,7 +60,7 @@ Entry: blob-named-by-arg: A blob entry which gets its filename property from its
Properties / Entry arguments: - <xxx>-path: Filename containing the contents of this entry (optional, - defaults to 0) + defaults to None)
where <xxx> is the blob_fname argument to the constructor.
@@ -691,6 +691,11 @@ Properties / Entry arguments: (see binman README for more information) name-prefix: Adds a prefix to the name of every entry in the section when writing out the map
+Properties: + _allow_missing: True if this section permits external blobs to be + missing their contents. The second will produce an image but of + course it will not work. + Since a section is also an entry, it inherits all the properies of entries too.
diff --git a/tools/binman/etype/blob_named_by_arg.py b/tools/binman/etype/blob_named_by_arg.py index e95dabe4d07..7c486b2dc91 100644 --- a/tools/binman/etype/blob_named_by_arg.py +++ b/tools/binman/etype/blob_named_by_arg.py @@ -17,7 +17,7 @@ class Entry_blob_named_by_arg(Entry_blob):
Properties / Entry arguments: - <xxx>-path: Filename containing the contents of this entry (optional, - defaults to 0) + defaults to None)
where <xxx> is the blob_fname argument to the constructor.
@@ -28,7 +28,9 @@ class Entry_blob_named_by_arg(Entry_blob):
See cros_ec_rw for an example of this. """ - def __init__(self, section, etype, node, blob_fname): + def __init__(self, section, etype, node, blob_fname, required=False): super().__init__(section, etype, node) - self._filename, = self.GetEntryArgsOrProps( - [EntryArg('%s-path' % blob_fname, str)]) + filename, = self.GetEntryArgsOrProps( + [EntryArg('%s-path' % blob_fname, str)], required=required) + if filename: + self._filename = filename diff --git a/tools/binman/etype/cros_ec_rw.py b/tools/binman/etype/cros_ec_rw.py index 741372e1af4..bf676b2d1a7 100644 --- a/tools/binman/etype/cros_ec_rw.py +++ b/tools/binman/etype/cros_ec_rw.py @@ -7,7 +7,6 @@
from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
- class Entry_cros_ec_rw(Entry_blob_named_by_arg): """A blob entry which contains a Chromium OS read-write EC image
@@ -18,5 +17,5 @@ class Entry_cros_ec_rw(Entry_blob_named_by_arg): updating the EC on startup via software sync. """ def __init__(self, section, etype, node): - super().__init__(section, etype, node, 'cros-ec-rw') + super().__init__(section, etype, node, 'cros-ec-rw', required=True) self.external = True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 5f650b5f94c..2f989c3e4f1 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1382,8 +1382,9 @@ class TestFunctional(unittest.TestCase): } with self.assertRaises(ValueError) as e: self._DoReadFileDtb('064_entry_args_required.dts') - self.assertIn("Node '/binman/_testing': Missing required " - 'properties/entry args: test-str-arg, test-int-fdt, test-int-arg', + self.assertIn("Node '/binman/_testing': " + 'Missing required properties/entry args: test-str-arg, ' + 'test-int-fdt, test-int-arg', str(e.exception))
def testEntryArgsInvalidFormat(self): @@ -1487,8 +1488,7 @@ class TestFunctional(unittest.TestCase): entry_args = { 'cros-ec-rw-path': 'ecrw.bin', } - data, _, _, _ = self._DoReadFileDtb('068_blob_named_by_arg.dts', - entry_args=entry_args) + self._DoReadFileDtb('068_blob_named_by_arg.dts', entry_args=entry_args)
def testFill(self): """Test for an fill entry type""" @@ -3477,5 +3477,12 @@ class TestFunctional(unittest.TestCase): fnode = dtb.GetNode('/images/kernel') self.assertNotIn('data', fnode.props)
+ def testBlobNamedByArgMissing(self): + """Test handling of a missing entry arg""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('068_blob_named_by_arg.dts') + self.assertIn("Missing required properties/entry args: cros-ec-rw-path", + str(e.exception)) + if __name__ == "__main__": unittest.main()

Tidy up a few test functions which lack argument comments. Rename one that has the same name as a different test.
Also fix up the comment for PrepareImagesAndDtbs().
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/control.py | 5 +++++ tools/binman/ftest.py | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/tools/binman/control.py b/tools/binman/control.py index 60e89d3776b..0e0de6be71d 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -344,6 +344,11 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt): dtb_fname: Filename of the device tree file to use (.dts or .dtb) selected_images: List of images to output, or None for all update_fdt: True to update the FDT wth entry offsets, etc. + + Returns: + OrderedDict of images: + key: Image name (str) + value: Image object """ # Import these here in case libfdt.py is not available, in which case # the above help option still works. diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2f989c3e4f1..6c63cd10c68 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -298,6 +298,13 @@ class TestFunctional(unittest.TestCase): key: arg name value: value of that arg images: List of image names to build + use_real_dtb: True to use the test file as the contents of + the u-boot-dtb entry. Normally this is not needed and the + test contents (the U_BOOT_DTB_DATA string) can be used. + But in some test we need the real contents. + 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 """ args = [] if debug: @@ -357,6 +364,13 @@ class TestFunctional(unittest.TestCase): We still want the DTBs for SPL and TPL to be different though, since otherwise it is confusing to know which one we are looking at. So add an 'spl' or 'tpl' property to the top-level node. + + Args: + dtb_data: dtb data to modify (this should be a value devicetree) + name: Name of a new property to add + + Returns: + New dtb data with the property added """ dtb = fdt.Fdt.FromData(dtb_data) dtb.Scan() @@ -384,6 +398,12 @@ class TestFunctional(unittest.TestCase): map: True to output map files for the images update_dtb: Update the offset and size of each entry in the device tree before packing it into the image + entry_args: Dict of entry args to supply to binman + key: arg name + value: value of that arg + 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
Returns: Tuple: @@ -3467,7 +3487,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(len(U_BOOT_SPL_DTB_DATA), int(data_sizes[1].split()[0]))
def testFitExternal(self): - """Test an image with an FIT""" + """Test an image with an FIT with external images""" data = self._DoReadFile('162_fit_external.dts') fit_data = data[len(U_BOOT_DATA):-2] # _testing is 2 bytes

At present the Python sequential-write interface can produce an error when it calls fdt_finish(), since this needs to add a terminating tag to the end of the struct section.
Fix this by automatically expanding the buffer if needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
scripts/dtc/pylibfdt/libfdt.i_shipped | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/dtc/pylibfdt/libfdt.i_shipped b/scripts/dtc/pylibfdt/libfdt.i_shipped index fae0b27d7d0..1d69ad38e2e 100644 --- a/scripts/dtc/pylibfdt/libfdt.i_shipped +++ b/scripts/dtc/pylibfdt/libfdt.i_shipped @@ -786,7 +786,8 @@ class FdtSw(FdtRo): Fdt object allowing access to the newly created device tree """ fdtsw = bytearray(self._fdt) - check_err(fdt_finish(fdtsw)) + while self.check_space(fdt_finish(fdtsw)): + fdtsw = bytearray(self._fdt) return Fdt(fdtsw)
def check_space(self, val):

At present we have an Entry_blob_ext which implement a blob which holds an external binary. We need to support other entry types that hold external binaries, e.g. Entry_blob_named_by_arg. Move the support into the base Entry class to allow this.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add new patch to move 'external' support into base class
tools/binman/README.entries | 2 +- tools/binman/entry.py | 14 ++++++++++++++ tools/binman/etype/blob.py | 8 +++++++- tools/binman/etype/blob_ext.py | 11 ----------- tools/binman/etype/section.py | 14 ++------------ 5 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 97bfae16116..1086a6a9790 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -692,7 +692,7 @@ Properties / Entry arguments: (see binman README for more information) when writing out the map
Properties: - _allow_missing: True if this section permits external blobs to be + allow_missing: True if this section permits external blobs to be missing their contents. The second will produce an image but of course it will not work.
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 3434a3f8048..7e4b0ab29ae 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -59,6 +59,10 @@ class Entry(object): compress: Compression algoithm used (e.g. 'lz4'), 'none' if none orig_offset: Original offset value read from node orig_size: Original size value read from node + missing: True if this entry is missing its contents + allow_missing: Allow children of this entry to be missing (used by + subclasses such as Entry_section) + external: True if this entry contains an external binary blob """ def __init__(self, section, etype, node, name_prefix=''): # Put this here to allow entry-docs and help to work without libfdt @@ -85,6 +89,8 @@ class Entry(object): self._expand_size = False self.compress = 'none' self.missing = False + self.external = False + self.allow_missing = False
@staticmethod def Lookup(node_path, etype): @@ -815,3 +821,11 @@ features to produce new behaviours. """ if self.missing: missing_list.append(self) + + def GetAllowMissing(self): + """Get whether a section allows missing external blobs + + Returns: + True if allowed, False if not allowed + """ + return self.allow_missing diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index e507203709f..c5f97c85a38 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -37,7 +37,13 @@ class Entry_blob(Entry):
def ObtainContents(self): self._filename = self.GetDefaultFilename() - self._pathname = tools.GetInputFilename(self._filename) + self._pathname = tools.GetInputFilename(self._filename, + self.section.GetAllowMissing()) + # Allow the file to be missing + if self.external and not self._pathname: + self.SetContents(b'') + self.missing = True + return True self.ReadBlobContents() return True
diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py index 8d641001a9e..e372445f300 100644 --- a/tools/binman/etype/blob_ext.py +++ b/tools/binman/etype/blob_ext.py @@ -26,14 +26,3 @@ class Entry_blob_ext(Entry_blob): def __init__(self, section, etype, node): Entry_blob.__init__(self, section, etype, node) self.external = True - - def ObtainContents(self): - self._filename = self.GetDefaultFilename() - self._pathname = tools.GetInputFilename(self._filename, - self.section.GetAllowMissing()) - # Allow the file to be missing - if not self._pathname: - self.SetContents(b'') - self.missing = True - return True - return super().ObtainContents() diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 73c5553c812..328e2cfad01 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -35,7 +35,7 @@ class Entry_section(Entry): when writing out the map
Properties: - _allow_missing: True if this section permits external blobs to be + allow_missing: True if this section permits external blobs to be missing their contents. The second will produce an image but of course it will not work.
@@ -54,8 +54,6 @@ class Entry_section(Entry): self._sort = False self._skip_at_start = None self._end_4gb = False - self._allow_missing = False - self.missing = False
def ReadNode(self): """Read properties from the image node""" @@ -549,18 +547,10 @@ class Entry_section(Entry): Args: allow_missing: True if allowed, False if not allowed """ - self._allow_missing = allow_missing + self.allow_missing = allow_missing for entry in self._entries.values(): entry.SetAllowMissing(allow_missing)
- def GetAllowMissing(self): - """Get whether a section allows missing external blobs - - Returns: - True if allowed, False if not allowed - """ - return self._allow_missing - def CheckMissing(self, missing_list): """Check if any entries in this section have missing external blobs

Add an entry for ARM Trusted Firmware's 'BL31' payload, which is the device's main firmware. Typically this is U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Update docs to mention both bl31.bin and bl31.elf - Add the URL of ARM Trusted Firmware and mention of U-Boot docs - Fix copyright year - Update docs to indicate that BL31 is loaded from SPL
tools/binman/README.entries | 14 ++++++++++++++ tools/binman/etype/atf_bl31.py | 24 ++++++++++++++++++++++++ tools/binman/ftest.py | 9 +++++++++ tools/binman/test/165_atf_bl31.dts | 16 ++++++++++++++++ 4 files changed, 63 insertions(+) create mode 100644 tools/binman/etype/atf_bl31.py create mode 100644 tools/binman/test/165_atf_bl31.dts
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 1086a6a9790..8999d5eb387 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -11,6 +11,20 @@ features to produce new behaviours.
+Entry: atf-bl31: Entry containing an ARM Trusted Firmware (ATF) BL31 blob +------------------------------------------------------------------------- + +Properties / Entry arguments: + - atf-bl31-path: Filename of file to read into entry. This is typically + called bl31.bin or bl31.elf + +This entry holds the run-time firmware, typically started by U-Boot SPL. +See the U-Boot README for your architecture or board for how to use it. See +https://github.com/ARM-software/arm-trusted-firmware for more information +about ATF. + + + Entry: blob: Entry containing an arbitrary binary blob ------------------------------------------------------
diff --git a/tools/binman/etype/atf_bl31.py b/tools/binman/etype/atf_bl31.py new file mode 100644 index 00000000000..195adc714b5 --- /dev/null +++ b/tools/binman/etype/atf_bl31.py @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2020 Google LLC +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for Intel Management Engine binary blob +# + +from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg + +class Entry_atf_bl31(Entry_blob_named_by_arg): + """Entry containing an ARM Trusted Firmware (ATF) BL31 blob + + Properties / Entry arguments: + - atf-bl31-path: Filename of file to read into entry. This is typically + called bl31.bin or bl31.elf + + This entry holds the run-time firmware, typically started by U-Boot SPL. + See the U-Boot README for your architecture or board for how to use it. See + https://github.com/ARM-software/arm-trusted-firmware for more information + about ATF. + """ + def __init__(self, section, etype, node): + super().__init__(section, etype, node, 'atf-bl31') + self.external = True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 6c63cd10c68..4fe0316ba19 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -74,6 +74,7 @@ REFCODE_DATA = b'refcode' FSP_M_DATA = b'fsp_m' FSP_S_DATA = b'fsp_s' FSP_T_DATA = b'fsp_t' +ATF_BL31_DATA = b'bl31'
# The expected size for the device tree in some tests EXTRACT_DTB_SIZE = 0x3c9 @@ -167,6 +168,7 @@ class TestFunctional(unittest.TestCase): os.path.join(cls._indir, 'files'))
TestFunctional._MakeInputFile('compress', COMPRESS_DATA) + TestFunctional._MakeInputFile('bl31.bin', ATF_BL31_DATA)
# Travis-CI may have an old lz4 cls.have_lz4 = True @@ -3504,5 +3506,12 @@ class TestFunctional(unittest.TestCase): self.assertIn("Missing required properties/entry args: cros-ec-rw-path", str(e.exception))
+ def testPackBl31(self): + """Test that an image with an ATF BL31 binary can be created""" + data = self._DoReadFile('165_atf_bl31.dts') + self.assertEqual(ATF_BL31_DATA, data[:len(ATF_BL31_DATA)]) + +ATF_BL31_DATA + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/165_atf_bl31.dts b/tools/binman/test/165_atf_bl31.dts new file mode 100644 index 00000000000..2b7547d70f9 --- /dev/null +++ b/tools/binman/test/165_atf_bl31.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + atf-bl31 { + filename = "bl31.bin"; + }; + }; +};

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 ---
Changes in v2: - Add a check for a missing fit,fdt-list property - Add a check for a missing of-list property - Add a check for an empty of-list
tools/binman/README.entries | 48 +++++++- tools/binman/etype/fit.py | 94 +++++++++++++++- tools/binman/ftest.py | 106 +++++++++++++++++- tools/binman/test/166_fit_fdt.dts | 55 +++++++++ .../binman/test/167_fit_fdt_missing_prop.dts | 54 +++++++++ 5 files changed, 346 insertions(+), 11 deletions(-) create mode 100644 tools/binman/test/166_fit_fdt.dts create mode 100644 tools/binman/test/167_fit_fdt_missing_prop.dts
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 8999d5eb387..d2628dffd5d 100644 --- a/tools/binman/README.entries +++ b/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. diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 75712f44092..d31b741a623 100644 --- a/tools/binman/etype/fit.py +++ b/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_content = defaultdict(list) 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,'): - self._fit_props[pname] = prop - else: + if not pname.startswith('fit,'): 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 for subnode in node.subnodes: if has_images and not (subnode.name.startswith('hash') or subnode.name.startswith('signature')): @@ -100,6 +156,32 @@ class Entry_fit(Entry): entry = Entry.Create(self.section, subnode) entry.ReadNode() self._fit_content[rel_path].append(entry) + 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) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 4fe0316ba19..1dbda57dc72 100644 --- a/tools/binman/ftest.py +++ b/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')
@@ -3511,7 +3529,87 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('165_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( + '166_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('166_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('166_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('167_fit_fdt_missing_prop.dts') + self.assertIn("Generator node requires 'fit,fdt-list' property", + str(e.exception))
if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/166_fit_fdt.dts b/tools/binman/test/166_fit_fdt.dts new file mode 100644 index 00000000000..89142e91017 --- /dev/null +++ b/tools/binman/test/166_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 { + }; + }; +}; diff --git a/tools/binman/test/167_fit_fdt_missing_prop.dts b/tools/binman/test/167_fit_fdt_missing_prop.dts new file mode 100644 index 00000000000..c36134715c6 --- /dev/null +++ b/tools/binman/test/167_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 { + }; + }; +};

At present binman warns about missing external blobs only when the BUILD_ROM is defined. Enable this behaviour always, since many boards are starting to use these (e.g. ARM Trusted Firmware's BL31).
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add new patch to support missing external blobs always
Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 5dd4c6bd402..5b4e60496d6 100644 --- a/Makefile +++ b/Makefile @@ -1334,8 +1334,7 @@ quiet_cmd_binman = BINMAN $@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ --toolpath $(objtree)/tools \ $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \ - build -u -d u-boot.dtb -O . \ - $(if $(BUILD_ROM),,-m --allow-missing) \ + build -u -d u-boot.dtb -O . -m --allow-missing \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ $(BINMAN_$(@F))

In some cases a FIT may use an external binary that is not available. Handle this case by issuing a warning, as is done for sections.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add new patch to allow FITs to be missing binaries
tools/binman/etype/fit.py | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index d31b741a623..bfea02a152b 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -244,3 +244,15 @@ class Entry_fit(Entry): fdt.Sync(auto_resize=True) data = fdt.GetContents() return data + + def CheckMissing(self, missing_list): + """Check if any entries in this FIT have missing external blobs + + If there are missing blobs, the entries are added to the list + + Args: + missing_list: List of Entry objects to be added to + """ + for path, entries in self._fit_content.items(): + for entry in entries: + entry.CheckMissing(missing_list)

At present 64-bit sunxi boards use the Makefile to create a FIT, using USE_SPL_FIT_GENERATOR. This is deprecated.
Update sunxi to use binman instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add a 'fit-fdt-list' property - Fix 'board' typo in commit message
Kconfig | 3 +- Makefile | 18 ++-------- arch/arm/dts/sunxi-u-boot.dtsi | 61 +++++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 19 deletions(-)
diff --git a/Kconfig b/Kconfig index 883e3f71d01..837b2f517ae 100644 --- a/Kconfig +++ b/Kconfig @@ -659,12 +659,11 @@ config SPL_FIT_SOURCE
config USE_SPL_FIT_GENERATOR bool "Use a script to generate the .its script" - default y if SPL_FIT + default y if SPL_FIT && !ARCH_SUNXI
config SPL_FIT_GENERATOR string ".its file generator script for U-Boot FIT image" depends on USE_SPL_FIT_GENERATOR - default "board/sunxi/mksunxi_fit_atf.sh" if SPL_LOAD_FIT && ARCH_SUNXI default "arch/arm/mach-rockchip/make_fit_atf.py" if SPL_LOAD_FIT && ARCH_ROCKCHIP default "arch/arm/mach-zynqmp/mkimage_fit_atf.sh" if SPL_LOAD_FIT && ARCH_ZYNQMP default "arch/riscv/lib/mkimage_fit_opensbi.sh" if SPL_LOAD_FIT && RISCV diff --git a/Makefile b/Makefile index 5b4e60496d6..65024c74089 100644 --- a/Makefile +++ b/Makefile @@ -923,11 +923,6 @@ INPUTS-$(CONFIG_REMAKE_ELF) += u-boot.elf INPUTS-$(CONFIG_EFI_APP) += u-boot-app.efi INPUTS-$(CONFIG_EFI_STUB) += u-boot-payload.efi
-# Build a combined spl + u-boot image for sunxi -ifeq ($(CONFIG_ARCH_SUNXI)$(CONFIG_ARM64)$(CONFIG_SPL),yyy) -INPUTS-y += u-boot-sunxi-with-spl.bin -endif - # Generate this input file for binman ifeq ($(CONFIG_SPL),) INPUTS-$(CONFIG_ARCH_MEDIATEK) += u-boot-mtk.bin @@ -1024,13 +1019,9 @@ PHONY += inputs inputs: $(INPUTS-y)
all: .binman_stamp inputs -# Hack for sunxi which doesn't have a proper binman definition for -# 64-bit boards -ifneq ($(CONFIG_ARCH_SUNXI)$(CONFIG_ARM64),yy) ifeq ($(CONFIG_BINMAN),y) $(call if_changed,binman) endif -endif
# Timestamp file to make sure that binman always runs .binman_stamp: FORCE @@ -1336,6 +1327,8 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \ build -u -d u-boot.dtb -O . -m --allow-missing \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ + -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \ + -a atf-bl31-path=${BL31} \ $(BINMAN_$(@F))
OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex @@ -1625,13 +1618,6 @@ u-boot-x86-reset16.bin: u-boot FORCE
endif # CONFIG_X86
-ifneq ($(CONFIG_ARCH_SUNXI),) -ifeq ($(CONFIG_ARM64),y) -u-boot-sunxi-with-spl.bin: spl/sunxi-spl.bin u-boot.itb FORCE - $(call if_changed,cat) -endif -endif - OBJCOPYFLAGS_u-boot-app.efi := $(OBJCOPYFLAGS_EFI) u-boot-app.efi: u-boot FORCE $(call if_changed,zobjcopy) diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi index fdd4c80aa46..1d1c3691099 100644 --- a/arch/arm/dts/sunxi-u-boot.dtsi +++ b/arch/arm/dts/sunxi-u-boot.dtsi @@ -5,14 +5,73 @@ mmc1 = &mmc2; };
- binman { + binman: binman { + multiple-images; + }; +}; + +&binman { + u-boot-sunxi-with-spl { filename = "u-boot-sunxi-with-spl.bin"; pad-byte = <0xff>; blob { filename = "spl/sunxi-spl.bin"; }; +#ifdef CONFIG_ARM64 + fit { + description = "Configuration to load ATF before U-Boot"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + + images { + uboot { + description = "U-Boot (64-bit)"; + type = "standalone"; + arch = "arm64"; + compression = "none"; + load = <0x4a000000>; + + u-boot-nodtb { + }; + }; + atf { + description = "ARM Trusted Firmware"; + type = "firmware"; + arch = "arm64"; + compression = "none"; +/* TODO: Do this with an overwrite in this board's dtb? */ +#ifdef CONFIG_MACH_SUN50I_H6 + load = <0x104000>; + entry = <0x104000>; +#else + load = <0x44000>; + entry = <0x44000>; +#endif + atf-bl31 { + }; + }; + + @fdt-SEQ { + description = "NAME"; + type = "flat_dt"; + compression = "none"; + }; + }; + + configurations { + default = "config-1"; + @config-SEQ { + description = "NAME"; + firmware = "uboot"; + loadables = "atf"; + fdt = "fdt-SEQ"; + }; + }; + }; +#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };

On 8/31/20 1:20 AM, Simon Glass wrote:
At present 64-bit sunxi boards use the Makefile to create a FIT, using USE_SPL_FIT_GENERATOR. This is deprecated.
Update sunxi to use binman instead.
Signed-off-by: Simon Glass sjg@chromium.org
Tested on pine64-lts_defconfig
When BL31 is not defined I see a warning:
Image 'main-section' is missing external blobs and is non-functional: atf-bl31 Some images are invalid
But this does not indicate that environment variable BL31 should be set. The old message was more helpful:
WARNING: BL31 file $BL31 NOT found, resulting binary is non-functional Please read the section on ARM Trusted Firmware (ATF) in board/sunxi/README.sunxi64
Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de
Changes in v2:
- Add a 'fit-fdt-list' property
- Fix 'board' typo in commit message
Kconfig | 3 +- Makefile | 18 ++-------- arch/arm/dts/sunxi-u-boot.dtsi | 61 +++++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 19 deletions(-)
diff --git a/Kconfig b/Kconfig index 883e3f71d01..837b2f517ae 100644 --- a/Kconfig +++ b/Kconfig @@ -659,12 +659,11 @@ config SPL_FIT_SOURCE
config USE_SPL_FIT_GENERATOR bool "Use a script to generate the .its script"
- default y if SPL_FIT
- default y if SPL_FIT && !ARCH_SUNXI
config SPL_FIT_GENERATOR string ".its file generator script for U-Boot FIT image" depends on USE_SPL_FIT_GENERATOR
- default "board/sunxi/mksunxi_fit_atf.sh" if SPL_LOAD_FIT && ARCH_SUNXI default "arch/arm/mach-rockchip/make_fit_atf.py" if SPL_LOAD_FIT && ARCH_ROCKCHIP default "arch/arm/mach-zynqmp/mkimage_fit_atf.sh" if SPL_LOAD_FIT && ARCH_ZYNQMP default "arch/riscv/lib/mkimage_fit_opensbi.sh" if SPL_LOAD_FIT && RISCV
diff --git a/Makefile b/Makefile index 5b4e60496d6..65024c74089 100644 --- a/Makefile +++ b/Makefile @@ -923,11 +923,6 @@ INPUTS-$(CONFIG_REMAKE_ELF) += u-boot.elf INPUTS-$(CONFIG_EFI_APP) += u-boot-app.efi INPUTS-$(CONFIG_EFI_STUB) += u-boot-payload.efi
-# Build a combined spl + u-boot image for sunxi -ifeq ($(CONFIG_ARCH_SUNXI)$(CONFIG_ARM64)$(CONFIG_SPL),yyy) -INPUTS-y += u-boot-sunxi-with-spl.bin -endif
# Generate this input file for binman ifeq ($(CONFIG_SPL),) INPUTS-$(CONFIG_ARCH_MEDIATEK) += u-boot-mtk.bin @@ -1024,13 +1019,9 @@ PHONY += inputs inputs: $(INPUTS-y)
all: .binman_stamp inputs -# Hack for sunxi which doesn't have a proper binman definition for -# 64-bit boards -ifneq ($(CONFIG_ARCH_SUNXI)$(CONFIG_ARM64),yy) ifeq ($(CONFIG_BINMAN),y) $(call if_changed,binman) endif -endif
# Timestamp file to make sure that binman always runs .binman_stamp: FORCE @@ -1336,6 +1327,8 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \ build -u -d u-boot.dtb -O . -m --allow-missing \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
-I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \
$(BINMAN_$(@F))-a atf-bl31-path=${BL31} \
OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex @@ -1625,13 +1618,6 @@ u-boot-x86-reset16.bin: u-boot FORCE
endif # CONFIG_X86
-ifneq ($(CONFIG_ARCH_SUNXI),) -ifeq ($(CONFIG_ARM64),y) -u-boot-sunxi-with-spl.bin: spl/sunxi-spl.bin u-boot.itb FORCE
- $(call if_changed,cat)
-endif -endif
OBJCOPYFLAGS_u-boot-app.efi := $(OBJCOPYFLAGS_EFI) u-boot-app.efi: u-boot FORCE $(call if_changed,zobjcopy) diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi index fdd4c80aa46..1d1c3691099 100644 --- a/arch/arm/dts/sunxi-u-boot.dtsi +++ b/arch/arm/dts/sunxi-u-boot.dtsi @@ -5,14 +5,73 @@ mmc1 = &mmc2; };
- binman {
- binman: binman {
multiple-images;
- };
+};
+&binman {
- u-boot-sunxi-with-spl { filename = "u-boot-sunxi-with-spl.bin"; pad-byte = <0xff>; blob { filename = "spl/sunxi-spl.bin"; };
+#ifdef CONFIG_ARM64
fit {
description = "Configuration to load ATF before U-Boot";
#address-cells = <1>;
fit,fdt-list = "of-list";
images {
uboot {
description = "U-Boot (64-bit)";
type = "standalone";
arch = "arm64";
compression = "none";
load = <0x4a000000>;
u-boot-nodtb {
};
};
atf {
description = "ARM Trusted Firmware";
type = "firmware";
arch = "arm64";
compression = "none";
+/* TODO: Do this with an overwrite in this board's dtb? */ +#ifdef CONFIG_MACH_SUN50I_H6
load = <0x104000>;
entry = <0x104000>;
+#else
load = <0x44000>;
entry = <0x44000>;
+#endif
atf-bl31 {
};
};
@fdt-SEQ {
description = "NAME";
type = "flat_dt";
compression = "none";
};
};
configurations {
default = "config-1";
@config-SEQ {
description = "NAME";
firmware = "uboot";
loadables = "atf";
fdt = "fdt-SEQ";
};
};
};
+#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };

Hi Heinrich,
On Mon, 31 Aug 2020 at 04:54, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/31/20 1:20 AM, Simon Glass wrote:
At present 64-bit sunxi boards use the Makefile to create a FIT, using USE_SPL_FIT_GENERATOR. This is deprecated.
Update sunxi to use binman instead.
Signed-off-by: Simon Glass sjg@chromium.org
Tested on pine64-lts_defconfig
When BL31 is not defined I see a warning:
Image 'main-section' is missing external blobs and is non-functional: atf-bl31 Some images are invalid
But this does not indicate that environment variable BL31 should be set. The old message was more helpful:
WARNING: BL31 file $BL31 NOT found, resulting binary is non-functional Please read the section on ARM Trusted Firmware (ATF) in board/sunxi/README.sunxi64
We could have a property in the DT to point to that message I suppose. It would be nice to point people directly to documentation instead of making them figure it out.
Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de
Changes in v2:
- Add a 'fit-fdt-list' property
- Fix 'board' typo in commit message
Kconfig | 3 +- Makefile | 18 ++-------- arch/arm/dts/sunxi-u-boot.dtsi | 61 +++++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 19 deletions(-)
Regards, Simon

This file is no-longer used. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
board/sunxi/mksunxi_fit_atf.sh | 87 ---------------------------------- 1 file changed, 87 deletions(-) delete mode 100755 board/sunxi/mksunxi_fit_atf.sh
diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh deleted file mode 100755 index 88ad7197470..00000000000 --- a/board/sunxi/mksunxi_fit_atf.sh +++ /dev/null @@ -1,87 +0,0 @@ -#!/bin/sh -# -# script to generate FIT image source for 64-bit sunxi boards with -# ARM Trusted Firmware and multiple device trees (given on the command line) -# -# usage: $0 <dt_name> [<dt_name> [<dt_name] ...] - -[ -z "$BL31" ] && BL31="bl31.bin" - -if [ ! -f $BL31 ]; then - echo "WARNING: BL31 file $BL31 NOT found, resulting binary is non-functional" >&2 - echo "Please read the section on ARM Trusted Firmware (ATF) in board/sunxi/README.sunxi64" >&2 - BL31=/dev/null -fi - -if grep -q "^CONFIG_MACH_SUN50I_H6=y" .config; then - BL31_ADDR=0x104000 -else - BL31_ADDR=0x44000 -fi - -cat << __HEADER_EOF -/dts-v1/; - -/ { - description = "Configuration to load ATF before U-Boot"; - #address-cells = <1>; - - images { - uboot { - description = "U-Boot (64-bit)"; - data = /incbin/("u-boot-nodtb.bin"); - type = "standalone"; - arch = "arm64"; - compression = "none"; - load = <0x4a000000>; - }; - atf { - description = "ARM Trusted Firmware"; - data = /incbin/("$BL31"); - type = "firmware"; - arch = "arm64"; - compression = "none"; - load = <$BL31_ADDR>; - entry = <$BL31_ADDR>; - }; -__HEADER_EOF - -cnt=1 -for dtname in $* -do - cat << __FDT_IMAGE_EOF - fdt_$cnt { - description = "$(basename $dtname .dtb)"; - data = /incbin/("$dtname"); - type = "flat_dt"; - compression = "none"; - }; -__FDT_IMAGE_EOF - cnt=$((cnt+1)) -done - -cat << __CONF_HEADER_EOF - }; - configurations { - default = "config_1"; - -__CONF_HEADER_EOF - -cnt=1 -for dtname in $* -do - cat << __CONF_SECTION_EOF - config_$cnt { - description = "$(basename $dtname .dtb)"; - firmware = "uboot"; - loadables = "atf"; - fdt = "fdt_$cnt"; - }; -__CONF_SECTION_EOF - cnt=$((cnt+1)) -done - -cat << __ITS_EOF - }; -}; -__ITS_EOF

Add a new entry argument to the fit entry which allows selection of the default configuration to use. This is the 'default' property in the 'configurations' node.
Update the Makefile to pass in the value of DEVICE_TREE or CONFIG_DEFAULT_DEVICE_TREE to provide this information.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Michal Simek michal.simek@xilinx.com ---
Changes in v2: - Add new patch to allow selecting default FIT configuration
Makefile | 2 ++ tools/binman/README.entries | 4 ++++ tools/binman/etype/fit.py | 22 ++++++++++++++++++ tools/binman/ftest.py | 37 ++++++++++++++++++++++++++++++- tools/binman/test/166_fit_fdt.dts | 2 +- 5 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 65024c74089..fcc559ce7fa 100644 --- a/Makefile +++ b/Makefile @@ -1321,6 +1321,7 @@ u-boot.ldr: u-boot # binman # --------------------------------------------------------------------------- # Use 'make BINMAN_DEBUG=1' to enable debugging +default_dt := $(if $(DEVICE_TREE),$(DEVICE_TREE),$(CONFIG_DEFAULT_DEVICE_TREE)) quiet_cmd_binman = BINMAN $@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ --toolpath $(objtree)/tools \ @@ -1329,6 +1330,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \ -a atf-bl31-path=${BL31} \ + -a default-dt=$(default_dt) \ $(BINMAN_$(@F))
OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex diff --git a/tools/binman/README.entries b/tools/binman/README.entries index d2628dffd5d..c1d436563e8 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -402,6 +402,10 @@ Available substitutions for '@' nodes are: Note that if no devicetree files are provided (with '-a of-list' as above) then no nodes will be generated.
+The 'default' property, if present, will be automatically set to the name +if of configuration whose devicetree matches the 'default-dt' entry +argument, e.g. with '-a default-dt=sun50i-a64-pine64-lts'. +
Properties (in the 'fit' node itself): fit,external-offset: Indicates that the contents of the FIT are external diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index bfea02a152b..7ebab5c6c81 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -90,6 +90,10 @@ class Entry_fit(Entry): Note that if no devicetree files are provided (with '-a of-list' as above) then no nodes will be generated.
+ The 'default' property, if present, will be automatically set to the name + if of configuration whose devicetree matches the 'default-dt' entry + argument, e.g. with '-a default-dt=sun50i-a64-pine64-lts'. +
Properties (in the 'fit' node itself): fit,external-offset: Indicates that the contents of the FIT are external @@ -121,6 +125,8 @@ class Entry_fit(Entry): [EntryArg(self._fit_list_prop.value, str)]) if fdts is not None: self._fdts = fdts.split() + self._fit_default_dt = self.GetEntryArgsOrProps([EntryArg('default-dt', + str)])[0]
def ReadNode(self): self._ReadSubnodes() @@ -142,6 +148,22 @@ class Entry_fit(Entry): """ for pname, prop in node.props.items(): if not pname.startswith('fit,'): + if pname == 'default': + val = prop.value + # Handle the 'default' property + if val.startswith('@'): + if not self._fdts: + continue + if not self._fit_default_dt: + self.Raise("Generated 'default' node requires default-dt entry argument") + if self._fit_default_dt not in self._fdts: + self.Raise("default-dt entry argument '%s' not found in fdt list: %s" % + (self._fit_default_dt, + ', '.join(self._fdts))) + seq = self._fdts.index(self._fit_default_dt) + val = val[1:].replace('DEFAULT-SEQ', str(seq + 1)) + fsw.property_string(pname, val) + continue fsw.property(pname, prop.bytes)
rel_path = node.path[len(base_node.path):] diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1dbda57dc72..84145ec5a93 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3556,7 +3556,7 @@ class TestFunctional(unittest.TestCase): """ cnode = dtb.GetNode('/configurations') self.assertIn('default', cnode.props) - self.assertEqual('config-1', cnode.props['default'].value) + self.assertEqual('config-2', cnode.props['default'].value)
name = 'config-%d' % seq fnode = dtb.GetNode('/configurations/%s' % name) @@ -3569,6 +3569,7 @@ class TestFunctional(unittest.TestCase):
entry_args = { 'of-list': 'test-fdt1 test-fdt2', + 'default-dt': 'test-fdt2', } data = self._DoReadFileDtb( '166_fit_fdt.dts', @@ -3611,5 +3612,39 @@ class TestFunctional(unittest.TestCase): self.assertIn("Generator node requires 'fit,fdt-list' property", str(e.exception))
+ def testFitFdtEmptyList(self): + """Test handling of an empty 'of-list' entry arg""" + entry_args = { + 'of-list': '', + } + data = self._DoReadFileDtb('166_fit_fdt.dts', entry_args=entry_args)[0] + + def testFitFdtMissing(self): + """Test handling of a missing 'default-dt' entry arg""" + entry_args = { + 'of-list': 'test-fdt1 test-fdt2', + } + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb( + '166_fit_fdt.dts', + entry_args=entry_args, + extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0] + self.assertIn("Generated 'default' node requires default-dt entry argument", + str(e.exception)) + + def testFitFdtNotInList(self): + """Test handling of a default-dt that is not in the of-list""" + entry_args = { + 'of-list': 'test-fdt1 test-fdt2', + 'default-dt': 'test-fdt3', + } + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb( + '166_fit_fdt.dts', + entry_args=entry_args, + extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0] + self.assertIn("default-dt entry argument 'test-fdt3' not found in fdt list: test-fdt1, test-fdt2", + str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/166_fit_fdt.dts b/tools/binman/test/166_fit_fdt.dts index 89142e91017..99d710c57e9 100644 --- a/tools/binman/test/166_fit_fdt.dts +++ b/tools/binman/test/166_fit_fdt.dts @@ -40,7 +40,7 @@ };
configurations { - default = "config-1"; + default = "@config-DEFAULT-SEQ"; @config-SEQ { description = "conf-NAME.dtb"; firmware = "uboot";
participants (2)
-
Heinrich Schuchardt
-
Simon Glass