Re: [PATCH 06/12] binman: Convert mkimage to Entry_section

Hi Jonas,

On Wed, 28 Jun 2023 at 23:35, Jonas Karlman jonas@kwiboo.se wrote:

Hi Simon, On 2023-06-28 13:41, Simon Glass wrote:

...

From: Marek Vasut marex@denx.de

This is needed to handle mkimage with inner section located itself in a section.

Signed-off-by: Marek Vasut marex@denx.de Use BuildSectionData() instead of ObtainContents(), add tests and a few other minor fixes: Signed-off-by: Simon Glass sjg@chromium.org

---

tools/binman/entry.py | 6 +- tools/binman/etype/mkimage.py | 76 ++++++++++++++--------- tools/binman/ftest.py | 45 +++++++++++++- tools/binman/test/283_mkimage_special.dts | 24 +++++++ 4 files changed, 117 insertions(+), 34 deletions(-) create mode 100644 tools/binman/test/283_mkimage_special.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 328b5bc568a9..8f06fea51ad4 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1311,10 +1311,8 @@ features to produce new behaviours. """ 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(fake_size=fake_size):
  

•            return None, None, None
  

•        # First get the input data and put it in a file
  

•        entry.ObtainContents(fake_size=fake_size)
         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 cb3e10672ad7..8311fed59762 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -8,10 +8,11 @@ from collections import OrderedDict

from binman.entry import Entry +from binman.etype.section import Entry_section from dtoc import fdt_util from u_boot_pylib import tools

-class Entry_mkimage(Entry): +class Entry_mkimage(Entry_section): """Binary produced by mkimage

 Properties / Entry arguments:

@@ -121,10 +122,8 @@ class Entry_mkimage(Entry): """ def __init__(self, section, etype, node): super().__init__(section, etype, node)

•    self._mkimage_entries = OrderedDict()
     self._imagename = None
  

•    self._filename = fdt_util.GetString(self._node, 'filename')
  

•    self.align_default = None
  

•    self._multiple_data_files = False
  
  

  def ReadNode(self): super().ReadNode()

@@ -135,41 +134,60 @@ class Entry_mkimage(Entry): '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): """Read the subnodes to find out what should go in this image""" for node in self._node.subnodes:

•        entry = Entry.Create(self, node)
  

•        if self.IsSpecialSubnode(node):
  

•            continue
  

•        entry = Entry.Create(self, node,
  

•                             expanded=self.GetImage().use_expanded,
  

•                             missing_etype=self.GetImage().missing_etype)
         entry.ReadNode()
  

•        entry.SetPrefix(self._name_prefix)
         if entry.name == 'imagename':
             self._imagename = entry
         else:
  

•            self._mkimage_entries[entry.name] = entry
  

•            self._entries[entry.name] = entry
  
  

• def ObtainContents(self):

• def BuildSectionData(self, required):

•    """Build mkimage entry contents
  

•    Runs mkimage to build the entry contents
  

•    Args:
  

•        required (bool): True if the data must be present, False if it is OK
  

•            to return None
  

•    Returns:
  

•        bytes: Contents of the section
  

•    """
     # Use a non-zero size for any fake files to keep mkimage happy
     # Note that testMkimageImagename() relies on this 'mkimage' parameter
     fake_size = 1024
     if self._multiple_data_files:
         fnames = []
         uniq = self.GetUniqueName()
  

•        for entry in self._mkimage_entries.values():
  

•            if not entry.ObtainContents(fake_size=fake_size):
  

•                return False
  

•            if entry._pathname:
  

•                fnames.append(entry._pathname)
  

•        for entry in self._entries.values():
  

•            entry.ObtainContents(fake_size=fake_size)
  

•            # If this is a section, put the contents in a temporary file.
  

•            # Otherwise, assume it is a blob and use the pathname
  

•            if isinstance(entry, Entry_section):
  

•                ename = f'mkimage-in-{uniq}-{entry.name}'
  

•                fname = tools.get_output_filename(ename)
  

•                tools.write_file(fname, entry.data)
  

•            elif entry._pathname:
  

•                fname = entry._pathname
  

•            fnames.append(fname)
         input_fname = ":".join(fnames)
  

•        data = b''
     else:
         data, input_fname, uniq = self.collect_contents_to_file(
  

•            self._mkimage_entries.values(), 'mkimage', fake_size)
  

•        if data is None:
  

•            return False
  

•            self._entries.values(), 'mkimage', fake_size)
     if self._imagename:
         image_data, imagename_fname, _ = self.collect_contents_to_file(
             [self._imagename], 'mkimage-n', 1024)
  

•        if image_data is None:
  

•            return False
     outfile = self._filename if self._filename else 'mkimage-out.%s' % uniq
     output_fname = tools.get_output_filename(outfile)
  
  

@@ -177,8 +195,7 @@ class Entry_mkimage(Entry): self.CheckMissing(missing_list) self.missing = bool(missing_list) if self.missing:

•        self.SetContents(b'')
  

•        return self.allow_missing
  

•        return b''
  
     args = ['-d', input_fname]
     if self._data_to_imagename:
  

@@ -187,17 +204,15 @@ class Entry_mkimage(Entry): 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))
  

•        return tools.read_file(output_fname)
     else:
         # Bintool is missing; just use the input data as the output
         self.record_missing_bintool(self.mkimage)
  

•        self.SetContents(data)
  

•    return True
  

•        return data
  
  

  def GetEntries(self): # Make a copy so we don't change the original

•    entries = OrderedDict(self._mkimage_entries)
  

•    entries = OrderedDict(self._entries)
     if self._imagename:
         entries['imagename'] = self._imagename
     return entries
  

@@ -209,7 +224,7 @@ class Entry_mkimage(Entry): allow_missing: True if allowed, False if not allowed """ self.allow_missing = allow_missing

•    for entry in self._mkimage_entries.values():
  

•    for entry in self._entries.values():
  

With the changes to use GetEntries() in Entry_section for this and following Check and Set functions, I suspect these can be removed and use the base implementation from Entry_section.

GetEntries() returns self._entries + self._imagename

Yes, good idea, will do in v2. Tht special code has annoyed me.

Regards, Simon