* [PATCH v3 01/12] binman: Allow entry args to be required
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
@ 2020-09-01 11:13 ` Simon Glass
2020-09-01 11:13 ` [PATCH v3 02/12] binman: Fix up a few missing comments Simon Glass
` (18 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:13 UTC (permalink / raw)
To: u-boot
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()
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 02/12] binman: Fix up a few missing comments
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
2020-09-01 11:13 ` [PATCH v3 01/12] binman: Allow entry args to be required Simon Glass
@ 2020-09-01 11:13 ` Simon Glass
2020-09-01 11:13 ` [PATCH v3 03/12] libfdt: Detected out-of-space with fdt_finish() Simon Glass
` (17 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:13 UTC (permalink / raw)
To: u-boot
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
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 03/12] libfdt: Detected out-of-space with fdt_finish()
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
2020-09-01 11:13 ` [PATCH v3 01/12] binman: Allow entry args to be required Simon Glass
2020-09-01 11:13 ` [PATCH v3 02/12] binman: Fix up a few missing comments Simon Glass
@ 2020-09-01 11:13 ` Simon Glass
2020-09-01 11:13 ` [PATCH v3 04/12] binman: Move 'external' support into base class Simon Glass
` (16 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:13 UTC (permalink / raw)
To: u-boot
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):
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 04/12] binman: Move 'external' support into base class
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (2 preceding siblings ...)
2020-09-01 11:13 ` [PATCH v3 03/12] libfdt: Detected out-of-space with fdt_finish() Simon Glass
@ 2020-09-01 11:13 ` Simon Glass
2020-09-01 11:13 ` [PATCH v3 05/12] binman: Add support for ATF BL31 Simon Glass
` (15 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:13 UTC (permalink / raw)
To: u-boot
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
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 05/12] binman: Add support for ATF BL31
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (3 preceding siblings ...)
2020-09-01 11:13 ` [PATCH v3 04/12] binman: Move 'external' support into base class Simon Glass
@ 2020-09-01 11:13 ` Simon Glass
2020-09-05 22:57 ` Samuel Holland
2020-09-01 11:13 ` [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs Simon Glass
` (14 subsequent siblings)
19 siblings, 1 reply; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:13 UTC (permalink / raw)
To: u-boot
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";
+ };
+ };
+};
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 05/12] binman: Add support for ATF BL31
2020-09-01 11:13 ` [PATCH v3 05/12] binman: Add support for ATF BL31 Simon Glass
@ 2020-09-05 22:57 ` Samuel Holland
2020-09-06 0:17 ` Simon Glass
0 siblings, 1 reply; 39+ messages in thread
From: Samuel Holland @ 2020-09-05 22:57 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 05/12] binman: Add support for ATF BL31
2020-09-05 22:57 ` Samuel Holland
@ 2020-09-06 0:17 ` Simon Glass
0 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-06 0:17 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (4 preceding siblings ...)
2020-09-01 11:13 ` [PATCH v3 05/12] binman: Add support for ATF BL31 Simon Glass
@ 2020-09-01 11:13 ` Simon Glass
2020-09-05 22:41 ` Samuel Holland
2020-09-01 11:14 ` [PATCH v3 07/12] Makefile: Support missing external blobs always Simon Glass
` (13 subsequent siblings)
19 siblings, 1 reply; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:13 UTC (permalink / raw)
To: u-boot
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 at 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 at 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 {
+ };
+ };
+};
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs
2020-09-01 11:13 ` [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs Simon Glass
@ 2020-09-05 22:41 ` Samuel Holland
2020-09-05 23:19 ` Samuel Holland
2020-09-06 0:18 ` Simon Glass
0 siblings, 2 replies; 39+ messages in thread
From: Samuel Holland @ 2020-09-05 22:41 UTC (permalink / raw)
To: u-boot
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]
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs
2020-09-05 22:41 ` Samuel Holland
@ 2020-09-05 23:19 ` Samuel Holland
2020-09-06 0:17 ` Simon Glass
2020-09-06 0:18 ` Simon Glass
1 sibling, 1 reply; 39+ messages in thread
From: Samuel Holland @ 2020-09-05 23:19 UTC (permalink / raw)
To: u-boot
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]
>
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs
2020-09-05 23:19 ` Samuel Holland
@ 2020-09-06 0:17 ` Simon Glass
0 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-06 0:17 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs
2020-09-05 22:41 ` Samuel Holland
2020-09-05 23:19 ` Samuel Holland
@ 2020-09-06 0:18 ` Simon Glass
1 sibling, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-06 0:18 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 07/12] Makefile: Support missing external blobs always
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (5 preceding siblings ...)
2020-09-01 11:13 ` [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs Simon Glass
@ 2020-09-01 11:14 ` Simon Glass
2020-09-01 11:14 ` [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman Simon Glass
` (12 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:14 UTC (permalink / raw)
To: u-boot
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))
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (6 preceding siblings ...)
2020-09-01 11:14 ` [PATCH v3 07/12] Makefile: Support missing external blobs always Simon Glass
@ 2020-09-01 11:14 ` Simon Glass
2020-09-05 23:10 ` Samuel Holland
2020-09-01 11:14 ` [PATCH v3 09/12] sunxi: Drop the FIT-generator script Simon Glass
` (11 subsequent siblings)
19 siblings, 1 reply; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:14 UTC (permalink / raw)
To: u-boot
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
};
};
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-01 11:14 ` [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman Simon Glass
@ 2020-09-05 23:10 ` Samuel Holland
2020-09-05 23:42 ` Samuel Holland
2020-09-06 0:18 ` Simon Glass
0 siblings, 2 replies; 39+ messages in thread
From: Samuel Holland @ 2020-09-05 23:10 UTC (permalink / raw)
To: u-boot
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 {
> + };
> + };
> +
> + @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
> };
> };
>
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-05 23:10 ` Samuel Holland
@ 2020-09-05 23:42 ` Samuel Holland
2020-09-06 0:17 ` Simon Glass
2020-09-06 0:18 ` Simon Glass
1 sibling, 1 reply; 39+ messages in thread
From: Samuel Holland @ 2020-09-05 23:42 UTC (permalink / raw)
To: u-boot
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.
>> + };
>> + };
>> +
>> + @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
>> };
>> };
>>
>
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-05 23:42 ` Samuel Holland
@ 2020-09-06 0:17 ` Simon Glass
0 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-06 0:17 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-05 23:10 ` Samuel Holland
2020-09-05 23:42 ` Samuel Holland
@ 2020-09-06 0:18 ` Simon Glass
2020-09-06 1:49 ` Samuel Holland
2020-09-07 13:01 ` Michal Simek
1 sibling, 2 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-06 0:18 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-06 0:18 ` Simon Glass
@ 2020-09-06 1:49 ` Samuel Holland
2020-09-06 2:22 ` Simon Glass
2020-09-07 13:01 ` Michal Simek
1 sibling, 1 reply; 39+ messages in thread
From: Samuel Holland @ 2020-09-06 1:49 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-06 1:49 ` Samuel Holland
@ 2020-09-06 2:22 ` Simon Glass
0 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-06 2:22 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-06 0:18 ` Simon Glass
2020-09-06 1:49 ` Samuel Holland
@ 2020-09-07 13:01 ` Michal Simek
2020-09-07 13:57 ` Simon Glass
1 sibling, 1 reply; 39+ messages in thread
From: Michal Simek @ 2020-09-07 13:01 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman
2020-09-07 13:01 ` Michal Simek
@ 2020-09-07 13:57 ` Simon Glass
0 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-07 13:57 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 09/12] sunxi: Drop the FIT-generator script
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (7 preceding siblings ...)
2020-09-01 11:14 ` [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman Simon Glass
@ 2020-09-01 11:14 ` Simon Glass
2020-09-01 11:14 ` [PATCH v3 10/12] binman: Allow selecting default FIT configuration Simon Glass
` (10 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:14 UTC (permalink / raw)
To: u-boot
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
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 10/12] binman: Allow selecting default FIT configuration
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (8 preceding siblings ...)
2020-09-01 11:14 ` [PATCH v3 09/12] sunxi: Drop the FIT-generator script Simon Glass
@ 2020-09-01 11:14 ` Simon Glass
2020-09-01 11:14 ` [PATCH v3 11/12] binman: Support help messages for missing blobs Simon Glass
` (9 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:14 UTC (permalink / raw)
To: u-boot
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";
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 11/12] binman: Support help messages for missing blobs
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (9 preceding siblings ...)
2020-09-01 11:14 ` [PATCH v3 10/12] binman: Allow selecting default FIT configuration Simon Glass
@ 2020-09-01 11:14 ` Simon Glass
2020-09-01 11:14 ` [PATCH v3 12/12] binman: sunxi: Add help message for missing sunxi ATF BL31 Simon Glass
` (8 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:14 UTC (permalink / raw)
To: u-boot
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";
+ };
};
};
};
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 12/12] binman: sunxi: Add help message for missing sunxi ATF BL31
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (10 preceding siblings ...)
2020-09-01 11:14 ` [PATCH v3 11/12] binman: Support help messages for missing blobs Simon Glass
@ 2020-09-01 11:14 ` Simon Glass
2020-09-02 10:26 ` [PATCH v3 00/12] binman: Add support for generating more complex FITs Michal Simek
` (7 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-01 11:14 UTC (permalink / raw)
To: u-boot
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
--
2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 00/12] binman: Add support for generating more complex FITs
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (11 preceding siblings ...)
2020-09-01 11:14 ` [PATCH v3 12/12] binman: sunxi: Add help message for missing sunxi ATF BL31 Simon Glass
@ 2020-09-02 10:26 ` Michal Simek
2020-09-02 17:07 ` Simon Glass
2020-09-05 21:10 ` [PATCH v3 07/12] Makefile: Support missing external blobs always Simon Glass
` (6 subsequent siblings)
19 siblings, 1 reply; 39+ messages in thread
From: Michal Simek @ 2020-09-02 10:26 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 00/12] binman: Add support for generating more complex FITs
2020-09-02 10:26 ` [PATCH v3 00/12] binman: Add support for generating more complex FITs Michal Simek
@ 2020-09-02 17:07 ` Simon Glass
2020-09-03 13:31 ` Michal Simek
0 siblings, 1 reply; 39+ messages in thread
From: Simon Glass @ 2020-09-02 17:07 UTC (permalink / raw)
To: u-boot
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
>
>
>
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 00/12] binman: Add support for generating more complex FITs
2020-09-02 17:07 ` Simon Glass
@ 2020-09-03 13:31 ` Michal Simek
0 siblings, 0 replies; 39+ messages in thread
From: Michal Simek @ 2020-09-03 13:31 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 07/12] Makefile: Support missing external blobs always
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (12 preceding siblings ...)
2020-09-02 10:26 ` [PATCH v3 00/12] binman: Add support for generating more complex FITs Michal Simek
@ 2020-09-05 21:10 ` Simon Glass
2020-09-05 21:10 ` [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs Simon Glass
` (5 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-05 21:10 UTC (permalink / raw)
To: u-boot
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!
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (13 preceding siblings ...)
2020-09-05 21:10 ` [PATCH v3 07/12] Makefile: Support missing external blobs always Simon Glass
@ 2020-09-05 21:10 ` Simon Glass
2020-09-05 21:10 ` [PATCH v3 05/12] binman: Add support for ATF BL31 Simon Glass
` (4 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-05 21:10 UTC (permalink / raw)
To: u-boot
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!
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 05/12] binman: Add support for ATF BL31
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (14 preceding siblings ...)
2020-09-05 21:10 ` [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs Simon Glass
@ 2020-09-05 21:10 ` Simon Glass
2020-09-05 21:10 ` [PATCH v3 04/12] binman: Move 'external' support into base class Simon Glass
` (3 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-05 21:10 UTC (permalink / raw)
To: u-boot
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!
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 04/12] binman: Move 'external' support into base class
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (15 preceding siblings ...)
2020-09-05 21:10 ` [PATCH v3 05/12] binman: Add support for ATF BL31 Simon Glass
@ 2020-09-05 21:10 ` Simon Glass
2020-09-05 21:10 ` [PATCH v3 03/12] libfdt: Detected out-of-space with fdt_finish() Simon Glass
` (2 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-05 21:10 UTC (permalink / raw)
To: u-boot
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!
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 03/12] libfdt: Detected out-of-space with fdt_finish()
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (16 preceding siblings ...)
2020-09-05 21:10 ` [PATCH v3 04/12] binman: Move 'external' support into base class Simon Glass
@ 2020-09-05 21:10 ` Simon Glass
2020-09-05 21:10 ` [PATCH v3 02/12] binman: Fix up a few missing comments Simon Glass
2020-09-05 21:10 ` [PATCH v3 01/12] binman: Allow entry args to be required Simon Glass
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-05 21:10 UTC (permalink / raw)
To: u-boot
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!
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 02/12] binman: Fix up a few missing comments
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (17 preceding siblings ...)
2020-09-05 21:10 ` [PATCH v3 03/12] libfdt: Detected out-of-space with fdt_finish() Simon Glass
@ 2020-09-05 21:10 ` Simon Glass
2020-09-05 21:10 ` [PATCH v3 01/12] binman: Allow entry args to be required Simon Glass
19 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-05 21:10 UTC (permalink / raw)
To: u-boot
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!
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 01/12] binman: Allow entry args to be required
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
` (18 preceding siblings ...)
2020-09-05 21:10 ` [PATCH v3 02/12] binman: Fix up a few missing comments Simon Glass
@ 2020-09-05 21:10 ` Simon Glass
2020-09-07 6:31 ` Michal Simek
19 siblings, 1 reply; 39+ messages in thread
From: Simon Glass @ 2020-09-05 21:10 UTC (permalink / raw)
To: u-boot
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!
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH v3 01/12] binman: Allow entry args to be required
2020-09-05 21:10 ` [PATCH v3 01/12] binman: Allow entry args to be required Simon Glass
@ 2020-09-07 6:31 ` Michal Simek
2020-09-07 13:57 ` Simon Glass
0 siblings, 1 reply; 39+ messages in thread
From: Michal Simek @ 2020-09-07 6:31 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 01/12] binman: Allow entry args to be required
2020-09-07 6:31 ` Michal Simek
@ 2020-09-07 13:57 ` Simon Glass
0 siblings, 0 replies; 39+ messages in thread
From: Simon Glass @ 2020-09-07 13:57 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 39+ messages in thread