Hi Tom,
On Mon, 12 Dec 2022 at 16:43, Tom Rini trini@konsulko.com wrote:
On Tue, Dec 06, 2022 at 10:03:37AM -0500, Tom Rini wrote:
On Tue, Dec 06, 2022 at 03:36:49PM +1300, Simon Glass wrote:
Hi Tom,
On Tue, 6 Dec 2022 at 15:03, Tom Rini trini@konsulko.com wrote:
When the user builds with BINMAN_ALLOW_MISSING=1 they're explicitly setting the flag to allow for additional binaries to be missing and so have acknowledged the output might not work. In this case we want to default to not passing a non-zero exit code.
Cc: Simon Glass sjg@chromium.org Reported-by: Peter Robinson pbrobinson@gmail.com Signed-off-by: Tom Rini trini@konsulko.com
This passes CI as-is: https://source.denx.de/u-boot/u-boot/-/pipelines/14340
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index de5746399a63..03de1da1bfd0 100644 --- a/Makefile +++ b/Makefile @@ -1334,7 +1334,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ --toolpath $(objtree)/tools \ $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \ build -u -d u-boot.dtb -O . -m \
$(if $(BINMAN_ALLOW_MISSING),--allow-missing --fake-ext-blobs) \
$(if $(BINMAN_ALLOW_MISSING),--allow-missing --ignore-missing) \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \ $(foreach f,$(BINMAN_INDIRS),-I $(f)) \-- 2.25.1
I believe we need the --fake-ext-blobs flag as well, since otherwise boards which use tools (like mkimage) on things that don't exist will not work.
So, we need to list out all the use cases perhaps, as we had missed one before. In my mind we have:
- Board requires 1 or more blobs, developer will be running this on hardware, expects output from U-Boot 'make' (or buildman) to boot. Blobs must be present or non-zero exist status by default. We didn't have this before, and we do now.
- Board requires 1 or more blobs AND has optional blobs, will be running this on hardware, expects output from U-Boot 'make' (or buildman) to boot. This is the case we had missed before as allwinner requires bl31 and has optional PMIC firmware. This is the case Peter reported and we had missed before, which this patch allows for, with the caveat that if you forget BL31 you're not going to boot and make won't complain exit-code wise. Another way to resolve this would be a property in the binman node to mark a blob as optional and warn if missing, rather than error if missing.
- Board requires 1 or more blobs, output will NOT be run. This is the CI case and the compile testing lots of board cases. This is what CI passes still, with the above.
- Board requires 1 or more blobs, developer will be running this on hardware, BUT will be injecting the blobs later on. I think this is the use case you're talking about?
Simon, can you test this last case by chance? I think that's the one that's unclear about if this patch breaks or not and I want to know if I need to respin to still include the fake blobs flag still too or not. Or does anyone else use this case and can test?
Yes I have your email printed out but travel and family things have made it hard to get to it. I do think you need the flag, but the extra case you mention need another look and probably a test, as you say.
Regards, Simon