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

Module for CMSIS v6 #77220

Closed
tomi-font opened this issue Aug 19, 2024 · 12 comments · Fixed by #82022
Closed

Module for CMSIS v6 #77220

tomi-font opened this issue Aug 19, 2024 · 12 comments · Fixed by #82022
Assignees
Labels
area: CMSIS-Core TSC Topics that need TSC discussion

Comments

@tomi-font
Copy link
Collaborator

Origin

https://github.com/ARM-software/CMSIS_6

Purpose

At version 2.1.0 TF-M switched to using CMSIS v6. It's hosted by Arm in a different repository than CMSIS v5.

Mode of integration

As a module. Name suggestion: CMSISv6
Also, the CMSIS v5 module could be considered for renaming from cmsis to specify the version.

Maintainership

(based on the current West project: cmsis entry)

Description

A quick fix was made right before the 3.7.0 release for TF-M: pushing the CMSIS v6 sources to Zephyr's fork of TF-M repository (#76094).
The proper way is to fork the CMSIS v6 repository and make it a new module.

Revision

d0c460c169

License

Apache-2.0

@tomi-font tomi-font added the TSC Topics that need TSC discussion label Aug 19, 2024
@nordicjm
Copy link
Collaborator

Why can't https://github.com/zephyrproject-rtos/cmsis be updated to version 6 and pulled in? Why do we need a completely duplicate module of the same thing and to pull it in twice?

@tomi-font
Copy link
Collaborator Author

Because it's not there for TF-M. TF-M used to push the CMSIS sources to their repository. The existing CMSIS repository is used by/for something else.

@nordicjm
Copy link
Collaborator

It's for CMSIS in general for zephyr, so if it can be updated to version 6 then tf-m can use it?

@nordicjm
Copy link
Collaborator

@stephanosio can you update cmsis to version 6?

@tomi-font
Copy link
Collaborator Author

It's for CMSIS in general for zephyr, so if it can be updated to version 6 then tf-m can use it?

Yes from TF-M's point of view, it doesn't matter. I just haven't dug into whether the already existing CMSIS module could be updated, assuming that no.

@tomi-font
Copy link
Collaborator Author

ping @stephanosio

@stephanosio
Copy link
Member

For updating the existing CMSIS module ecosystem to v6, we will need to:

  1. Ensure that the ARM arch port, which makes use of many CMSIS-Core functions, is compatible with CMSIS-Core v6.
  2. Update CMSIS-DSP module to v6 (because we do not want to patch "CMSIS-DSP v5" to be compatible with "CMSIS-Core v6").
    • Ensure that all in-tree CMSIS-DSP tests and samples are compatible with CMSIS-DSP v6.
    • Ensure that the Zephyr DSP subsystem CMSIS-DSP backend is compatible with CMSIS-DSP v6.
  3. CMSIS-NN module is updated to v6.
    • Ensure that all in-tree CMSIS-NN tests and samples are compatible with CMSIS-NN v6.

Unfortunately, I do not have the bandwidth for the above tasks at this time.

Since Arm is now an active member of the project, I would prefer if someone from Arm takes over the maintainership of this module (maybe @ithinuel?).

Alternatively, we can create a new module for CMSIS-Core v6 (cmsis_6?), while keeping the CMSIS-Core v5 for use by the existing components relying on the v5, and only reference it where needed (e.g. TF-M).

@ithinuel
Copy link
Collaborator

ithinuel commented Sep 6, 2024

  • cmsis-nn’s repo is a fork of the upstream. Its branches are:

    • zephyr which is about 50 commits behind upstream.
    • main which is 5 commits behind.
    • zephyr-v6.0.0 which is 6 commits behind. This is the branch Zephyr’s west.yaml points to.
      This is already aligned with CMSIS-6.
  • cmsis-dsp’s repo is a fork of the upstream. Its branches are:

    • main, the default branch, 53 commits behind is where Zephyr’s west.yaml points to.
    • zephyr is 89 commits behind.

    Zephyr’s fork is currently pointing somewhere between the upstream tag for versions 1.5.0 and 1.6.0.
    This is already aligned with CMSIS-6.

  • Zephyr’s cmsis repo does not fork and does not share the history of the upstream.
    One of the major difference is that Zephyr’s version only keeps the Core elements from the upstream which also contains CMSIS-NN, CMSIS-DSP etc.

Some parts of CMSIS-Core require soc specific headers and definitions.
Although CMSIS-5 and CMSIS-6 functionnality are essentially the same, there has been some breaking change introduced. A list can be found at https://arm-software.github.io/CMSIS_6/latest/Core/core_revisionHistory.html#core6_changes.

IMHO, I think it’d be better to get a fork of the upstream repository to ease tracking of the history.

We have a pretty limited bandwidth so I cannot give a timeframe for when we would be able to implement this. However, I’ll be happy to help with reviews 👍

Copy link

github-actions bot commented Nov 6, 2024

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Nov 6, 2024
@RobinKastberg
Copy link
Contributor

This is needed for IAR support or https://github.com/ARM-software/CMSIS_6/pull/102/files cherry-picked into cmsis_5 module.

@nordicjm nordicjm removed the Stale label Nov 19, 2024
@tomi-font tomi-font linked a pull request Nov 26, 2024 that will close this issue
@piotrrak
Copy link

Since I've wanted use clang from arm 19.1.1 on Renesas ek_ra8m1 devboard and hit missing cmsis5 include (pac_armv81.h), so I gave it a shot.

After replacing CMSIS v5 with v6 I've built and run few examples successfully for -march thumbv8.1-....
That's probably way to go for cortex-m55/cortex-m85 in my case.
For this target it seems to be drop in replacement, with no real changes required so far.

While clang needs some changes (in cmake/compiler/clang/target.cmake specify correct '-mfpu' rather than -mfpu=auto which works only for gcc). Rest is minor stuff like ignoring few of the different warnings, etc.

Extent of changes that was needed was adding:
cat ../modules/lib/cmsis_6/CMakeLists.txt
zephyr_include_directories("CMSIS/Core/Include/")
And trivial zephyr/module.yaml

If other hals don't use any of header symbols that have changed (there is small list of few struct members)
switching should be pretty straightforward.

From clang support perspective would be nice to have CMSIS v6 since it started to distinguish gcc/clang differences rather than treating clang as if it was gcc compatible.

Maybe starting at some point on per hal basis would be a good idea?

@tomi-font
Copy link
Collaborator Author

@piotrrak Not sure that's what you're asking for, but I have recently put up a PR to add the CMSIS v6 repo as a module (separate from the v5): #82022
It should get merged pretty soon, so anything in Zephyr will be able to use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: CMSIS-Core TSC Topics that need TSC discussion
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants