[Buildroot] [PATCH v5 1/3] package: add rockchip-rkbin package
Quentin Schulz
quentin.schulz at theobroma-systems.com
Wed Jul 12 09:42:54 UTC 2023
Hi Kilian,
On 7/9/23 14:33, Kilian Zinnecker wrote:
> This patch adds a package for the Rockchip ATF binary blobs. These
> binaries are needed to build U-Boot for some Rockchip SoCs (e.g.,
> RK3588). One can config a custom version and manually define which
> blobs (for bl31, tpl and optee) to use from the repository. The
> U-Boot package is modified so that it takes the chosen binaries and
> uses them during build.
>
> Signed-off-by: Kilian Zinnecker <kilian.zinnecker at mail.de>
> ---
> DEVELOPERS | 3 ++
> boot/uboot/Config.in | 9 ++++
> boot/uboot/uboot.mk | 14 +++++
> package/Config.in | 1 +
> package/rockchip-rkbin/Config.in | 62 ++++++++++++++++++++++
> package/rockchip-rkbin/rockchip-rkbin.hash | 3 ++
> package/rockchip-rkbin/rockchip-rkbin.mk | 46 ++++++++++++++++
> 7 files changed, 138 insertions(+)
> create mode 100644 package/rockchip-rkbin/Config.in
> create mode 100644 package/rockchip-rkbin/rockchip-rkbin.hash
> create mode 100644 package/rockchip-rkbin/rockchip-rkbin.mk
>
> diff --git a/DEVELOPERS b/DEVELOPERS
> index 188c579010..8e603ff3f8 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -1776,6 +1776,9 @@ F: package/ramsmp/
> N: Kieran Bingham <kieran.bingham at ideasonboard.com>
> F: package/libcamera/
>
> +N: Kilian Zinnecker <kilian.zinnecker at mail.de>
> +F: package/rockchip-rkbin/
> +
> N: Klaus Heinrich Kiwi <klaus at linux.vnet.ibm.com>
> F: package/wqy-zenhei/
>
> diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
> index 085397d03d..7733f8501f 100644
> --- a/boot/uboot/Config.in
> +++ b/boot/uboot/Config.in
> @@ -262,6 +262,15 @@ config BR2_TARGET_UBOOT_NEEDS_IMX_FIRMWARE
> This option makes sure that the i.MX firmwares are copied into
> the U-Boot source directory.
>
> +config BR2_TARGET_UBOOT_NEEDS_ROCKCHIP_RKBIN
> + bool "U-Boot needs rockchip-rkbin"
> + depends on BR2_PACKAGE_ROCKCHIP_RKBIN
> + help
> + For some Rockchip SoCs U-Boot needs binary blobs from
> + Rockchip.
> + This option makes sure that the needed binary blobs are copied
> + into the U-Boot source directory.
> +
> menu "U-Boot binary format"
>
> config BR2_TARGET_UBOOT_FORMAT_AIS
> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> index 4eae8e95c3..143432d55b 100644
> --- a/boot/uboot/uboot.mk
> +++ b/boot/uboot/uboot.mk
> @@ -207,6 +207,20 @@ endef
> UBOOT_PRE_BUILD_HOOKS += UBOOT_COPY_IMX_FW_FILES
> endif
>
> +ifeq ($(BR2_TARGET_UBOOT_NEEDS_ROCKCHIP_RKBIN),y)
> +UBOOT_DEPENDENCIES += rockchip-rkbin
> +ifeq ($(BR2_PACKAGE_ROCKCHIP_RKBIN_BL31_FILE),y)
> +UBOOT_MAKE_OPTS += BL31=$(BINARIES_DIR)/bl31.elf
> +endif
> +ifeq ($(BR2_PACKAGE_ROCKCHIP_RKBIN_TPL_FILE),y)
> +UBOOT_MAKE_OPTS += ROCKCHIP_TPL=$(BINARIES_DIR)/ddr.bin
> +endif
> +ifeq ($(BR2_PACKAGE_ROCKCHIP_RKBIN_OPTEE_FILE),y)
> +UBOOT_DEPENDENCIES += optee-os
I think this is a mistake? We want to use the one from rockchip-=rkbin
package and so we don't need to build optee-os package dont' we?
> +UBOOT_MAKE_OPTS += TEE=$(BINARIES_DIR)/tee.elf
> +endif
> +endif
> +
> ifeq ($(BR2_TARGET_UBOOT_NEEDS_DTC),y)
> UBOOT_DEPENDENCIES += host-dtc
> endif
> diff --git a/package/Config.in b/package/Config.in
> index bff090a661..80221d0406 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -581,6 +581,7 @@ endmenu
> source "package/read-edid/Config.in"
> source "package/rng-tools/Config.in"
> source "package/rockchip-mali/Config.in"
> + source "package/rockchip-rkbin/Config.in"
> source "package/rpi-userland/Config.in"
> source "package/rs485conf/Config.in"
> source "package/rtc-tools/Config.in"
> diff --git a/package/rockchip-rkbin/Config.in b/package/rockchip-rkbin/Config.in
> new file mode 100644
> index 0000000000..5cc9bd0ab5
> --- /dev/null
> +++ b/package/rockchip-rkbin/Config.in
> @@ -0,0 +1,62 @@
> +config BR2_PACKAGE_ROCKCHIP_RKBIN
> + bool "Rockchip rkbin binary blobs"
> + depends on BR2_arm || BR2_aarch64
> + help
> + This package provides Rockchip SoC binary blobs for U-Boot.
> +
> +if BR2_PACKAGE_ROCKCHIP_RKBIN
> +
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_LATEST_VERSION
> + bool "d6ccfe401ca84a98ca3b85c12b9554a1a43a166c"
> +
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_CUSTOM_VERSION
> + bool "Custom version"
> + help
> + This option allows to use a specific version.
> +
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_CUSTOM_VERSION_VALUE
> + string "Rockchip rkbin version"
> + depends on BR2_PACKAGE_ROCKCHIP_RKBIN_CUSTOM_VERSION
> +
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_VERSION
> + string
> + default "d6ccfe401ca84a98ca3b85c12b9554a1a43a166c" \
> + if BR2_PACKAGE_ROCKCHIP_RKBIN_LATEST_VERSION
It'd be nice if we could reuse the version already in
BR2_PACKAGE_ROCKCHIP_RKBIN_LATEST_VERSION symbol but I looked 5min and
didn't find anything :/
> + default BR2_PACKAGE_ROCKCHIP_RKBIN_CUSTOM_VERSION_VALUE \
> + if BR2_PACKAGE_ROCKCHIP_RKBIN_CUSTOM_VERSION
> +
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_TPL_FILE
> + bool "Rockchip rkbin tpl file"
> + default n
> +
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_TPL_FILENAME
> + string "ddr.bin file path"
> + depends on BR2_PACKAGE_ROCKCHIP_RKBIN_TPL_FILE
> + help
> + Full path to the tpl file inside the rkbin repository. The
> + specified file will be copied and used by U-Boot as tpl.
> +
I think we can merge those two together? If it's an empty string, then
we don't use it?
ifneq ($(BR2_PACKAGE_ROCKCHIP_RKBIN_TPL),"")
instead of
ifeq ($((BR2_PACKAGE_ROCKCHIP_RKBIN_TPL_FILE),y)
Just a suggestion.
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_BL31_FILE
> + bool "Rockchip rkbin bl31 file"
> + default n
> +
Shoudl this depends on !BR2_TARGET_UBOOT_NEEDS_ATF_BL31 ?
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_BL31_FILENAME
> + string "bl31.elf file path"
> + depends on BR2_PACKAGE_ROCKCHIP_RKBIN_BL31_FILE
> + help
> + Full path to the bl31 file inside the rkbin repository. The
> + specified file will be copied and used by U-Boot as bl31.
> +
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_OPTEE_FILE
> + bool "Rockchip rkbin optee file"
> + default n
> +
> +config BR2_PACKAGE_ROCKCHIP_RKBIN_OPTEE_FILENAME
> + string "optee.elf file path"
> + depends on BR2_PACKAGE_ROCKCHIP_RKBIN_OPTEE_FILE
> + help
> + Full path to the optee file inside the rkbin repository.
> + The specified file will be copied and used by U-Boot as
> + optee.
Issue around the indent, probably tabs vs spaces?
Also, to U-Boot, it's a tee not op-tee. OP-TEE OS is just a possible
implementation of a tee OS. You could have something else than OP-TEE OS
(though I am personally not aware of any).
> +
> +endif # BR2_PACKAGE_ROCKCHIP_RKBIN
> diff --git a/package/rockchip-rkbin/rockchip-rkbin.hash b/package/rockchip-rkbin/rockchip-rkbin.hash
> new file mode 100644
> index 0000000000..5659ecf719
> --- /dev/null
> +++ b/package/rockchip-rkbin/rockchip-rkbin.hash
> @@ -0,0 +1,3 @@
> +# Locally computed
> +sha256 6f7b58fe35108101031ebfd3cc6eb7a186f258a1cdbd93c4256888997ab52c8f rockchip-rkbin-d6ccfe401ca84a98ca3b85c12b9554a1a43a166c-br1.tar.gz
> +
> diff --git a/package/rockchip-rkbin/rockchip-rkbin.mk b/package/rockchip-rkbin/rockchip-rkbin.mk
> new file mode 100644
> index 0000000000..ea1afd56d3
> --- /dev/null
> +++ b/package/rockchip-rkbin/rockchip-rkbin.mk
> @@ -0,0 +1,46 @@
> +################################################################################
> +#
> +# rockchip-rkbin
> +#
> +################################################################################
> +
> +
> +ROCKCHIP_RKBIN_VERSION = $(call qstrip,$(BR2_PACKAGE_ROCKCHIP_RKBIN_VERSION))
> +ROCKCHIP_RKBIN_SITE = https://github.com/rockchip-linux/rkbin.git
> +ROCKCHIP_RKBIN_SITE_METHOD = git
> +ROCKCHIP_RKBIN_LICENSE = PROPRIETARY
> +ROCKCHIP_RKBIN_REDISTRIBUTE = NO
> +
I think it's fine to redistribute it since it's already public? Any
reason not to?
> +ROCKCHIP_RKBIN_INSTALL_STAGING = YES
I think this is incorrect and should rather be:
ROCKCHIP_RKBIN_INSTALL_IMAGES_CMDS?
c.f. documentation
(https://buildroot.org/downloads/manual/manual.html#generic-package-reference)
"""
LIBFOO_INSTALL_STAGING_CMDS lists the actions to be performed to
install the package to the staging directory, when the package is a
target package. The package must install its files to the directory
given by $(STAGING_DIR). All development files should be installed,
since they might be needed to compile other packages.
LIBFOO_INSTALL_IMAGES_CMDS lists the actions to be performed to install
the package to the images directory, when the package is a target
package. The package must install its files to the directory given by
$(BINARIES_DIR). Only files that are binary images (aka images) that do
not belong in the TARGET_DIR but are necessary for booting the board
should be placed here. For example, a package should utilize this step
if it has binaries which would be similar to the kernel image,
bootloader or root filesystem images.
"""
What I'm afraid of is that you're installing to BINARIES_DIR but
STAGING_CMDS should install only in STAGING_DIR.
At the same time, I don't know how this will fly with parallel builds
(which are experimental still, so do we care?) because u-boot dependency
would probably be rather on staging commands from dependencies rather
than image commands from dependencies.
> +ROCKCHIP_RKBIN_INSTALL_TARGET = NO
> +
> +ifeq ($(BR2_PACKAGE_ROCKCHIP_RKBIN_BL31_FILE),y)
> +ROCKCHIP_RKBIN_BL31_FILENAME = $(call qstrip,$(BR2_PACKAGE_ROCKCHIP_RKBIN_BL31_FILENAME))
> +endif
> +
> +ifeq ($(BR2_PACKAGE_ROCKCHIP_RKBIN_TPL_FILE),y)
> +ROCKCHIP_RKBIN_TPL_FILENAME = $(call qstrip,$(BR2_PACKAGE_ROCKCHIP_RKBIN_TPL_FILENAME))
> +endif
> +
> +ifeq ($(BR2_PACKAGE_ROCKCHIP_RKBIN_OPTEE_FILE),y)
> +ROCKCHIP_RKBIN_OPTEE_FILENAME = $(call qstrip,$(BR2_PACKAGE_ROCKCHIP_RKBIN_OPTEE_FILENAME))
> +endif
> +
> +define ROCKCHIP_RKBIN_INSTALL_STAGING_CMDS
> + $(if $(filter y, $(BR2_PACKAGE_ROCKCHIP_RKBIN_BL31_FILE)), \
> + cp $(@D)/$(ROCKCHIP_RKBIN_BL31_FILENAME) $(BINARIES_DIR)/bl31.elf)
> + $(if $(filter y, $(BR2_PACKAGE_ROCKCHIP_RKBIN_TPL_FILE)), \
> + cp $(@D)/$(ROCKCHIP_RKBIN_TPL_FILENAME) $(BINARIES_DIR)/ddr.bin)
> + $(if $(filter y, $(BR2_PACKAGE_ROCKCHIP_RKBIN_OPTEE_FILE)), \
> + cp $(@D)/$(ROCKCHIP_RKBIN_OPTEE_FILENAME) $(BINARIES_DIR)/tee.elf)
I honestly would prefer to keep the original file extension. Not sure
how to handle this on U-Boot package makefile side though.
Cheers,
Quentin
More information about the buildroot
mailing list