Re: [yocto-patches] [meta-rockchip PATCH 5/6] bsp: rkbin: ddr: factor out do_deploy to be SoC-agnostic

[Trevor Woerner <twoerner@gmail.com>]

On Tue 2025-03-11 @ 12:26:34 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The do_deploy task is essentially the same for all SoCs, install a file
> from a specific path to another one.
> 
> No magic involved, so let's rather have one generic do_deploy task. For
> this to work nicely, we check that all necessary variables are set and
> notify the developer otherwise. This may be useful whenever a new SoC
> will be supported by this recipe.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  recipes-bsp/rkbin/rockchip-rkbin-ddr_git.bb | 33 +++++++++++++++--------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/recipes-bsp/rkbin/rockchip-rkbin-ddr_git.bb b/recipes-bsp/rkbin/rockchip-rkbin-ddr_git.bb
> index 193d0f00a6868f050a0ff4531278b3296d3eae94..17131b6d5a6b443409eacb5806e0613b292852d7 100644
> --- a/recipes-bsp/rkbin/rockchip-rkbin-ddr_git.bb
> +++ b/recipes-bsp/rkbin/rockchip-rkbin-ddr_git.bb
> @@ -8,40 +8,41 @@ DDRBIN_FILE:rk3308 ?= "rk3308_ddr_589MHz_uart4_m0_${DDRBIN_VERS}.bin"
>  DDRBIN_DEPLOY_FILENAME:rk3308 ?= "ddr-rk3308.bin"
>  DDRBIN_TOOL_SOC:rk3308 ?= "rk3308"
>  
> -do_deploy:rk3308() {
> -	# Prebuilt U-Boot TPL (DDR init)
> -	install -m 644 ${S}/${DDRBIN_DIR}${DDRBIN_FILE} ${DEPLOYDIR}/${DDRBIN_DEPLOY_FILENAME}
> -}
> -
>  DDRBIN_DIR:rk3566 ?= "bin/rk35/"
>  DDRBIN_VERS:rk3566 ?= "v1.23"
>  DDRBIN_FILE:rk3566 ?= "rk3566_ddr_1056MHz_${DDRBIN_VERS}.bin"
>  DDRBIN_DEPLOY_FILENAME:rk3566 ?= "ddr-rk3566.bin"
>  DDRBIN_TOOL_SOC:rk3566 ?= "rk356x"
>  
> -do_deploy:rk3566() {
> -	# Prebuilt U-Boot TPL (DDR init)
> -	install -m 644 ${S}/${DDRBIN_DIR}${DDRBIN_FILE} ${DEPLOYDIR}/${DDRBIN_DEPLOY_FILENAME}
> -}
> -
>  DDRBIN_DIR:rk3568 ?= "bin/rk35/"
>  DDRBIN_VERS:rk3568 ?= "v1.23"
>  DDRBIN_FILE:rk3568 ?= "rk3568_ddr_1560MHz_${DDRBIN_VERS}.bin"
>  DDRBIN_DEPLOY_FILENAME:rk3568 ?= "ddr-rk3568.bin"
>  DDRBIN_TOOL_SOC:rk3568 ?= "rk356x"
>  
> -do_deploy:rk3568() {
> -	# Prebuilt U-Boot TPL (DDR init)
> -	install -m 644 ${S}/${DDRBIN_DIR}${DDRBIN_FILE} ${DEPLOYDIR}/${DDRBIN_DEPLOY_FILENAME}
> -}
> -
>  DDRBIN_DIR:rk3588s ?= "bin/rk35/"
>  DDRBIN_VERS:rk3588s ?= "v1.18"
>  DDRBIN_FILE:rk3588s ?= "rk3588_ddr_lp4_2112MHz_lp5_2400MHz_${DDRBIN_VERS}.bin"
>  DDRBIN_DEPLOY_FILENAME:rk3588s ?= "ddr-rk3588.bin"
>  DDRBIN_TOOL_SOC:rk3588s ?= "rk3588"
>  
> -do_deploy:rk3588s() {
> +do_deploy() {
> +	if [ -z "${DDRBIN_DIR}" ]; then
> +		bbfatal "Non-empty DDRBIN_DIR:<MACHINE> required!"
> +	fi
> +
> +	if [ -z "${DDRBIN_VERS}" ]; then
> +		bbfatal "Non-empty DDRBIN_VERS:<MACHINE> required!"
> +	fi
> +
> +	if [ -z "${DDRBIN_FILE}" ]; then
> +		bbfatal "Non-empty DDRBIN_FILE:<MACHINE> required!"
> +	fi
> +
> +	if [ -z "${DDRBIN_DEPLOY_FILENAME}" ]; then
> +		bbfatal "Non-empty DDRBIN_DEPLOY_FILENAME:<MACHINE> required!"
> +	fi
> +
>  	# Prebuilt U-Boot TPL (DDR init)
>  	install -m 644 ${S}/${DDRBIN_DIR}${DDRBIN_FILE} ${DEPLOYDIR}/${DDRBIN_DEPLOY_FILENAME}
>  }

This is nice, but the problem with this patch is that the rkbin handling
involves 4 files:
	- rockchip-rkbin.inc
	- rockchip-rkbin-ddr_git.bb
	- rockchip-rkbin-optee-os_git.bb
	- rockchip-rkbin-tf-a_git.bb

In the *inc file it says:

	 28 do_deploy() {
	 29         bbfatal "COMPATIBLE_MACHINE requires a corresponding do_deploy:<MACHINE>() override"
	 30 }

These machine-specific overrides are found in both:
	- rockchip-rkbin-optee-os_git.bb and -
	rockchip-rkbin-tf-a_git.bb
but, with this patch, not in rockchip-rkbin-ddr_git.bb.

I like what you've done, but I would prefer they all follow the same
"template". Either:
	1) modify rockchip-rkbin-optee-os_git.bb and
	rockchip-rkbin-tf-a_git.bb to match and update the bbfatal in
	rockchip-rkbin.inc so it makes sense, or

	2) skip this patch

I don't have a strong preference either way, but I do have a strong desire
that they all match.