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

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Tue, Feb 11, 2025 at 10:33:49AM +0100, Dario Binacchi wrote:
> 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.
> 
> 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>
> ---
>  boot/uboot/Config.in                   | 8 ++++++++
>  boot/uboot/uboot.mk                    | 3 ++-
>  configs/olimex_a64_olinuxino_defconfig | 2 +-
>  configs/orangepi_pc2_defconfig         | 2 +-
>  configs/orangepi_zero2w_defconfig      | 2 +-
>  configs/orangepi_zero3_defconfig       | 2 +-
>  configs/orangepi_zero_plus2_defconfig  | 2 +-
>  configs/orangepi_zero_plus_defconfig   | 2 +-
>  configs/pine64_defconfig               | 2 +-
>  support/testing/tests/boot/test_atf.py | 2 +-
>  10 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
> index f17f932c9e95..6087534fee2d 100644
> --- a/boot/uboot/Config.in
> +++ b/boot/uboot/Config.in
> @@ -744,4 +744,12 @@ config BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS
>  	  List of custom make options passed at build time. Can be
>  	  used for example to pass a DEVICE_TREE= value.
>  
> +config BR2_TARGET_UBOOT_SCP_FIRMWARE
> +	string "System Control Processor (SCP) firmware location"
> +	help
> +	  Location of a SCP firmware binary.
> +
> +	  If not empty, holds firmware for an external platform-specific
> +	  coprocessor.
> +
>  endif # BR2_TARGET_UBOOT
> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> index 2796b0a31010..18997cbdbe26 100644
> --- a/boot/uboot/uboot.mk
> +++ b/boot/uboot/uboot.mk
> @@ -176,7 +176,8 @@ UBOOT_MAKE_OPTS += \
>  	ARCH=$(UBOOT_ARCH) \
>  	HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem /,$(HOST_CFLAGS)))" \
>  	HOSTLDFLAGS="$(HOST_LDFLAGS)" \
> -	$(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS))
> +	$(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS)) \
> +	SCP=$(BR2_TARGET_UBOOT_SCP_FIRMWARE)
>  
>  # Disable FDPIC if enabled by default in toolchain
>  ifeq ($(BR2_BINFMT_FDPIC),y)
> diff --git a/configs/olimex_a64_olinuxino_defconfig b/configs/olimex_a64_olinuxino_defconfig
> index ad446ea18c82..b63ea51bc6d3 100644
> --- a/configs/olimex_a64_olinuxino_defconfig
> +++ b/configs/olimex_a64_olinuxino_defconfig
> @@ -29,7 +29,7 @@ BR2_TARGET_UBOOT_NEEDS_OPENSSL=y
>  BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
>  BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
>  BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot-sunxi-with-spl.bin"
> -BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"
> +BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"
>  BR2_PACKAGE_HOST_DOSFSTOOLS=y
>  BR2_PACKAGE_HOST_GENIMAGE=y
>  BR2_PACKAGE_HOST_MTOOLS=y
> diff --git a/configs/orangepi_pc2_defconfig b/configs/orangepi_pc2_defconfig
> index 46d04b34c845..d61ffd0f5f24 100644
> --- a/configs/orangepi_pc2_defconfig
> +++ b/configs/orangepi_pc2_defconfig
> @@ -33,5 +33,5 @@ BR2_TARGET_UBOOT_NEEDS_OPENSSL=y
>  BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
>  BR2_TARGET_UBOOT_SPL=y
>  BR2_TARGET_UBOOT_SPL_NAME="u-boot-sunxi-with-spl.bin"
> -BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"
> +BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"
>  BR2_PACKAGE_HOST_GENIMAGE=y
> diff --git a/configs/orangepi_zero2w_defconfig b/configs/orangepi_zero2w_defconfig
> index c2030b05c20a..64c06d42315a 100644
> --- a/configs/orangepi_zero2w_defconfig
> +++ b/configs/orangepi_zero2w_defconfig
> @@ -37,6 +37,6 @@ BR2_TARGET_UBOOT_NEEDS_GNUTLS=y
>  BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
>  BR2_TARGET_UBOOT_SPL=y
>  BR2_TARGET_UBOOT_SPL_NAME="u-boot-sunxi-with-spl.bin"
> -BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"
> +BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"
>  BR2_PACKAGE_HOST_GENIMAGE=y
>  BR2_PACKAGE_HOST_UBOOT_TOOLS=y
> diff --git a/configs/orangepi_zero3_defconfig b/configs/orangepi_zero3_defconfig
> index c6aa9c991a6c..78c7d6daff00 100644
> --- a/configs/orangepi_zero3_defconfig
> +++ b/configs/orangepi_zero3_defconfig
> @@ -38,6 +38,6 @@ BR2_TARGET_UBOOT_NEEDS_GNUTLS=y
>  BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
>  BR2_TARGET_UBOOT_SPL=y
>  BR2_TARGET_UBOOT_SPL_NAME="u-boot-sunxi-with-spl.bin"
> -BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"
> +BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"
>  BR2_PACKAGE_HOST_GENIMAGE=y
>  BR2_PACKAGE_HOST_UBOOT_TOOLS=y
> diff --git a/configs/orangepi_zero_plus2_defconfig b/configs/orangepi_zero_plus2_defconfig
> index 2b7ad2064d96..73b18533a28e 100644
> --- a/configs/orangepi_zero_plus2_defconfig
> +++ b/configs/orangepi_zero_plus2_defconfig
> @@ -45,5 +45,5 @@ BR2_TARGET_UBOOT_NEEDS_GNUTLS=y
>  BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
>  BR2_TARGET_UBOOT_SPL=y
>  BR2_TARGET_UBOOT_SPL_NAME="u-boot-sunxi-with-spl.bin"
> -BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"
> +BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"
>  BR2_PACKAGE_HOST_GENIMAGE=y
> diff --git a/configs/orangepi_zero_plus_defconfig b/configs/orangepi_zero_plus_defconfig
> index 48c8e993f020..c6eee1d4115b 100644
> --- a/configs/orangepi_zero_plus_defconfig
> +++ b/configs/orangepi_zero_plus_defconfig
> @@ -34,5 +34,5 @@ BR2_TARGET_UBOOT_NEEDS_OPENSSL=y
>  BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
>  BR2_TARGET_UBOOT_SPL=y
>  BR2_TARGET_UBOOT_SPL_NAME="u-boot-sunxi-with-spl.bin"
> -BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"
> +BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"
>  BR2_PACKAGE_HOST_GENIMAGE=y
> diff --git a/configs/pine64_defconfig b/configs/pine64_defconfig
> index 8975f3b1a211..f719cd9d2fef 100644
> --- a/configs/pine64_defconfig
> +++ b/configs/pine64_defconfig
> @@ -29,7 +29,7 @@ BR2_TARGET_UBOOT_NEEDS_OPENSSL=y
>  BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
>  BR2_TARGET_UBOOT_SPL=y
>  BR2_TARGET_UBOOT_SPL_NAME="u-boot-sunxi-with-spl.bin"
> -BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"
> +BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"
>  BR2_PACKAGE_HOST_DOSFSTOOLS=y
>  BR2_PACKAGE_HOST_GENIMAGE=y
>  BR2_PACKAGE_HOST_MTOOLS=y
> diff --git a/support/testing/tests/boot/test_atf.py b/support/testing/tests/boot/test_atf.py
> index 6142dd36c12a..d00bfa0ff931 100644
> --- a/support/testing/tests/boot/test_atf.py
> +++ b/support/testing/tests/boot/test_atf.py
> @@ -25,7 +25,7 @@ class TestATFAllwinner(infra.basetest.BRTest):
>          BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
>          BR2_TARGET_UBOOT_SPL=y
>          BR2_TARGET_UBOOT_SPL_NAME="u-boot-sunxi-with-spl.bin"
> -        BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"
> +        BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"
>          """
>  
>      def test_run(self):
> -- 

Hello Dario,

here:
https://lore.kernel.org/buildroot/a023971c7c8bfa4826a9a8721500c7ff@free.fr/

Julien said that it does not need to be /dev/null,
and that it works fine with just an empty string
(if the default is the empty string).


So, shouldn't you simply be able to remove the:

BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="SCP=/dev/null"

lines from the defconfigs, without any need to add any
BR2_TARGET_UBOOT_SCP_FIRMWARE="/dev/null"

lines to the defconfigs?


Kind regards,
Niklas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot