Put this at the end so it is easier to copy it from the terminal.

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

(no changes since v1)

tools/patman/test_util.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index c27e0b39e5f..7df2aec6705 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -81,8 +81,7 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None print(coverage) if coverage != '100%': print(stdout) - print("Type 'python3-coverage html' to get a report in " - 'htmlcov/index.html') + print("To get a report in 'htmlcov/index.html', type: python3-coverage html") print('Coverage error: %s, but should be 100%%' % coverage) ok = False if not ok:

When a single test is run we don't need to buffer the test output. This has the unfortunate side effect of suppressing test output, in particular the binman output directory normally printed with the -X option. This is a huge problem since it blocks debugging of tests.

We don't actually know how many tests will be run when we set up the suite, so as a work-around, assume that test_name being specified indicates that there is likely only one.

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

(no changes since v1)

tools/patman/test_util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index 7df2aec6705..0f6d1aa902d 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -208,14 +208,14 @@ def run_test_suites(toolname, debug, verbosity, test_preserve_dirs, processes, runner = unittest.TextTestRunner( stream=sys.stdout, verbosity=(1 if verbosity is None else verbosity), - buffer=buffer_outputs, + buffer=False if test_name else buffer_outputs, resultclass=FullTextTestResult, )

if use_concurrent and processes != 1: suite = ConcurrentTestSuite(suite, fork_for_tests(processes or multiprocessing.cpu_count(), - buffer=buffer_outputs)) + buffer=False if test_name else buffer_outputs))

for module in class_and_module_list: if isinstance(module, str) and (not test_name or test_name == module):

This has got out of sync and needs a line wrap. Fix it.

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

(no changes since v1)

tools/binman/entries.rst | 3 ++- tools/binman/etype/pre_load.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index ae4305c99e4..a77e61800dd 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1159,7 +1159,8 @@ Entry: pre-load: Pre load image header --------------------------------------

Properties / Entry arguments: - - key-path: Path of the directory that store key (provided by the environment variable KEY_PATH) + - pre-load-key-path: Path of the directory that store key (provided by + the environment variable PRE_LOAD_KEY_PATH) - content: List of phandles to entries to sign - algo-name: Hash and signature algo to use for the signature - padding-name: Name of the padding (pkcs-1.5 or pss) diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py index 245ee755259..b6222811592 100644 --- a/tools/binman/etype/pre_load.py +++ b/tools/binman/etype/pre_load.py @@ -37,7 +37,8 @@ class Entry_pre_load(Entry_collection): """Pre load image header

Properties / Entry arguments: - - pre-load-key-path: Path of the directory that store key (provided by the environment variable PRE_LOAD_KEY_PATH) + - pre-load-key-path: Path of the directory that store key (provided by + the environment variable PRE_LOAD_KEY_PATH) - content: List of phandles to entries to sign - algo-name: Hash and signature algo to use for the signature - padding-name: Name of the padding (pkcs-1.5 or pss)

Some new entries are likely to have required properties. Support this in a standard way, with a list of required properties which can be set up by base classes. Check for missing properties when the entry is read.

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

Changes in v2: - Use a dict to hold required properties

tools/binman/entry.py | 20 ++++++++++++++++++++ tools/binman/etype/fill.py | 3 +-- tools/binman/ftest.py | 2 +- 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 41f0eb58ae0..5424a0e6e32 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -84,6 +84,8 @@ class Entry(object): update_hash: True if this entry's "hash" subnode should be updated with a hash of the entry contents fake_fname: Fake filename, if one was created, else None + required_props (dict of str): Properties which must be present. This can + be added to by subclasses """ fake_dir = None

@@ -121,6 +123,7 @@ class Entry(object): self.missing_bintools = [] self.update_hash = True self.fake_fname = None + self.required_props = []

@staticmethod def FindEntryClass(etype, expanded): @@ -238,6 +241,7 @@ class Entry(object):

This reads all the fields we recognise from the node, ready for use. """ + self.ensure_props() if 'pos' in self._node.props: self.Raise("Please use 'offset' instead of 'pos'") if 'expand-size' in self._node.props: @@ -1179,3 +1183,19 @@ features to produce new behaviours. if not os.path.exists(cls.fake_dir): os.mkdir(cls.fake_dir) tout.notice(f"Fake-blob dir is '{cls.fake_dir}'") + + def ensure_props(self): + """Raise an exception if properties are missing + + Args: + prop_list (list of str): List of properties to check for + + Raises: + ValueError: Any property is missing + """ + not_present = [] + for prop in self.required_props: + if not prop in self._node.props: + not_present.append(prop) + if not_present: + self.Raise(f"'{self.etype}' entry is missing properties: {' '.join(not_present)}") diff --git a/tools/binman/etype/fill.py b/tools/binman/etype/fill.py index cd382799199..c91d0152a8a 100644 --- a/tools/binman/etype/fill.py +++ b/tools/binman/etype/fill.py @@ -23,11 +23,10 @@ class Entry_fill(Entry): """ def __init__(self, section, etype, node): super().__init__(section, etype, node) + self.required_props = ['size']

def ReadNode(self): super().ReadNode() - if self.size is None: - self.Raise("'fill' entry must have a size property") self.fill_value = fdt_util.GetByte(self._node, 'fill-byte', 0)

def ObtainContents(self): diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index c8bab5c9416..4f696c68600 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1693,7 +1693,7 @@ class TestFunctional(unittest.TestCase): """Test for an fill entry type with no size""" with self.assertRaises(ValueError) as e: self._DoReadFile('070_fill_no_size.dts') - self.assertIn("'fill' entry must have a size property", + self.assertIn("'fill' entry is missing properties: size", str(e.exception))

def _HandleGbbCommand(self, pipe_list):

Since this is implemented as a section, it should really be split into several functions, one to read the node and one to read the entries. Do this so that it matches how Entry_section works.

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

(no changes since v1)

tools/binman/etype/mkimage.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 5f6def2287f..f3b3df6fe04 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -45,11 +45,21 @@ class Entry_mkimage(Entry): """ def __init__(self, section, etype, node): super().__init__(section, etype, node) - self._args = fdt_util.GetArgs(self._node, 'args') self._mkimage_entries = OrderedDict() self.align_default = None + + def ReadNode(self): + super().ReadNode() + self._args = fdt_util.GetArgs(self._node, 'args') self.ReadEntries()

+ def ReadEntries(self): + """Read the subnodes to find out what should go in this image""" + for node in self._node.subnodes: + entry = Entry.Create(self, node) + entry.ReadNode() + self._mkimage_entries[entry.name] = entry + def ObtainContents(self): # Use a non-zero size for any fake files to keep mkimage happy data, input_fname, uniq = self.collect_contents_to_file( @@ -67,13 +77,6 @@ class Entry_mkimage(Entry):

return True

- def ReadEntries(self): - """Read the subnodes to find out what should go in this image""" - for node in self._node.subnodes: - entry = Entry.Create(self, node) - entry.ReadNode() - self._mkimage_entries[entry.name] = entry - def SetAllowMissing(self, allow_missing): """Set whether a section allows missing external blobs

The testReplaceSectionSimple() test is the only one which expects failure. It looks odd in the output and takes time to glance at it to see that all is in fact well. Also it does not check that the right exception is generated.

Use the more common (in binman) approach of checking for an exception.

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

(no changes since v1)

tools/binman/ftest.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 4f696c68600..ac54183c399 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5712,14 +5712,15 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertIsNotNone(path) self.assertEqual(expected_fdtmap, fdtmap)

- @unittest.expectedFailure def testReplaceSectionSimple(self): """Test replacing a simple section with arbitrary data""" new_data = b'w' * len(COMPRESS_DATA + U_BOOT_DATA) - data, expected_fdtmap, _ = self._RunReplaceCmd( - 'section', new_data, - dts='234_replace_section_simple.dts') - self.assertEqual(new_data, data) + with self.assertRaises(ValueError) as exc: + self._RunReplaceCmd('section', new_data, + dts='234_replace_section_simple.dts') + self.assertIn( + "Node '/section': Replacing sections is not implemented yet", + str(exc.exception))

if __name__ == "__main__":

Expand this a little to make things clearer. Also drop the invalid entry arg.

Series-changes 2 - Make it clear that -d data is concatenated/collected by binman - Fix mulitple typoe - Reword a sentence for grammar

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

(no changes since v1)

tools/binman/entries.rst | 28 +++++++++++++++++++++------- tools/binman/etype/mkimage.py | 31 ++++++++++++++++++++++++------- 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index a77e61800dd..8d7cbdc2e75 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1100,11 +1100,10 @@ Entry: mkimage: Binary produced by mkimage ------------------------------------------

Properties / Entry arguments: - - datafile: Filename for -d argument - - args: Other arguments to pass + - args: Arguments to pass

-The data passed to mkimage is collected from subnodes of the mkimage node, -e.g.:: +The data passed to mkimage via the -d flag is collected from subnodes of the +mkimage node, e.g.::

mkimage { args = "-n test -T imximage"; @@ -1113,9 +1112,24 @@ e.g.:: }; };

-This calls mkimage to create an imximage with u-boot-spl.bin as the input -file. The output from mkimage then becomes part of the image produced by -binman. +This calls mkimage to create an imximage with `u-boot-spl.bin` as the data +file, which mkimage being called like this:: + + mkimage -d <data_file> -n test -T imximage <output_file> + +The output from mkimage then becomes part of the image produced by +binman. If you need to put mulitple things in the data file, you can use +a section, or just multiple subnodes like this:: + + mkimage { + args = "-n test -T imximage"; + + u-boot-spl { + }; + + u-boot-tpl { + }; + };

To use CONFIG options in the arguments, use a string list instead, as in this example which also produces four arguments:: diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index f3b3df6fe04..a5d94da6a91 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -15,11 +15,10 @@ class Entry_mkimage(Entry): """Binary produced by mkimage

Properties / Entry arguments: - - datafile: Filename for -d argument - - args: Other arguments to pass + - args: Arguments to pass

- The data passed to mkimage is collected from subnodes of the mkimage node, - e.g.:: + The data passed to mkimage via the -d flag is collected from subnodes of the + mkimage node, e.g.::

mkimage { args = "-n test -T imximage"; @@ -28,9 +27,27 @@ class Entry_mkimage(Entry): }; };

- This calls mkimage to create an imximage with u-boot-spl.bin as the input - file. The output from mkimage then becomes part of the image produced by - binman. + This calls mkimage to create an imximage with `u-boot-spl.bin` as the data + file, with mkimage being called like this:: + + mkimage -d <data_file> -n test -T imximage <output_file> + + The output from mkimage then becomes part of the image produced by + binman. If you need to put multiple things in the data file, you can use + a section, or just multiple subnodes like this:: + + mkimage { + args = "-n test -T imximage"; + + u-boot-spl { + }; + + u-boot-tpl { + }; + }; + + Note that binman places the contents (here SPL and TPL) into a single file + and passes that to mkimage using the -d option.

To use CONFIG options in the arguments, use a string list instead, as in this example which also produces four arguments::

Some image types use the -n parameter to pass in the data file. Add support for this, with a new property.

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

Changes in v2: - Drop spurious quote - Reword explanation of what is passed with -d and -n

tools/binman/entries.rst | 15 ++++++++++++++ tools/binman/etype/mkimage.py | 27 ++++++++++++++++++++++++-- tools/binman/ftest.py | 17 ++++++++++++++++ tools/binman/test/235_mkimage_name.dts | 18 +++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 tools/binman/test/235_mkimage_name.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 8d7cbdc2e75..1d38c513ffa 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1101,6 +1101,8 @@ Entry: mkimage: Binary produced by mkimage

Properties / Entry arguments: - args: Arguments to pass + - data-to-imagename: Indicates that the -d data should be passed in as + the image name also (-n)

The data passed to mkimage via the -d flag is collected from subnodes of the mkimage node, e.g.:: @@ -1141,6 +1143,19 @@ this example which also produces four arguments:: }; };

+If you need to pass the input data in with the -n argument as well, then use +the 'data-to-imagename' property:: + + mkimage { + args = "-T imximage"; + data-to-imagename'; + + u-boot-spl { + }; + }; + +That will pass the data to mkimage both as the data file (with -d) and as +the image name (with -n).

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index a5d94da6a91..53622546dc0 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -16,6 +16,8 @@ class Entry_mkimage(Entry):

Properties / Entry arguments: - args: Arguments to pass + - data-to-imagename: Indicates that the -d data should be passed in as + the image name also (-n)

The data passed to mkimage via the -d flag is collected from subnodes of the mkimage node, e.g.:: @@ -59,6 +61,20 @@ class Entry_mkimage(Entry): }; };

+ If you need to pass the input data in with the -n argument as well, then use + the 'data-to-imagename' property:: + + mkimage { + args = "-T imximage"; + data-to-imagename; + + u-boot-spl { + }; + }; + + That will pass the data to mkimage both as the data file (with -d) and as + the image name (with -n). In both cases, a filename is passed as the + argument, with the actual data being in that file. """ def __init__(self, section, etype, node): super().__init__(section, etype, node) @@ -68,6 +84,8 @@ class Entry_mkimage(Entry): def ReadNode(self): super().ReadNode() self._args = fdt_util.GetArgs(self._node, 'args') + self._data_to_imagename = fdt_util.GetBool(self._node, + 'data-to-imagename') self.ReadEntries()

def ReadEntries(self): @@ -79,13 +97,18 @@ class Entry_mkimage(Entry):

def ObtainContents(self): # Use a non-zero size for any fake files to keep mkimage happy + # Note that testMkimageImagename() relies on this 'mkimage' parameter data, input_fname, uniq = self.collect_contents_to_file( self._mkimage_entries.values(), 'mkimage', 1024) if data is None: return False output_fname = tools.get_output_filename('mkimage-out.%s' % uniq) - if self.mkimage.run_cmd('-d', input_fname, *self._args, - output_fname) is not None: + + args = ['-d', input_fname] + if self._data_to_imagename: + args += ['-n', input_fname] + args += self._args + [output_fname] + if self.mkimage.run_cmd(*args) is not None: self.SetContents(tools.read_file(output_fname)) else: # Bintool is missing; just use the input data as the output diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ac54183c399..e88eedff51b 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5722,6 +5722,23 @@ fdt fdtmap Extract the devicetree blob from the fdtmap "Node '/section': Replacing sections is not implemented yet", str(exc.exception))

+ def testMkimageImagename(self): + """Test using mkimage with -n holding the data too""" + data = self._DoReadFile('235_mkimage_name.dts') + + # Check that the data appears in the file somewhere + self.assertIn(U_BOOT_SPL_DATA, data) + + # Get struct image_header -> ih_name + name = data[0x20:0x40] + + # Build the filename that we expect to be placed in there, by virtue of + # the -n paraameter + expect = os.path.join(tools.get_output_dir(), 'mkimage.mkimage') + + # Check that the image name is set to the temporary filename used + self.assertEqual(expect.encode('utf-8')[:0x20], name) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/235_mkimage_name.dts b/tools/binman/test/235_mkimage_name.dts new file mode 100644 index 00000000000..fbc82f1f8d6 --- /dev/null +++ b/tools/binman/test/235_mkimage_name.dts @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + mkimage { + args = "-T script"; + data-to-imagename; + + u-boot-spl { + }; + }; + }; +};

Also control over what goes in the file passed with -n using a separate imagename subnode. This can include a section or any other entry type.

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

(no changes since v1)

tools/binman/entries.rst | 18 +++++++++ tools/binman/etype/mkimage.py | 39 ++++++++++++++++++- tools/binman/ftest.py | 34 ++++++++++++++++ tools/binman/test/236_mkimage_image.dts | 21 ++++++++++ .../test/237_mkimage_image_no_content.dts | 22 +++++++++++ tools/binman/test/238_mkimage_image_bad.dts | 22 +++++++++++ 6 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/236_mkimage_image.dts create mode 100644 tools/binman/test/237_mkimage_image_no_content.dts create mode 100644 tools/binman/test/238_mkimage_image_bad.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 1d38c513ffa..682159ac6d3 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1157,6 +1157,24 @@ the 'data-to-imagename' property:: That will pass the data to mkimage both as the data file (with -d) and as the image name (with -n).

+If need to pass different data in with -n, then use an imagename subnode:: + + mkimage { + args = "-T imximage"; + + imagename { + blob { + filename = "spl/u-boot-spl.cfgout" + }; + }; + + u-boot-spl { + }; + }; + +This will pass in u-boot-spl as the input data and the .cfgout file as the +-n data. +

Entry: opensbi: RISC-V OpenSBI fw_dynamic blob diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 53622546dc0..437fcdacfd7 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -75,10 +75,29 @@ class Entry_mkimage(Entry): That will pass the data to mkimage both as the data file (with -d) and as the image name (with -n). In both cases, a filename is passed as the argument, with the actual data being in that file. + + If need to pass different data in with -n, then use an `imagename` subnode:: + + mkimage { + args = "-T imximage"; + + imagename { + blob { + filename = "spl/u-boot-spl.cfgout" + }; + }; + + u-boot-spl { + }; + }; + + This will pass in u-boot-spl as the input data and the .cfgout file as the + -n data. """ def __init__(self, section, etype, node): super().__init__(section, etype, node) self._mkimage_entries = OrderedDict() + self._imagename = None self.align_default = None

def ReadNode(self): @@ -86,6 +105,8 @@ class Entry_mkimage(Entry): self._args = fdt_util.GetArgs(self._node, 'args') self._data_to_imagename = fdt_util.GetBool(self._node, 'data-to-imagename') + if self._data_to_imagename and self._node.FindNode('imagename'): + self.Raise('Cannot use both imagename node and data-to-imagename') self.ReadEntries()

def ReadEntries(self): @@ -93,7 +114,10 @@ class Entry_mkimage(Entry): for node in self._node.subnodes: entry = Entry.Create(self, node) entry.ReadNode() - self._mkimage_entries[entry.name] = entry + if entry.name == 'imagename': + self._imagename = entry + else: + self._mkimage_entries[entry.name] = entry

def ObtainContents(self): # Use a non-zero size for any fake files to keep mkimage happy @@ -102,11 +126,18 @@ class Entry_mkimage(Entry): self._mkimage_entries.values(), 'mkimage', 1024) if data is None: return False + if self._imagename: + image_data, imagename_fname, _ = self.collect_contents_to_file( + [self._imagename], 'mkimage-n', 1024) + if image_data is None: + return False output_fname = tools.get_output_filename('mkimage-out.%s' % uniq)

args = ['-d', input_fname] if self._data_to_imagename: args += ['-n', input_fname] + elif self._imagename: + args += ['-n', imagename_fname] args += self._args + [output_fname] if self.mkimage.run_cmd(*args) is not None: self.SetContents(tools.read_file(output_fname)) @@ -126,6 +157,8 @@ class Entry_mkimage(Entry): self.allow_missing = allow_missing for entry in self._mkimage_entries.values(): entry.SetAllowMissing(allow_missing) + if self._imagename: + self._imagename.SetAllowMissing(allow_missing)

def SetAllowFakeBlob(self, allow_fake): """Set whether the sub nodes allows to create a fake blob @@ -135,6 +168,8 @@ class Entry_mkimage(Entry): """ for entry in self._mkimage_entries.values(): entry.SetAllowFakeBlob(allow_fake) + if self._imagename: + self._imagename.SetAllowFakeBlob(allow_fake)

def CheckFakedBlobs(self, faked_blobs_list): """Check if any entries in this section have faked external blobs @@ -146,6 +181,8 @@ class Entry_mkimage(Entry): """ for entry in self._mkimage_entries.values(): entry.CheckFakedBlobs(faked_blobs_list) + if self._imagename: + self._imagename.CheckFakedBlobs(faked_blobs_list)

def AddBintools(self, btools): self.mkimage = self.AddBintool(btools, 'mkimage') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index e88eedff51b..9b10fd8698d 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5739,6 +5739,40 @@ fdt fdtmap Extract the devicetree blob from the fdtmap # Check that the image name is set to the temporary filename used self.assertEqual(expect.encode('utf-8')[:0x20], name)

+ def testMkimageImage(self): + """Test using mkimage with -n holding the data too""" + data = self._DoReadFile('236_mkimage_image.dts') + + # Check that the data appears in the file somewhere + self.assertIn(U_BOOT_SPL_DATA, data) + + # Get struct image_header -> ih_name + name = data[0x20:0x40] + + # Build the filename that we expect to be placed in there, by virtue of + # the -n paraameter + expect = os.path.join(tools.get_output_dir(), 'mkimage-n.mkimage') + + # Check that the image name is set to the temporary filename used + self.assertEqual(expect.encode('utf-8')[:0x20], name) + + # Check the corect data is in the imagename file + self.assertEqual(U_BOOT_DATA, tools.read_file(expect)) + + def testMkimageImageNoContent(self): + """Test using mkimage with -n and no data""" + with self.assertRaises(ValueError) as exc: + self._DoReadFile('237_mkimage_image_no_content.dts') + self.assertIn('Could not complete processing of contents', + str(exc.exception)) + + def testMkimageImageBad(self): + """Test using mkimage with imagename node and data-to-imagename""" + with self.assertRaises(ValueError) as exc: + self._DoReadFile('238_mkimage_image_bad.dts') + self.assertIn('Cannot use both imagename node and data-to-imagename', + str(exc.exception)) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/236_mkimage_image.dts b/tools/binman/test/236_mkimage_image.dts new file mode 100644 index 00000000000..6b8f4a4a401 --- /dev/null +++ b/tools/binman/test/236_mkimage_image.dts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + mkimage { + args = "-T script"; + + imagename { + type = "u-boot"; + }; + + u-boot-spl { + }; + }; + }; +}; diff --git a/tools/binman/test/237_mkimage_image_no_content.dts b/tools/binman/test/237_mkimage_image_no_content.dts new file mode 100644 index 00000000000..7306c06af45 --- /dev/null +++ b/tools/binman/test/237_mkimage_image_no_content.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + mkimage { + args = "-T script"; + + imagename { + type = "_testing"; + return-unknown-contents; + }; + + u-boot-spl { + }; + }; + }; +}; diff --git a/tools/binman/test/238_mkimage_image_bad.dts b/tools/binman/test/238_mkimage_image_bad.dts new file mode 100644 index 00000000000..54d2c99d628 --- /dev/null +++ b/tools/binman/test/238_mkimage_image_bad.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + mkimage { + args = "-T script"; + data-to-imagename; + + imagename { + type = "u-boot"; + }; + + u-boot-spl { + }; + }; + }; +};

At present the collections etype only works with entries in the same section. This can be limiting, since in some cases the data may be inside a subsection, e.g. if there are alignment constraints.

Add a function to find the entries in an etype and have it search recursively. Make use of this for mkimage also.

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

(no changes since v1)

tools/binman/entries.rst | 3 +++ tools/binman/entry.py | 23 +++++++++++++++++ tools/binman/etype/collection.py | 3 +++ tools/binman/etype/mkimage.py | 7 ++++++ tools/binman/etype/section.py | 8 +++--- tools/binman/ftest.py | 14 +++++++++++ tools/binman/test/239_collection_other.dts | 29 ++++++++++++++++++++++ tools/binman/test/240_mkimage_coll.dts | 27 ++++++++++++++++++++ 8 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/239_collection_other.dts create mode 100644 tools/binman/test/240_mkimage_coll.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 682159ac6d3..3fa027a241c 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -427,6 +427,9 @@ listed entries are combined to form this entry. This serves as a useful base class for entry types which need to process data from elsewhere in the image, not necessarily child entries.

+The entries can generally be anywhere in the same image, even if they are in +a different section from this entry. +

Entry: cros-ec-rw: A blob entry which contains a Chromium OS read-write EC image diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 5424a0e6e32..6d305bfb611 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -679,6 +679,7 @@ class Entry(object): self.WriteMapLine(fd, indent, self.name, self.offset, self.size, self.image_pos)

+ # pylint: disable=assignment-from-none def GetEntries(self): """Return a list of entries contained by this entry

@@ -688,6 +689,28 @@ class Entry(object): """ return None

+ def FindEntryByNode(self, find_node): + """Find a node in an entry, searching all subentries + + This does a recursive search. + + Args: + find_node (fdt.Node): Node to find + + Returns: + Entry: entry, if found, else None + """ + entries = self.GetEntries() + if entries: + for entry in entries.values(): + if entry._node == find_node: + return entry + found = entry.FindEntryByNode(find_node) + if found: + return found + + return None + def GetArg(self, name, datatype=str): """Get the value of an entry argument or device-tree-node property

diff --git a/tools/binman/etype/collection.py b/tools/binman/etype/collection.py index 442b40b48b3..c532aafe3e7 100644 --- a/tools/binman/etype/collection.py +++ b/tools/binman/etype/collection.py @@ -21,6 +21,9 @@ class Entry_collection(Entry): listed entries are combined to form this entry. This serves as a useful base class for entry types which need to process data from elsewhere in the image, not necessarily child entries. + + The entries can generally be anywhere in the same image, even if they are in + a different section from this entry. """ def __init__(self, section, etype, node): super().__init__(section, etype, node) diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 437fcdacfd7..d298776741e 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -148,6 +148,13 @@ class Entry_mkimage(Entry):

return True

+ def GetEntries(self): + # Make a copy so we don't change the original + entries = OrderedDict(self._mkimage_entries) + if self._imagename: + entries['imagename'] = self._imagename + return entries + def SetAllowMissing(self, allow_missing): """Set whether a section allows missing external blobs

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index bd67238b919..5c326a75e8c 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -506,10 +506,10 @@ class Entry_section(Entry): node = self._node.GetFdt().LookupPhandle(phandle) if not node: source_entry.Raise("Cannot find node for phandle %d" % phandle) - for entry in self._entries.values(): - if entry._node == node: - return entry.GetData(required) - source_entry.Raise("Cannot find entry for node '%s'" % node.name) + entry = self.FindEntryByNode(node) + if not entry: + source_entry.Raise("Cannot find entry for node '%s'" % node.name) + return entry.GetData(required)

def LookupSymbol(self, sym_name, optional, msg, base_addr, entries=None): """Look up a symbol in an ELF file diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 9b10fd8698d..737dbcc2466 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5773,6 +5773,20 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertIn('Cannot use both imagename node and data-to-imagename', str(exc.exception))

+ def testCollectionOther(self): + """Test a collection where the data comes from another section""" + data = self._DoReadFile('239_collection_other.dts') + self.assertEqual(U_BOOT_NODTB_DATA + U_BOOT_DTB_DATA + + tools.get_bytes(0xff, 2) + U_BOOT_NODTB_DATA + + tools.get_bytes(0xfe, 3) + U_BOOT_DTB_DATA, + data) + + def testMkimageCollection(self): + """Test using a collection referring to an entry in a mkimage entry""" + data = self._DoReadFile('240_mkimage_coll.dts') + expect = U_BOOT_SPL_DATA + U_BOOT_DATA + self.assertEqual(expect, data[:len(expect)]) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/239_collection_other.dts b/tools/binman/test/239_collection_other.dts new file mode 100644 index 00000000000..09de20e5bca --- /dev/null +++ b/tools/binman/test/239_collection_other.dts @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + collection { + content = <&u_boot_nodtb &dtb>; + }; + section { + fill { + size = <2>; + fill-byte = [ff]; + }; + u_boot_nodtb: u-boot-nodtb { + }; + fill2 { + type = "fill"; + size = <3>; + fill-byte = [fe]; + }; + }; + dtb: u-boot-dtb { + }; + }; +}; diff --git a/tools/binman/test/240_mkimage_coll.dts b/tools/binman/test/240_mkimage_coll.dts new file mode 100644 index 00000000000..30860118860 --- /dev/null +++ b/tools/binman/test/240_mkimage_coll.dts @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + collection { + content = <&spl &u_boot>; + }; + mkimage { + args = "-T script"; + + spl: u-boot-spl { + }; + + imagename { + type = "section"; + + u_boot: u-boot { + }; + }; + }; + }; +};