Re: [Buildroot] [PATCH 1/1] boot/uboot: support System Control Processor (SCP) firmware blob

From: Peter Korsgaard <peter@korsgaard.com>
To: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: buildroot@buildroot.org,
	 Giulio Benetti <giulio.benetti@benettiengineering.com>,
	 Jagan Teki <jagan@amarulasolutions.com>,
	 Javad Rahimi <javad321javad@gmail.com>,
	Marcin Niestroj <m.niestroj@grinn-global.com>,
	 Sergey Matyukevich <geomatsi@gmail.com>,
	 linux-amarula@amarulasolutions.com,
	 Julien Olivain <ju.o@free.fr>
Subject: Re: [Buildroot] [PATCH 1/1] boot/uboot: support System Control Processor (SCP) firmware blob
Date: Tue, 11 Feb 2025 17:48:15 +0100	[thread overview]
Message-ID: <87zfis2zsg.fsf@dell.be.48ers.dk> (raw)
In-Reply-To: <20250211093349.2153008-1-dario.binacchi@amarulasolutions.com> (Dario Binacchi's message of "Tue, 11 Feb 2025 10:33:49 +0100")

>>>>> "Dario" == Dario Binacchi <dario.binacchi@amarulasolutions.com> writes:

 > The commit 4bce3270d680 ("Config.in: timeout earlier when connecting to
 > download servers"), among other things, adds the connection timeout
 > parameter to the default scp command:

 > config BR2_SCP
 >        string "Secure copy (scp) command"
 >        default "scp -o ConnectTimeout=10"

 > Since the package/pkg-download.mk file exports this command using the
 > SCP variable:

 > export SCP := $(call qstrip,$(BR2_SCP))

 > and the U-Boot Makefile uses this variable as the path for the system
 > control processor (SCP) firmware blob:

 > cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
 > 		$(foreach f,$(BINMAN_TOOLPATHS),--toolpath $(f)) \
 >                 --toolpath $(objtree)/tools \
 > 		$(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \
 > 		build -u -d $(binman_dtb) -O . -m \
 > 		--allow-missing --fake-ext-blobs \
 > 		$(if $(BINMAN_ALLOW_MISSING),--ignore-missing) \
 > 		-I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
 > 		$(foreach f,$(of_list_dirs),-I $(f)) -a of-list=$(of_list) \
 > 		$(foreach f,$(BINMAN_INDIRS),-I $(f)) \
 > 		-a atf-bl31-path=${BL31} \
 > 		-a tee-os-path=${TEE} \
 > 		-a ti-dm-path=${TI_DM} \
 > 		-a opensbi-path=${OPENSBI} \
 > 		-a default-dt=$(default_dt) \
 > 		-a scp-path=$(SCP) \

 > the following error occurs:

 >   BINMAN  .binman_stamp
 > usage: binman [-h] [-B BUILD_DIR] [-D] [-H] [--tooldir TOOLDIR] [--toolpath TOOLPATH] [-T THREADS] [--test-section-timeout] [-v VERBOSITY] [-V]
 >               {build,bintool-docs,entry-docs,ls,extract,replace,sign,test,tool} ...
 > binman: error: unrecognized arguments: -o ConnectTimeout=10
 > make[2]: *** [Makefile:1126: .binman_stamp] Error 2

 > The issue was already present, as the scp command was being passed
 > instead of the path to the SCP firmware, but it is now detected due to
 > the compilation error.

Indeed.

 > Adding the BR2_TARGET_UBOOT_SCP_FIRMWARE option allows passing the
 > correct parameter to U-Boot, even if it is not set. This also requires
 > removing the SCP setting from BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS for
 > configurations that previously set this variable explicitly.

 > Fixes: 4bce3270d680 ("Config.in: timeout earlier when connecting to download servers")
 > Suggested-by: Julien Olivain <ju.o@free.fr>
 > Co-Developed-by: Julien Olivain <ju.o@free.fr>
 > Signed-off-by: Julien Olivain <ju.o@free.fr>
 > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

So the real fix is presumably to stop globally exporting all these
variables in package/pkg-download.mk. I don't right away see why they
couldn't just be passed in $(PKG)_DL_ENV to the dl-wrapper instead.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot