Hi Jonas, On Tue, 14 Feb 2023 at 03:34, Jonas Karlman <jo...@kwiboo.se> wrote: > > Implement CheckMissing and CheckOptional methods that is adapted to > Entry_mkimage in order to improve support for allow missing flag. > > Use collect_contents_to_file in multiple-data-files handling to improve > support for allow missing and fake blobs handling. > > Signed-off-by: Jonas Karlman <jo...@kwiboo.se> > --- > Building for RK3568 without ROCKCHIP_TPL will result in the following > error message, regardless if BINMAN_ALLOW_MISSING is used or not. > > binman: Filename 'rockchip-tpl' not found in input path (...) > > With this patch and using BINMAN_ALLOW_MISSING=1 a new external blob > with no content is created and then passed to mkimage, resulting in an > error from the mkimage command. > > binman: Error 255 running 'mkimage -d > ./mkimage-0.simple-bin.mkimage:./mkimage-1.simple-bin.mkimage -n rk3568 -T > rksd ./idbloader.img': mkimage: Can't read ./mkimage-0.simple-bin.mkimage: > Invalid argument > > If the --fake-ext-blobs argument is also used I get the message I was > expecting to see from the beginning. > > Image 'main-section' is missing external blobs and is non-functional: > rockchip-tpl > > /binman/simple-bin/mkimage/rockchip-tpl: > An external TPL is required to initialize DRAM. Get the external TPL > binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source > for the external TPL binary is https://github.com/rockchip-linux/rkbin. > Image 'main-section' has faked external blobs and is non-functional: > rockchip-tpl > > Some images are invalid > > Not sure how this should work, but I was expecting to see the message > about the missing rockchip-tpl from the beginning instead of the generic > "not found in input path" message. At leas with BINMAN_ALLOW_MISSING=1. > > tools/binman/etype/mkimage.py | 37 +++++++++++++++++++++++++++++++---- > 1 file changed, 33 insertions(+), 4 deletions(-) > > diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py > index cb264c3cad0b..44510a8c40ba 100644 > --- a/tools/binman/etype/mkimage.py > +++ b/tools/binman/etype/mkimage.py > @@ -153,10 +153,12 @@ class Entry_mkimage(Entry): > if self._multiple_data_files: > fnames = [] > uniq = self.GetUniqueName() > - for entry in self._mkimage_entries.values(): > - if not entry.ObtainContents(fake_size=fake_size): > + for idx, entry in enumerate(self._mkimage_entries.values()): > + entry_data, entry_fname, _ = self.collect_contents_to_file( > + [entry], 'mkimage-%s' % idx, fake_size) > + if entry_data is None:
This is OK, I suppose, but I'm not quite sure what actually changes here, other than writing each entry to a file? Also, if you do this, please add / extend a test that checks that the output files are written, since there is otherwise no coverage here. What test uses these optional mkimage pieces? > return False > - > fnames.append(tools.get_input_filename(entry.GetDefaultFilename())) > + fnames.append(entry_fname) > input_fname = ":".join(fnames) > else: > data, input_fname, uniq = self.collect_contents_to_file( > @@ -165,7 +167,7 @@ class Entry_mkimage(Entry): > return False > if self._imagename: > image_data, imagename_fname, _ = self.collect_contents_to_file( > - [self._imagename], 'mkimage-n', 1024) > + [self._imagename], 'mkimage-n', fake_size) > if image_data is None: > return False > outfile = self._filename if self._filename else 'mkimage-out.%s' % > uniq > @@ -216,6 +218,20 @@ class Entry_mkimage(Entry): > if self._imagename: > self._imagename.SetAllowFakeBlob(allow_fake) > > + def CheckMissing(self, missing_list): > + """Check if any entries in this section have missing external blobs > + > + If there are missing (non-optional) blobs, the entries are added to > the > + list > + > + Args: > + missing_list: List of Entry objects to be added to > + """ > + for entry in self._mkimage_entries.values(): > + entry.CheckMissing(missing_list) > + if self._imagename: > + self._imagename.CheckMissing(missing_list) > + > def CheckFakedBlobs(self, faked_blobs_list): > """Check if any entries in this section have faked external blobs > > @@ -229,6 +245,19 @@ class Entry_mkimage(Entry): > if self._imagename: > self._imagename.CheckFakedBlobs(faked_blobs_list) > > + def CheckOptional(self, optional_list): > + """Check the section for missing but optional external blobs > + > + If there are missing (optional) blobs, the entries are added to the > list > + > + Args: > + optional_list (list): List of Entry objects to be added to > + """ > + for entry in self._mkimage_entries.values(): > + entry.CheckOptional(optional_list) > + if self._imagename: > + self._imagename.CheckOptional(optional_list) > + > def AddBintools(self, btools): > super().AddBintools(btools) > self.mkimage = self.AddBintool(btools, 'mkimage') > -- > 2.39.1 > Regards, Simon