Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to NXP's iMX93 SoC A1 revision #1700

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
From e3803a81326cece3b4cdd086ab009e9858fec21e Mon Sep 17 00:00:00 2001
From: Joao Marcos Costa <[email protected]>
Date: Thu, 7 Dec 2023 15:45:12 +0100
Subject: [PATCH] iMX9/soc.mak: dynamically set AHAB_IMG

Instead of assigning a constant value to AHAB_IMG (i.e.,
mx93a0-ahab-container.img), set it to SECO_FIRMWARE_NAME.

This variable is defined in
use-imx-security-controller-firmware.bbclass, then it is passed as a
make parameter in imx-boot's do_compile() task.

As of now, there are revisions beyond A0 (namely A1), so the firmware
must be compiled accordingly. The firmware name is already handled in
Yocto side, but not in imx-boot/imx-mkimage side. This patch implements
such feature.

Signed-off-by: Joao Marcos Costa <[email protected]>
---
iMX9/soc.mak | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iMX9/soc.mak b/iMX9/soc.mak
index 7c4086c..fa13391 100644
--- a/iMX9/soc.mak
+++ b/iMX9/soc.mak
@@ -16,7 +16,7 @@ else
RENAME = rename
endif

-AHAB_IMG = mx93a0-ahab-container.img
+AHAB_IMG = ${SECO_FIRMWARE_NAME}
MCU_IMG = m33_image.bin

SPL_LOAD_ADDR ?= 0x2049A000
--
2.43.0

1 change: 1 addition & 0 deletions recipes-bsp/imx-mkimage/imx-mkimage_git.inc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ DEPENDS = "zlib-native openssl-native"

SRC_URI = "git://github.com/nxp-imx/imx-mkimage.git;protocol=https;branch=${SRCBRANCH} \
file://0001-iMX8M-soc.mak-use-native-mkimage-from-sysroot.patch \
file://0001-iMX9-soc.mak-dynamically-set-AHAB_IMG.patch \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @thochstein, could you please ensure that you apply those changes for the upcoming release? Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes to imx-boot and imx-mkimage are not necessary. The root cause is there is some other meta-data that did not get upstreamed, namely the setting of IMX_SOC_REV:

diff --git a/conf/machine/include/imx-base.inc b/conf/machine/include/imx-base.inc
index 71c2aa80..7d63472f 100644
--- a/conf/machine/include/imx-base.inc
+++ b/conf/machine/include/imx-base.inc
@@ -180,6 +180,7 @@ IMX_SOC_REV:mx8dx-generic-bsp  ??= "C0"
 IMX_SOC_REV:mx8ulp-generic-bsp ??= \
     "${@bb.utils.contains('MACHINE_FEATURES', 'soc-reva0', 'A0', \
                                                            'A2', d)}"
+IMX_SOC_REV:mx93-generic-bsp   ??= "A1"

 IMX_SOC_REV_LOWER   = "${@d.getVar('IMX_SOC_REV').lower()}"
 IMX_SOC_REV_UPPER   = "${@d.getVar('IMX_SOC_REV').upper()}"

I'm testing now, but unfortunately my build is delayed by rust rebuild.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

I'm going to test this tomorrow morning, but I was getting the very same error (which I'll try to reproduce and send the logs here) even though I was setting IMX_SOC_REV to "A1" by hand in my local.conf, so I don't really see how setting this elsewhere would give a different result.

Kind regards,
João

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I did run into the error as well. It seems that imx-mkimage is not up to date either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully this is fixed now by #1701.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmarcoscosta please test #1701

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@otavio @thochstein I just tested it, and it works fine. Thanks!

Still, I must admit I'm feeling a bit bothered with keeping this hardcoded firmware name in the makefile I fixed.

Besides, shouldn't we keep at least the commit messages, for documentation's sake?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jmarcoscosta,

Glad to hear it works!

The firmware name is not hard-coded anymore [1] and is driven by IMX_SOC_REV, which sets SECO_FIRMWARE_NAME and gets passed to imx-mkimage [2].

[1] 0688f79
[2] https://github.com/nxp-imx/imx-mkimage/blob/lf-6.1.36_2.1.0/iMX9/soc.mak#L20-L21

In the end this error should never have happened had we done the upstreaming of NXP release 6.1.36-2.1.0 properly. I can try to flesh out the new commit messages with some of this information.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks!

Could you please give me a heads-up once this is all merged into meta-freescale's master?

"
SRCBRANCH = "lf-6.1.22_2.0.0"
SRCREV = "5cfd218012e080fb907d9cc301fbb4ece9bc17a9"
Expand Down