Re: [Buildroot] [PATCH v4 18/20] boot/ti-k3: move TI_K3_{SECTYPE, SOC} out of the image-gen scope

[Dario Binacchi <dario.binacchi@amarulasolutions.com>]

Hi Andreas, Romain, All

On Tue, Feb 20, 2024 at 2:28 AM Andreas Dannenberg <dannenberg@ti.com> wrote:
>
> Romain, Dario,
>
> On Mon, Feb 19, 2024 at 11:28:59AM +0100, Romain Naour wrote:
> > Hello Dario,
> >
> > Le 17/02/2024 à 17:02, Dario Binacchi a écrit :
> > > Recent versions of U-Boot can use Binman to compile and sign the generated
> > > binaries, thus making the use of custom tools like ti-k3-image-gen no
> > > longer necessary. In ti-k3-image-gen, options such as SECTYPE and SOC were
> > > configured and used outside of it. This patch allows setting such options
> > > even if the package is not enabled.
> >
> > I'm agree that Buildroot doesn't provide any "Platform selection" as the Linux
> > kernel and I'm not sure where those TI_K3_{SECTYPE,SOC} should belong.
>
> There were discussions/suggestions around this before by Yann E Morin
> [1] and Thomas Petazzoni [2]. I think the gist of it was to move the
> selection to a new package like BR2_PACKAGE_TI_K3_PLATFORM containing
> all the K3 SoC specific selections. And then other packages and Kconfig
> menues can depend on it more cleanly. Like it's currently done with
> package/freescale-imx/Config.in.  This would also bring some consistency
> in how those NXP and TI device families are managed, which will make for
> a better end-user experience I think.

If I'm not mistaken, I think the patches:
[18/20] boot/ti-k3: move TI_K3_{SECTYPE,SOC} out of the image-gen scope
[17/20] boot: move ti-k3-* packages to 'ti-k3' directory
Are heading in the direction of what we find in
package/freescale-imx/, with the only difference being
that in our case, we're under boot.
So, can I consider these two patches correct?

Thanks and regards,
Dario

>
>
>
> --
> Andreas Dannenberg
> Texas Instruments Inc
>
> [1] http://lists.busybox.net/pipermail/buildroot/2023-June/669342.html
> [2] http://lists.busybox.net/pipermail/buildroot/2023-August/673292.html
>
>
> >
> > Best regards,
> > Romain
> >
> >
> > >
> > > ---
> > >
> > > Added in v4
> > >
> > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > ---
> > >  boot/ti-k3/Config.in                          | 46 ++++++++++++++
> > >  boot/ti-k3/ti-k3-image-gen/Config.in          | 63 ++-----------------
> > >  boot/ti-k3/ti-k3-image-gen/ti-k3-image-gen.mk |  6 +-
> > >  boot/uboot/Config.in                          |  7 +--
> > >  configs/ti_am62x_sk_defconfig                 |  2 +-
> > >  configs/ti_am64x_sk_defconfig                 |  2 +-
> > >  6 files changed, 59 insertions(+), 67 deletions(-)
> > >
> > > diff --git a/boot/ti-k3/Config.in b/boot/ti-k3/Config.in
> > > index 06dda2f156a9..9e508326c9c8 100644
> > > --- a/boot/ti-k3/Config.in
> > > +++ b/boot/ti-k3/Config.in
> > > @@ -5,6 +5,52 @@ config BR2_TARGET_TI_K3
> > >
> > >  if BR2_TARGET_TI_K3
> > >
> > > +choice
> > > +   prompt "SoC family"
> > > +
> > > +config BR2_TARGET_TI_K3_SOC_AM62AX
> > > +   bool "am62ax"
> > > +
> > > +config BR2_TARGET_TI_K3_SOC_AM62X
> > > +   bool "am62x"
> > > +
> > > +config BR2_TARGET_TI_K3_SOC_AM64X
> > > +   bool "am64x"
> > > +
> > > +config BR2_TARGET_TI_K3_SOC_AM65X
> > > +   bool "am65x"
> > > +
> > > +endchoice
> > > +
> > > +choice
> > > +   prompt "Security type"
> > > +   help
> > > +     The target SoC security type option.
> > > +
> > > +config BR2_TARGET_TI_K3_SECTYPE_GP
> > > +   bool "gp"
> > > +
> > > +config BR2_TARGET_TI_K3_SECTYPE_HS_FS
> > > +   bool "hs-fs"
> > > +
> > > +config BR2_TARGET_TI_K3_SECTYPE_HS
> > > +   bool "hs"
> > > +
> > > +endchoice
> > > +
> > > +config BR2_TARGET_TI_K3_SOC
> > > +   string
> > > +   default "am62ax" if BR2_TARGET_TI_K3_SOC_AM62AX
> > > +   default "am62x"  if BR2_TARGET_TI_K3_SOC_AM62X
> > > +   default "am64x"  if BR2_TARGET_TI_K3_SOC_AM64X
> > > +   default "am65x"  if BR2_TARGET_TI_K3_SOC_AM65X
> > > +
> > > +config BR2_TARGET_TI_K3_SECTYPE
> > > +   string
> > > +   default "gp"    if BR2_TARGET_TI_K3_SECTYPE_GP
> > > +   default "hs-fs" if BR2_TARGET_TI_K3_SECTYPE_HS_FS
> > > +   default "hs"    if BR2_TARGET_TI_K3_SECTYPE_HS
> > > +
> > >  source "boot/ti-k3/ti-k3-boot-firmware/Config.in"
> > >  source "boot/ti-k3/ti-k3-core-secdev/Config.in"
> > >  source "boot/ti-k3/ti-k3-image-gen/Config.in"
> > > diff --git a/boot/ti-k3/ti-k3-image-gen/Config.in b/boot/ti-k3/ti-k3-image-gen/Config.in
> > > index 38d1598d9fc0..2d53d62dd4ee 100644
> > > --- a/boot/ti-k3/ti-k3-image-gen/Config.in
> > > +++ b/boot/ti-k3/ti-k3-image-gen/Config.in
> > > @@ -2,7 +2,7 @@ config BR2_TARGET_TI_K3_IMAGE_GEN
> > >     bool "ti-k3-image-gen"
> > >     depends on BR2_TARGET_TI_K3_R5_LOADER
> > >     select BR2_TARGET_TI_K3_BOOT_FIRMWARE
> > > -   select BR2_TARGET_TI_K3_CORE_SECDEV if BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_HS_FS || BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_HS
> > > +   select BR2_TARGET_TI_K3_CORE_SECDEV if BR2_TARGET_TI_K3_SECTYPE_HS_FS || BR2_TARGET_TI_K3_SECTYPE_HS
> > >     # We need FIT support in uboot-tools, which is why we select a
> > >     # host package
> > >     select BR2_PACKAGE_HOST_UBOOT_TOOLS
> > > @@ -14,65 +14,12 @@ config BR2_TARGET_TI_K3_IMAGE_GEN
> > >       https://git.ti.com/cgit/k3-image-gen/k3-image-gen/
> > >
> > >  if BR2_TARGET_TI_K3_IMAGE_GEN
> > > -choice
> > > -   prompt "SoC family"
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62AX
> > > -   bool "am62ax"
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62X
> > > -   bool "am62x"
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM64X
> > > -   bool "am64x"
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM65X
> > > -   bool "am65x"
> > > -
> > > -endchoice
> > > -
> > > -choice
> > > -   prompt "Security type"
> > > -   help
> > > -     The target SoC security type option for image gen.  Valid
> > > -     options are "gp" for General Purpose devices, "hs-fs" for
> > > -     High Security - Field Securable devices, or "hs" for High
> > > -     Security - Security Enforcing devices.  Note for all High
> > > -     Security device variants the TI_SECURE_DEV_PKG environmental
> > > -     variable must be defined at build time pointing to a valid
> > > -     core-secdev-k3 folder location, otherwise the build will
> > > -     fail, see
> > > -     https://git.ti.com/cgit/security-development-tools/core-secdev-k3
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_GP
> > > -   bool "gp"
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_HS_FS
> > > -   bool "hs-fs"
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_HS
> > > -   bool "hs"
> > > -
> > > -endchoice
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SOC
> > > -   string
> > > -   default "am62ax" if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62AX
> > > -   default "am62x"  if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62X
> > > -   default "am64x"  if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM64X
> > > -   default "am65x"  if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM65X
> > >
> > >  config BR2_TARGET_TI_K3_IMAGE_GEN_FW_TYPE
> > >     string
> > > -   default "ti-fs"  if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62AX
> > > -   default "ti-fs"  if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62X
> > > -   default "ti-sci" if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM64X
> > > -   default "ti-sci" if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM65X
> > > -
> > > -config BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE
> > > -   string
> > > -   default "gp"    if BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_GP
> > > -   default "hs-fs" if BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_HS_FS
> > > -   default "hs"    if BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_HS
> > > +   default "ti-fs"  if BR2_TARGET_TI_K3_SOC_AM62AX
> > > +   default "ti-fs"  if BR2_TARGET_TI_K3_SOC_AM62X
> > > +   default "ti-sci" if BR2_TARGET_TI_K3_SOC_AM64X
> > > +   default "ti-sci" if BR2_TARGET_TI_K3_SOC_AM65X
> > >
> > >  endif
> > > diff --git a/boot/ti-k3/ti-k3-image-gen/ti-k3-image-gen.mk b/boot/ti-k3/ti-k3-image-gen/ti-k3-image-gen.mk
> > > index 74b4fcb4bf89..834d8f7d5cd5 100644
> > > --- a/boot/ti-k3/ti-k3-image-gen/ti-k3-image-gen.mk
> > > +++ b/boot/ti-k3/ti-k3-image-gen/ti-k3-image-gen.mk
> > > @@ -26,10 +26,10 @@ TI_K3_IMAGE_GEN_DEPENDENCIES = \
> > >     $(BR2_MAKE_HOST_DEPENDENCY)
> > >
> > >  TI_K3_IMAGE_GEN_FW_TYPE = $(call qstrip,$(BR2_TARGET_TI_K3_IMAGE_GEN_FW_TYPE))
> > > -TI_K3_IMAGE_GEN_SOC = $(call qstrip,$(BR2_TARGET_TI_K3_IMAGE_GEN_SOC))
> > > -TI_K3_IMAGE_GEN_SECTYPE = $(call qstrip,$(BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE))
> > > +TI_K3_IMAGE_GEN_SOC = $(call qstrip,$(BR2_TARGET_TI_K3_SOC))
> > > +TI_K3_IMAGE_GEN_SECTYPE = $(call qstrip,$(BR2_TARGET_TI_K3_SECTYPE))
> > >
> > > -ifeq ($(BR2_TARGET_TI_K3_IMAGE_GEN_SECTYPE_GP),y)
> > > +ifeq ($(BR2_TARGET_TI_K3_SECTYPE_GP),y)
> > >  TI_K3_IMAGE_GEN_SYSFW = \
> > >     $(TI_K3_IMAGE_GEN_FW_TYPE)-firmware-$(TI_K3_IMAGE_GEN_SOC)-$(TI_K3_IMAGE_GEN_SECTYPE).bin
> > >  else
> > > diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
> > > index beafb38930db..1a9d21e7c6d1 100644
> > > --- a/boot/uboot/Config.in
> > > +++ b/boot/uboot/Config.in
> > > @@ -244,8 +244,7 @@ config BR2_TARGET_UBOOT_NEEDS_TI_K3_DM
> > >     bool "U-Boot needs TI K3 Device Manager (DM)"
> > >     # We use the SoC selection defined for the ti-k3-image-gen
> > >     # package
> > > -   depends on BR2_TARGET_TI_K3_IMAGE_GEN
> > > -   depends on BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62AX || BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62X
> > > +   depends on BR2_TARGET_TI_K3_SOC_AM62AX || BR2_TARGET_TI_K3_SOC_AM62X
> > >     help
> > >       Some TI K3 devices need the Device Manager (DM) firmware to
> > >       be available for the U-Boot build.
> > > @@ -254,8 +253,8 @@ if BR2_TARGET_UBOOT_NEEDS_TI_K3_DM
> > >
> > >  config BR2_TARGET_UBOOT_TI_K3_DM_SOCNAME
> > >     string
> > > -   default "am62axx" if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62AX
> > > -   default "am62xx"  if BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62X
> > > +   default "am62axx" if BR2_TARGET_TI_K3_SOC_AM62AX
> > > +   default "am62xx"  if BR2_TARGET_TI_K3_SOC_AM62X
> > >
> > >  endif
> > >
> > > diff --git a/configs/ti_am62x_sk_defconfig b/configs/ti_am62x_sk_defconfig
> > > index cd26235342e4..cfefce0a77a4 100644
> > > --- a/configs/ti_am62x_sk_defconfig
> > > +++ b/configs/ti_am62x_sk_defconfig
> > > @@ -24,8 +24,8 @@ BR2_TARGET_OPTEE_OS=y
> > >  BR2_TARGET_OPTEE_OS_PLATFORM="k3-am62x"
> > >  BR2_TARGET_OPTEE_OS_PLATFORM_FLAVOR="am62x"
> > >  BR2_TARGET_TI_K3=y
> > > +BR2_TARGET_TI_K3_SOC_AM62X=y
> > >  BR2_TARGET_TI_K3_IMAGE_GEN=y
> > > -BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM62X=y
> > >  BR2_TARGET_TI_K3_R5_LOADER=y
> > >  BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_VERSION=y
> > >  BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_VERSION_VALUE="2024.01"
> > > diff --git a/configs/ti_am64x_sk_defconfig b/configs/ti_am64x_sk_defconfig
> > > index 1befeb4ab96c..daf78dbffff8 100644
> > > --- a/configs/ti_am64x_sk_defconfig
> > > +++ b/configs/ti_am64x_sk_defconfig
> > > @@ -24,8 +24,8 @@ BR2_TARGET_OPTEE_OS=y
> > >  BR2_TARGET_OPTEE_OS_PLATFORM="k3-am64x"
> > >  BR2_TARGET_OPTEE_OS_PLATFORM_FLAVOR="am64x"
> > >  BR2_TARGET_TI_K3=y
> > > +BR2_TARGET_TI_K3_SOC_AM64X=y
> > >  BR2_TARGET_TI_K3_IMAGE_GEN=y
> > > -BR2_TARGET_TI_K3_IMAGE_GEN_SOC_AM64X=y
> > >  BR2_TARGET_TI_K3_R5_LOADER=y
> > >  BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_VERSION=y
> > >  BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_VERSION_VALUE="2022.10"
> >



-- 

Dario Binacchi

Senior Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com

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