diff mbox series

[07/10] binman: Support reading an image with entry args

Message ID 20210106213503.7.I1e24d731dc789a02dc72134d1beac84f24830eb8@changeid
State Accepted
Commit 939d1062d05fb4990ca7898613bcc753574f7c56
Delegated to: Simon Glass
Headers show
Series binman: Various minor fixes and improvement | expand

Commit Message

Simon Glass Jan. 7, 2021, 4:35 a.m. UTC
Normally when an entry is created, any entry arguments it has are required
to be provided, so it can actually generate its contents correctly.

However when an existing image is read, Entry objects are created for each
of the entries in the image. This happens as part of the process of
reading the image into binman.

In this case we don't need the entry arguments, since we do not intend to
regenerate the entries, or at least not unless requested. So there is no
sense in reporting an error for missing entry arguments.

Add a new property for the Image to handle this case. Update the error
reporting to be conditional on this property.

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

 tools/binman/entry.py                    |  3 +--
 tools/binman/etype/section.py            | 15 +++++++++++++++
 tools/binman/ftest.py                    | 19 +++++++++++++++++++
 tools/binman/image.py                    | 10 ++++++++--
 tools/binman/test/188_image_entryarg.dts | 21 +++++++++++++++++++++
 5 files changed, 64 insertions(+), 4 deletions(-)
 create mode 100644 tools/binman/test/188_image_entryarg.dts

Comments

Simon Glass Jan. 23, 2021, 5:27 p.m. UTC | #1
Normally when an entry is created, any entry arguments it has are required
to be provided, so it can actually generate its contents correctly.

However when an existing image is read, Entry objects are created for each
of the entries in the image. This happens as part of the process of
reading the image into binman.

In this case we don't need the entry arguments, since we do not intend to
regenerate the entries, or at least not unless requested. So there is no
sense in reporting an error for missing entry arguments.

Add a new property for the Image to handle this case. Update the error
reporting to be conditional on this property.

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

 tools/binman/entry.py                    |  3 +--
 tools/binman/etype/section.py            | 15 +++++++++++++++
 tools/binman/ftest.py                    | 19 +++++++++++++++++++
 tools/binman/image.py                    | 10 ++++++++--
 tools/binman/test/188_image_entryarg.dts | 21 +++++++++++++++++++++
 5 files changed, 64 insertions(+), 4 deletions(-)
 create mode 100644 tools/binman/test/188_image_entryarg.dts

Applied to u-boot-dm, thanks!
diff mbox series

Patch

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 2be0d8e0532..d58a730f3d5 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -434,8 +434,7 @@  class Entry(object):
                 missing.append(prop.name)
             values.append(value)
         if missing:
-            self.Raise('Missing required properties/entry args: %s' %
-                       (', '.join(missing)))
+            self.GetImage().MissingArgs(self, missing)
         return values
 
     def GetPath(self):
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 9c6334c4504..28a888776af 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -679,3 +679,18 @@  class Entry_section(Entry):
             for entry in to_add.values():
                 self._CollectEntries(entries, entries_by_name, entry)
         entries_by_name[add_entry.name] = add_entry
+
+    def MissingArgs(self, entry, missing):
+        """Report a missing argument, if enabled
+
+        For entries which require arguments, this reports an error if some are
+        missing. If missing entries are being ignored (e.g. because we read the
+        entry from an image rather than creating it), this function does
+        nothing.
+
+        Args:
+            missing: List of missing properties / entry args, each a string
+        """
+        if not self._ignore_missing:
+            entry.Raise('Missing required properties/entry args: %s' %
+                       (', '.join(missing)))
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 5016060ea68..8b928eb406d 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -4154,6 +4154,25 @@  class TestFunctional(unittest.TestCase):
                     U_BOOT_SPL_DATA[20:])
         self.assertEqual(expected, data)
 
+    def testReadImageEntryArg(self):
+        """Test reading an image that would need an entry arg to generate"""
+        entry_args = {
+            'cros-ec-rw-path': 'ecrw.bin',
+        }
+        data = self.data = self._DoReadFileDtb(
+            '188_image_entryarg.dts',use_real_dtb=True, update_dtb=True,
+            entry_args=entry_args)
+
+        image_fname = tools.GetOutputFilename('image.bin')
+        orig_image = control.images['image']
+
+        # This should not generate an error about the missing 'cros-ec-rw-path'
+        # since we are reading the image from a file. Compare with
+        # testEntryArgsRequired()
+        image = Image.FromFile(image_fname)
+        self.assertEqual(orig_image.GetEntries().keys(),
+                         image.GetEntries().keys())
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index 3622efc6862..3c2fe5ea620 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -43,8 +43,13 @@  class Image(section.Entry_section):
         test: True if this is being called from a test of Images. This this case
             there is no device tree defining the structure of the section, so
             we create a section manually.
+        ignore_missing: Ignore any missing entry arguments (i.e. don't raise an
+            exception). This should be used if the Image is being loaded from
+            a file rather than generated. In that case we obviously don't need
+            the entry arguments since the contents already exists.
     """
-    def __init__(self, name, node, copy_to_orig=True, test=False):
+    def __init__(self, name, node, copy_to_orig=True, test=False,
+                 ignore_missing=False):
         super().__init__(None, 'section', node, test=test)
         self.copy_to_orig = copy_to_orig
         self.name = 'main-section'
@@ -53,6 +58,7 @@  class Image(section.Entry_section):
         self.fdtmap_dtb = None
         self.fdtmap_data = None
         self.allow_repack = False
+        self._ignore_missing = ignore_missing
         if not test:
             self.ReadNode()
 
@@ -100,7 +106,7 @@  class Image(section.Entry_section):
 
         # Return an Image with the associated nodes
         root = dtb.GetRoot()
-        image = Image('image', root, copy_to_orig=False)
+        image = Image('image', root, copy_to_orig=False, ignore_missing=True)
 
         image.image_node = fdt_util.GetString(root, 'image-node', 'image')
         image.fdtmap_dtb = dtb
diff --git a/tools/binman/test/188_image_entryarg.dts b/tools/binman/test/188_image_entryarg.dts
new file mode 100644
index 00000000000..29d81491623
--- /dev/null
+++ b/tools/binman/test/188_image_entryarg.dts
@@ -0,0 +1,21 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		u-boot {
+		};
+		cros-ec-rw {
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};