Re: [Buildroot] [PATCH v10 06/12] boot/ti-k3/ti-k3-r5-loader: install tiboot3.bin

From: "Michael Walle" <mwalle@kernel.org>
To: "Dario Binacchi" <dario.binacchi@amarulasolutions.com>,
	<buildroot@buildroot.org>
Cc: Alexander Sverdlin <alexander.sverdlin@gmail.com>,
	Anand Gadiyar <gadiyar@ti.com>,
	heiko.thiery@gmail.com, Asaf Kahlon <asafka7@gmail.com>,
	Xuanhao Shi <X15000177@gmail.com>,
	James Hilliard <james.hilliard1@gmail.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Romain Naour <romain.naour@smile.fr>,
	michael@amarulasolutions.com, linux-amarula@amarulasolutions.com,
	"Yann E . Morin" <yann.morin.1998@free.fr>,
	bryce@redpinelabs.com, Andreas Dannenberg <dannenberg@ti.com>
Subject: Re: [Buildroot] [PATCH v10 06/12] boot/ti-k3/ti-k3-r5-loader: install tiboot3.bin
Date: Wed, 03 Apr 2024 15:05:46 +0200	[thread overview]
Message-ID: <D0AICZ7BOW8A.2PLPZEPK1YG2E@kernel.org> (raw)
In-Reply-To: <20240330160244.3214733-7-dario.binacchi@amarulasolutions.com>

[-- Attachment #1.1: Type: text/plain, Size: 2495 bytes --]

[resend because mail got bounced on the ML, sorry]

Hi Dario,

On Sat Mar 30, 2024 at 5:02 PM CET, Dario Binacchi wrote:
> Recent versions of U-Boot are capable of building tiboot3.bin using
> Binman. In this case, let's copy it to the binaries directory.
>
> The use of `find' command is justified by the need to use a regex
> expression to select the correct file. This is not possible with a
> simple `cp' command.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>
> ---
>
> Changes in v8:
> - Use the `find' command in pipe with the `cp' command.
>
> Changes in v6:
> - Replace `find' command with `cp' one.
>
> Changes in v5:
> - Replace tiboot3-*-$(TI_K3_R5_LOADER_SECTYPE)-*.bin with
>   tiboot3-$(TI_K3_R5_LOADER_SOC)-$(TI_K3_R5_LOADER_SECTYPE)-*.bin
>
> Added in v4
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>  boot/ti-k3-r5-loader/ti-k3-r5-loader.mk | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk b/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
> index fdb058f3b72a..9a05d005067a 100644
> --- a/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
> +++ b/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
> @@ -70,8 +70,12 @@ define TI_K3_R5_LOADER_BUILD_CMDS
>  	$(TARGET_CONFIGURE_OPTS) $(TI_K3_R5_LOADER_MAKE) -C $(@D) $(TI_K3_R5_LOADER_MAKE_OPTS)
>  endef
>  
> +TI_K3_R5_LOADER_SECTYPE = $(call qstrip,$(BR2_PACKAGE_TI_K3_SECTYPE))
> +TI_K3_R5_LOADER_SOC = $(call qstrip,$(BR2_PACKAGE_TI_K3_SOC))
> +
>  define TI_K3_R5_LOADER_INSTALL_IMAGES_CMDS
>  	cp $(@D)/spl/u-boot-spl.bin $(BINARIES_DIR)/r5-u-boot-spl.bin
> +	cd $(@D); find ./ -type f -regex "\.\/tiboot3-$(TI_K3_R5_LOADER_SOC)-$(TI_K3_R5_LOADER_SECTYPE)-[^-]*..bin" -exec cp {} $(BINARIES_DIR)/tiboot3.bin \;

The filename should *not* be hardcoded to look like
<soc>-<sectype>.bin. This is just an artifact how TI is naming their
files in *their* u-boot binman files. Instead this should just be a
Kconfig variable (maybe K3_R5_LOADER_FILENAME?) which can be set by
the user/board config and which defaults to "tiboot3.bin".
"tiboot3.bin" is the filename which is used by the ROM in filesystem
boot mode and for which u-boot is already creating a symlink.
So this will then just be another simple "cp".

On a side node, we also found out that the "find . -regex" is rather
fragile in the sense that it might not copy anything at all without
generating an error.

-michael

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot