[PATCH v2] binman: Skip node generation for images read from files

From: Jan Kiszka jan.kiszka@siemens.com
We can and should run the node generator only when creating a new image. When we read it back, there is no need to generate nodes - they already exits, and binman does not dive that deep into the image - and there is no way to provide the required fdt-list. So forward the mode from the image to every Entry object it contains so that Entry_fit can simply skip generator nodes when reading them from an fdtmap.
This unbreaks all read-backs of images that contain generator nodes in their fdtmap.
Signed-off-by: Jan Kiszka jan.kiszka@siemens.com ---
Changes in v2: - more verbose commit log
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: + generate = section.generate if section else True + self.generate = generate
@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('@'): 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): + 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

Hi Jan,
On Mon, 17 Jan 2022 at 00:28, Jan Kiszka jan.kiszka@siemens.com wrote:
From: Jan Kiszka jan.kiszka@siemens.com
We can and should run the node generator only when creating a new image. When we read it back, there is no need to generate nodes - they already exits, and binman does not dive that deep into the image - and there is no way to provide the required fdt-list. So forward the mode from the image to every Entry object it contains so that Entry_fit can simply skip generator nodes when reading them from an fdtmap.
This unbreaks all read-backs of images that contain generator nodes in their fdtmap.
Signed-off-by: Jan Kiszka jan.kiszka@siemens.com
Changes in v2:
- more verbose commit log
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(-)
Did you miss my other comments and a test?
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:
generate = section.generate if section else True
self.generate = generate @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('@'): 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):
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

On 26.01.22 16:57, Simon Glass wrote:
Hi Jan,
On Mon, 17 Jan 2022 at 00:28, Jan Kiszka jan.kiszka@siemens.com wrote:
From: Jan Kiszka jan.kiszka@siemens.com
We can and should run the node generator only when creating a new image. When we read it back, there is no need to generate nodes - they already exits, and binman does not dive that deep into the image - and there is no way to provide the required fdt-list. So forward the mode from the image to every Entry object it contains so that Entry_fit can simply skip generator nodes when reading them from an fdtmap.
This unbreaks all read-backs of images that contain generator nodes in their fdtmap.
Signed-off-by: Jan Kiszka jan.kiszka@siemens.com
Changes in v2:
- more verbose commit log
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(-)
Did you miss my other comments and a test?
Sorry, I indeed the other comments on the code - after ignoring the one about a test case. ENOTIME.
Jan
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:
generate = section.generate if section else True
self.generate = generate @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('@'): 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):
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
participants (2)
-
Jan Kiszka
-
Simon Glass