[PATCH v2 4/5] binman: Convert FIT entry type to a subclass of Section entry type

Jan Kiszka jan.kiszka at siemens.com
Mon Feb 14 10:09:51 CET 2022


On 07.02.22 23:08, Alper Nebi Yasak wrote:
> The binman FIT entry type shares some code with the Section entry type.
> This shared code is bound to grow, since FIT entries are conceptually a
> variation of Section entries.
> 
> Make FIT entry type a subclass of Section entry type, simplifying it a
> bit and providing us the features that Section implements. Also fix the
> subentry alignment test which now attempts to write symbols to a
> nonexistent SPL ELF test file by creating it first.
> 
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak at gmail.com>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2:
> - Add tag: "Reviewed-by: Simon Glass <sjg at chromium.org>"
> 
>  tools/binman/etype/fit.py | 70 +++++++++++----------------------------
>  tools/binman/ftest.py     |  1 +
>  2 files changed, 20 insertions(+), 51 deletions(-)
> 
> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> index 9dffcd5adbd6..0f8c88a9720e 100644
> --- a/tools/binman/etype/fit.py
> +++ b/tools/binman/etype/fit.py
> @@ -9,11 +9,12 @@
>  import libfdt
>  
>  from binman.entry import Entry, EntryArg
> +from binman.etype.section import Entry_section
>  from dtoc import fdt_util
>  from dtoc.fdt import Fdt
>  from patman import tools
>  
> -class Entry_fit(Entry):
> +class Entry_fit(Entry_section):
>      """Flat Image Tree (FIT)
>  
>      This calls mkimage to create a FIT (U-Boot Flat Image Tree) based on the
> @@ -112,15 +113,15 @@ def __init__(self, section, etype, node):
>          """
>          Members:
>              _fit: FIT file being built
> -            _fit_sections: dict:
> +            _entries: dict from Entry_section:
>                  key: relative path to entry Node (from the base of the FIT)
>                  value: Entry_section object comprising the contents of this
>                      node
>          """
>          super().__init__(section, etype, node)
>          self._fit = None
> -        self._fit_sections = {}
>          self._fit_props = {}
> +
>          for pname, prop in self._node.props.items():
>              if pname.startswith('fit,'):
>                  self._fit_props[pname] = prop
> @@ -185,7 +186,7 @@ def _AddNode(base_node, depth, node):
>                  # 'data' property later.
>                  entry = Entry.Create(self.section, node, etype='section')
>                  entry.ReadNode()
> -                self._fit_sections[rel_path] = entry
> +                self._entries[rel_path] = entry
>  
>              for subnode in node.subnodes:
>                  if has_images and not (subnode.name.startswith('hash') or
> @@ -237,18 +238,19 @@ def _AddNode(base_node, depth, node):
>          self._fdt = Fdt.FromData(fdt.as_bytearray())
>          self._fdt.Scan()
>  
> -    def ExpandEntries(self):
> -        super().ExpandEntries()
> -        for section in self._fit_sections.values():
> -            section.ExpandEntries()
> -
> -    def ObtainContents(self):
> -        """Obtain the contents of the FIT
> +    def BuildSectionData(self, required):
> +        """Build FIT entry contents
>  
>          This adds the 'data' properties to the input ITB (Image-tree Binary)
>          then runs mkimage to process it.
> +
> +        Args:
> +            required: True if the data must be present, False if it is OK to
> +                return None
> +
> +        Returns:
> +            Contents of the section (bytes)
>          """
> -        # self._BuildInput() either returns bytes or raises an exception.
>          data = self._BuildInput(self._fdt)
>          uniq = self.GetUniqueName()
>          input_fname = tools.GetOutputFilename('%s.itb' % uniq)
> @@ -264,14 +266,12 @@ def ObtainContents(self):
>                  'pad': fdt_util.fdt32_to_cpu(ext_offset.value)
>                  }
>          if self.mkimage.run(reset_timestamp=True, output_fname=output_fname,
> -                            **args) is not None:
> -            self.SetContents(tools.ReadFile(output_fname))
> -        else:
> +                            **args) is None:
>              # Bintool is missing; just use empty data as the output
>              self.record_missing_bintool(self.mkimage)
> -            self.SetContents(tools.GetBytes(0, 1024))
> +            return tools.GetBytes(0, 1024)
>  
> -        return True
> +        return tools.ReadFile(output_fname)
>  
>      def _BuildInput(self, fdt):
>          """Finish the FIT by adding the 'data' properties to it
> @@ -282,12 +282,8 @@ def _BuildInput(self, fdt):
>          Returns:
>              New fdt contents (bytes)
>          """
> -        for path, section in self._fit_sections.items():
> +        for path, section in self._entries.items():
>              node = fdt.GetNode(path)
> -            # Entry_section.ObtainContents() either returns True or
> -            # raises an exception.
> -            section.ObtainContents()
> -            section.Pack(0)
>              data = section.GetData()
>              node.AddData('data', data)
>  
> @@ -295,34 +291,6 @@ def _BuildInput(self, fdt):
>          data = fdt.GetContents()
>          return data
>  
> -    def CheckMissing(self, missing_list):
> -        """Check if any entries in this FIT have missing external blobs
> -
> -        If there are missing blobs, the entries are added to the list
> -
> -        Args:
> -            missing_list: List of Entry objects to be added to
> -        """
> -        for path, section in self._fit_sections.items():
> -            section.CheckMissing(missing_list)
> -
> -    def SetAllowMissing(self, allow_missing):
> -        for section in self._fit_sections.values():
> -            section.SetAllowMissing(allow_missing)
> -
>      def AddBintools(self, tools):
> -        for section in self._fit_sections.values():
> -            section.AddBintools(tools)
> +        super().AddBintools(tools)
>          self.mkimage = self.AddBintool(tools, 'mkimage')
> -
> -    def check_missing_bintools(self, missing_list):
> -        """Check if any entries in this section have missing bintools
> -
> -        If there are missing bintools, these are added to the list
> -
> -        Args:
> -            missing_list: List of Bintool objects to be added to
> -        """
> -        super().check_missing_bintools(missing_list)
> -        for entry in self._fit_sections.values():
> -            entry.check_missing_bintools(missing_list)
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index bc073587cc07..f16798960b84 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -3850,6 +3850,7 @@ def testPadInSections(self):
>  
>      def testFitImageSubentryAlignment(self):
>          """Test relative alignability of FIT image subentries"""
> +        self._SetupSplElf()
>          entry_args = {
>              'test-id': TEXT_DATA,
>          }

Only came to testing this now, and it causes a regression.

Before this commit (I've added fdtmap to
arch/arm/dts/k3-am65-iot2050-boot-image.dtsi for this):

$ source/tools/binman/binman -D ls -i flash.bin
Name                 Image-pos  Size     Entry-type         Offset   Uncomp-size
----------------------------------------------------------------------------------
main-section                 0   8c0000  section                  0
  blob-ext at 0x000000          0    37373  blob-ext at 0x000000        0
  blob at 0x080000          80000    84484  blob at 0x080000        80000
  fit at 0x280000          280000    f445f  fit at 0x280000        280000
  fdtmap                37445f      cf9  fdtmap              37445f
  fill at 0x680000         680000    20000  fill at 0x680000       680000
  fill at 0x6a0000         6a0000    20000  fill at 0x6a0000       6a0000
  blob-ext at 0x6c0000     6c0000    415de  blob-ext at 0x6c0000   6c0000
  blob-ext at 0x740000     740000    43952  blob-ext at 0x740000   740000
  blob-ext at 0x7c0000     7c0000    415e2  blob-ext at 0x7c0000   7c0000
  blob-ext at 0x840000     840000    4395a  blob-ext at 0x840000   840000

With this commit:

$ source/tools/binman/binman -D ls -i flash.bin
Name                          Image-pos  Size     Entry-type         Offset   Uncomp-size
-------------------------------------------------------------------------------------------
main-section                          0   8c0000  section                  0
  blob-ext at 0x000000                   0    37373  blob-ext at 0x000000        0
  blob at 0x080000                   80000    84484  blob at 0x080000        80000
  fit at 0x280000                                    fit at 0x280000        280000
    u-boot                                        section
      u-boot-nodtb                                u-boot-nodtb
    fdt-iot2050-basic                             section
      blob                                        blob
    fdt-iot2050-basic-pg2                         section
      blob                                        blob
    fdt-iot2050-advanced                          section
      blob                                        blob
    fdt-iot2050-advanced-pg2                      section
      blob                                        blob
    k3-rti-wdt-firmware                           section
      blob-ext                                    blob-ext
  fdtmap                         37445f      cd9  fdtmap              37445f
  fill at 0x680000                  680000    20000  fill at 0x680000       680000
  fill at 0x6a0000                  6a0000    20000  fill at 0x6a0000       6a0000
  blob-ext at 0x6c0000              6c0000    415de  blob-ext at 0x6c0000   6c0000
  blob-ext at 0x740000              740000    43952  blob-ext at 0x740000   740000
  blob-ext at 0x7c0000              7c0000    415e2  blob-ext at 0x7c0000   7c0000
  blob-ext at 0x840000              840000    4395a  blob-ext at 0x840000   840000

The fit is nicely shown but its Image-pos column is now empty. And that 
leads to:

$ source/tools/binman/binman -D extract -i flash.bin -f extr.bin fit at 0x280000
binman: unsupported operand type(s) for +: 'int' and 'NoneType'

Traceback (most recent call last):
  File "source/tools/binman/binman", line 141, in RunBinman
    ret_code = control.Binman(args)
  File "/data/u-boot/tools/binman/control.py", line 639, in Binman
    not args.uncompressed, args.format)
  File "/data/u-boot/tools/binman/control.py", line 260, in ExtractEntries
    data = entry.ReadData(decomp, alt_format)
  File "/data/u-boot/tools/binman/etype/section.py", line 763, in ReadData
    data = parent_data[offset:offset + self.size]
TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'

Similar breakages for "replace" and likely more sub-commands.

I have to revert this locally for now - no time to debug.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


More information about the U-Boot mailing list