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

Update CMSIS to 5.8.0 #14900

Merged
merged 16 commits into from
Jul 30, 2021
Merged

Update CMSIS to 5.8.0 #14900

merged 16 commits into from
Jul 30, 2021

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Jul 9, 2021 •
edited
Loading

Summary of changes

Update CMSIS to 5.8.0. It brings lot of fixes, see https://github.com/ARM-software/CMSIS_5/releases/tag/5.8.0

We again had some breakages, mainly it was arm compat (workaround implemented).

Impact of changes

Migration actions required

Documentation

Pull request type 

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results 

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@mergify mergify bot added the do not merge label Jul 9, 2021
0xc0170
0xc0170 commented Jul 9, 2021
View reviewed changes
Copy link
Contributor Author

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am able to compile a simple test now if I comment out rtx_def, so at least something

cmsis/CMSIS_5/CMSIS/RTOS2/RTX/Source/TOOLCHAIN_GCC/TARGET_M0/irq_cm0.S
*
* -----------------------------------------------------------------------------
*/


.syntax unified

#include "rtx_def.h"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is causing a problem as we somehow get some C/C++ headers to this .S file and it just errors with strange errors.

We ignored the inclusion previously because of this, will need to check how to fix it this time.

@0xc0170 0xc0170 force-pushed the feature_CMSIS_5_13b9f72f2 branch from 2902af9 to 14beb29 Compare July 9, 2021 10:53
0xc0170
0xc0170 commented Jul 9, 2021
View reviewed changes
cmsis/CMSIS_5/CMSIS/TARGET_CORTEX_M/Source/mbed_tz_context.c Outdated
@@ -22,7 +22,7 @@
* limitations under the License.
*/

#if !FEATURE_TFM
#if !TARGET_TFM
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one should be reverted as well, will check how it got in

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@0xc0170 0xc0170 force-pushed the feature_CMSIS_5_13b9f72f2 branch from 14beb29 to 551bceb Compare July 9, 2021 10:58
@ciarmcom ciarmcom requested review from a team July 9, 2021 11:00
@ciarmcom
Copy link
Member

ciarmcom commented Jul 9, 2021

@0xc0170, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

@Patater
Copy link
Contributor

Patater commented Jul 9, 2021

CI started

@rajkan01
Copy link
Contributor

rajkan01 commented Jul 9, 2021 •
edited
Loading

I will check the CI failures, as Martin is looking

Patater
Patater reviewed Jul 9, 2021
View reviewed changes
cmsis/CMSIS_5/CMSIS/RTOS2/RTX/Include/rtx_def.h
* Title: RTX derived definitions
*
* -----------------------------------------------------------------------------
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file failed CI due to missing SPDX header

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am reporting this upstream, it should be fixed asap. I assume as this is already released and new version might take some time, we can patch this file manually. I'll check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for upstream ARM-software/CMSIS_5#1238

@mbed-ci
Copy link

mbed-ci commented Jul 9, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ❌
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ❌
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ❌
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM ❌
jenkins-ci/mbed-os-ci_build-greentea-ARM ❌
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ❌
jenkins-ci/mbed-os-ci_cmake-example-ARM ❌
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ❌
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ❌
jenkins-ci/mbed-os-ci_build-example-ARM ❌

@0xc0170 0xc0170 force-pushed the feature_CMSIS_5_13b9f72f2 branch from 9639d83 to 2b3bb8b Compare July 9, 2021 14:57
@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 9, 2021

I rebased instead of fixup 🤦

I patched rtx_def, we had similar patch in place but they introduced this via the new version. I provided details in commit msg. I already asked cmsis team for clarification and will create an issue once I get details what should be fixed to get this fix upstream or our codebase needs some refactoring. Commit 89d9642

@ciarmcom
Copy link
Member

ciarmcom commented Jul 9, 2021

@0xc0170, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested review from a team July 9, 2021 15:00
@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 9, 2021

I started another round of tests.

I can locally build Gcc Arm without errors (I assume TFM related things might have issues or FPU - both things were patched previously in Mbed OS, I reverted some patches so lets see what we still need or do not need).

ARMClang reports an error with one overload, I am checking it now.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 9, 2021

I fixed also locally ARMClang (removing old compat header, we should use CMSIS functionality instead)

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 9, 2021

Aborting the current build and reruning CI

@mergify mergify bot dismissed Patater’s stale review July 29, 2021 13:33

Pull request has been modified.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 29, 2021

Fixed both

Patater
Patater previously approved these changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I guess remaining work is to update the patch list in cmsis_importer.json with final hashes.

@mergify mergify bot added needs: CI and removed needs: work labels Jul 29, 2021
cmsis importer: add commits for Mbed OS changes
049d1f1 
See specific SHA for details.
@mergify mergify bot dismissed Patater’s stale review July 30, 2021 07:36

Pull request has been modified.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 30, 2021

SHAs added to importer. I'll rerun CI

@mbed-ci
Copy link

mbed-ci commented Jul 30, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 12 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️
jenkins-ci/mbed-os-ci_tfm-integration ✔️

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 30, 2021

Lets get this in and monitor nightly over the weekend.

@Patater Patater merged commit 744814f into ARMmbed:master Jul 30, 2021
@mergify mergify bot removed the ready for merge label Jul 30, 2021
@0xc0170 0xc0170 deleted the feature_CMSIS_5_13b9f72f2 branch July 30, 2021 10:49
@jeromecoutant
Copy link
Collaborator

This PR has dropped IAR support.... again.... :-(

jeromecoutant added a commit to jeromecoutant/mbed that referenced this pull request Aug 20, 2021
@jeromecoutant
Remove IAR patches from ARMmbed#14900
0341316
@jeromecoutant jeromecoutant mentioned this pull request Aug 20, 2021
Closed
jeromecoutant added a commit to jeromecoutant/mbed that referenced this pull request Aug 23, 2021
@jeromecoutant
Remove IAR patches from ARMmbed#14900
5c0ca65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Patater Patater Patater approved these changes

Assignees
No one assigned
Labels
component: core component: tools release-version: 6.14.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@0xc0170 @ciarmcom @Patater @rajkan01 @mbed-ci @jeromecoutant @mbedmain @bulislaw