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 ---

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):

Add a new function to Entry to check for missing properties, since this is likely to be come a common requirement.

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

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

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 41f0eb58ae0..3be074ccd66 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1179,3 +1179,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, prop_list): + """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 prop_list: + 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..0b068ba3268 100644 --- a/tools/binman/etype/fill.py +++ b/tools/binman/etype/fill.py @@ -26,8 +26,7 @@ class Entry_fill(Entry):

def ReadNode(self): super().ReadNode() - if self.size is None: - self.Raise("'fill' entry must have a size property") + self.ensure_props(['size']) 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 ---

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

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

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

tools/binman/entries.rst | 28 +++++++++++++++++++++------- tools/binman/etype/mkimage.py | 28 +++++++++++++++++++++------- 2 files changed, 42 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..28f7eb31343 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,24 @@ 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, 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::

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 ---

tools/binman/entries.rst | 15 +++++++++++++++ tools/binman/etype/mkimage.py | 26 ++++++++++++++++++++++++-- tools/binman/ftest.py | 17 +++++++++++++++++ tools/binman/test/235_mkimage_name.dts | 18 ++++++++++++++++++ 4 files changed, 74 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 28f7eb31343..3295f451f79 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.:: @@ -56,6 +58,19 @@ 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). """ def __init__(self, section, etype, node): super().__init__(section, etype, node) @@ -65,6 +80,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): @@ -76,13 +93,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 { + }; + }; + }; +};

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 ---

tools/binman/entries.rst | 3 +++ tools/binman/entry.py | 22 ++++++++++++++++ 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 ++++++++++++++++++++++ 7 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/239_collection_other.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 3be074ccd66..b9995254982 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -684,6 +684,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 dd82d51bdb6..36eef973f21 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -144,6 +144,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 { + }; + }; +};