xtensa-build-zephyr: add imx by marc-hb · Pull Request #4525 · thesofproject/sof

marc-hb · 2021-07-22T02:36:46Z

Building in CI is required to test changes like #4524 and others

-p is required when the real (not symbolic) directories are not nested in one another. The documentation gap was clearly demonstrated by me getting confused even though _I_ implemented this logic some time back. Signed-off-by: Marc Herbert <marc.herbert@intel.com>

Building imx is critical to test changes like thesofproject#4524 Note not signing imx means sof-imx.ldc is staged for deployment but no imx firmware is, example: $ scripts/xtensa-build-zephyr.sh imx apl build-sof-staging ├── sof │ ├── community │ │ └── sof-apl.ri │ ├── sof-apl.ldc │ └── sof-imx.ldc └── tools └── sof-logger Signed-off-by: Marc Herbert <marc.herbert@intel.com>

marc-hb · 2021-07-22T04:59:21Z

scripts/xtensa-build-zephyr.sh

 				PLAT_CONFIG='intel_adsp_cavs15'
 				;;
+			imx)
+				PLAT_CONFIG='intel_adsp_cavs15'


Hasty copy/paste... @iuliana-prodan which --board parameter have you been using?

I believe it's nxp_adsp_imx8.

yes, is PLAT_CONFIG='nxp_adsp_imx8'

dbaluta · 2021-07-22T05:40:53Z

scripts/xtensa-build-zephyr.sh


-SUPPORTED_PLATFORMS=(apl cnl icl tgl-h)
+SUPPORTED_PLATFORMS=(apl cnl icl tgl-h imx)
 # Default value, can (and sometimes must) be overridden with -p


Shouldn't we add imx8 instead of imx here?

yes, should be imx8:

-SUPPORTED_PLATFORMS=(apl cnl icl tgl-h) +SUPPORTED_PLATFORMS=(apl cnl icl tgl-h imx8)

dbaluta · 2021-07-22T05:41:23Z

scripts/xtensa-build-zephyr.sh

 			       "$bdir"/zephyr/zephyr.elf

+                        test "$RIMAGE_KEY" = 'None' ||
 			west sign  --build-dir "$bdir" \


Indentation looks odd here. Same with line below.

Tabs vs spaces, the new line seems to be indented with spaces.

iuliana-prodan · 2021-07-22T08:31:21Z

@marc-hb I already have a patch for this, but is only local...since Zephyr pull request is not yet merged - see zephyrproject-rtos/zephyr#35467

Here's my patch:

 scripts/xtensa-build-zephyr.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/xtensa-build-zephyr.sh b/scripts/xtensa-build-zephyr.sh
index ce27053d2..6016a49ad 100755
--- a/scripts/xtensa-build-zephyr.sh
+++ b/scripts/xtensa-build-zephyr.sh
@@ -7,7 +7,7 @@ set -e
 
 SOF_TOP=$(cd "$(dirname "$0")" && cd .. && pwd)
 
-SUPPORTED_PLATFORMS=(apl cnl icl tgl-h)
+SUPPORTED_PLATFORMS=(apl cnl icl tgl-h imx8)
 # Default value, can be overridden on the command line
 WEST_TOP="${SOF_TOP}"/zephyrproject
 BUILD_JOBS=$(nproc --all)
@@ -104,6 +104,10 @@ build()
 				PLAT_CONFIG='intel_adsp_cavs25'
 				RIMAGE_KEY=modules/audio/sof/keys/otc_private_key_3k.pem
 				;;
+                        imx8)
+                                PLAT_CONFIG='nxp_adsp_imx8'
+                                ;;
+
 			*)
 				echo "Unsupported platform: $platform"
 				exit 1
-- 
2.17.1

With this you compile imx8 with:
zephyrproject$ ./modules/audio/sof/scripts/xtensa-build-zephyr.sh -p . imx8

The commands for west are:

zephyrproject$ west build -d imx -b nxp_adsp_imx8 zephyr/samples/subsys/audio/sof/
zephyrproject$ west sign -d imx -p rimage/build/rimage -t rimage -D rimage/config -- -k no

marc-hb · 2021-07-23T00:45:09Z

@marc-hb I already have a patch for this, but is only local...since Zephyr pull request is not yet merged - see zephyrproject-rtos/zephyr#35467

Thanks this helps. You can already submit the PLAT_CONFIG='nxp_adsp_imx8' right now, that takes away the guesswork and makes your local diffs (and others' including mine!) smaller. As long as you don't change the SUPPORTED_PLATFORMS yet it won't get built by default. For the icing on the cake you could add a WIP link to zephyrproject-rtos/zephyr#35467 next to PLAT_CONFIG='nxp_adsp_imx8'.

BTW what about the RIMAGE_KEY? You don't care for now that this script signs sof-imx with the Intel key? Just curious.

So I cherry-picked zephyrproject-rtos/zephyr#35467 and #4524 but now I'm gettting this with the Zephyr SDK 0.12.2, any clue?

zephyr/arch/xtensa/core/offsets/offsets.c
In file included from zephyr/include/arch/xtensa/arch.h:27,
                 from zephyr/include/arch/cpu.h:27,
                 from zephyr/include/kernel_includes.h:33,
                 from zephyr/include/kernel.h:17,
                 from zephyr/include/init.h:11,
                 from zephyr/include/device.h:29,
                 from zephyr/kernel/include/kernel_offsets.h:6,
                 from zephyr/arch/xtensa/core/offsets/offsets.c:7:
zephyr/include/arch/xtensa/irq.h:10:10: fatal error: xtensa/config/core-isa.h: No such file or directory
   10 | #include <xtensa/config/core-isa.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~

iuliana-prodan · 2021-07-23T12:35:46Z

@marc-hb I already have a patch for this, but is only local...since Zephyr pull request is not yet merged - see zephyrproject-rtos/zephyr#35467

Thanks this helps. You can already submit the PLAT_CONFIG='nxp_adsp_imx8' right now, that takes away the guesswork and makes your local diffs (and others' including mine!) smaller. As long as you don't change the SUPPORTED_PLATFORMS yet it won't get built by default. For the icing on the cake you could add a WIP link to zephyrproject-rtos/zephyr#35467 next to PLAT_CONFIG='nxp_adsp_imx8'.

Ok, I will make a draft PR for adding PLAT_CONFIG='nxp_adsp_imx8' in xtensa_build_zephyr.sh.

BTW what about the RIMAGE_KEY? You don't care for now that this script signs sof-imx with the Intel key? Just curious.

TBH I thought that if I don't specify a RIMAGE_KEY here, the .ri image is not signed. I'll have to double check this.

So I cherry-picked zephyrproject-rtos/zephyr#35467 and #4524 but now I'm gettting this with the Zephyr SDK 0.12.2, any clue?

zephyr/arch/xtensa/core/offsets/offsets.c
In file included from zephyr/include/arch/xtensa/arch.h:27,
                 from zephyr/include/arch/cpu.h:27,
                 from zephyr/include/kernel_includes.h:33,
                 from zephyr/include/kernel.h:17,
                 from zephyr/include/init.h:11,
                 from zephyr/include/device.h:29,
                 from zephyr/kernel/include/kernel_offsets.h:6,
                 from zephyr/arch/xtensa/core/offsets/offsets.c:7:
zephyr/include/arch/xtensa/irq.h:10:10: fatal error: xtensa/config/core-isa.h: No such file or directory
   10 | #include <xtensa/config/core-isa.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~

For this you'll have to update the xtensa_hal, also. I had a PR on that repo, also. See here.
I see that xtensa_build_zephyr.sh is doing west update zephyr hal_xtensa, but maybe is not taking the latest changes?
Check if you have the latest commit from hal_xtensa.

marc-hb · 2021-07-23T19:19:12Z

TBH I thought that if I don't specify a RIMAGE_KEY here, the .ri image is not signed. I'll have to double check this.

No there is a default value at the top. I don't think the build can produce any unsigned file with .ri extension right now (which would be confusing).

I was confused by this default value too which is why I added a comment in this PR. Maybe the default value should simply be moved down next to the relevant code.

For this you'll have to update the xtensa_hal, also. I had a PR on that repo, also. See here.
I see that xtensa_build_zephyr.sh is doing west update zephyr hal_xtensa, but maybe is not taking the latest changes?

It won't because you didn't update the west manifest in the Zephyr PR. If you want your Zephyr PR to update hal_extensa there must be a west manifest update in your Zephyr PR pointing at the new hal extensa version.

https://docs.zephyrproject.org/latest/guides/modules.html#submitting-changes-to-modules

Or probably even better: you could simply submit and fast-track the manifest update separately, upgrading everyone to the new hal_xtensa even before your Zephyr PR is merged.

It works after cherry-picking everything, thanks!

marc-hb · 2021-07-25T20:18:14Z

I see that xtensa_build_zephyr.sh is doing west update zephyr hal_xtensa, but maybe is not taking the latest changes?
Check if you have the latest commit from hal_xtensa.

In fact if you run west update zephyr hal_xtensa then you should get xtensa/config/core-isa.h: No such file or directory right now. Try it, as long as you've committed or git stashed everything in hal_xtensa you can't lose anything.

marc-hb added 2 commits July 22, 2021 02:34

marc-hb marked this pull request as ready for review July 22, 2021 02:50

marc-hb requested review from dbaluta, iuliana-prodan, kv2019i, lgirdwood, lyakh and paulstelian97 July 22, 2021 02:50

marc-hb marked this pull request as draft July 22, 2021 04:52

marc-hb commented Jul 22, 2021

View reviewed changes

marc-hb mentioned this pull request Jul 22, 2021

Fixes for SOF with Zephyr on i.MX #4524

Merged

dbaluta reviewed Jul 22, 2021

View reviewed changes

marc-hb mentioned this pull request Jul 22, 2021

zephyr: sof-logger #4503

Merged

marc-hb closed this Jul 23, 2021

marc-hb deleted the zephyr-build-imx branch July 26, 2021 06:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xtensa-build-zephyr: add imx#4525

xtensa-build-zephyr: add imx#4525
marc-hb wants to merge 2 commits intothesofproject:mainfrom
marc-hb:zephyr-build-imx

marc-hb commented Jul 22, 2021

Uh oh!

marc-hb Jul 22, 2021

Uh oh!

paulstelian97 Jul 22, 2021

Uh oh!

iuliana-prodan Jul 22, 2021

Uh oh!

dbaluta Jul 22, 2021

Uh oh!

iuliana-prodan Jul 22, 2021

Uh oh!

dbaluta Jul 22, 2021

Uh oh!

paulstelian97 Jul 22, 2021

Uh oh!

iuliana-prodan commented Jul 22, 2021

Uh oh!

marc-hb commented Jul 23, 2021 •

edited

Loading

Uh oh!

iuliana-prodan commented Jul 23, 2021

Uh oh!

marc-hb commented Jul 23, 2021

Uh oh!

marc-hb commented Jul 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

marc-hb commented Jul 22, 2021

Uh oh!

marc-hb Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

paulstelian97 Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

iuliana-prodan Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

dbaluta Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

iuliana-prodan Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

dbaluta Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

paulstelian97 Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

iuliana-prodan commented Jul 22, 2021

Uh oh!

marc-hb commented Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iuliana-prodan commented Jul 23, 2021

Uh oh!

marc-hb commented Jul 23, 2021

Uh oh!

marc-hb commented Jul 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marc-hb commented Jul 23, 2021 •

edited

Loading