Hi Alper, On Tue, 8 Feb 2022 at 16:07, Alper Nebi Yasak <alpernebiya...@gmail.com> wrote: > > Binman's FIT entry type can have image subentries with "hash" subnodes > intended to be processed by mkimage, but not binman. However, the Entry > class and any subclass that reuses its implementation tries to process > these unconditionally. This can lead to an error when boards specify > hash algorithms that binman doesn't support, but mkimage supports. > > Let entries skip processing these "hash" subnodes based on an instance > variable, and set this instance variable for FIT subsections. Also > re-enable processing of calculated and missing properties of FIT entries > which was disabled to mitigate this issue. > > Signed-off-by: Alper Nebi Yasak <alpernebiya...@gmail.com> > --- > This applies on top of u-boot-dm/master, and does not resend my > "binman: Update image positions of FIT subentries" patch [1] which > should be applied on top of this. > > [1] > https://patchwork.ozlabs.org/project/uboot/patch/20220207220809.4497-6-alpernebiya...@gmail.com/
Perfect, thanks > > tools/binman/entry.py | 15 +++++++++++---- > tools/binman/etype/fit.py | 12 ++---------- > 2 files changed, 13 insertions(+), 14 deletions(-) > > diff --git a/tools/binman/entry.py b/tools/binman/entry.py > index dc26f8f167b0..6f98353c8569 100644 > --- a/tools/binman/entry.py > +++ b/tools/binman/entry.py > @@ -78,6 +78,8 @@ class Entry(object): > external: True if this entry contains an external binary blob > bintools: Bintools used by this entry (only populated for Image) > missing_bintools: List of missing bintools for this entry > + update_hash: True if this entry's "hash" subnode should be > + updated with a hash of the entry contents > """ > def __init__(self, section, etype, node, name_prefix=''): > # Put this here to allow entry-docs and help to work without libfdt > @@ -111,6 +113,7 @@ def __init__(self, section, etype, node, name_prefix=''): > self.allow_fake = False > self.bintools = {} > self.missing_bintools = [] > + self.update_hash = True > > @staticmethod > def FindEntryClass(etype, expanded): > @@ -315,9 +318,11 @@ def AddMissingProperties(self, have_image_pos): > > if self.compress != 'none': > state.AddZeroProp(self._node, 'uncomp-size') > - err = state.CheckAddHashProp(self._node) > - if err: > - self.Raise(err) > + > + if self.update_hash: > + err = state.CheckAddHashProp(self._node) > + if err: > + self.Raise(err) > > def SetCalculatedProperties(self): > """Set the value of device-tree properties calculated by binman""" > @@ -333,7 +338,9 @@ def SetCalculatedProperties(self): > state.SetInt(self._node, 'orig-size', self.orig_size, True) > if self.uncomp_size is not None: > state.SetInt(self._node, 'uncomp-size', self.uncomp_size) > - state.CheckSetHashValue(self._node, self.GetData) > + > + if self.update_hash: > + state.CheckSetHashValue(self._node, self.GetData) > > def ProcessFdt(self, fdt): > """Allow entries to adjust the device tree > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py > index a56b0564f9a1..a979d0f1a613 100644 > --- a/tools/binman/etype/fit.py > +++ b/tools/binman/etype/fit.py > @@ -186,6 +186,8 @@ def _AddNode(base_node, depth, node): > # 'data' property later. > entry = Entry.Create(self.section, node, etype='section') > entry.ReadNode() > + # The hash subnodes here are for mkimage, not binman. > + entry.update_hash = False I know it is less Pythonic but I would like to have this be a function in Entry, like SetUpdateHash(bool), so that it feels more like that class has control over things. > self._entries[rel_path] = entry > Also can you please add a test that uses a FIT containing hash{} nodes? > for subnode in node.subnodes: > @@ -294,13 +296,3 @@ def _BuildInput(self, fdt): > def AddBintools(self, tools): > super().AddBintools(tools) > self.mkimage = self.AddBintool(tools, 'mkimage') > - > - def AddMissingProperties(self, have_image_pos): > - # We don't want to interfere with any hash properties in the FIT, so > - # disable this for now. > - pass > - > - def SetCalculatedProperties(self): > - # We don't want to interfere with any hash properties in the FIT, so > - # disable this for now. > - pass > -- > 2.34.1 > Regards, Simon