Hi Jan,

On Sun, 16 Jan 2022 at 08:51, Jan Kiszka jan.kiszka@siemens.com wrote:

From: Jan Kiszka jan.kiszka@siemens.com

This unbreaks all read-backs of images that contain generator nodes in their fdtmap.

This issue is subtle enough that I think it could use a few lines of explanation.

Signed-off-by: Jan Kiszka jan.kiszka@siemens.com

I tried to write some test case as well, but the testsuite is too fragile and too non-intuitive to me to extend it. E.g., just adding a fdtmap to 170_fit_fdt.dts made existing tests fail, for unclear reasons. I have to leave that to someone who understands the mechanics better.

That's because a fake FDT is used in most tests, to save time and reduce complexity. In your case you need a real one so that you don't just get fake junk in the fdtmap. The -9 FDT_ERR_BADMAGIC is produced because the fdt is not really an fdt, but is U_BOOT_DTB_DATA (i.e. 'udtb').

You can call _DoReadFileRealDtb() to make things work - see testFdpmap().

But note that you should not reuse an existing dts for new tests. Create a new one with the things you want in it and use that in your new test.

tools/binman/entry.py | 5 ++++- tools/binman/etype/fit.py | 2 +- tools/binman/etype/section.py | 4 ++-- tools/binman/image.py | 7 ++++--- 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index bac90bbbcd..fdb9746fda 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -75,7 +75,7 @@ class Entry(object): available. This is mainly used for testing. external: True if this entry contains an external binary blob """

• def __init__(self, section, etype, node, name_prefix=''):

• def __init__(self, section, etype, node, name_prefix='', generate=None): # Put this here to allow entry-docs and help to work without libfdt global state from binman import state

@@ -105,6 +105,9 @@ class Entry(object): self.external = False self.allow_missing = False self.allow_fake = False

•    if generate == None:
  

is None

•        generate = section.generate if section else True
  

•    self.generate = generate
  

But I think it would be simpler to have a flag in the Image (as you have) and not copy it elsewhere.

  @staticmethod
  def FindEntryClass(etype, expanded):

diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index b41187df80..4e4d2f9c22 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -193,7 +193,7 @@ class Entry_fit(Entry): # the FIT (e.g. "/images/kernel/u-boot"), so don't call # fsw.add_node() or _AddNode() for it. pass

•            elif subnode.name.startswith('@'):
  

•            elif self.generate and subnode.name.startswith('@'):
  

You can call self.GetImage().generate here so you don't need to copy the 'generate' flag.

                  if self._fdts:
                      # Generate notes for each FDT
                      for seq, fdt_fname in enumerate(self._fdts):

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 7a55d03231..319156a09a 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -154,9 +154,9 @@ class Entry_section(Entry): available. This is set by the `SetAllowMissing()` method, if `--allow-missing` is passed to binman. """

• def __init__(self, section, etype, node, test=False):

• def __init__(self, section, etype, node, test=False, generate=None): if not test:

•        super().__init__(section, etype, node)
  

•        super().__init__(section, etype, node, generate=generate)
      self._entries = OrderedDict()
      self._pad_byte = 0
      self._sort = False
  

diff --git a/tools/binman/image.py b/tools/binman/image.py index f0a7d65299..1ff97e687c 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -69,8 +69,9 @@ class Image(section.Entry_section): version which does not support all the entry types. """ def __init__(self, name, node, copy_to_orig=True, test=False,

•             ignore_missing=False, use_expanded=False, missing_etype=False):
  

•    super().__init__(None, 'section', node, test=test)
  

•             ignore_missing=False, use_expanded=False, missing_etype=False,
  

•             generate=True):
  

Please remember to document 'generate' in the comments for class Image.

•    super().__init__(None, 'section', node, test=test, generate=generate)
      self.copy_to_orig = copy_to_orig
      self.name = 'main-section'
      self.image_name = name
  

@@ -130,7 +131,7 @@ class Image(section.Entry_section): # Return an Image with the associated nodes root = dtb.GetRoot() image = Image('image', root, copy_to_orig=False, ignore_missing=True,

•                  missing_etype=True)
  

•                  missing_etype=True, generate=False)
  
      image.image_node = fdt_util.GetString(root, 'image-node', 'image')
      image.fdtmap_dtb = dtb
  

-- 2.31.1

Regards, Simon