Skip to content

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented May 10, 2023

We import hal_xtensa indirectly through zephyr/west.yml.

When overriding the Zephyr revision in sof/west.yml to test the Zephyr main branch "zmain", we must also update the version of hal_xtensa specified in zephyr/west.yml. Stop doing a manual, single repo git fetch and use a submanifest to perform this correctly. This is exactly why submanifests/ were added in the first place.

These fixes the imx8m compilation error spotted and discussed in (unrelated) #7579 following the IMX rename

@marc-hb

This comment was marked as resolved.

@marc-hb marc-hb force-pushed the fix-zmain-hal branch 2 times, most recently from 697c59e to 91ed161 Compare May 11, 2023 00:16
@marc-hb marc-hb requested a review from iuliana-prodan May 11, 2023 00:43
@marc-hb

This comment was marked as resolved.

@iuliana-prodan
Copy link
Contributor

@marc-hb can you please, help me a bit with this failure?

I see here that is using Zephyr's revision: HEAD is now at 3e02d48e4 ("driver: adc: infineon: Adding ADC driver"), which doesn't have the commits from zephyrproject-rtos/zephyr#57084, but it complaints on "fatal error: nxp/nxp_imx/mimx8ml8dvnlz-pinctrl.dtsi: No such file or directory" - this dtsi is added in this commit, part of the aforementioned PR.

Originally posted by @iuliana-prodan in #7579 (comment)

There is a mimx8ml8dvnlz.dtsi file in this commit but I don't see any mimx8ml8dvnlz-pinctrl.dtsi. Did you run "git status"?

The mimx8ml8dvnlz-pinctrl.dtsi is taken from nxp_hal - sorry, I was wrong when saying that it is added in this commit.
On my side I did a west update in zephyr and all modules (and hals) were brought so the dtsi was found, therefore no error.

Let me see how to fix this, I don't want to bring nxp_hal when compiling SOF with zephyr - for now it is not needed.

I'll answer here to this comment, also.
The west build -p always -b nxp_adsp_imx8m ../modules/audio/sof/ -- -DTOOLCHAIN=/opt/zephyr-sdk-0.15.2/xtensa-nxp_imx8m_adsp_zephyr-elf/bin/xtensa-nxp_imx8m_adsp_zephyr-elf -DINIT_CONFIG=imx8m_defconfig is incomplete.
As discussed here, the correct way to give SOF as a sample app in Zephyr is modules/audio/sof/**app**.
Also, some of these warnings/errors were solved here: #7533

The

 compiler_info.h:40:17: error: initializer-string for array of 'unsigned char' is too long [-Werror]
   40 | #define CC_DESC " " XCC_TOOLS_VERSION

was not fixed (only locally on my machine :) ) because it was only related with NXP i.MX8MP, since the XCC_TOOLS_VERSION is too long (xtensa-nxp_imx8m_adsp_zephyr...), more than 32 as defined here.

@iuliana-prodan
Copy link
Contributor

@marc-hb I've fix it in zephyrproject-rtos/zephyr#57795
Added also a PR, in SOF, for testing this: #7591

@marc-hb
Copy link
Collaborator Author

marc-hb commented May 11, 2023

https://github.com/thesofproject/sof/actions/runs/4951673081/jobs/8856954534?pr=7586 is now passing \o/

One final touch and will mark "ready for review" today.

We import hal_xtensa indirectly through zephyr/west.yml.

When overriding the Zephyr revision in sof/west.yml to test the Zephyr
main branch "zmain", we must also update the version of hal_xtensa
specified in zephyr/west.yml. Stop doing a manual, single repo git fetch
and use a submanifest to perform this correctly. This is exactly why
submanifests/ were added in the first place.

These fixes the imx8m compilation error spotted and discussed
in (unrelated) thesofproject#7579 following the IMX rename in
zephyrproject-rtos/zephyr#57084 and
zephyrproject-rtos/zephyr#57795

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
Copy link
Collaborator Author

marc-hb commented May 11, 2023

@marc-hb marc-hb marked this pull request as ready for review May 11, 2023 21:37
@ranj063
Copy link
Collaborator

ranj063 commented May 12, 2023

PR testing skipped as this is pure .github/action change

@ranj063 ranj063 merged commit fca84b8 into thesofproject:main May 12, 2023
@marc-hb marc-hb deleted the fix-zmain-hal branch May 12, 2023 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants