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.

Sign up for GitHub

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

Jump to bottom

xtensa: nxp_imx8ulp_adsp: Add missing include header #712

Merged
merged 2 commits into from
Nov 13, 2023
Merged

xtensa: nxp_imx8ulp_adsp: Add missing include header #712

merged 2 commits into from
Nov 13, 2023

Conversation

iuliana-prodan
Copy link
Contributor

Add missing header to fix imx8ulp_adsp toolchain.

Fixes: e88c208 ("xtensa: Add 8ulp_adsp toolchain")

@iuliana-prodan
Copy link
Contributor Author

@nashif @dcpleung @keith-packard @tejlmand @dbaluta @dleach02 Please review this
Thanks!

@keith-packard
Copy link
Collaborator

Yup, this appears to be included in other xtensa toolchains, so it makes sense to have it here as well.

@iuliana-prodan
Copy link
Contributor Author

I don't understand why some CI tests are failing (they are, definitely, not related with this fix) so I did an amend just to retrigger the CI.

@stephanosio
Copy link
Member

I don't understand why some CI tests are failing (they are, definitely, not related with this fix) so I did an amend just to retrigger the CI.

ERROR   - platform_filter - unrecognized platform - qemu_xtensa_mmu

It looks like the qemu_xtensa_mmu platform, which is required by the SDK CI, has been either renamed or removed from Zephyr. I will look into fixing this.

@iuliana-prodan
Copy link
Contributor Author

I don't understand why some CI tests are failing (they are, definitely, not related with this fix) so I did an amend just to retrigger the CI.

ERROR   - platform_filter - unrecognized platform - qemu_xtensa_mmu

It looks like the qemu_xtensa_mmu platform, which is required by the SDK CI, has been either renamed or removed from Zephyr. I will look into fixing this.

Thanks!
I thought it might be related with this: zephyrproject-rtos/zephyr#64464, but I did a rebase on Friday and still failing.

@dcpleung
Copy link
Member

I don't understand why some CI tests are failing (they are, definitely, not related with this fix) so I did an amend just to retrigger the CI.

ERROR   - platform_filter - unrecognized platform - qemu_xtensa_mmu

It looks like the qemu_xtensa_mmu platform, which is required by the SDK CI, has been either renamed or removed from Zephyr. I will look into fixing this.

That should have been updated in #708.

This was referenced Nov 3, 2023
Merged
Merged
@dbaluta
Copy link
Contributor

dbaluta commented Nov 8, 2023 •
edited
Loading

@dcpleung I really don't understand the problem.

So, Zephyr renamed qemu_xtensa_dc233c to qemu_xtensa_mmu but we get an error that qemu_xtensa_mmu is not a recognized platform.

So, maybe CI isn't using an updated version of zephyr for testing?

@dcpleung
Copy link
Member

dcpleung commented Nov 8, 2023

@dcpleung I really don't understand the problem.

So, Zephyr renamed qemu_xtensa_dc233c to qemu_xtensa_mmu but we get an error that qemu_xtensa_mmu is not a recognized platform.

So, maybe CI isn't using an updated version of zephyr for testing?

I think I know why... SDK CI uses topic-sdk-dev branch on Zephyr repo, which does not contain the renaming commit, and thus failing. I am not sure the process on updating topic-sdk-dev.

@iuliana-prodan
Copy link
Contributor Author

@stephanosio @dcpleung @nashif @dbaluta I've created a test PR (#715) in which I reverted 1068dfb hoping to get an error like: unrecognized platform - qemu_xtensa_dc233c.
But no, I'm getting the same unrecognized platform - qemu_xtensa_mmu - see https://github.com/zephyrproject-rtos/sdk-ng/actions/runs/6798463782/job/18487083028?pr=715

Also, I see the zephyr version is Zephyr version: zephyr-v3.4.0-2888-g755bdf80c696 - isn't this a bit old, I believe this commit zephyrproject-rtos/zephyr@1c0178a is from 3.5.

@iuliana-prodan
Copy link
Contributor Author

@stephanosio @dcpleung @nashif @dbaluta I've created a test PR (#715) in which I reverted 1068dfb hoping to get an error like: unrecognized platform - qemu_xtensa_dc233c. But no, I'm getting the same unrecognized platform - qemu_xtensa_mmu - see https://github.com/zephyrproject-rtos/sdk-ng/actions/runs/6798463782/job/18487083028?pr=715

Also, I see the zephyr version is Zephyr version: zephyr-v3.4.0-2888-g755bdf80c696 - isn't this a bit old, I believe this commit zephyrproject-rtos/zephyr@1c0178a is from 3.5.

I've updated PR #715 to use main branch for zephyr (see 2nd commit here) but I get the same error and the same zephyr version:

image

So... I'm out of ideas :(
What am I doing wrong?

@stephanosio
Copy link
Member

So... I'm out of ideas :(
What am I doing wrong?

@iuliana-prodan Nothing. It is still using the topic-sdk-dev branch even though you have updated the workflow to use main in your PR because the workflow runs in the context of the base branch, and not the PR branch.

I am busy working on something else at the moment; once I get back to the SDK work (likely going to be next week, I will either fast-forward the topic-sdk-dev branch, or modify the SDK CI to use the Zephyr main branch and merge this PR.

@iuliana-prodan @stephanosio
xtensa: nxp_imx8ulp_adsp: Add missing include header
0f8d32a 
Add missing header to fix imx8ulp_adsp toolchain.

Fixes: e88c208 ("xtensa: Add 8ulp_adsp toolchain")
Signed-off-by: Iuliana Prodan <[email protected]>
@stephanosio
Copy link
Member

Rebased to fix CI issues.

@stephanosio stephanosio mentioned this pull request Nov 13, 2023
Merged
@iuliana-prodan
Copy link
Contributor Author

Thank you very much for your help @stephanosio.
The error in the CI - ERROR - platform_filter - unrecognized platform - nxp_adsp_imx8ulp might be to the fact that the Zephyr support for 8ulp is not yet merged -see zephyrproject-rtos/zephyr#63751?

@stephanosio
ci: Disable testing of nxp_adsp_imx8ulp
3b9e9bf 
This commit disables the testing of `nxp_adsp_imx8ulp` platform
because it has not been merged upstream yet.

Re-enable this platform when the zephyrproject-rtos/zephyr#63751 is
merged.

Signed-off-by: Stephanos Ioannidis <[email protected]>
@stephanosio
Copy link
Member

Thank you very much for your help @stephanosio. The error in the CI - ERROR - platform_filter - unrecognized platform - nxp_adsp_imx8ulp might be to the fact that the Zephyr support for 8ulp is not yet merged -see zephyrproject-rtos/zephyr#63751?

@iuliana-prodan Yes. I have just pushed a commit to disable the testing of that platform now.

@stephanosio stephanosio merged commit dc574db into zephyrproject-rtos:main Nov 13, 2023
27 of 29 checks passed
@dbaluta
Copy link
Contributor

dbaluta commented Nov 13, 2023

Thanks a lot @stephanosio !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcpleung dcpleung dcpleung approved these changes

@tejlmand tejlmand tejlmand approved these changes

@keith-packard keith-packard keith-packard approved these changes

@dbaluta dbaluta dbaluta approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@iuliana-prodan @keith-packard @stephanosio @dcpleung @dbaluta @tejlmand