Re: [Buildroot] [PATCH v4 14/20] boot/uboot: set BINMAN_INDIRS for TI K3 DM

From: Andreas Dannenberg via buildroot <buildroot@buildroot.org>
To: Romain Naour <romain.naour@smile.fr>
Cc: michael@amarulasolutions.com, linux-amarula@amarulasolutions.com,
	Asaf Kahlon <asafka7@gmail.com>,
	Xuanhao Shi <X15000177@gmail.com>,
	James Hilliard <james.hilliard1@gmail.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	buildroot@buildroot.org,
	Dario Binacchi <dario.binacchi@amarulasolutions.com>,
	Alexander Sverdlin <alexander.sverdlin@gmail.com>,
	bryce@redpinelabs.com, Anand Gadiyar <gadiyar@ti.com>
Subject: Re: [Buildroot] [PATCH v4 14/20] boot/uboot: set BINMAN_INDIRS for TI K3 DM
Date: Mon, 19 Feb 2024 20:00:11 -0600	[thread overview]
Message-ID: <20240220020011.x7mfxcs5oxbr5s3d@dasso> (raw)
In-Reply-To: <63d28259-ff96-4450-bdc5-e1bc209d8e59@smile.fr>

On Mon, Feb 19, 2024 at 10:34:52PM +0100, Romain Naour wrote:
> Hello Andreas,
> 
> Le 19/02/2024 à 21:51, Andreas Dannenberg a écrit :
> > On Sat, Feb 17, 2024 at 09:44:29PM +0100, Alexander Sverdlin wrote:
> >> Hi Dario,
> >>
> >> On Sat, 2024-02-17 at 17:02 +0100, Dario Binacchi wrote:
> >>> As explained in [1], adding support for AM62x LP SK in U-Boot, requires
> >>> binman for building bootloader images. The BINMAN_INDIRS environment
> >>> variable is used to provide a space-separated list of directories to
> >>> search for binary blobs.
> >>>
> >>> [1] https://patchwork.ozlabs.org/project/uboot/patch/20231030110138.1347603-3-n-yadav@ti.com/
> >>> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> >>>
> >>> ---
> >>>
> >>> no changes since v1:
> >>>
> >>> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> >>> ---
> >>>  boot/uboot/uboot.mk | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> >>> index dda606a88059..cf70f86a4132 100644
> >>> --- a/boot/uboot/uboot.mk
> >>> +++ b/boot/uboot/uboot.mk
> >>> @@ -189,7 +189,8 @@ endif
> >>>  ifeq ($(BR2_TARGET_UBOOT_NEEDS_TI_K3_DM),y)
> >>>  UBOOT_TI_K3_DM_SOCNAME = $(call qstrip,$(BR2_TARGET_UBOOT_TI_K3_DM_SOCNAME))
> >>>  UBOOT_DEPENDENCIES += ti-k3-boot-firmware
> >>> -UBOOT_MAKE_OPTS += DM=$(BINARIES_DIR)/ti-dm/$(UBOOT_TI_K3_DM_SOCNAME)/ipc_echo_testb_mcu1_0_release_strip.xer5f
> >>> +UBOOT_MAKE_OPTS += DM=$(BINARIES_DIR)/ti-dm/$(UBOOT_TI_K3_DM_SOCNAME)/ipc_echo_testb_mcu1_0_release_strip.xer5f \
> >>
> >> Is the above actually still required with binman?
> >> The names are hardcoded in U-Boot in arch/arm/dts/k3-am625-sk-binman.dtsi
> >>
> >>> +	BINMAN_INDIRS=$(BINARIES_DIR)
> >>
> >> This seems to be the only required part.
> > 
> > Correct. 'DM=' is no longer used, also see the official U-Boot build
> > instructions for TI platforms in the SDK documentation [1] (technically
> > speaking this documentation is for the U-Boot vendor tree, but the same
> > principle applies to upstream U-Boot builds).
> > 
> > Reviewed-by: Andreas Dannenberg <dannenberg@ti.com>
> > 
> > [1] https://software-dl.ti.com/processor-sdk-linux/esd/AM62X/09_01_00_08/exports/docs/linux/Foundational_Components/U-Boot/UG-General-Info.html
> 
> It means that we can remove BR2_TARGET_UBOOT_NEEDS_TI_K3_DM and
> UBOOT_TI_K3_DM_SOCNAME too.
> 
> meta-ti still use DM= but probably to support u-boot-ti-staging_2023.04 that is
> still available [1].
> 
> The u-boot documentation also contains a similar note [2].

Yes you are right, this option is still available as an override. But
best will be to standardize on the new way of doing things, which is
BINMAN_INDIRS.

> In the end we only need an option to select which SoC type we want to use
> (GP,HS,HS-FS).

Yes. There's one small twist to that, technically for HS devices (a.k.a.
"HS-SE" devices), which is what an HS-FS device turns into _after_ the
fuses are burned with a custom key, the end user will need to provide
their own key for signing the boot image, which will then be used
instead of './board/ti/keys/custMpk.pem' in the U-Boot tree. Let me
think about some more if there isn't a good way that can be done through
Builroot, based on how actual end customers manage their private keys
(which can be very elaborate, even involving HSMs in some cases). It
might be helpful to expose such an override using a config option that
can be used to point to an external key. Anyways just food for thought,
this shouldn't gate anything on Dario's series here.

--
Andreas Dannenberg
Texas Instruments Inc

> 
> [1]
> https://git.yoctoproject.org/meta-ti/tree/meta-ti-bsp/recipes-bsp/u-boot/u-boot-ti.inc?h=cicd.master.202402130100#n52
> 
> [2]
> https://gitlab.com/u-boot/u-boot/-/blob/master/doc/board/ti/k3.rst?ref_type=heads&plain=1#L372
> 
> Best regards,
> Romain
> 
> > 
> > 
> > 
> >>
> >>>  endif
> >>>  
> >>>  ifeq ($(BR2_TARGET_UBOOT_NEEDS_OPENSBI),y)
> >>
> >> -- 
> >> Alexander Sverdlin.
> >>
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot