All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gary Bisson <bisson.gary@gmail.com>
To: "Sébastien Szymanski" <sebastien.szymanski@armadeus.com>
Cc: Refik Tuzakli <tuzakli.refik@gmail.com>,
	Erik Larsson <karl.erik.larsson@gmail.com>,
	Fabio Estevam <festevam@gmail.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 3/4] package/freescale-imx/firmware-sentinel: new package
Date: Tue, 5 Dec 2023 22:43:05 +0100	[thread overview]
Message-ID: <ZW-ZaX3SRi8mnSdP@p1g2> (raw)
In-Reply-To: <20231011110932.4425-4-sebastien.szymanski@armadeus.com>

Hi,

On Wed, Oct 11, 2023 at 01:09:31PM +0200, Sébastien Szymanski wrote:
> This package provides firmware blobs for the i.MX9 Edgelock secure
> enclave (ELE).

This actually not only for i.MX 9 but also for i.MX 8ULP.
I suggest adding the release this version comes from (6.1.36-2.1.0).

> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
> ---
>  package/freescale-imx/Config.in               |  1 +
>  .../freescale-imx/firmware-sentinel/Config.in | 34 +++++++++++++++++++
>  .../firmware-sentinel/firmware-sentinel.hash  |  4 +++
>  .../firmware-sentinel/firmware-sentinel.mk    | 27 +++++++++++++++
>  4 files changed, 66 insertions(+)
>  create mode 100644 package/freescale-imx/firmware-sentinel/Config.in
>  create mode 100644 package/freescale-imx/firmware-sentinel/firmware-sentinel.hash
>  create mode 100644 package/freescale-imx/firmware-sentinel/firmware-sentinel.mk
> 
> diff --git a/package/freescale-imx/Config.in b/package/freescale-imx/Config.in
> index 192b1c0d70d1..4d31bcc4f4cf 100644
> --- a/package/freescale-imx/Config.in
> +++ b/package/freescale-imx/Config.in
> @@ -126,6 +126,7 @@ source "package/freescale-imx/imx-vpu/Config.in"
>  source "package/freescale-imx/imx-vpu-hantro/Config.in"
>  source "package/freescale-imx/imx-vpuwrap/Config.in"
>  source "package/freescale-imx/firmware-imx/Config.in"
> +source "package/freescale-imx/firmware-sentinel/Config.in"
>  source "package/freescale-imx/imx-sc-firmware/Config.in"
>  source "package/freescale-imx/imx-seco/Config.in"
>  source "package/freescale-imx/imx-vpu-hantro-daemon/Config.in"
> diff --git a/package/freescale-imx/firmware-sentinel/Config.in b/package/freescale-imx/firmware-sentinel/Config.in
> new file mode 100644
> index 000000000000..9ceecd5a39dc
> --- /dev/null
> +++ b/package/freescale-imx/firmware-sentinel/Config.in
> @@ -0,0 +1,34 @@
> +config BR2_PACKAGE_FIRMWARE_SENTINEL
> +	bool "firmware-sentinel"
> +	depends on BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX91 || \
> +		BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX93
> +	help
> +	  Firmware blobs for the i.MX9 Edgelock secure enclave (ELE).

ditto.

> +	  This library is provided by Freescale as-is and doesn't have
> +	  an upstream.

s/Freescale/NXP/

> +if BR2_PACKAGE_FIRMWARE_SENTINEL
> +
> +choice
> +	prompt "i.MX Sentinel Firmware Release"
> +	help
> +	  Select the appropriate ahab container image to install
> +	  to match the iMX asics revision.
> +
> +	  Note - mismatches result in a failure to boot
> +
> +config BR2_PACKAGE_FIRMWARE_SENTINEL_A0
> +	bool "A0"
> +
> +config BR2_PACKAGE_FIRMWARE_SENTINEL_A1
> +	bool "A1"
> +
> +endchoice

I don't think this is the right approach as this package exists for
several CPU types (8ULP & 93).
Moreover, for the 8ULP, which I plan on adding soon-ish, there is even
more revisions which are used in more than 1 package (firmware-upower).
Also, that revision really is a Silicon version more than a firmware
release like the title suggests.

Overall, my suggestion would be to move the Silicon version to
freescale-imx/Config.in to be selected at the same time the CPU is
selected. Also, the default Silicon revision should always be the
latest.

> +config BR2_PACKAGE_FIRMWARE_SENTINEL_AHAB_CONTAINER_IMAGE
> +	string
> +	default "mx93a0-ahab-container.img" if BR2_PACKAGE_FIRMWARE_SENTINEL_A0
> +	default "mx93a1-ahab-container.img" if BR2_PACKAGE_FIRMWARE_SENTINEL_A1
> +
> +endif # BR2_PACKAGE_FIRMWARE_SENTINEL
> diff --git a/package/freescale-imx/firmware-sentinel/firmware-sentinel.hash b/package/freescale-imx/firmware-sentinel/firmware-sentinel.hash
> new file mode 100644
> index 000000000000..1a9ff0543250
> --- /dev/null
> +++ b/package/freescale-imx/firmware-sentinel/firmware-sentinel.hash
> @@ -0,0 +1,4 @@
> +# Locally calculated
> +sha256  269480417a8ae9aa4cc4101ab947287fc33455a931021dbdc4d9badb5212bceb  firmware-sentinel-0.11.bin
> +sha256  de37a0bcbf1717b910c1a53ea6eab853c404e61e8143bb6c081d39f532571e54  COPYING
> +sha256  c800aaca3a7e9f470d99d7cde0a48c95982ed601d4c306f7b8f43f3710054f28  SCR.txt
> diff --git a/package/freescale-imx/firmware-sentinel/firmware-sentinel.mk b/package/freescale-imx/firmware-sentinel/firmware-sentinel.mk
> new file mode 100644
> index 000000000000..f68467194cbc
> --- /dev/null
> +++ b/package/freescale-imx/firmware-sentinel/firmware-sentinel.mk
> @@ -0,0 +1,27 @@
> +################################################################################
> +#
> +# firmware-sentinel
> +#
> +################################################################################
> +
> +FIRMWARE_SENTINEL_VERSION = 0.11
> +FIRMWARE_SENTINEL_SITE = $(FREESCALE_IMX_SITE)
> +FIRMWARE_SENTINEL_SOURCE = firmware-sentinel-$(FIRMWARE_SENTINEL_VERSION).bin
> +
> +FIRMWARE_SENTINEL_LICENSE = NXP Semiconductor Software License Agreement
> +FIRMWARE_SENTINEL_LICENSE_FILES = COPYING SCR.txt
> +FIRMWARE_SENTINEL_REDISTRIBUTE = NO
> +
> +FIRMWARE_SENTINEL_INSTALL_IMAGES = YES
> +
> +define FIRMWARE_SENTINEL_EXTRACT_CMDS
> +	$(call NXP_EXTRACT_HELPER,$(FIRMWARE_SENTINEL_DL_DIR)/$(FIRMWARE_SENTINEL_SOURCE))
> +endef
> +
> +FIRMWARE_SENTINEL_AHAB_CONTAINER_IMAGE = $(call qstrip,$(BR2_PACKAGE_FIRMWARE_SENTINEL_AHAB_CONTAINER_IMAGE))
> +
> +define FIRMWARE_SENTINEL_INSTALL_IMAGES_CMDS
> +	cp $(@D)/$(FIRMWARE_SENTINEL_AHAB_CONTAINER_IMAGE) $(BINARIES_DIR)/ahab-container.img
> +endef
> +
> +$(eval $(generic-package))

The rest of the patch looks good!

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

  reply	other threads:[~2023-12-05 21:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-11 11:09 [Buildroot] [PATCH 0/4] i.MX91 and i.MX93 support Sébastien Szymanski
2023-10-11 11:09 ` [Buildroot] [PATCH 1/4] package/imx-mkimage: bump version to lf-6.1.36-2.1.0 Sébastien Szymanski
2023-11-03  9:16   ` Thomas Petazzoni via buildroot
2023-11-06  8:56     ` Sébastien Szymanski
2023-10-11 11:09 ` [Buildroot] [PATCH 2/4] package/freescale-imx: add i.MX91 and i.MX93 SoC support Sébastien Szymanski
2023-10-11 11:09 ` [Buildroot] [PATCH 3/4] package/freescale-imx/firmware-sentinel: new package Sébastien Szymanski
2023-12-05 21:43   ` Gary Bisson [this message]
2023-10-11 11:09 ` [Buildroot] [PATCH 4/4] configs/freescale_imx93evk: new defconfig Sébastien Szymanski
2023-10-11 13:43   ` Nicolas Cavallari

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZW-ZaX3SRi8mnSdP@p1g2 \
    --to=bisson.gary@gmail.com \
    --cc=buildroot@buildroot.org \
    --cc=festevam@gmail.com \
    --cc=karl.erik.larsson@gmail.com \
    --cc=sebastien.szymanski@armadeus.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tuzakli.refik@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.