[PATCH v2 11/11] binman: Allow collection to use entries from other sections

[Simon Glass <sjg@chromium.org>]

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