Skip to content

refactor: extract shared CMSIS to libs/3rdparty/cmsis#445

Merged
rolandreichweinbmw merged 3 commits into
eclipse-openbsw:mainfrom
nhuvaoanh123:pr0-cmsis-shared-lib
May 26, 2026
Merged

refactor: extract shared CMSIS to libs/3rdparty/cmsis#445
rolandreichweinbmw merged 3 commits into
eclipse-openbsw:mainfrom
nhuvaoanh123:pr0-cmsis-shared-lib

Conversation

@nhuvaoanh123
Copy link
Copy Markdown
Contributor

Summary

Moves CMSIS 6.1.0 headers from `platforms/s32k1xx/bsp/bspMcu/include/3rdparty/cmsis/` to `libs/3rdparty/cmsis/` so that future Cortex-M platforms can share the same CMSIS core without duplication.

This is the preparatory refactor referenced in PR #413 discussion and the strategy doc under `docs/stm32-pr-strategy.md` on the STM32 series branch.

Changes

  • Move 9 CMSIS files to `libs/3rdparty/cmsis/` (6 byte-identical, 3 with trivial Doxygen comment-style diffs that are preserved as-is)
  • Add `libs/3rdparty/cmsis/module.spec` following the `libs/3rdparty/googletest/module.spec` pattern (unit_test: false, format_check: false, oss: true)
  • Update `platforms/s32k1xx/bsp/bspMcu/CMakeLists.txt` to add the new CMSIS paths to `target_include_directories`
  • Update `platforms/s32k1xx/bsp/bspMcu/include/mcu/mcu.h`: change hardcoded `#include "3rdparty/cmsis/core_cm4.h"` to bare `#include "core_cm4.h"` (resolved via the new include path)
  • Remove CMSIS entries from `platforms/s32k1xx/bsp/bspMcu/module.spec` exclude globs (files are no longer under this module)
  • Consolidate `NOTICE.md`: single CMSIS entry pointing to `libs/3rdparty/cmsis/LICENSE`

Non-goals

  • No functional changes
  • No STM32 code
  • No new CMake targets

Test plan

  • s32k148 FreeRTOS cross-compile succeeds locally (`cmake --preset s32k148-freertos-gcc && cmake --build --preset s32k148-freertos-gcc`) — `app.referenceApp.elf` produced (231 KB text + 100 KB BSS, fits well within S32K148 flash)
  • CI: `tests-s32k1xx-debug` host unit tests pass (awaiting CI — blocked locally by pre-existing mingw64 toolchain issue in `libs/bsw/util`, unrelated to this change)
  • CI: s32k148 ThreadX cross-compile succeeds
  • CI: clang-tidy passes

🤖 Generated with Claude Code

@rolandreichweinbmw
Copy link
Copy Markdown
Contributor

Libs under libs/3rdparty mostly have .riminfo files available to be able to check consistency with upstream.

Can you provide that one also?

nhuvaoanh123 added a commit to nhuvaoanh123/openbsw that referenced this pull request May 1, 2026
Fix treefmt CI gate on PR eclipse-openbsw#445. cmake-format collapses target_include_directories
arguments onto fewer lines than the manual layout produced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nhuvaoanh123
Copy link
Copy Markdown
Contributor Author

Hi @rolandreichweinbmw — quick question before producing the .riminfo. The CMSIS files in this PR were relocated from platforms/s32k1xx/bsp/bspMcu/include/3rdparty/cmsis/, not freshly imported via RIM, so there's no original RIM provenance to preserve.

Two options:

  • (a) Run RIM retroactively against ARM-software/CMSIS_6 v6.1.0 — would require locating the RIM CLI on our side and confirming whether the vendored copy is vanilla v6.1.0 or carries local patches (any divergence would need to land in the ignores field).
  • (b) A stub .riminfo pointing at the upstream URL/tag without RIM-validated checksum, acknowledging this is a relocation rather than a fresh import.

Which would you prefer?

Also pushed 473d0713 to clear the cmake-format treefmt failure.

@rolandreichweinbmw
Copy link
Copy Markdown
Contributor

Hi @rolandreichweinbmw — quick question before producing the .riminfo. The CMSIS files in this PR were relocated from platforms/s32k1xx/bsp/bspMcu/include/3rdparty/cmsis/, not freshly imported via RIM, so there's no original RIM provenance to preserve.

Two options:

  • (a) Run RIM retroactively against ARM-software/CMSIS_6 v6.1.0 — would require locating the RIM CLI on our side and confirming whether the vendored copy is vanilla v6.1.0 or carries local patches (any divergence would need to land in the ignores field).
  • (b) A stub .riminfo pointing at the upstream URL/tag without RIM-validated checksum, acknowledging this is a relocation rather than a fresh import.

Which would you prefer?

Also pushed 473d0713 to clear the cmake-format treefmt failure.

I propose (a). Checksumming and providing traceability to a certain upstream version/commit is the whole point of RIM.

@rolandreichweinbmw
Copy link
Copy Markdown
Contributor

@nhuvaoanh123 : We were just asked about the missing RIM support for CMSIS in the Eclipse review process: https://gitlab.eclipse.org/eclipsefdn/emo-team/emo/-/issues/1125

Can you please update the PR regarding .riminfo?

rolandreichweinbmw pushed a commit to nhuvaoanh123/openbsw that referenced this pull request May 25, 2026
Fix treefmt CI gate on PR eclipse-openbsw#445. cmake-format collapses target_include_directories
arguments onto fewer lines than the manual layout produced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rolandreichweinbmw
Copy link
Copy Markdown
Contributor

Content-wise, looks good now! Thank you for the update.

Formally, can you please update the commit messages according to https://github.com/eclipse-openbsw/openbsw/blob/main/doc/dev/guidelines/commit_message.rst :

  • Headline: Start uppercase, and don't prefix with "refactor:" or similar. E.g. "Extract shared CMSIS to libs/3rdparty/cmsis"
  • Don't list the "Changes:" or "Verified:"
  • Don't include "Co-Authored-By:" by AI tools. We need to assign copyright by a natural or corporate person, to assert the Open Source license. This can't be done for AI generally. If you insist on listing your development tools, you can use "Assisted-By:".

Move the CMSIS 6.1.0 headers from the s32k1xx BSP into
libs/3rdparty/cmsis so future Cortex-M platforms can share the
same CMSIS Core files without duplicating the import.

The module metadata and NOTICE entry are updated with the new path.
The s32k1xx BSP now uses the shared include directory.
Format the updated target_include_directories call so treefmt
accepts the CMSIS include path change.
Add .riminfo metadata for the shared CMSIS Core subset. The
metadata points to the exact CMSIS_6 commit behind version 6.1.0
and ignores upstream files that are not part of the OpenBSW import.
@nhuvaoanh123 nhuvaoanh123 force-pushed the pr0-cmsis-shared-lib branch from 1f3c015 to 5508d7e Compare May 26, 2026 08:55
@nhuvaoanh123
Copy link
Copy Markdown
Contributor Author

Thanks for the guidance. I updated the commit messages accordingly and removed the AI Co-Authored-By trailers.

@rolandreichweinbmw rolandreichweinbmw merged commit 0afc5da into eclipse-openbsw:main May 26, 2026
127 checks passed
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.

2 participants