Re: [ptxdist] [PATCH v4 4/4] barebox: add integration of firmware blobs

[Michael Olbrich <m.olbrich@pengutronix.de>]

On Mon, Dec 20, 2021 at 01:08:57PM +0100, Michael Riesch wrote:
> In some cases barebox requires firmware blobs, which may be
> provided in binary form by the vendor or compiled in a
> preceding step. Add the possibility to specify files (in
> separate rule files) which are injected in the barebox source
> directory during preparation.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  platforms/barebox.in          |  8 ++++++++
>  platforms/barebox.rockchip.in | 10 ++++++++++
>  rules/barebox.make            |  4 ++++
>  rules/barebox.rockchip.make   | 16 ++++++++++++++++
>  4 files changed, 38 insertions(+)
>  create mode 100644 platforms/barebox.rockchip.in
>  create mode 100644 rules/barebox.rockchip.make
> 
> diff --git a/platforms/barebox.in b/platforms/barebox.in
> index d35d16501..8a81be40f 100644
> --- a/platforms/barebox.in
> +++ b/platforms/barebox.in
> @@ -55,6 +55,14 @@ config BAREBOX_CONFIG
>  	  This entry specifies the .config file used to compile
>  	  barebox.
>  
> +menuconfig BAREBOX_FIRMWARE
> +	bool
> +	prompt "integrate firmware blobs      "
> +
> +if BAREBOX_FIRMWARE
> +source "generated/barebox_firmware.in"
> +endif

If you split out the generic parts and add empty files with just the
"## SECTION=" header (for both new sections) then we could maybe get that
merged while we figure out how to handle the rockchip part.

> +
>  config BAREBOX_EXTRA_ENV
>  	prompt "extend the builtin barebox environment"
>  	bool
> diff --git a/platforms/barebox.rockchip.in b/platforms/barebox.rockchip.in
> new file mode 100644
> index 000000000..238798ce1
> --- /dev/null
> +++ b/platforms/barebox.rockchip.in
> @@ -0,0 +1,10 @@
> +## SECTION=barebox_firmware
> +
> +config BAREBOX_NEEDS_FIRMWARE_ROCKCHIP
> +	prompt "barebox needs firmware-rockchip"
> +	bool
> +	depends on ARCH_ARM64
> +	select FIRMWARE_ROCKCHIP if BAREBOX_NEEDS_FIRMWARE_ROCKCHIP
> +	help
> +	  Select this if barebox needs the non-free Rockchip firmware
> +	  blobs.
> diff --git a/rules/barebox.make b/rules/barebox.make
> index bea9f3adc..cc70a6ff5 100644
> --- a/rules/barebox.make
> +++ b/rules/barebox.make
> @@ -94,6 +94,10 @@ ifdef PTXCONF_BAREBOX_EXTRA_ENV
>  	@rm -rf $(BAREBOX_BUILD_DIR)/defaultenv/barebox_default_env
>  endif
>  
> +ifdef PTXCONF_BAREBOX_FIRMWARE
> +	@$(call world/inject, BAREBOX)
> +endif
> +
>  	@$(call touch)
>  
>  # ----------------------------------------------------------------------------
> diff --git a/rules/barebox.rockchip.make b/rules/barebox.rockchip.make
> new file mode 100644
> index 000000000..33795bee4
> --- /dev/null
> +++ b/rules/barebox.rockchip.make
> @@ -0,0 +1,16 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2021 by Michael Riesch <michael.riesch@wolfvision.net>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +ifdef PTXCONF_BAREBOX_NEEDS_FIRMWARE_ROCKCHIP
> +
> +BAREBOX_INJECT_PATH	+= ${PTXDIST_SYSROOT_TARGET}/usr/lib/firmware

This should be in barebox.make. BAREBOX_INJECT_PATH is a ':' separated
path, so expanding it like this doesn't work anyways.

So in general I've been thinking which parts make sense in PTXdist upstream
and which parts should be in the BSPs. I mean it doesn't really scale, if
we add firmware files for all boards into PTXdist upstream. So I had the
vague idea that we could add files that are SoC specific to PTXdist and
leave the board specific files in the BSP. Then I saw this:

> +BAREBOX_INJECT_FILES	+= rk3568_ddr_1560MHz_v1.08.bin:arch/arm/boards/rockchip-rk3568-evb/sdram-init.bin

My understanding was that this file is the code that does the SDRAM timing
calibration. So I expected it to be SoC specific and therefore in a more
generic path, not the board directory.
Am I missing something here?

> +BAREBOX_INJECT_FILES	+= rk3568_bl31_v1.24.elf:firmware/rk3568-bl31.bin

That's TF-A right?

> +BAREBOX_INJECT_FILES	+= rk3568_bl32_v1.05.bin:firmware/rk3568-op-tee.bin

I expected these files to be board specific, but they are in a generic
directory? Also, I would expect those to be built from source.

And about the suboptions: Do we actually need them? I mean, if we copy a
few extra files, then barebox will just ignore them, right?

Regards,
Michael

> +
> +endif
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de
> To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de