On Thu, Dec 09, 2021 at 07:10:49AM +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 which
> are injected in the barebox source directory during
> preparation.

So, I've been thinking about this some more. And I think this needs a
different approach.

With SDRAM calibration and TF-A and stuff like that, there will be a lot of
blobs that need to be integrated into barebox.
And it will be different for each vendor and possibly SOC family. And maybe
a upstream and a downstream version.

Modifying barebox.in for each of them will not scale. Especially because in
most cases we'll carry a modified version until it's merged into ptxdist
master and the BSP is updated to the new version.

And from what I've seen with TF-A, it's not uncommon to start with some
ugly downstream stuff and switch to upstream later.

So I'd like this to be more flexible. More below.

> Signed-off-by: Michael Riesch <[email protected]>
> ---
>  platforms/barebox.in                   | 35 +++++++++++++++++++++
>  rules/barebox.make                     |  6 ++++
>  rules/post/ptxd_make_world_inject.make | 19 ++++++++++++
>  scripts/lib/ptxd_make_world_inject.sh  | 42 ++++++++++++++++++++++++++
>  4 files changed, 102 insertions(+)
>  create mode 100644 rules/post/ptxd_make_world_inject.make
>  create mode 100644 scripts/lib/ptxd_make_world_inject.sh
> 
> diff --git a/platforms/barebox.in b/platforms/barebox.in
> index d35d16501..63e9929e5 100644
> --- a/platforms/barebox.in
> +++ b/platforms/barebox.in
> @@ -15,6 +15,7 @@ menuconfig BAREBOX
>       select HOST_IMX_CST if BAREBOX_NEEDS_HOST_IMX_CST
>       select HOST_LZOP if BAREBOX_NEEDS_HOST_LZOP
>       select CODE_SIGNING if BAREBOX_NEEDS_KEYS
> +     select FIRMWARE_ROCKCHIP if BAREBOX_NEEDS_FIRMWARE_ROCKCHIP
>       prompt "barebox                       "
>       bool
>       help
> @@ -55,6 +56,32 @@ config BAREBOX_CONFIG
>         This entry specifies the .config file used to compile
>         barebox.
>  
> +menuconfig BAREBOX_FIRMWARE
> +     bool
> +     prompt "integrate firmware blobs      "
> +
> +if BAREBOX_FIRMWARE
> +config BAREBOX_FIRMWARE_PATH
> +     string "path(s) to firmware blobs"
> +     default "${PTXDIST_SYSROOT_TARGET}/usr/lib/firmware"
> +     help
> +       Define path to the firmware blob(s). Multiple directories can
> +       be specified separated by ':'. A relative path will be expanded 
> relative
> +       to the workspace and all other layers. Only one of the specified paths
> +       can be a relative path.

I think we can just define a fixed directory for this. For exceptions, and
absolute file path will work as well.

> +config BAREBOX_FIRMWARE_FILES
> +     string "firmware blob file(s)"
> +     default "<vendorblob>.bin:firmware"

I don't like having this in the config. The problem ist, that the user will
need to know, which files to copy and where to.

> +     help
> +       Select the firmware blob(s) to be integrated into the barebox
> +       source before compilation. Each entry consists of <file>:<target>,
> +       where <target> is an optional path relative to the barebox source
> +       directory. Multiple entries can be specified, separated by spaces.
> +
> +endif
> +
>  config BAREBOX_EXTRA_ENV
>       prompt "extend the builtin barebox environment"
>       bool
> @@ -146,4 +173,12 @@ config BAREBOX_NEEDS_HOST_LZOP
>         lzop is used in order to compile lzop for your development
>         host.
>  
> +config BAREBOX_NEEDS_FIRMWARE_ROCKCHIP
> +     prompt "barebox needs firmware-rockchip"
> +     bool
> +     depends on ARCH_ARM64
> +     help
> +       Select this if barebox needs the non-free Rockchip firmware
> +       blobs.

As I said, I don't want to modify barebox.in for each vendor / SOC family.

So how about something like this:

------------------------------------
menu "firmware blobs"

source barebox_firmware.in

endmenu
------------------------------------

And the firmware package will have an extra menu file, e.g.
barebox-firmware-rockchip.in:

------------------------------------
## SECTION=barebox_firmware

config BAREBOX
        select FIRMWARE_ROCKCHIP if BAREBOX_NEEDS_FIRMWARE_ROCKCHIP

config BAREBOX_NEEDS_FIRMWARE_ROCKCHIP
        prompt "barebox needs firmware-rockchip"
        ...

------------------------------------
Maybe some suboptions for individual SoCs / boards.

And an extra rule file, e.g. barebox.rockchip.make:
------------------------------------

BAREBOX_INJECT_FILES += source-file:target-file

------------------------------------
Maybe with some conditions depending on the suboptions.
And if absolutely necessary, the source-file could expand to an absolute
path here.

Once the core support is upstream in ptxdist. These small drop-ins can be
easily maintained in a BSP but are also easier to upstream because there
are no conflicting changes to barebox.in.

And with the latest ptxdist version, <pkg>.<something>.make can be
overwritten in the BSP just like the regular rule file. So if the upstream
version of such a drop-in has a bug, fixing that in the BSP is possible.

Michael

> +
>  endif
> diff --git a/rules/barebox.make b/rules/barebox.make
> index bea9f3adc..a81fc86b3 100644
> --- a/rules/barebox.make
> +++ b/rules/barebox.make
> @@ -26,6 +26,8 @@ BAREBOX_BUILD_DIR   := $(BAREBOX_DIR)-build
>  BAREBOX_LICENSE              := GPL-2.0-only
>  BAREBOX_DEVPKG               := NO
>  BAREBOX_BUILD_OOT    := KEEP
> +BAREBOX_INJECT_PATH  :=$(call remove_quotes,$(PTXCONF_BAREBOX_FIRMWARE_PATH))
> +BAREBOX_INJECT_FILES :=$(call 
> remove_quotes,$(PTXCONF_BAREBOX_FIRMWARE_FILES))
>  
>  BAREBOX_CONFIG               := $(call ptx/in-platformconfigdir, \
>               $(call remove_quotes, $(PTXCONF_BAREBOX_CONFIG)))
> @@ -94,6 +96,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/post/ptxd_make_world_inject.make 
> b/rules/post/ptxd_make_world_inject.make
> new file mode 100644
> index 000000000..b7d28e92f
> --- /dev/null
> +++ b/rules/post/ptxd_make_world_inject.make
> @@ -0,0 +1,19 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2021 by Michael Riesch <[email protected]>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +world/inject/env = \
> +     $(call world/env, $(1)) \
> +     pkg_inject_path="$($(1)_INJECT_PATH)" \
> +     pkg_inject_files="$($(1)_INJECT_FILES)" \
> +     pkg_source="$($(1)_DIR)"
> +
> +world/inject = \
> +     $(call world/inject/env,$(strip $(1))) \
> +     ptxd_make_world_inject
> +
> +# vim: syntax=make
> diff --git a/scripts/lib/ptxd_make_world_inject.sh 
> b/scripts/lib/ptxd_make_world_inject.sh
> new file mode 100644
> index 000000000..c2e45ad42
> --- /dev/null
> +++ b/scripts/lib/ptxd_make_world_inject.sh
> @@ -0,0 +1,42 @@
> +#!/bin/bash
> +#
> +# Copyright (C) 2021 by Michael Riesch <[email protected]>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +ptxd_make_inject() {
> +    local source target
> +
> +    source="$(echo ${inject_file} | cut -d ":" -f 1)"
> +    target="${pkg_source}/$(echo ${inject_file} | cut -d ":" -f 2)"
> +    if [ -z "${target}" ]; then
> +     target="${source}"
> +    fi
> +
> +    if [[ "${source}" =~ ^/.* ]]; then
> +     ptxd_bailout "'${source}' must not be an absolute path!" \
> +         "Use <PKG>_INJECT_PATH to specify the search path."
> +    fi
> +
> +    if ! ptxd_in_path pkg_inject_path "${source}"; then
> +     ptxd_bailout "Blob '${source}' not found in '${pkg_inject_path}'."
> +    fi
> +    source="${ptxd_reply}"
> +
> +    echo -e "\nInject file $(ptxd_print_path ${source}) into" \
> +      "$(ptxd_print_path ${target})..."
> +    cp ${source} ${target}
> +}
> +export -f ptxd_make_inject
> +
> +
> +ptxd_make_world_inject() {
> +    ptxd_make_world_init || break
> +
> +    for inject_file in ${pkg_inject_files}; do
> +     ptxd_make_inject || break
> +    done
> +}
> +export -f ptxd_make_world_inject
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> ptxdist mailing list
> [email protected]
> To unsubscribe, send a mail with subject "unsubscribe" to 
> [email protected]
> 

-- 
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
[email protected]
To unsubscribe, send a mail with subject "unsubscribe" to 
[email protected]

Reply via email to