Hi Alper, On Tue, 8 Feb 2022 at 11:54, Alper Nebi Yasak <alpernebiya...@gmail.com> wrote: > > On 08/02/2022 20:59, Simon Glass wrote: > > This shows an internal type at present, rather than the algorithm name. > > Fix it and update the test to catch this. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > --- > > > > tools/binman/ftest.py | 2 +- > > tools/binman/state.py | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > Reviewed-by: Alper Nebi Yasak <alpernebiya...@gmail.com> > > I saw the failing build for my series [1]. Looks to me like binman > doesn't support crc32 in hash nodes, and turning FIT into a Section > simply exposed that. I tried a sloppy fix, see below. > > [1] https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/388771 > > > diff --git a/tools/binman/state.py b/tools/binman/state.py > > index af0a65e841..843aefd28d 100644 > > --- a/tools/binman/state.py > > +++ b/tools/binman/state.py > > @@ -397,7 +397,7 @@ def CheckAddHashProp(node): > > if algo.value == 'sha256': > > size = 32 > > If I add here: > > + elif algo.value == 'crc32': > + size = 8 > > and a crc32 implementation attempt to the next function: > > > else: > > - return "Unknown hash algorithm '%s'" % algo > > + return "Unknown hash algorithm '%s'" % algo.value > > for n in GetUpdateNodes(hash_node): > > n.AddEmptyProp('value', size) > > def CheckSetHashValue(node, get_data_func): > hash_node = node.FindNode('hash') > if hash_node: > algo = hash_node.props.get('algo').value > if algo == 'sha256': > m = hashlib.sha256() > m.update(get_data_func()) > data = m.digest() > + elif algo == 'crc32': > + data = zlib.crc32(get_data_func()).to_bytes(8, 'little') > for n in GetUpdateNodes(hash_node): > n.SetData('value', data) > > the failing boards start building again. Not sure how correct this is > though (especially the endianness). Do we need this and maybe a test > with crc32 hash now?
I can get your series in without the final patch (testing at u-boot-dm/testing now). The thing is that mkimage creates the hashes so we don't want binman doing that too. My current feeling is that we need a way to tell sections not to add calculated properties for hashes...but just for the FIT section itself, so its children should still add these. Regards, Simon