All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/11] binman: Enhancements to binman mkimage
@ 2022-08-13 17:40 Simon Glass
  2022-08-13 17:40 ` [PATCH v2 01/11] doc: Build documentation in parallel Simon Glass
                   ` (20 more replies)
  0 siblings, 21 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass, Heiko Thiery

This includes enhancements to binman's 'mkimage' support so that it can
provide control over what is passed using the -n (imagename) argument.

It also includes various minor fixes and tweaks.

The entry-docs must be regenerated regularly with binman and this is very
slow now. A patch is included to speed up 'make htmldocs'.

Changes in v2:
- Use a dict to hold required properties
- Drop spurious quote
- Reword explanation of what is passed with -d and -n

Simon Glass (11):
  doc: Build documentation in parallel
  patman: Put the coverage command-line last
  patman: Don't buffer test output with a single test
  binman: Fix up the entry-docs for Entry_pre_load
  binman: Add a way to check for missing properties
  binman: Adjust mkimage etype node reading
  binman: Avoid use of expected failure
  binman: Improve mkimage documentation
  binman: Allow the image name to be the data file
  binman: Allow passing entries using -n
  binman: Allow collection to use entries from other sections

 doc/Makefile                                  |   1 +
 tools/binman/entries.rst                      |  67 ++++++++--
 tools/binman/entry.py                         |  43 +++++++
 tools/binman/etype/collection.py              |   3 +
 tools/binman/etype/fill.py                    |   3 +-
 tools/binman/etype/mkimage.py                 | 119 +++++++++++++++---
 tools/binman/etype/pre_load.py                |   3 +-
 tools/binman/etype/section.py                 |   8 +-
 tools/binman/ftest.py                         |  78 +++++++++++-
 tools/binman/test/235_mkimage_name.dts        |  18 +++
 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 ++++
 tools/binman/test/239_collection_other.dts    |  29 +++++
 tools/binman/test/240_mkimage_coll.dts        |  27 ++++
 tools/patman/test_util.py                     |   7 +-
 16 files changed, 430 insertions(+), 41 deletions(-)
 create mode 100644 tools/binman/test/235_mkimage_name.dts
 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
 create mode 100644 tools/binman/test/239_collection_other.dts
 create mode 100644 tools/binman/test/240_mkimage_coll.dts

-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 01/11] doc: Build documentation in parallel
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-20 21:33   ` Simon Glass
  2022-08-27  1:59   ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 02/11] patman: Put the coverage command-line last Simon Glass
                   ` (19 subsequent siblings)
  20 siblings, 2 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass

With the addition of the revision stats this now takes over a minute. Use
a parallel build to reduce it a bit (24 seconds for me).

Series-changes; 2
- Use '-j auto' instead

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

(no changes since v1)

 doc/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/Makefile b/doc/Makefile
index 050d9dd2391..a0680f9a376 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -57,6 +57,7 @@ quiet_cmd_sphinx = SPHINX  $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
 	BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath $(srctree)/$(src)/$5/$(SPHINX_CONF)) \
 	$(SPHINXBUILD) \
 	-b $2 \
+	-j auto \
 	-c $(abspath $(srctree)/$(src)) \
 	-d $(abspath $(BUILDDIR)/.doctrees/$3) \
 	-D version=$(KERNELVERSION) -D release=$(KERNELRELEASE) \
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 02/11] patman: Put the coverage command-line last
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
  2022-08-13 17:40 ` [PATCH v2 01/11] doc: Build documentation in parallel Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 03/11] patman: Don't buffer test output with a single test Simon Glass
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass

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:
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 03/11] patman: Don't buffer test output with a single test
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
  2022-08-13 17:40 ` [PATCH v2 01/11] doc: Build documentation in parallel Simon Glass
  2022-08-13 17:40 ` [PATCH v2 02/11] patman: Put the coverage command-line last Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 04/11] binman: Fix up the entry-docs for Entry_pre_load Simon Glass
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass

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):
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 04/11] binman: Fix up the entry-docs for Entry_pre_load
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (2 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 03/11] patman: Don't buffer test output with a single test Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 05/11] binman: Add a way to check for missing properties Simon Glass
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass

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)
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 05/11] binman: Add a way to check for missing properties
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (3 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 04/11] binman: Fix up the entry-docs for Entry_pre_load Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 06/11] binman: Adjust mkimage etype node reading Simon Glass
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass

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):
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 06/11] binman: Adjust mkimage etype node reading
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (4 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 05/11] binman: Add a way to check for missing properties Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 07/11] binman: Avoid use of expected failure Simon Glass
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass, Heiko Thiery

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
 
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 07/11] binman: Avoid use of expected failure
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (5 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 06/11] binman: Adjust mkimage etype node reading Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 08/11] binman: Improve mkimage documentation Simon Glass
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass

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__":
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 08/11] binman: Improve mkimage documentation
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (6 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 07/11] binman: Avoid use of expected failure Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 09/11] binman: Allow the image name to be the data file Simon Glass
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass, Heiko Thiery

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::
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 09/11] binman: Allow the image name to be the data file
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (7 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 08/11] binman: Improve mkimage documentation Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 10/11] binman: Allow passing entries using -n Simon Glass
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass, Heiko Thiery

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 {
+			};
+		};
+	};
+};
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 10/11] binman: Allow passing entries using -n
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (8 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 09/11] binman: Allow the image name to be the data file Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-13 17:40 ` [PATCH v2 11/11] binman: Allow collection to use entries from other sections Simon Glass
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass, Heiko Thiery

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 {
+			};
+		};
+	};
+};
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 11/11] binman: Allow collection to use entries from other sections
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (9 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 10/11] binman: Allow passing entries using -n Simon Glass
@ 2022-08-13 17:40 ` Simon Glass
  2022-08-21  0:10 ` Simon Glass
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-13 17:40 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Simon Glass, Heiko Thiery

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 {
+				};
+			};
+		};
+	};
+};
-- 
2.37.1.595.g718a3a8f04-goog


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 01/11] doc: Build documentation in parallel
  2022-08-13 17:40 ` [PATCH v2 01/11] doc: Build documentation in parallel Simon Glass
@ 2022-08-20 21:33   ` Simon Glass
  2022-08-27  1:59   ` Simon Glass
  1 sibling, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-20 21:33 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut

+Heinrich Schuchardt

On Sat, 13 Aug 2022 at 11:41, Simon Glass <sjg@chromium.org> wrote:
>
> With the addition of the revision stats this now takes over a minute. Use
> a parallel build to reduce it a bit (24 seconds for me).
>
> Series-changes; 2
> - Use '-j auto' instead
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  doc/Makefile | 1 +
>  1 file changed, 1 insertion(+)

Does this look OK to you?

>
> diff --git a/doc/Makefile b/doc/Makefile
> index 050d9dd2391..a0680f9a376 100644
> --- a/doc/Makefile
> +++ b/doc/Makefile
> @@ -57,6 +57,7 @@ quiet_cmd_sphinx = SPHINX  $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
>         BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath $(srctree)/$(src)/$5/$(SPHINX_CONF)) \
>         $(SPHINXBUILD) \
>         -b $2 \
> +       -j auto \
>         -c $(abspath $(srctree)/$(src)) \
>         -d $(abspath $(BUILDDIR)/.doctrees/$3) \
>         -D version=$(KERNELVERSION) -D release=$(KERNELRELEASE) \
> --
> 2.37.1.595.g718a3a8f04-goog
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 11/11] binman: Allow collection to use entries from other sections
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (10 preceding siblings ...)
  2022-08-13 17:40 ` [PATCH v2 11/11] binman: Allow collection to use entries from other sections Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 09/11] binman: Allow the image name to be the data file Simon Glass
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Heiko Thiery, U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 09/11] binman: Allow the image name to be the data file
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (11 preceding siblings ...)
  2022-08-21  0:10 ` Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 10/11] binman: Allow passing entries using -n Simon Glass
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Heiko Thiery, U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 10/11] binman: Allow passing entries using -n
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (12 preceding siblings ...)
  2022-08-21  0:10 ` [PATCH v2 09/11] binman: Allow the image name to be the data file Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 08/11] binman: Improve mkimage documentation Simon Glass
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Heiko Thiery, U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 08/11] binman: Improve mkimage documentation
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (13 preceding siblings ...)
  2022-08-21  0:10 ` [PATCH v2 10/11] binman: Allow passing entries using -n Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 07/11] binman: Avoid use of expected failure Simon Glass
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Heiko Thiery, U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 07/11] binman: Avoid use of expected failure
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (14 preceding siblings ...)
  2022-08-21  0:10 ` [PATCH v2 08/11] binman: Improve mkimage documentation Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 06/11] binman: Adjust mkimage etype node reading Simon Glass
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 06/11] binman: Adjust mkimage etype node reading
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (15 preceding siblings ...)
  2022-08-21  0:10 ` [PATCH v2 07/11] binman: Avoid use of expected failure Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 05/11] binman: Add a way to check for missing properties Simon Glass
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	Heiko Thiery, U-Boot Mailing 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(-)

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 05/11] binman: Add a way to check for missing properties
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (16 preceding siblings ...)
  2022-08-21  0:10 ` [PATCH v2 06/11] binman: Adjust mkimage etype node reading Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 04/11] binman: Fix up the entry-docs for Entry_pre_load Simon Glass
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 03/11] patman: Don't buffer test output with a single test
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (18 preceding siblings ...)
  2022-08-21  0:10 ` [PATCH v2 04/11] binman: Fix up the entry-docs for Entry_pre_load Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 02/11] patman: Put the coverage command-line last Simon Glass
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 04/11] binman: Fix up the entry-docs for Entry_pre_load
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (17 preceding siblings ...)
  2022-08-21  0:10 ` [PATCH v2 05/11] binman: Add a way to check for missing properties Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  2022-08-21  0:10 ` [PATCH v2 03/11] patman: Don't buffer test output with a single test Simon Glass
  2022-08-21  0:10 ` [PATCH v2 02/11] patman: Put the coverage command-line last Simon Glass
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 02/11] patman: Put the coverage command-line last
  2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
                   ` (19 preceding siblings ...)
  2022-08-21  0:10 ` [PATCH v2 03/11] patman: Don't buffer test output with a single test Simon Glass
@ 2022-08-21  0:10 ` Simon Glass
  20 siblings, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-21  0:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	U-Boot Mailing List

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 01/11] doc: Build documentation in parallel
  2022-08-13 17:40 ` [PATCH v2 01/11] doc: Build documentation in parallel Simon Glass
  2022-08-20 21:33   ` Simon Glass
@ 2022-08-27  1:59   ` Simon Glass
  1 sibling, 0 replies; 24+ messages in thread
From: Simon Glass @ 2022-08-27  1:59 UTC (permalink / raw)
  To: Simon Glass
  Cc: Quentin Schulz, Alper Nebi Yasak, Philippe Reynes, Marek Vasut,
	U-Boot Mailing List

+Heinrich Schuchardt

On Sat, 13 Aug 2022 at 11:41, Simon Glass <sjg@chromium.org> wrote:
>
> With the addition of the revision stats this now takes over a minute. Use
> a parallel build to reduce it a bit (24 seconds for me).
>
> Series-changes; 2
> - Use '-j auto' instead
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  doc/Makefile | 1 +
>  1 file changed, 1 insertion(+)

Does this look OK to you?

>
Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-08-27  2:00 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-13 17:40 [PATCH v2 00/11] binman: Enhancements to binman mkimage Simon Glass
2022-08-13 17:40 ` [PATCH v2 01/11] doc: Build documentation in parallel Simon Glass
2022-08-20 21:33   ` Simon Glass
2022-08-27  1:59   ` Simon Glass
2022-08-13 17:40 ` [PATCH v2 02/11] patman: Put the coverage command-line last Simon Glass
2022-08-13 17:40 ` [PATCH v2 03/11] patman: Don't buffer test output with a single test Simon Glass
2022-08-13 17:40 ` [PATCH v2 04/11] binman: Fix up the entry-docs for Entry_pre_load Simon Glass
2022-08-13 17:40 ` [PATCH v2 05/11] binman: Add a way to check for missing properties Simon Glass
2022-08-13 17:40 ` [PATCH v2 06/11] binman: Adjust mkimage etype node reading Simon Glass
2022-08-13 17:40 ` [PATCH v2 07/11] binman: Avoid use of expected failure Simon Glass
2022-08-13 17:40 ` [PATCH v2 08/11] binman: Improve mkimage documentation Simon Glass
2022-08-13 17:40 ` [PATCH v2 09/11] binman: Allow the image name to be the data file Simon Glass
2022-08-13 17:40 ` [PATCH v2 10/11] binman: Allow passing entries using -n Simon Glass
2022-08-13 17:40 ` [PATCH v2 11/11] binman: Allow collection to use entries from other sections Simon Glass
2022-08-21  0:10 ` Simon Glass
2022-08-21  0:10 ` [PATCH v2 09/11] binman: Allow the image name to be the data file Simon Glass
2022-08-21  0:10 ` [PATCH v2 10/11] binman: Allow passing entries using -n Simon Glass
2022-08-21  0:10 ` [PATCH v2 08/11] binman: Improve mkimage documentation Simon Glass
2022-08-21  0:10 ` [PATCH v2 07/11] binman: Avoid use of expected failure Simon Glass
2022-08-21  0:10 ` [PATCH v2 06/11] binman: Adjust mkimage etype node reading Simon Glass
2022-08-21  0:10 ` [PATCH v2 05/11] binman: Add a way to check for missing properties Simon Glass
2022-08-21  0:10 ` [PATCH v2 04/11] binman: Fix up the entry-docs for Entry_pre_load Simon Glass
2022-08-21  0:10 ` [PATCH v2 03/11] patman: Don't buffer test output with a single test Simon Glass
2022-08-21  0:10 ` [PATCH v2 02/11] patman: Put the coverage command-line last Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.