[PATCH v3 00/12] 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.
Note: This series is available at u-boot-dm/binman-working and is based on u-boot-dm/testing
Changes in v3: - Add a way to show help messages for missing blobs - Rebase on top of earlier binman series
Changes in v2: - Add a 'fit-fdt-list' property - 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 allow selecting default FIT configuration - Add new patch to move 'external' support into base class - Add new patch to support missing external blobs always - Add the URL of ARM Trusted Firmware and mention of U-Boot docs - Fix 'board' typo in commit message - Fix copyright year - Update docs to indicate that BL31 is loaded from SPL - Update docs to mention both bl31.bin and bl31.elf
Simon Glass (12): 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 sunxi: Convert 64-bit boards to use binman sunxi: Drop the FIT-generator script binman: Allow selecting default FIT configuration binman: Support help messages for missing blobs binman: sunxi: Add help message for missing sunxi ATF BL31
Kconfig | 3 +- Makefile | 23 +- arch/arm/dts/sunxi-u-boot.dtsi | 62 +++++- board/sunxi/mksunxi_fit_atf.sh | 87 -------- scripts/dtc/pylibfdt/libfdt.i_shipped | 3 +- tools/binman/README | 6 + tools/binman/README.entries | 73 ++++++- tools/binman/control.py | 74 ++++++- tools/binman/entry.py | 23 ++ 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 | 116 +++++++++- tools/binman/etype/section.py | 14 +- tools/binman/ftest.py | 203 +++++++++++++++++- tools/binman/missing-blob-help | 15 ++ tools/binman/test/168_fit_missing_blob.dts | 9 +- tools/binman/test/169_atf_bl31.dts | 16 ++ .../binman/test/171_fit_fdt_missing_prop.dts | 54 +++++ tools/binman/test/172_fit_fdt.dts | 55 +++++ 22 files changed, 734 insertions(+), 158 deletions(-) delete mode 100755 board/sunxi/mksunxi_fit_atf.sh create mode 100644 tools/binman/etype/atf_bl31.py create mode 100644 tools/binman/missing-blob-help create mode 100644 tools/binman/test/169_atf_bl31.dts create mode 100644 tools/binman/test/171_fit_fdt_missing_prop.dts create mode 100644 tools/binman/test/172_fit_fdt.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 e672967dbaa..f000a794325 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""" @@ -3523,5 +3523,12 @@ class TestFunctional(unittest.TestCase): err = stderr.getvalue() self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
+ 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()

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(-)
Applied to u-boot-dm/next, thanks!

Hi Simon,
On 05. 09. 20 23:10, Simon Glass wrote:
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(-)
Applied to u-boot-dm/next, thanks!
Did you start to use patman for sending this? Just curios because I am missing indentation in origin message.
Thanks, Michal

Hi Michal,
On Mon, 7 Sep 2020 at 00:31, Michal Simek michal.simek@xilinx.com wrote:
Hi Simon,
On 05. 09. 20 23:10, Simon Glass wrote:
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(-)
Applied to u-boot-dm/next, thanks!
Did you start to use patman for sending this? Just curios because I am missing indentation in origin message.
No that is done with a gmail script. It would be interesting to use patman though. I am using patman to collect review tags from patchwork, so using it to send out the 'applied' emails would help complete the circle.
Regards, Simon

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 3b523266417..15bfac582a7 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -346,6 +346,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 f000a794325..ceeee1dfe63 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

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(-)
Applied to u-boot-dm/next, thanks!

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 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(-)
Applied to u-boot-dm/next, thanks!

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 ---
(no changes since v2)
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 c17a98958bd..0f128c4cf5e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -57,6 +57,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 @@ -83,6 +87,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): @@ -813,3 +819,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 72600b1ef3a..515c97f9290 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

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 ---
(no changes since v2)
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(-)
Applied to u-boot-dm/next, thanks!

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 v3: - Rebase on top of earlier binman series
Changes in v2: - 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 - Update docs to mention both bl31.bin and bl31.elf
tools/binman/README.entries | 14 ++++++++++++++ tools/binman/etype/atf_bl31.py | 24 ++++++++++++++++++++++++ tools/binman/ftest.py | 9 +++++++++ tools/binman/test/169_atf_bl31.dts | 16 ++++++++++++++++ 4 files changed, 63 insertions(+) create mode 100644 tools/binman/etype/atf_bl31.py create mode 100644 tools/binman/test/169_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 ceeee1dfe63..2b3690e88e0 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 @@ -3550,5 +3552,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('169_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/169_atf_bl31.dts b/tools/binman/test/169_atf_bl31.dts new file mode 100644 index 00000000000..2b7547d70f9 --- /dev/null +++ b/tools/binman/test/169_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"; + }; + }; +};

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 v3: - Rebase on top of earlier binman series
Changes in v2: - 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 - Update docs to mention both bl31.bin and bl31.elf
tools/binman/README.entries | 14 ++++++++++++++ tools/binman/etype/atf_bl31.py | 24 ++++++++++++++++++++++++ tools/binman/ftest.py | 9 +++++++++ tools/binman/test/169_atf_bl31.dts | 16 ++++++++++++++++ 4 files changed, 63 insertions(+) create mode 100644 tools/binman/etype/atf_bl31.py create mode 100644 tools/binman/test/169_atf_bl31.dts
Applied to u-boot-dm/next, thanks!

On 9/1/20 6:13 AM, Simon Glass wrote:
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 v3:
- Rebase on top of earlier binman series
Changes in v2:
- 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
- Update docs to mention both bl31.bin and bl31.elf
tools/binman/README.entries | 14 ++++++++++++++ tools/binman/etype/atf_bl31.py | 24 ++++++++++++++++++++++++ tools/binman/ftest.py | 9 +++++++++ tools/binman/test/169_atf_bl31.dts | 16 ++++++++++++++++ 4 files changed, 63 insertions(+) create mode 100644 tools/binman/etype/atf_bl31.py create mode 100644 tools/binman/test/169_atf_bl31.dts
Does this mean every kind of firmware blob referenced by FIT generator scripts (TEE fimware, SCP firmware, OpenSBI firmware, etc.) needs its own Python package?
What if you need multiple versions of ATF BL31 and TEE firmware for different configurations, like Rockchip currently does? You would need dynamic argument names, but then how do you get them in the Makefile?
This approach doesn't seem very flexible or scalable.
Why not have a generic Entry_blob_named_by_env, with a "filename-var" (or similar) property in addition to "filename"? Then the existing interface of the FIT generator scripts could be maintained without tons of boilerplate.

Hi Samuel,
On Sat, 5 Sep 2020 at 16:57, Samuel Holland samuel@sholland.org wrote:
On 9/1/20 6:13 AM, Simon Glass wrote:
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 v3:
- Rebase on top of earlier binman series
Changes in v2:
- 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
- Update docs to mention both bl31.bin and bl31.elf
tools/binman/README.entries | 14 ++++++++++++++ tools/binman/etype/atf_bl31.py | 24 ++++++++++++++++++++++++ tools/binman/ftest.py | 9 +++++++++ tools/binman/test/169_atf_bl31.dts | 16 ++++++++++++++++ 4 files changed, 63 insertions(+) create mode 100644 tools/binman/etype/atf_bl31.py create mode 100644 tools/binman/test/169_atf_bl31.dts
Does this mean every kind of firmware blob referenced by FIT generator scripts (TEE fimware, SCP firmware, OpenSBI firmware, etc.) needs its own Python package?
No, but in general I would like to do that if possible. It is easier that having random filenames in the description.
What if you need multiple versions of ATF BL31 and TEE firmware for different configurations, like Rockchip currently does?
You can always specify a filename in the node.
You would need dynamic argument names, but then how do you get them in the Makefile?
Using the -a flag.
This approach doesn't seem very flexible or scalable.
Why not have a generic Entry_blob_named_by_env, with a "filename-var" (or similar) property in addition to "filename"? Then the existing interface of the FIT generator scripts could be maintained without tons of boilerplate.
You can do that if you like (see blob-named-by-arg and -a), but the idea with binman is that it knows how to deal with various types of binaries, and each one has a name. This makes it easier to see what is going on, I think.
Also we get documentation about each binary type in README.entries
Regards, Simon

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 v3: - Rebase on top of earlier binman series
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/170_fit_fdt.dts | 55 +++++++++ .../binman/test/171_fit_fdt_missing_prop.dts | 54 +++++++++ 5 files changed, 346 insertions(+), 11 deletions(-) create mode 100644 tools/binman/test/170_fit_fdt.dts create mode 100644 tools/binman/test/171_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 615b2fd8778..c291eb26bad 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_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,'): - 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 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) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2b3690e88e0..78d0e9c2b93 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')
@@ -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() diff --git a/tools/binman/test/170_fit_fdt.dts b/tools/binman/test/170_fit_fdt.dts new file mode 100644 index 00000000000..89142e91017 --- /dev/null +++ b/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 { + }; + }; +}; diff --git a/tools/binman/test/171_fit_fdt_missing_prop.dts b/tools/binman/test/171_fit_fdt_missing_prop.dts new file mode 100644 index 00000000000..c36134715c6 --- /dev/null +++ b/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 { + }; + }; +};

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 v3: - Rebase on top of earlier binman series
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/170_fit_fdt.dts | 55 +++++++++ .../binman/test/171_fit_fdt_missing_prop.dts | 54 +++++++++ 5 files changed, 346 insertions(+), 11 deletions(-) create mode 100644 tools/binman/test/170_fit_fdt.dts create mode 100644 tools/binman/test/171_fit_fdt_missing_prop.dts
Applied to u-boot-dm/next, thanks!

On 9/1/20 6:13 AM, Simon Glass wrote:
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 v3:
- Rebase on top of earlier binman series
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/170_fit_fdt.dts | 55 +++++++++ .../binman/test/171_fit_fdt_missing_prop.dts | 54 +++++++++ 5 files changed, 346 insertions(+), 11 deletions(-) create mode 100644 tools/binman/test/170_fit_fdt.dts create mode 100644 tools/binman/test/171_fit_fdt_missing_prop.dts
[snip]
@@ -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.
Is this output written anywhere? The compiled DTB has the unprocessed template in it, and the final image created by binman requires some dissection to get to the FIT ITS.
+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')
There is no mention of DEFAULT-SEQ here.
+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.
[snip]

On 9/5/20 5:41 PM, Samuel Holland wrote:
On 9/1/20 6:13 AM, Simon Glass wrote:
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 v3:
- Rebase on top of earlier binman series
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/170_fit_fdt.dts | 55 +++++++++ .../binman/test/171_fit_fdt_missing_prop.dts | 54 +++++++++ 5 files changed, 346 insertions(+), 11 deletions(-) create mode 100644 tools/binman/test/170_fit_fdt.dts create mode 100644 tools/binman/test/171_fit_fdt_missing_prop.dts
[snip]
@@ -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.
Is this output written anywhere? The compiled DTB has the unprocessed template in it, and the final image created by binman requires some dissection to get to the FIT ITS.
+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')
There is no mention of DEFAULT-SEQ here.
I see now that you added DEFAULT-SEQ in a later patch, even though it's present in the example above(?). The comment here and on the sunxi dtsi still apply, whichever patch the change should go in.
+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.
[snip]

Hi Samuel,
On Sat, 5 Sep 2020 at 17:19, Samuel Holland samuel@sholland.org wrote:
On 9/5/20 5:41 PM, Samuel Holland wrote:
On 9/1/20 6:13 AM, Simon Glass wrote:
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 v3:
- Rebase on top of earlier binman series
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/170_fit_fdt.dts | 55 +++++++++ .../binman/test/171_fit_fdt_missing_prop.dts | 54 +++++++++ 5 files changed, 346 insertions(+), 11 deletions(-) create mode 100644 tools/binman/test/170_fit_fdt.dts create mode 100644 tools/binman/test/171_fit_fdt_missing_prop.dts
[snip]
@@ -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.
Is this output written anywhere? The compiled DTB has the unprocessed template in it, and the final image created by binman requires some dissection to get to the FIT ITS.
+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')
There is no mention of DEFAULT-SEQ here.
I see now that you added DEFAULT-SEQ in a later patch, even though it's present in the example above(?). The comment here and on the sunxi dtsi still apply, whichever patch the change should go in.
OK. Well the existing has something that is documented in a later patch, unfortunately.
+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.
[snip]
Regards, Simon

Hi Samuel,
On Sat, 5 Sep 2020 at 16:41, Samuel Holland samuel@sholland.org wrote:
On 9/1/20 6:13 AM, Simon Glass wrote:
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 v3:
- Rebase on top of earlier binman series
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/170_fit_fdt.dts | 55 +++++++++ .../binman/test/171_fit_fdt_missing_prop.dts | 54 +++++++++ 5 files changed, 346 insertions(+), 11 deletions(-) create mode 100644 tools/binman/test/170_fit_fdt.dts create mode 100644 tools/binman/test/171_fit_fdt_missing_prop.dts
[snip]
@@ -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.
Is this output written anywhere? The compiled DTB has the unprocessed template in it, and the final image created by binman requires some dissection to get to the FIT ITS.
Yes both the input to mkimage and the output are written out. See the build directory for these files, which are named after the node names.
+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')
There is no mention of DEFAULT-SEQ here.
That feature is added in a later patch. I will make sure it is documented here.
+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.
[snip]
Regards, Simon

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 ---
(no changes since v2)
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))

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 ---
(no changes since v2)
Changes in v2: - Add new patch to support missing external blobs always
Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Applied to u-boot-dm/next, thanks!

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 ---
(no changes since v2)
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 9/1/20 6:14 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
(no changes since v2)
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
Now `make u-boot.itb` doesn't work.
u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at least).
Is there a way to make binman also write the FIT without the SPL? Would that require duplicating the whole binman node?
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
Now `make u-boot-sunxi-with-spl.bin` doesn't work.
This is less of an issue, but still probably breaks some scripts. It breaks mine, at least.
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>;
style: blank line here (and above "atf" and "@config-SEQ" below).
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? */
This address is determined by the physical SRAM layout, so it is per-SoC, not per-board. I would suggest omitting load/entry here entirely, and putting them in the $SOC-u-boot.dtsi.
+#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";
I would expect @DEFAULT-SEQ here.
@config-SEQ {
description = "NAME";
firmware = "uboot";
loadables = "atf";
fdt = "fdt-SEQ";
};
};
};
+#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };

On 9/5/20 6:10 PM, Samuel Holland wrote:
On 9/1/20 6:14 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
(no changes since v2)
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
Now `make u-boot.itb` doesn't work.
u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at least).
Is there a way to make binman also write the FIT without the SPL? Would that require duplicating the whole binman node?
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
Now `make u-boot-sunxi-with-spl.bin` doesn't work.
This is less of an issue, but still probably breaks some scripts. It breaks mine, at least.
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>;
style: blank line here (and above "atf" and "@config-SEQ" below).
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? */
This address is determined by the physical SRAM layout, so it is per-SoC, not per-board. I would suggest omitting load/entry here entirely, and putting them in the $SOC-u-boot.dtsi.
+#ifdef CONFIG_MACH_SUN50I_H6
load = <0x104000>;
entry = <0x104000>;
+#else
load = <0x44000>;
entry = <0x44000>;
+#endif
atf-bl31 {
Another regression: you need
filename = "bl31.bin";
here to match the behavior when the environment variable is not defined.
};
};
@fdt-SEQ {
description = "NAME";
type = "flat_dt";
compression = "none";
};
};
configurations {
default = "config-1";
I would expect @DEFAULT-SEQ here.
@config-SEQ {
description = "NAME";
firmware = "uboot";
loadables = "atf";
fdt = "fdt-SEQ";
};
};
};
+#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };

Hi Samuel,
On Sat, 5 Sep 2020 at 17:42, Samuel Holland samuel@sholland.org wrote:
On 9/5/20 6:10 PM, Samuel Holland wrote:
On 9/1/20 6:14 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
(no changes since v2)
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
Now `make u-boot.itb` doesn't work.
u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at least).
Is there a way to make binman also write the FIT without the SPL? Would that require duplicating the whole binman node?
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
Now `make u-boot-sunxi-with-spl.bin` doesn't work.
This is less of an issue, but still probably breaks some scripts. It breaks mine, at least.
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>;
style: blank line here (and above "atf" and "@config-SEQ" below).
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? */
This address is determined by the physical SRAM layout, so it is per-SoC, not per-board. I would suggest omitting load/entry here entirely, and putting them in the $SOC-u-boot.dtsi.
+#ifdef CONFIG_MACH_SUN50I_H6
load = <0x104000>;
entry = <0x104000>;
+#else
load = <0x44000>;
entry = <0x44000>;
+#endif
atf-bl31 {
Another regression: you need
filename = "bl31.bin";
here to match the behavior when the environment variable is not defined.
That would be better to go in the code. If U-Boot passes an empty filename to binman then it needs special handling, if we want a default.
};
};
@fdt-SEQ {
description = "NAME";
type = "flat_dt";
compression = "none";
};
};
configurations {
default = "config-1";
I would expect @DEFAULT-SEQ here.
@config-SEQ {
description = "NAME";
firmware = "uboot";
loadables = "atf";
fdt = "fdt-SEQ";
};
};
};
+#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };
Regards, Simon

Hi Samuel,
On Sat, 5 Sep 2020 at 17:10, Samuel Holland samuel@sholland.org wrote:
On 9/1/20 6:14 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
(no changes since v2)
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
Now `make u-boot.itb` doesn't work.
u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at least).
It is generated, just with a different filename.
Is there a way to make binman also write the FIT without the SPL? Would that require duplicating the whole binman node?
Yes it would. We could get more complicated and allow an image to build on another perhaps. I'm not sure what is easiest here.
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
Now `make u-boot-sunxi-with-spl.bin` doesn't work.
This is less of an issue, but still probably breaks some scripts. It breaks mine, at least.
Why do you specify a target? Doesn't it build the file without the target?
One problem with buildman is that there is no definitely of what files it will produce when run, or at least there is, but it is in the binman description itself.
This means that 'make clean' doesn't work fully, for example. I can think of a few ways to implement this. One would be to put a list of target files into a text file and have 'make clean' use that. We could also have an option to tell binman to produce a list of files it would generate if run. Then we might be able to tell binman to generate a particular file.
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>;
style: blank line here (and above "atf" and "@config-SEQ" below).
I'll send a fix-up patch.
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? */
This address is determined by the physical SRAM layout, so it is per-SoC, not per-board. I would suggest omitting load/entry here entirely, and putting them in the $SOC-u-boot.dtsi.
That's fine also. I just don't like having #ifdefs here.
+#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";
I would expect @DEFAULT-SEQ here.
For some reason the old script just used config-1.
@config-SEQ {
description = "NAME";
firmware = "uboot";
loadables = "atf";
fdt = "fdt-SEQ";
};
};
};
+#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };
Regards, Simon

Simon,
On 9/5/20 7:18 PM, Simon Glass wrote:
On Sat, 5 Sep 2020 at 17:10, Samuel Holland samuel@sholland.org wrote:
On 9/1/20 6:14 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
(no changes since v2)
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
Now `make u-boot.itb` doesn't work.
u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at least).
It is generated, just with a different filename.
Thanks. From looking at the code and comparing with u-boot-sunxi-with-spl.bin, it seems that u-boot-sunxi-with-spl.fit.fit is the "final" ITB file. My only hesitation is that it seems like an implementation detail, but I guess it's fine for now.
Is there a way to make binman also write the FIT without the SPL? Would that require duplicating the whole binman node?
Yes it would. We could get more complicated and allow an image to build on another perhaps. I'm not sure what is easiest here.
u-boot-sunxi-with-spl.fit.fit is good enough for my purposes, but others may have an opinion.
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
Now `make u-boot-sunxi-with-spl.bin` doesn't work.
This is less of an issue, but still probably breaks some scripts. It breaks mine, at least.
Why do you specify a target? Doesn't it build the file without the target?
Yes, the file is built either way. I provide a specific target to avoid building other files I don't need -- for example, a plain `make` used to rebuild the hello world EFI application every time.
One problem with buildman is that there is no definitely of what files it will produce when run, or at least there is, but it is in the binman description itself.
This means that 'make clean' doesn't work fully, for example. I can think of a few ways to implement this. One would be to put a list of target files into a text file and have 'make clean' use that. We could also have an option to tell binman to produce a list of files it would generate if run. Then we might be able to tell binman to generate a particular file.
Yes, having binman generate a Makefile fragment would be ideal. That would also solve binman being forced to re-run every `make` invocation. For now a plain `make`/`make all` is fine. The forced rebuilds seem to be better under control now.
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>;
style: blank line here (and above "atf" and "@config-SEQ" below).
I'll send a fix-up patch.
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? */
This address is determined by the physical SRAM layout, so it is per-SoC, not per-board. I would suggest omitting load/entry here entirely, and putting them in the $SOC-u-boot.dtsi.
That's fine also. I just don't like having #ifdefs here.
I tried implementing this, and I ran into the problem that sunxi doesn't define CONFIG_SYS_VENDOR. So this file (from CONFIG_SYS_SOC) and the board-DTS-specific file are the only two magic u-boot.dtsi files available, and I don't think we want a u-boot.dtsi for every board.
A possible improvement would be defining BL31_ADDR (and later SCP_ADDR) as macros at the top of the file, like the shell script does.
+#ifdef CONFIG_MACH_SUN50I_H6
load = <0x104000>;
entry = <0x104000>;
+#else
load = <0x44000>;
entry = <0x44000>;
+#endif
atf-bl31 {
Another regression: you need
filename = "bl31.bin";
here to match the behavior when the environment variable is not defined.
That would be better to go in the code. If U-Boot passes an empty filename to binman then it needs special handling, if we want a default.
(merging the threads here)
What special handling are you thinking of? In blob_named_by_arg.py, the default filename is only overwritten from the arg if the arg is not empty. So the code already does the right thing, matching the baseline script: if no environment variable was defined, use a file with the default name in the current directory. It just needs a default name defined here.
};
};
@fdt-SEQ {
description = "NAME";
type = "flat_dt";
compression = "none";
};
};
configurations {
default = "config-1";
I would expect @DEFAULT-SEQ here.
For some reason the old script just used config-1.
Probably because determining the index of CONFIG_DEFAULT_DEVICE_TREE in CONFIG_OF_LIST in POSIX sh is nontrivial. sunxi SPL always explicitly chooses the config matching CONFIG_DEFAULT_DEVICE_TREE, so "1" vs "DEFAULT-SEQ" would only make a difference if CONFIG_DEFAULT_DEVICE_TREE was not included in CONFIG_OF_LIST. (If using "DEFAULT-SEQ", that would be an error, so it would require that the SPL and FIT were built separately).
@config-SEQ {
description = "NAME";
firmware = "uboot";
loadables = "atf";
fdt = "fdt-SEQ";
};
};
};
+#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };
Regards, Simon
Regards, Samuel

Hi Samuel,
On Sat, 5 Sep 2020 at 19:49, Samuel Holland samuel@sholland.org wrote:
Simon,
On 9/5/20 7:18 PM, Simon Glass wrote:
On Sat, 5 Sep 2020 at 17:10, Samuel Holland samuel@sholland.org wrote:
On 9/1/20 6:14 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
(no changes since v2)
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
Now `make u-boot.itb` doesn't work.
u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at least).
It is generated, just with a different filename.
Thanks. From looking at the code and comparing with u-boot-sunxi-with-spl.bin, it seems that u-boot-sunxi-with-spl.fit.fit is the "final" ITB file. My only hesitation is that it seems like an implementation detail, but I guess it's fine for now.
Is there a way to make binman also write the FIT without the SPL? Would that require duplicating the whole binman node?
Yes it would. We could get more complicated and allow an image to build on another perhaps. I'm not sure what is easiest here.
u-boot-sunxi-with-spl.fit.fit is good enough for my purposes, but others may have an opinion.
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
Now `make u-boot-sunxi-with-spl.bin` doesn't work.
This is less of an issue, but still probably breaks some scripts. It breaks mine, at least.
Why do you specify a target? Doesn't it build the file without the target?
Yes, the file is built either way. I provide a specific target to avoid building other files I don't need -- for example, a plain `make` used to rebuild the hello world EFI application every time.
One problem with buildman is that there is no definitely of what files it will produce when run, or at least there is, but it is in the binman description itself.
This means that 'make clean' doesn't work fully, for example. I can think of a few ways to implement this. One would be to put a list of target files into a text file and have 'make clean' use that. We could also have an option to tell binman to produce a list of files it would generate if run. Then we might be able to tell binman to generate a particular file.
Yes, having binman generate a Makefile fragment would be ideal. That would also solve binman being forced to re-run every `make` invocation. For now a plain `make`/`make all` is fine. The forced rebuilds seem to be better under control now.
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>;
style: blank line here (and above "atf" and "@config-SEQ" below).
I'll send a fix-up patch.
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? */
This address is determined by the physical SRAM layout, so it is per-SoC, not per-board. I would suggest omitting load/entry here entirely, and putting them in the $SOC-u-boot.dtsi.
That's fine also. I just don't like having #ifdefs here.
I tried implementing this, and I ran into the problem that sunxi doesn't define CONFIG_SYS_VENDOR. So this file (from CONFIG_SYS_SOC) and the board-DTS-specific file are the only two magic u-boot.dtsi files available, and I don't think we want a u-boot.dtsi for every board.
No that would be painful.
A possible improvement would be defining BL31_ADDR (and later SCP_ADDR) as macros at the top of the file, like the shell script does.
Yes that might be better I suppose.
+#ifdef CONFIG_MACH_SUN50I_H6
load = <0x104000>;
entry = <0x104000>;
+#else
load = <0x44000>;
entry = <0x44000>;
+#endif
atf-bl31 {
Another regression: you need
filename = "bl31.bin";
here to match the behavior when the environment variable is not defined.
That would be better to go in the code. If U-Boot passes an empty filename to binman then it needs special handling, if we want a default.
(merging the threads here)
What special handling are you thinking of? In blob_named_by_arg.py, the default filename is only overwritten from the arg if the arg is not empty. So the code already does the right thing, matching the baseline script: if no environment variable was defined, use a file with the default name in the current directory. It just needs a default name defined here.
Well Entry_aft_bl31 uses Entry_named_blob_by_arg which uses Entry_blob which reads the filename. So we would need to do some refactoring to avoid overriding the default filename, as I read it.
};
};
@fdt-SEQ {
description = "NAME";
type = "flat_dt";
compression = "none";
};
};
configurations {
default = "config-1";
I would expect @DEFAULT-SEQ here.
For some reason the old script just used config-1.
Probably because determining the index of CONFIG_DEFAULT_DEVICE_TREE in CONFIG_OF_LIST in POSIX sh is nontrivial. sunxi SPL always explicitly chooses the config matching CONFIG_DEFAULT_DEVICE_TREE, so "1" vs "DEFAULT-SEQ" would only make a difference if CONFIG_DEFAULT_DEVICE_TREE was not included in CONFIG_OF_LIST. (If using "DEFAULT-SEQ", that would be an error, so it would require that the SPL and FIT were built separately).
OK.
@config-SEQ {
description = "NAME";
firmware = "uboot";
loadables = "atf";
fdt = "fdt-SEQ";
};
};
};
+#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif }; };
Regards, Simon

Hi,
On 06. 09. 20 2:18, Simon Glass wrote:
Hi Samuel,
On Sat, 5 Sep 2020 at 17:10, Samuel Holland samuel@sholland.org wrote:
On 9/1/20 6:14 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
(no changes since v2)
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
Now `make u-boot.itb` doesn't work.
u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at least).
It is generated, just with a different filename.
Is there a way to make binman also write the FIT without the SPL? Would that require duplicating the whole binman node?
Yes it would. We could get more complicated and allow an image to build on another perhaps. I'm not sure what is easiest here.
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
Now `make u-boot-sunxi-with-spl.bin` doesn't work.
This is less of an issue, but still probably breaks some scripts. It breaks mine, at least.
Why do you specify a target? Doesn't it build the file without the target?
One problem with buildman is that there is no definitely of what files it will produce when run, or at least there is, but it is in the binman description itself.
This means that 'make clean' doesn't work fully, for example. I can think of a few ways to implement this. One would be to put a list of target files into a text file and have 'make clean' use that. We could also have an option to tell binman to produce a list of files it would generate if run. Then we might be able to tell binman to generate a particular file.
Why not just to generate all binman images to specific folder?
Thanks, Michal

Hi Michal,
On Mon, 7 Sep 2020 at 07:02, Michal Simek michal.simek@xilinx.com wrote:
Hi,
On 06. 09. 20 2:18, Simon Glass wrote:
Hi Samuel,
On Sat, 5 Sep 2020 at 17:10, Samuel Holland samuel@sholland.org wrote:
On 9/1/20 6:14 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
(no changes since v2)
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
Now `make u-boot.itb` doesn't work.
u-boot.itb is helpful to have because, with CONFIG_OF_LIST, it can be shared across all boards of a platform. Only SPL is board-specific (on arm64 sunxi, at least).
It is generated, just with a different filename.
Is there a way to make binman also write the FIT without the SPL? Would that require duplicating the whole binman node?
Yes it would. We could get more complicated and allow an image to build on another perhaps. I'm not sure what is easiest here.
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
Now `make u-boot-sunxi-with-spl.bin` doesn't work.
This is less of an issue, but still probably breaks some scripts. It breaks mine, at least.
Why do you specify a target? Doesn't it build the file without the target?
One problem with buildman is that there is no definitely of what files it will produce when run, or at least there is, but it is in the binman description itself.
This means that 'make clean' doesn't work fully, for example. I can think of a few ways to implement this. One would be to put a list of target files into a text file and have 'make clean' use that. We could also have an option to tell binman to produce a list of files it would generate if run. Then we might be able to tell binman to generate a particular file.
Why not just to generate all binman images to specific folder?
We need to put at least some of them in the build directory. We could perhaps have a 'binman' subdir of that where intermediate images go.
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 v3: - Rebase on top of earlier binman series
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 | 41 +++++++++++++++++-- .../test/{170_fit_fdt.dts => 172_fit_fdt.dts} | 2 +- 5 files changed, 67 insertions(+), 4 deletions(-) rename tools/binman/test/{170_fit_fdt.dts => 172_fit_fdt.dts} (95%)
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 c291eb26bad..bf73bfb9a56 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 78d0e9c2b93..a269a7497cb 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3602,7 +3602,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) @@ -3615,9 +3615,10 @@ class TestFunctional(unittest.TestCase):
entry_args = { 'of-list': 'test-fdt1 test-fdt2', + 'default-dt': 'test-fdt2', } data = self._DoReadFileDtb( - '170_fit_fdt.dts', + '172_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):]) @@ -3639,7 +3640,7 @@ class TestFunctional(unittest.TestCase): 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._DoReadFile('172_fit_fdt.dts') self.assertIn("Generator node requires 'of-list' entry argument", str(e.exception))
@@ -3657,5 +3658,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('172_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( + '172_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( + '172_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/170_fit_fdt.dts b/tools/binman/test/172_fit_fdt.dts similarity index 95% rename from tools/binman/test/170_fit_fdt.dts rename to tools/binman/test/172_fit_fdt.dts index 89142e91017..99d710c57e9 100644 --- a/tools/binman/test/170_fit_fdt.dts +++ b/tools/binman/test/172_fit_fdt.dts @@ -40,7 +40,7 @@ };
configurations { - default = "config-1"; + default = "@config-DEFAULT-SEQ"; @config-SEQ { description = "conf-NAME.dtb"; firmware = "uboot";

When an external blob is missing it can be quite confusing for the user. Add a way to provide a help message that is shown.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Add a way to show help messages for missing blobs
tools/binman/README | 6 ++ tools/binman/control.py | 69 +++++++++++++++++++++- tools/binman/entry.py | 9 +++ tools/binman/ftest.py | 18 +++++- tools/binman/missing-blob-help | 11 ++++ tools/binman/test/168_fit_missing_blob.dts | 9 ++- 6 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 tools/binman/missing-blob-help
diff --git a/tools/binman/README b/tools/binman/README index 37ee3fc2d3b..f7bf285a915 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -343,6 +343,12 @@ compress: Sets the compression algortihm to use (for blobs only). See the entry documentation for details.
+missing-msg: + Sets the tag of the message to show if this entry is missing. This is + used for external blobs. When they are missing it is helpful to show + information about what needs to be fixed. See missing-blob-help for the + message for each tag. + The attributes supported for images and sections are described below. Several are similar to those for entries.
diff --git a/tools/binman/control.py b/tools/binman/control.py index 15bfac582a7..ee5771e7292 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -9,6 +9,7 @@ from collections import OrderedDict import glob import os import pkg_resources +import re
import sys from patman import tools @@ -22,6 +23,11 @@ from patman import tout # Make this global so that it can be referenced from tests images = OrderedDict()
+# Help text for each type of missing blob, dict: +# key: Value of the entry's 'missing-msg' or entry name +# value: Text for the help +missing_blob_help = {} + def _ReadImageDesc(binman_node): """Read the image descriptions from the /binman node
@@ -54,6 +60,66 @@ def _FindBinmanNode(dtb): return node return None
+def _ReadMissingBlobHelp(): + """Read the missing-blob-help file + + This file containins help messages explaining what to do when external blobs + are missing. + + Returns: + Dict: + key: Message tag (str) + value: Message text (str) + """ + + def _FinishTag(tag, msg, result): + if tag: + result[tag] = msg.rstrip() + tag = None + msg = '' + return tag, msg + + my_data = pkg_resources.resource_string(__name__, 'missing-blob-help') + re_tag = re.compile('^([-a-z0-9]+):$') + result = {} + tag = None + msg = '' + for line in my_data.decode('utf-8').splitlines(): + if not line.startswith('#'): + m_tag = re_tag.match(line) + if m_tag: + _, msg = _FinishTag(tag, msg, result) + tag = m_tag.group(1) + elif tag: + msg += line + '\n' + _FinishTag(tag, msg, result) + return result + +def _ShowBlobHelp(path, text): + tout.Warning('\n%s:' % path) + for line in text.splitlines(): + tout.Warning(' %s' % line) + +def _ShowHelpForMissingBlobs(missing_list): + """Show help for each missing blob to help the user take action + + Args: + missing_list: List of Entry objects to show help for + """ + global missing_blob_help + + if not missing_blob_help: + missing_blob_help = _ReadMissingBlobHelp() + + for entry in missing_list: + tags = entry.GetHelpTags() + + # Show the first match help message + for tag in tags: + if tag in missing_blob_help: + _ShowBlobHelp(entry._node.path, missing_blob_help[tag]) + break + def GetEntryModules(include_testing=True): """Get a set of entry class implementations
@@ -478,6 +544,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, if missing_list: tout.Warning("Image '%s' is missing external blobs and is non-functional: %s" % (image.name, ' '.join([e.name for e in missing_list]))) + _ShowHelpForMissingBlobs(missing_list) return bool(missing_list)
@@ -563,7 +630,7 @@ def Binman(args): tools.WriteFile(dtb_item._fname, dtb_item.GetContents())
if missing: - tout.Warning("Some images are invalid") + tout.Warning("\nSome images are invalid") finally: tools.FinaliseOutputDir() finally: diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 0f128c4cf5e..f7adc3b1abb 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -178,6 +178,7 @@ class Entry(object): self.align_end = fdt_util.GetInt(self._node, 'align-end') self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset') self.expand_size = fdt_util.GetBool(self._node, 'expand-size') + self.missing_msg = fdt_util.GetString(self._node, 'missing-msg')
def GetDefaultFilename(self): return None @@ -827,3 +828,11 @@ features to produce new behaviours. True if allowed, False if not allowed """ return self.allow_missing + + def GetHelpTags(self): + """Get the tags use for missing-blob help + + Returns: + list of possible tags, most desirable first + """ + return list(filter(None, [self.missing_msg, self.name, self.etype])) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index a269a7497cb..95b17d0b749 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3561,7 +3561,7 @@ class TestFunctional(unittest.TestCase): self._DoTestFile('168_fit_missing_blob.dts', allow_missing=True) err = stderr.getvalue() - self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext") + self.assertRegex(err, "Image 'main-section'.*missing.*: atf-bl31")
def testBlobNamedByArgMissing(self): """Test handling of a missing entry arg""" @@ -3692,5 +3692,21 @@ class TestFunctional(unittest.TestCase): self.assertIn("default-dt entry argument 'test-fdt3' not found in fdt list: test-fdt1, test-fdt2", str(e.exception))
+ def testFitExtblobMissingHelp(self): + """Test display of help messages when an external blob is missing""" + control.missing_blob_help = control._ReadMissingBlobHelp() + control.missing_blob_help['wibble'] = 'Wibble test' + control.missing_blob_help['another'] = 'Another test' + with test_util.capture_sys_output() as (stdout, stderr): + self._DoTestFile('168_fit_missing_blob.dts', + allow_missing=True) + err = stderr.getvalue() + + # We can get the tag from the name, the type or the missing-msg + # property. Check all three. + self.assertIn('You may need to build ARM Trusted', err) + self.assertIn('Wibble test', err) + self.assertIn('Another test', err) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help new file mode 100644 index 00000000000..3de195c23c8 --- /dev/null +++ b/tools/binman/missing-blob-help @@ -0,0 +1,11 @@ +# This file contains help messages for missing external blobs. Each message has +# a tag (MUST be just lower-case text, digits and hyphens) starting in column 1, +# followed by a colon (:) to indicate its start. The message can include any +# number of lines, including blank lines. +# +# When looking for a tag, Binman uses the value of 'missing-msg' for the entry, +# the entry name or the entry type, in that order + +atf-bl31: +See the documentation for your board. You may need to build ARM Trusted +Firmware and build with BL31=/path/to/bl31.bin diff --git a/tools/binman/test/168_fit_missing_blob.dts b/tools/binman/test/168_fit_missing_blob.dts index e007aa41d8a..15f6cc07e5d 100644 --- a/tools/binman/test/168_fit_missing_blob.dts +++ b/tools/binman/test/168_fit_missing_blob.dts @@ -29,9 +29,16 @@ hash-2 { algo = "sha1"; }; - blob-ext { + atf-bl31 { filename = "missing"; }; + cros-ec-rw { + type = "atf-bl31"; + missing-msg = "wibble"; + }; + another { + type = "atf-bl31"; + }; }; }; };

Add a special help message pointing to the relevant README.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/arm/dts/sunxi-u-boot.dtsi | 1 + tools/binman/missing-blob-help | 4 ++++ 2 files changed, 5 insertions(+)
diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi index 1d1c3691099..c97943b3c19 100644 --- a/arch/arm/dts/sunxi-u-boot.dtsi +++ b/arch/arm/dts/sunxi-u-boot.dtsi @@ -48,6 +48,7 @@ entry = <0x44000>; #endif atf-bl31 { + missing-msg = "atf-bl31-sunxi"; }; };
diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help index 3de195c23c8..7cf1c346101 100644 --- a/tools/binman/missing-blob-help +++ b/tools/binman/missing-blob-help @@ -9,3 +9,7 @@ atf-bl31: See the documentation for your board. You may need to build ARM Trusted Firmware and build with BL31=/path/to/bl31.bin + +atf-bl31-sunxi: +Please read the section on ARM Trusted Firmware (ATF) in +board/sunxi/README.sunxi64

Hi Simon,
On 01. 09. 20 13:13, Simon Glass wrote:
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.
Note: This series is available at u-boot-dm/binman-working and is based on u-boot-dm/testing
I continue on testing this series on ZynqMP and reach one issue. One of our usecase is to put u-boot on memory above 32bit address space. To be accurate to this memory range. reg = <0x8 0x00000000 0x0 0x80000000>;
One way to get there is to setup TEXT_BASE to for example 0x808000000. Then u-boot.itb fragment for binmap looks like this.
u-boot-itb { filename = "u-boot.itb"; fit { description = "FIT image with ATF support"; fit,fdt-list = "of-list"; #address-cells = <2>;
images { uboot { description = "U-Boot (64-bit)"; type = "firmware"; os = "u-boot"; arch = "arm64"; compression = "none"; load = <8 0x8000000>; entry = <8 0x8000000>; u-boot-nodtb { }; }; ... The key properties are load/entry. In example above I have address-cells = 2 and entries written by hand and it works fine. But I want to use CONFIG_SYS_TEXT_BASE there which is already in 64bit format.
When this is used I get Error: arch/arm/dts/zynqmp-u-boot.dtsi:31.14-25 Value out of range for 32-bit array element Error: arch/arm/dts/zynqmp-u-boot.dtsi:32.15-26 Value out of range for 32-bit array element
Which is understandable but my question is how to handle 64bit addresses which are properly filled via Kconfig?
I have the same issue with one more DT property.
Thanks, Michal

Hi Michal,
On Wed, 2 Sep 2020 at 04:27, Michal Simek michal.simek@xilinx.com wrote:
Hi Simon,
On 01. 09. 20 13:13, Simon Glass wrote:
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.
Note: This series is available at u-boot-dm/binman-working and is based on u-boot-dm/testing
I continue on testing this series on ZynqMP and reach one issue. One of our usecase is to put u-boot on memory above 32bit address space. To be accurate to this memory range. reg = <0x8 0x00000000 0x0 0x80000000>;
One way to get there is to setup TEXT_BASE to for example 0x808000000. Then u-boot.itb fragment for binmap looks like this.
u-boot-itb { filename = "u-boot.itb"; fit { description = "FIT image with ATF support"; fit,fdt-list = "of-list"; #address-cells = <2>; images { uboot { description = "U-Boot (64-bit)"; type = "firmware"; os = "u-boot"; arch = "arm64"; compression = "none"; load = <8 0x8000000>; entry = <8 0x8000000>; u-boot-nodtb { }; };
... The key properties are load/entry. In example above I have address-cells = 2 and entries written by hand and it works fine. But I want to use CONFIG_SYS_TEXT_BASE there which is already in 64bit format.
When this is used I get Error: arch/arm/dts/zynqmp-u-boot.dtsi:31.14-25 Value out of range for 32-bit array element Error: arch/arm/dts/zynqmp-u-boot.dtsi:32.15-26 Value out of range for 32-bit array element
Which is understandable but my question is how to handle 64bit addresses which are properly filled via Kconfig?
If I understand the problem correctly, you can add 64-bit values like this:
load = /bits/ 64 <CONFIG_SYS_TEXT_BASE>;
I have the same issue with one more DT property.
Thanks, Michal

Hi Simon,
On 02. 09. 20 19:07, Simon Glass wrote:
Hi Michal,
On Wed, 2 Sep 2020 at 04:27, Michal Simek michal.simek@xilinx.com wrote:
Hi Simon,
On 01. 09. 20 13:13, Simon Glass wrote:
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.
Note: This series is available at u-boot-dm/binman-working and is based on u-boot-dm/testing
I continue on testing this series on ZynqMP and reach one issue. One of our usecase is to put u-boot on memory above 32bit address space. To be accurate to this memory range. reg = <0x8 0x00000000 0x0 0x80000000>;
One way to get there is to setup TEXT_BASE to for example 0x808000000. Then u-boot.itb fragment for binmap looks like this.
u-boot-itb { filename = "u-boot.itb"; fit { description = "FIT image with ATF support"; fit,fdt-list = "of-list"; #address-cells = <2>; images { uboot { description = "U-Boot (64-bit)"; type = "firmware"; os = "u-boot"; arch = "arm64"; compression = "none"; load = <8 0x8000000>; entry = <8 0x8000000>; u-boot-nodtb { }; };
... The key properties are load/entry. In example above I have address-cells = 2 and entries written by hand and it works fine. But I want to use CONFIG_SYS_TEXT_BASE there which is already in 64bit format.
When this is used I get Error: arch/arm/dts/zynqmp-u-boot.dtsi:31.14-25 Value out of range for 32-bit array element Error: arch/arm/dts/zynqmp-u-boot.dtsi:32.15-26 Value out of range for 32-bit array element
Which is understandable but my question is how to handle 64bit addresses which are properly filled via Kconfig?
If I understand the problem correctly, you can add 64-bit values like this:
load = /bits/ 64 <CONFIG_SYS_TEXT_BASE>;
ok. This works - thanks for that.
I have sent 2 patches for adding 64bit support to use this feature.
Thanks, Michal
participants (3)
-
Michal Simek
-
Samuel Holland
-
Simon Glass