[PATCH v2 07/25] binman: Tweak collect_contents_to_file() and docs

Simon Glass
2022-02-23T23:03:31Z

Update the return value of this function, fix the 'create' typo and
update the documentation for clarity.

Signed-off-by: Simon Glass <sjg@chromium.org>
Suggested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---

Changes in v2:
- Add new patch to tweak collect_contents_to_file() and docs

tools/binman/binman.rst | 25 +++++++++++++++----------
tools/binman/entry.py | 8 ++++----
tools/binman/etype/mkimage.py | 2 +-
3 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 771645380e..0061e43659 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -1375,18 +1375,20 @@ Some entry types deal with data obtained from others. For example,
};

This shows mkimage being passed a file consisting of SPL and U-Boot proper. It
-is create by calling `Entry.collect_contents_to_file()`. Note that in this case,
-the data is passed to mkimage for processing but does not appear separately in
-the image. It may not appear at all, depending on what mkimage does. The
-contents of the `mkimage` entry are entirely dependent on the processing done
-by the entry, with the provided subnodes (`u-boot-spl` and `u-boot`) simply
-providing the input data for that processing.
+is created by calling `Entry.collect_contents_to_file()`. Note that in this
+case, the data is passed to mkimage for processing but does not appear
+separately in the image. It may not appear at all, depending on what mkimage
+does. The contents of the `mkimage` entry are entirely dependent on the
+processing done by the entry, with the provided subnodes (`u-boot-spl` and
+`u-boot`) simply providing the input data for that processing.

Note that `Entry.collect_contents_to_file()` simply concatenates the data from
the different entries together, with no control over alignment, etc. Another
approach is to subclass `Entry_section` so that those features become available,
such as `size` and `pad-byte`. Then the contents of the entry can be obtained by
-calling `BuildSectionData()`.
+calling `super().BuildSectionData()` in the entry's BuildSectionData()
+implementation to get the input data, then write it to a file and process it
+however is desired.

There are other ways to obtain data also, depending on the situation. If the
entry type is simply signing data which exists elsewhere in the image, then
@@ -1396,6 +1398,7 @@ is used by `Entry_vblock`, for example::

u_boot: u-boot {
};
+
vblock {
content = <&u_boot &dtb>;
keyblock = "firmware.keyblock";
@@ -1440,9 +1443,11 @@ The `soc-fw` node is a `blob-ext` (i.e. it reads in a named binary file) whereas
a known blob type.

When adding new entry types you are encouraged to use subnodes to provide the
-data for processing, unless the `content` approach is more suitable. Ad-hoc
-properties and other methods of obtaining data are discouraged, since it adds to
-confusion for users.
+data for processing, unless the `content` approach is more suitable. Consider
+whether the input entries are contained within (or consumed by) the entry, vs
+just being 'referenced' by the entry. In the latter case, the `content` approach
+makes more sense. Ad-hoc properties and other methods of obtaining data are
+discouraged, since it adds to confusion for users.

History / Credits
-----------------
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index bf68a85b24..de8526618d 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1136,16 +1136,16 @@ features to produce new behaviours.

Returns:
Tuple:
- bytes: Concatenated data from all the entries (or False)
- str: Filename of file written (or False if no data)
- str: Unique portion of filename (or False if no data)
+ bytes: Concatenated data from all the entries (or None)
+ str: Filename of file written (or None if no data)
+ str: Unique portion of filename (or None if no data)
"""
data = b''
for entry in entries:
# First get the input data and put it in a file. If not available,
# try later.
if not entry.ObtainContents():
- return False, False, False
+ return None, None, None
data += entry.GetData()
uniq = self.GetUniqueName()
fname = tools.get_output_filename(f'{prefix}.{uniq}')
diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index 8e74ebb9c0..c28141ff69 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -53,7 +53,7 @@ class Entry_mkimage(Entry):
def ObtainContents(self):
data, input_fname, uniq = self.collect_contents_to_file(
self._mkimage_entries.values(), 'mkimage')
- if data is False:
+ 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,
--
2.35.1.574.g5d30c73bfb-goog