Add support to NXP's iMX93 SoC A1 revision #1700
Conversation
v0.11 supports A1 revision for iMX93 SoC. The previous version, however, only supports A0. Without the proper firmware version, it is impossible to boot the SoC. Upgrade firmware-sentinel from v0.10 to v0.11, which supports both A0 and A1 hardware revisions. Signed-off-by: Joao Marcos Costa <[email protected]>
In imx-boot's iMX9/soc.mak, the firmware name is set to a constant value, i.e. mx93a0-ahab-container.img. Instead, it should be set 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. This patch implements such feature. Signed-off-by: Joao Marcos Costa <[email protected]>
This is a follow-up to the previous commit, which handles firmware naming in Yocto side. Add patch to handle it in imx-boot/imx-mkimage side. Signed-off-by: Joao Marcos Costa <[email protected]>
jmarcoscosta
force-pushed
the
master-add-support-to-IMX_SOC_REV-A1
branch
from
3f8ddb1
to
25c496f
Compare
| @@ -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 \ | |||
There was a problem hiding this comment.
Hey @thochstein, could you please ensure that you apply those changes for the upcoming release? Thank you!
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Indeed, I did run into the error as well. It seems that imx-mkimage is not up to date either.
There was a problem hiding this comment.
Hopefully this is fixed now by #1701.
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I see, thanks!
Could you please give me a heads-up once this is all merged into meta-freescale's master?
|
Hey @jmarcoscosta, have you performed any tests on other Systems on Chips (SoCs)? |
otavio
requested review from
MrCry0,
angolini,
quaresmajose,
junzhuimx and
thochstein
|
@otavio not really, as the A1 revision is the only one I have in hands. |
|
I don't have access to any imx93 any longer, so I cannot test it on the board |
|
Replaced by #1701 |
|
Hello @otavio, Can you backport these changes on the Kirkstone (LTS) branch? Kind regards, |
|
Please prepare a PR for this. |
|
Hello @otavio,
I'm preparing a pull-request to backport SoC revision support and sentinel firmware update, otherwise boards with new SoC revisions aren't usable with the Kirkstone (LTS) branch. But what do you advise about Freescale EULA ? Because the sentinel firmware v0.11 is using the EULA V48 that isn't available on the Kirkstone branch. Kind regards, |
|
Could it be added? we must confirm it does not break old recipes. |
|
Hello @otavio, Moreover, it seems that the sentinel API has been modified since the 0.9 and the U-Boot 2022.04 used in the Kirkstone (LTS) branch doesn't use this new API. So, to be able to use new SoC revision with the Kirkstone (LTS) it should be also necessary to update U-Boot or to backport patches adding the new sentinel API.
Kind regards, |
|
We are using meta-freescale master with kirkstone (currently using based on the previous BSP rev, about to update to the latest), and it works fine with imx93 a1. |
|
I believe not only u-boot, but optee might need to be updated as well, which is why backporting is a pain. |
|
I'd rather avoid too invasive backports to reduce the risk of regressions. |
Hello,
I'm currently in a project where the target board is the A1 revision of iMX93, and I picked meta-freescale's BSP to work upon.
At first, I built a core-image-minimal and it didn't boot. As it turns out, the layer only supported building firmware for iMX93's A0 revision. Basically, firmware-sentinel only supports A1 starting at v0.11.
I'm sending this pull request to update firmware-sentinel according to meta-imx's, and I also added some tweaks to dynamically set the firmware name (e.g,
mx93a0-ahab-container.imgormx93a1-ahab-container.img), because the current implementation doesn't actually useSECO_FIRMWARE_NAME(and consequently neitherIMX_SOC_REV).Please do not hesitate to point any mistakes, or even to point a specific revision that already fixes (partially or entirely) this issue.
Best regards,
João Marcos Costa