Hi Jonas,
On Wed, 15 Feb 2023 at 11:25, Jonas Karlman jonas@kwiboo.se wrote:
Hi Simon,
On 2023-02-14 20:48, Simon Glass wrote:
Hi Jonas,
On Tue, 14 Feb 2023 at 03:34, Jonas Karlman jonas@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 jonas@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%3E%3E%3E 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?
The collect_contents_to_file function seemed to handle the external/missing/optional/faked entry flags. So I changed to use that function in order to see if that would change anything, see below.
Use of this function does make it possible to use entry type other then external blobs, not sure if that would ever be needed/useful.
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?
Sorry, I was not clear enough about the reason for these changes in my message above.
Building with --rockchip-tpl-path=/path/to/existing/tpl works as expected and generates a working image.
I was expecting that the missing-blob-help message added in patch 1 would be shown by binman when rockchip-tpl-path was empty/not-existing, or at least together with the allow-missing flag.
However, whatever I tested, empty/none-existing rockchip-tpl-path, allow-missing, fake-ext-blobs, I was not able to see this message. Instead, all I could get from binman was this "Filename 'rockchip-tpl' not found in input path" message.
Maybe my assumptions about when these missing messages should be shown is wrong?
Trying binman with the following two dts and --allow-missing gives different results. First one shows the missing message, second one show filename not found.
binman { rockchip-tpl { }; };
binman { mkimage { args = "-n rk3568 -T rksd"; multiple-data-files;
rockchip-tpl { }; };};
With the changes in this patch I instead get the missing message when I also add the --fake-ext-blobs flag, so it clearly needs more work or a completely different approach if we want to be able to see the missing message added in patch 1.
Adding a message that never will be displayed annoys me :-) Maybe I should just drop this rfc/patch for a v3 of this series?
Does the mkimage etype need a CheckMissing() function? That is how binman knows whether something is missing.
Regards, Simon [..]