[PATCH v8 07/13] binman: Support optional external blobs

[Simon Glass <sjg@chromium.org>]

Some blobs are actually not necessary for the board to work correctly. Add
a property to allow this to be indicated. Missing optional blobs do not
cause a build failure.

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

Changes in v8:
- Move support for optional external blobs into this series
- Support optional FIT images

 tools/binman/binman.rst                |  9 +++++++++
 tools/binman/control.py                | 11 +++++++++++
 tools/binman/entry.py                  | 25 +++++++++++++++++++++----
 tools/binman/etype/blob.py             |  2 +-
 tools/binman/etype/fit.py              | 11 ++++++-----
 tools/binman/etype/section.py          | 14 +++++++++++++-
 tools/binman/ftest.py                  | 10 ++++++++++
 tools/binman/test/266_blob_ext_opt.dts | 21 +++++++++++++++++++++
 8 files changed, 92 insertions(+), 11 deletions(-)
 create mode 100644 tools/binman/test/266_blob_ext_opt.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 1ab6d012b83..391494907ab 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -689,6 +689,15 @@ no-expanded:
     `no-expanded` property disables this just for a single entry. Put the
     `no-expanded` boolean property in the node to select this behaviour.
 
+optional:
+    External blobs are normally required to be present for the image to be
+    built (but see `External blobs`_). This properly allows an entry to be
+    optional, so that when it is cannot be found, this problem is ignored and
+    an empty file is used for this blob. This should be used only when the blob
+    is entirely optional and is not needed for correct operation of the image.
+    Note that missing, optional blobs do not produce a non-zero exit code from
+    binman, although it does show a warning about the missing external blob.
+
 The attributes supported for images and sections are described below. Several
 are similar to those for entries.
 
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 07225381146..e64740094f6 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -594,12 +594,14 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
     image.BuildImage()
     if write_map:
         image.WriteMap()
+
     missing_list = []
     image.CheckMissing(missing_list)
     if missing_list:
         tout.warning("Image '%s' is missing external blobs and is non-functional: %s" %
                      (image.name, ' '.join([e.name for e in missing_list])))
         _ShowHelpForMissingBlobs(missing_list)
+
     faked_list = []
     image.CheckFakedBlobs(faked_list)
     if faked_list:
@@ -607,6 +609,15 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
             "Image '%s' has faked external blobs and is non-functional: %s" %
             (image.name, ' '.join([os.path.basename(e.GetDefaultFilename())
                                    for e in faked_list])))
+
+    optional_list = []
+    image.CheckOptional(optional_list)
+    if optional_list:
+        tout.warning(
+            "Image '%s' is missing external blobs but is still functional: %s" %
+            (image.name, ' '.join([e.name for e in optional_list])))
+        _ShowHelpForMissingBlobs(optional_list)
+
     missing_bintool_list = []
     image.check_missing_bintools(missing_bintool_list)
     if missing_bintool_list:
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index de51d295891..d73f3013405 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -73,7 +73,9 @@ class Entry(object):
         compress: Compression algoithm used (e.g. 'lz4'), 'none' if none
         orig_offset: Original offset value read from node
         orig_size: Original size value read from node
-        missing: True if this entry is missing its contents
+        missing: True if this entry is missing its contents. Note that if it is
+            optional, this entry will not appear in the list generated by
+            entry.CheckMissing() since it is considered OK for it to be missing.
         allow_missing: Allow children of this entry to be missing (used by
             subclasses such as Entry_section)
         allow_fake: Allow creating a dummy fake file if the blob file is not
@@ -95,6 +97,7 @@ class Entry(object):
             the entry itself, allowing it to vanish in certain circumstances.
             An absent entry is removed during processing so that it does not
             appear in the map
+        optional (bool): True if this entry contains an optional external blob
     """
     fake_dir = None
 
@@ -138,6 +141,7 @@ class Entry(object):
         self.elf_fname = None
         self.auto_write_symbols = auto_write_symbols
         self.absent = False
+        self.optional = False
 
     @staticmethod
     def FindEntryClass(etype, expanded):
@@ -289,6 +293,7 @@ class Entry(object):
         self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset')
         self.extend_size = fdt_util.GetBool(self._node, 'extend-size')
         self.missing_msg = fdt_util.GetString(self._node, 'missing-msg')
+        self.optional = fdt_util.GetBool(self._node, 'optional')
 
         # This is only supported by blobs and sections at present
         self.compress = fdt_util.GetString(self._node, 'compress', 'none')
@@ -1039,14 +1044,15 @@ features to produce new behaviours.
         self.allow_fake = allow_fake
 
     def CheckMissing(self, missing_list):
-        """Check if any entries in this section have missing external blobs
+        """Check if the entry has missing external blobs
 
-        If there are missing blobs, the entries are added to the list
+        If there are missing (non-optional) blobs, the entries are added to the
+        list
 
         Args:
             missing_list: List of Entry objects to be added to
         """
-        if self.missing:
+        if self.missing and not self.optional:
             missing_list.append(self)
 
     def check_fake_fname(self, fname, size=0):
@@ -1085,6 +1091,17 @@ features to produce new behaviours.
         # This is meaningless for anything other than blobs
         pass
 
+    def CheckOptional(self, optional_list):
+        """Check if the entry has missing but optional external blobs
+
+        If there are missing (optional) blobs, the entries are added to the list
+
+        Args:
+            optional_list (list): List of Entry objects to be added to
+        """
+        if self.missing and self.optional:
+            optional_list.append(self)
+
     def GetAllowMissing(self):
         """Get whether a section allows missing external blobs
 
diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
index a50a8068901..70dea7158ee 100644
--- a/tools/binman/etype/blob.py
+++ b/tools/binman/etype/blob.py
@@ -39,7 +39,7 @@ class Entry_blob(Entry):
     def ObtainContents(self, fake_size=0):
         self._filename = self.GetDefaultFilename()
         self._pathname = tools.get_input_filename(self._filename,
-            self.external and self.section.GetAllowMissing())
+            self.external and (self.optional or self.section.GetAllowMissing()))
         # Allow the file to be missing
         if not self._pathname:
             self._pathname, faked = self.check_fake_fname(self._filename,
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index 21c769a1cbe..acd0466be5a 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -392,8 +392,8 @@ class Entry_fit(Entry_section):
 
         _add_entries(self._node, 0, self._node)
 
-        # Keep a copy of all entries, including generator entries, since these
-        # removed from self._entries later.
+        # Keep a copy of all entries, including generator entries, since those
+        # are removed from self._entries later.
         self._priv_entries = dict(self._entries)
 
     def BuildSectionData(self, required):
@@ -601,14 +601,15 @@ class Entry_fit(Entry_section):
                 # Entry_section.ObtainContents() either returns True or
                 # raises an exception.
                 data = None
-                missing_list = []
+                missing_opt_list = []
                 entry.ObtainContents()
                 entry.Pack(0)
-                entry.CheckMissing(missing_list)
+                entry.CheckMissing(missing_opt_list)
+                entry.CheckOptional(missing_opt_list)
 
                 # If any pieces are missing, skip this. The missing entries will
                 # show an error
-                if not missing_list:
+                if not missing_opt_list:
                     segs = entry.read_elf_segments()
                     if segs:
                         segments, entry_addr = segs
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 57bfee0b286..44dafaf7262 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -863,7 +863,8 @@ class Entry_section(Entry):
     def CheckMissing(self, missing_list):
         """Check if any entries in this section have missing external blobs
 
-        If there are missing blobs, the entries are added to the list
+        If there are missing (non-optional) blobs, the entries are added to the
+        list
 
         Args:
             missing_list: List of Entry objects to be added to
@@ -882,6 +883,17 @@ class Entry_section(Entry):
         for entry in self._entries.values():
             entry.CheckFakedBlobs(faked_blobs_list)
 
+    def CheckOptional(self, optional_list):
+        """Check the section for missing but optional external blobs
+
+        If there are missing (optional) blobs, the entries are added to the list
+
+        Args:
+            optional_list (list): List of Entry objects to be added to
+        """
+        for entry in self._entries.values():
+            entry.CheckOptional(optional_list)
+
     def check_missing_bintools(self, missing_list):
         """Check if any entries in this section have missing bintools
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index f893050e706..330e8e1ccb4 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6178,6 +6178,16 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
             "Node '/binman/fit/images/@tee-SEQ/tee-os': Invalid OP-TEE file: size mismatch (expected 0x4, have 0xe)",
             str(exc.exception))
 
+    def testExtblobOptional(self):
+        """Test an image with an external blob that is optional"""
+        with test_util.capture_sys_output() as (stdout, stderr):
+            data = self._DoReadFile('266_blob_ext_opt.dts')
+        self.assertEqual(REFCODE_DATA, data)
+        err = stderr.getvalue()
+        self.assertRegex(
+            err,
+            "Image '.*' is missing external blobs but is still functional: missing")
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/266_blob_ext_opt.dts b/tools/binman/test/266_blob_ext_opt.dts
new file mode 100644
index 00000000000..717153152ce
--- /dev/null
+++ b/tools/binman/test/266_blob_ext_opt.dts
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		ok {
+			type = "blob-ext";
+			filename = "refcode.bin";
+		};
+
+		missing {
+			type = "blob-ext";
+			filename = "missing.bin";
+			optional;
+		};
+	};
+};
-- 
2.39.0.314.g84b9a713c41-goog