Hi Simon, On 2023-06-28 13:41, Simon Glass wrote: > From: Marek Vasut <ma...@denx.de> > > This is needed to handle mkimage with inner section located itself in a > section. > > Signed-off-by: Marek Vasut <ma...@denx.de> > Use BuildSectionData() instead of ObtainContents(), add tests and a few > other minor fixes: > Signed-off-by: Simon Glass <s...@chromium.org> > --- > > tools/binman/entry.py | 6 +- > tools/binman/etype/mkimage.py | 76 ++++++++++++++--------- > tools/binman/ftest.py | 45 +++++++++++++- > tools/binman/test/283_mkimage_special.dts | 24 +++++++ > 4 files changed, 117 insertions(+), 34 deletions(-) > create mode 100644 tools/binman/test/283_mkimage_special.dts > > diff --git a/tools/binman/entry.py b/tools/binman/entry.py > index 328b5bc568a9..8f06fea51ad4 100644 > --- a/tools/binman/entry.py > +++ b/tools/binman/entry.py > @@ -1311,10 +1311,8 @@ features to produce new behaviours. > """ > data = b'' > for entry in entries: > - # First get the input data and put it in a file. If not > available, > - # try later. > - if not entry.ObtainContents(fake_size=fake_size): > - return None, None, None > + # First get the input data and put it in a file > + entry.ObtainContents(fake_size=fake_size) > data += entry.GetData() > uniq = self.GetUniqueName() > fname = tools.get_output_filename(f'{prefix}.{uniq}') > diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py > index cb3e10672ad7..8311fed59762 100644 > --- a/tools/binman/etype/mkimage.py > +++ b/tools/binman/etype/mkimage.py > @@ -8,10 +8,11 @@ > from collections import OrderedDict > > from binman.entry import Entry > +from binman.etype.section import Entry_section > from dtoc import fdt_util > from u_boot_pylib import tools > > -class Entry_mkimage(Entry): > +class Entry_mkimage(Entry_section): > """Binary produced by mkimage > > Properties / Entry arguments: > @@ -121,10 +122,8 @@ class Entry_mkimage(Entry): > """ > def __init__(self, section, etype, node): > super().__init__(section, etype, node) > - self._mkimage_entries = OrderedDict() > self._imagename = None > - self._filename = fdt_util.GetString(self._node, 'filename') > - self.align_default = None > + self._multiple_data_files = False > > def ReadNode(self): > super().ReadNode() > @@ -135,41 +134,60 @@ class Entry_mkimage(Entry): > 'data-to-imagename') > if self._data_to_imagename and self._node.FindNode('imagename'): > self.Raise('Cannot use both imagename node and > data-to-imagename') > - self.ReadEntries() > > def ReadEntries(self): > """Read the subnodes to find out what should go in this image""" > for node in self._node.subnodes: > - entry = Entry.Create(self, node) > + if self.IsSpecialSubnode(node): > + continue > + entry = Entry.Create(self, node, > + expanded=self.GetImage().use_expanded, > + missing_etype=self.GetImage().missing_etype) > entry.ReadNode() > + entry.SetPrefix(self._name_prefix) > if entry.name == 'imagename': > self._imagename = entry > else: > - self._mkimage_entries[entry.name] = entry > + self._entries[entry.name] = entry > > - def ObtainContents(self): > + def BuildSectionData(self, required): > + """Build mkimage entry contents > + > + Runs mkimage to build the entry contents > + > + Args: > + required (bool): True if the data must be present, False if it > is OK > + to return None > + > + Returns: > + bytes: Contents of the section > + """ > # Use a non-zero size for any fake files to keep mkimage happy > # Note that testMkimageImagename() relies on this 'mkimage' parameter > fake_size = 1024 > if self._multiple_data_files: > fnames = [] > uniq = self.GetUniqueName() > - for entry in self._mkimage_entries.values(): > - if not entry.ObtainContents(fake_size=fake_size): > - return False > - if entry._pathname: > - fnames.append(entry._pathname) > + for entry in self._entries.values(): > + entry.ObtainContents(fake_size=fake_size) > + > + # If this is a section, put the contents in a temporary file. > + # Otherwise, assume it is a blob and use the pathname > + if isinstance(entry, Entry_section): > + ename = f'mkimage-in-{uniq}-{entry.name}' > + fname = tools.get_output_filename(ename) > + tools.write_file(fname, entry.data) > + elif entry._pathname: > + fname = entry._pathname > + fnames.append(fname) > input_fname = ":".join(fnames) > + data = b'' > else: > data, input_fname, uniq = self.collect_contents_to_file( > - self._mkimage_entries.values(), 'mkimage', fake_size) > - if data is None: > - return False > + self._entries.values(), 'mkimage', fake_size) > if self._imagename: > image_data, imagename_fname, _ = self.collect_contents_to_file( > [self._imagename], 'mkimage-n', 1024) > - if image_data is None: > - return False > outfile = self._filename if self._filename else 'mkimage-out.%s' % > uniq > output_fname = tools.get_output_filename(outfile) > > @@ -177,8 +195,7 @@ class Entry_mkimage(Entry): > self.CheckMissing(missing_list) > self.missing = bool(missing_list) > if self.missing: > - self.SetContents(b'') > - return self.allow_missing > + return b'' > > args = ['-d', input_fname] > if self._data_to_imagename: > @@ -187,17 +204,15 @@ class Entry_mkimage(Entry): > args += ['-n', imagename_fname] > args += self._args + [output_fname] > if self.mkimage.run_cmd(*args) is not None: > - self.SetContents(tools.read_file(output_fname)) > + return tools.read_file(output_fname) > else: > # Bintool is missing; just use the input data as the output > self.record_missing_bintool(self.mkimage) > - self.SetContents(data) > - > - return True > + return data > > def GetEntries(self): > # Make a copy so we don't change the original > - entries = OrderedDict(self._mkimage_entries) > + entries = OrderedDict(self._entries) > if self._imagename: > entries['imagename'] = self._imagename > return entries > @@ -209,7 +224,7 @@ class Entry_mkimage(Entry): > allow_missing: True if allowed, False if not allowed > """ > self.allow_missing = allow_missing > - for entry in self._mkimage_entries.values(): > + for entry in self._entries.values():
With the changes to use GetEntries() in Entry_section for this and following Check and Set functions, I suspect these can be removed and use the base implementation from Entry_section. GetEntries() returns self._entries + self._imagename Regards, Jonas > entry.SetAllowMissing(allow_missing) > if self._imagename: > self._imagename.SetAllowMissing(allow_missing) > @@ -220,7 +235,7 @@ class Entry_mkimage(Entry): > Args: > allow_fake: True if allowed, False if not allowed > """ > - for entry in self._mkimage_entries.values(): > + for entry in self._entries.values(): > entry.SetAllowFakeBlob(allow_fake) > if self._imagename: > self._imagename.SetAllowFakeBlob(allow_fake) > @@ -234,7 +249,7 @@ class Entry_mkimage(Entry): > Args: > missing_list: List of Entry objects to be added to > """ > - for entry in self._mkimage_entries.values(): > + for entry in self._entries.values(): > entry.CheckMissing(missing_list) > if self._imagename: > self._imagename.CheckMissing(missing_list) > @@ -247,7 +262,7 @@ class Entry_mkimage(Entry): > Args: > faked_blobs_list: List of Entry objects to be added to > """ > - for entry in self._mkimage_entries.values(): > + for entry in self._entries.values(): > entry.CheckFakedBlobs(faked_blobs_list) > if self._imagename: > self._imagename.CheckFakedBlobs(faked_blobs_list) > @@ -255,3 +270,6 @@ class Entry_mkimage(Entry): > def AddBintools(self, btools): > super().AddBintools(btools) > self.mkimage = self.AddBintool(btools, 'mkimage') > + > + def CheckEntries(self): > + pass > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > index dabb3f689fdb..6d0ffda2f432 100644 > --- a/tools/binman/ftest.py > +++ b/tools/binman/ftest.py > @@ -3737,6 +3737,7 @@ class TestFunctional(unittest.TestCase): > > def testMkimage(self): > """Test using mkimage to build an image""" > + self._SetupSplElf() > data = self._DoReadFile('156_mkimage.dts') > > # Just check that the data appears in the file somewhere > @@ -3744,6 +3745,7 @@ class TestFunctional(unittest.TestCase): > > def testMkimageMissing(self): > """Test that binman still produces an image if mkimage is missing""" > + self._SetupSplElf() > with test_util.capture_sys_output() as (_, stderr): > self._DoTestFile('156_mkimage.dts', > force_missing_bintools='mkimage') > @@ -5952,6 +5954,7 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > > def testMkimageCollection(self): > """Test using a collection referring to an entry in a mkimage > entry""" > + self._SetupSplElf() > data = self._DoReadFile('247_mkimage_coll.dts') > expect = U_BOOT_SPL_DATA + U_BOOT_DATA > self.assertEqual(expect, data[:len(expect)]) > @@ -6051,6 +6054,39 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > expect += U_BOOT_SPL_DATA > self.assertEqual(expect, data[-len(expect):]) > > + def testMkimageMultipleExpanded(self): > + """Test passing multiple files to mkimage in a mkimage entry""" > + self._SetupSplElf() > + self._SetupTplElf() > + entry_args = { > + 'spl-bss-pad': 'y', > + 'spl-dtb': 'y', > + } > + data = self._DoReadFileDtb('252_mkimage_mult_data.dts', > + use_expanded=True, > entry_args=entry_args)[0] > + pad_len = 10 > + tpl_expect = U_BOOT_TPL_DATA > + spl_expect = U_BOOT_SPL_NODTB_DATA + tools.get_bytes(0, pad_len) > + spl_expect += U_BOOT_SPL_DTB_DATA > + > + content = data[0x40:] > + lens = struct.unpack('>III', content[:12]) > + > + # Size of files are packed in their 4B big-endian format > + # Size info is always followed by a 4B zero value. > + self.assertEqual(len(tpl_expect), lens[0]) > + self.assertEqual(len(spl_expect), lens[1]) > + self.assertEqual(0, lens[2]) > + > + rest = content[12:] > + self.assertEqual(tpl_expect, rest[:len(tpl_expect)]) > + > + rest = rest[len(tpl_expect):] > + align_pad = len(tpl_expect) % 4 > + self.assertEqual(tools.get_bytes(0, align_pad), rest[:align_pad]) > + rest = rest[align_pad:] > + self.assertEqual(spl_expect, rest) > + > def testMkimageMultipleNoContent(self): > """Test passing multiple data files to mkimage with one data file > having no content""" > with self.assertRaises(ValueError) as exc: > @@ -6696,6 +6732,13 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > entry_args=entry_args, use_expanded=True, > no_write_symbols=True) > > + def testMkimageSpecial(self): > + """Test mkimage ignores special hash-1 node""" > + data = self._DoReadFile('283_mkimage_special.dts') > + > + # Just check that the data appears in the file somewhere > + self.assertIn(U_BOOT_DATA, data) > + > > -if __name__ == "__main__": > +if __name__ == "_s_main__": > unittest.main() > diff --git a/tools/binman/test/283_mkimage_special.dts > b/tools/binman/test/283_mkimage_special.dts > new file mode 100644 > index 000000000000..c234093e6ece > --- /dev/null > +++ b/tools/binman/test/283_mkimage_special.dts > @@ -0,0 +1,24 @@ > +// SPDX-License-Identifier: GPL-2.0+ > + > +/dts-v1/; > + > +/ { > + #address-cells = <1>; > + #size-cells = <1>; > + > + binman { > + mkimage { > + args = "-T script"; > + > + u-boot { > + }; > + > + hash { > + }; > + > + imagename { > + type = "u-boot"; > + }; > + }; > + }; > +};