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

Simon Glass sjg at chromium.org
Thu Jun 29 21:09:35 CEST 2023


Hi Jonas,

On Wed, 28 Jun 2023 at 23:35, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Simon,
> On 2023-06-28 13:41, Simon Glass wrote:
> > From: Marek Vasut <marex at denx.de>
> >
> > This is needed to handle mkimage with inner section located itself in a
> > section.
> >
> > Signed-off-by: Marek Vasut <marex at denx.de>
> > Use BuildSectionData() instead of ObtainContents(), add tests and a few
> > other minor fixes:
> > Signed-off-by: Simon Glass <sjg at 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


More information about the U-Boot mailing list