Feature: Make changes for Cortex-A5 support #14718

Meano · 2021-06-01T03:00:45Z

Summary of changes

It's a PR that add Cortex-A5 support for mbed-os custom targets.
Unify some __CORTEX_A macros.
__MBED_CMSIS_RTOS_CM and __MBED_CMSIS_RTOS_CA9 have been removed from CMake as they're not used anymore.

Impact of changes

Migration actions required

Documentation

None

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

ciarmcom · 2021-06-01T03:30:12Z

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

ciarmcom · 2021-06-01T04:00:14Z

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

0xc0170

Otherwise looks good to me

tools/toolchains/mbed_toolchain.py

LDong-Arm

@Meano Thanks for the addition!

Checking what we have for A9 (grep -rnI CORTEX_A9), I also found the following

hal/tests/TESTS/mbed_hal/critical_section/main.cpp
platform/include/platform/mbed_application.h
platform/source/mbed_sdk_boot.c
platform/source/mbed_application.c
platform/tests/TESTS/mbed_platform/stats_cpu/main.cpp
cmsis/device/rtos/source/mbed_boot.c
rtos/tests/TESTS/mbed_rtos/threads/main.cpp

I think at least some (most?) of them would also need to be done for A5.

@ARMmbed/mbed-os-maintainers Who is the best to help with Cortex-A support? Is it possible to unify (at least some of) the macro checks to Cortex-A in general rather than targeting A9 and A5 specifically?

Pull request has been modified.

LDong-Arm · 2021-06-01T09:25:04Z

As a verification, it'd be really helpful to run Greentea tests on the following inside mbed-os

hal-tests-TESTS-mbed_hal-critical_section
platform-tests-TESTS-mbed_platform-stats_cpu
rtos-tests-TESTS-mbed_rtos-threads

with

mbed test -t <toolchain> -m <your custom target> -n <test name>

as those test cases contain Cortex-A specifics. Thanks!

LDong-Arm · 2021-06-01T09:28:08Z

tools/cmake/cores/Cortex-A5.cmake

+if(${MBED_TOOLCHAIN} STREQUAL "GCC_ARM")
+    list(APPEND common_options
+        "-mthumb-interwork"
+        "-marm"
+        "-mfpu=vfpv3"
+        "-mfloat-abi=softfp"
+        "-mno-unaligned-access"
+        "-mcpu=cortex-a5"
+    )
+elseif(${MBED_TOOLCHAIN} STREQUAL "ARM")
+    list(APPEND common_options
+        "-mfpu=vfpv3"
+        "-mfloat-abi=hard"
+        "-mcpu=cortex-a5"
+    )
+endif()


@ARMmbed/mbed-os-maintainers If we can get some expert to check the architectural features specific to A5, it'd be nice. But otherwise the flags look fairly generic and okay to me.

I compared with m cores and one a core we currently have, no difference.

LDong-Arm · 2021-06-01T09:31:10Z

tools/cmake/cores/Cortex-A5.cmake

+        INTERFACE
+            __CORTEX_A5
+            ARM_MATH_CA5
+            __FPU_PRESENT
+            __CMSIS_RTOS
+            __EVAL
+            __MBED_CMSIS_RTOS_CA5


(More of a note to the Mbed team for future reference: at some point we might check if all of them are still used and do a clean-up. They are defined for A9 too, so okay to keep for consistency)

This is not required, lets remove it.

The old symbols are not used anymore neither (__MBED_CMSIS_RTOS_CA9 or __MBED_CMSIS_RTOS_CM)

0xc0170 · 2021-06-01T10:06:20Z

@ARMmbed/mbed-os-maintainers Who is the best to help with Cortex-A support? Is it possible to unify (at least some of) the macro checks to Cortex-A in general rather than targeting A9 and A5 specifically?

@Meano + @ARMmbed/team-renesas-rz would be the best bet to align them. I'll rereview and find better macro to be used, good point

0xc0170 · 2021-06-01T10:07:32Z

hal/source/mbed_critical_section_api.c

@@ -26,7 +26,7 @@ static bool state_saved = false;

 static bool are_interrupts_enabled(void)
 {
-#if defined(__CORTEX_A9)
+#if defined(__CORTEX_A9) || defined(__CORTEX_A5)


This can become __CORTEX_A only in all where we distinguish cortex-m vs cortex-a cores. CMSIs headers provide __CORTEX_A and __CORTEX_M definition for each, it should be sufficient to check for these.

0xc0170 · 2021-06-01T10:57:13Z

tools/cmake/cores/Cortex-A5.cmake

+if(${MBED_TOOLCHAIN} STREQUAL "GCC_ARM")
+    list(APPEND common_options
+        "-mthumb-interwork"
+        "-marm"
+        "-mfpu=vfpv3"
+        "-mfloat-abi=softfp"
+        "-mno-unaligned-access"
+        "-mcpu=cortex-a5"
+    )
+elseif(${MBED_TOOLCHAIN} STREQUAL "ARM")
+    list(APPEND common_options
+        "-mfpu=vfpv3"
+        "-mfloat-abi=hard"
+        "-mcpu=cortex-a5"
+    )
+endif()


I compared with m cores and one a core we currently have, no difference.

0xc0170 · 2021-06-01T11:03:49Z

tools/cmake/cores/Cortex-A5.cmake

+        INTERFACE
+            __CORTEX_A5
+            ARM_MATH_CA5
+            __FPU_PRESENT
+            __CMSIS_RTOS
+            __EVAL
+            __MBED_CMSIS_RTOS_CA5


This is not required, lets remove it.

The old symbols are not used anymore neither (__MBED_CMSIS_RTOS_CA9 or __MBED_CMSIS_RTOS_CM)

Meano · 2021-06-01T13:33:58Z

@LDong-Arm @0xc0170 I'll try to unify the macro and run Greentea tests.

Meano · 2021-06-02T16:51:25Z

The CA9 board passed Greentea tests. Our CA5 chip will be tested as soon as possible.

LDong-Arm

Looks good to me. The main purpose of this PR is adding Cortex-A5 support, so any further refactoring can be done by the Mbed team in the future if we need.

The Travis failure is okay to ignore. The path tools/targets is for compatibility with the old build system and frozen from major changes, but we can allow it case by case.

Meano · 2021-06-21T16:07:12Z

tools/toolchains/mbed_toolchain.py

-    "Cortex-M0":       ["__CORTEX_M0", "ARM_MATH_CM0", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
-    "Cortex-M0+":      ["__CORTEX_M0PLUS", "ARM_MATH_CM0PLUS", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
-    "Cortex-M1":       ["__CORTEX_M3", "ARM_MATH_CM1", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
-    "Cortex-M3":       ["__CORTEX_M3", "ARM_MATH_CM3", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
-    "Cortex-M4":       ["__CORTEX_M4", "ARM_MATH_CM4", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
+    "Cortex-M0":       ["__CORTEX_M0", "ARM_MATH_CM0", "__CMSIS_RTOS"],
+    "Cortex-M0+":      ["__CORTEX_M0PLUS", "ARM_MATH_CM0PLUS", "__CMSIS_RTOS"],
+    "Cortex-M1":       ["__CORTEX_M3", "ARM_MATH_CM1", "__CMSIS_RTOS"],
+    "Cortex-M3":       ["__CORTEX_M3", "ARM_MATH_CM3", "__CMSIS_RTOS"],
+    "Cortex-M4":       ["__CORTEX_M4", "ARM_MATH_CM4", "__CMSIS_RTOS"],
    "Cortex-M4F":      ["__CORTEX_M4", "ARM_MATH_CM4", "__FPU_PRESENT=1",
-                        "__CMSIS_RTOS", "__MBED_CMSIS_RTOS_CM"],
-    "Cortex-M7":       ["__CORTEX_M7", "ARM_MATH_CM7", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
+                        "__CMSIS_RTOS"],
+    "Cortex-M7":       ["__CORTEX_M7", "ARM_MATH_CM7", "__CMSIS_RTOS"],
    "Cortex-M7F":      ["__CORTEX_M7", "ARM_MATH_CM7", "__FPU_PRESENT=1",
-                        "__CMSIS_RTOS", "__MBED_CMSIS_RTOS_CM"],
+                        "__CMSIS_RTOS"],
    "Cortex-M7FD":     ["__CORTEX_M7", "ARM_MATH_CM7", "__FPU_PRESENT=1",
-                        "__CMSIS_RTOS", "__MBED_CMSIS_RTOS_CM"],
+                        "__CMSIS_RTOS"],
    "Cortex-A9":       ["__CORTEX_A9", "ARM_MATH_CA9", "__FPU_PRESENT",
-                        "__CMSIS_RTOS", "__EVAL", "__MBED_CMSIS_RTOS_CA9"],
+                        "__CMSIS_RTOS", "__EVAL"],
    "Cortex-M23-NS":   ["__CORTEX_M23", "ARM_MATH_ARMV8MBL", "DOMAIN_NS=1",
-                        "__CMSIS_RTOS", "__MBED_CMSIS_RTOS_CM"],
-    "Cortex-M23":      ["__CORTEX_M23", "ARM_MATH_ARMV8MBL", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
+                        "__CMSIS_RTOS"],
+    "Cortex-M23":      ["__CORTEX_M23", "ARM_MATH_ARMV8MBL", "__CMSIS_RTOS"],
    "Cortex-M33-NS":   ["__CORTEX_M33", "ARM_MATH_ARMV8MML", "DOMAIN_NS=1",
-                        "__CMSIS_RTOS", "__MBED_CMSIS_RTOS_CM"],
-    "Cortex-M33":      ["__CORTEX_M33", "ARM_MATH_ARMV8MML", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
+                        "__CMSIS_RTOS"],
+    "Cortex-M33":      ["__CORTEX_M33", "ARM_MATH_ARMV8MML", "__CMSIS_RTOS"],
    "Cortex-M33F-NS":  ["__CORTEX_M33", "ARM_MATH_ARMV8MML", "DOMAIN_NS=1",
-                        "__FPU_PRESENT=1U", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
+                        "__FPU_PRESENT=1U", "__CMSIS_RTOS"],
    "Cortex-M33F":     ["__CORTEX_M33", "ARM_MATH_ARMV8MML",
-                        "__FPU_PRESENT=1U", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM"],
+                        "__FPU_PRESENT=1U", "__CMSIS_RTOS"],
    "Cortex-M33FE-NS": ["__CORTEX_M33", "ARM_MATH_ARMV8MML", "DOMAIN_NS=1",
                        "__FPU_PRESENT=1U", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM", "__DSP_PRESENT=1U"],
+                        "__DSP_PRESENT=1U"],
    "Cortex-M33FE":    ["__CORTEX_M33", "ARM_MATH_ARMV8MML",
                        "__FPU_PRESENT=1U", "__CMSIS_RTOS",
-                        "__MBED_CMSIS_RTOS_CM", "__DSP_PRESENT=1U"],
+                        "__DSP_PRESENT=1U"],


@Patater Revert this file in this commit?

Meano · 2021-06-21T16:12:32Z

tools/toolchains/mbed_toolchain.py

+    "Cortex-A5":       ["__CORTEX_A5", "ARM_MATH_CA5", "__FPU_PRESENT",
+                        "__CMSIS_RTOS", "__EVAL"],


Are these changes to mbed_toolchain.py could be approved? @Patater

It's a new core added without changing existing ones, so it should be okay

Meano · 2021-06-23T06:51:03Z

The modifications to "tools/toolchains/mbed_toolchain.py" aren't functionally necessary, right?

Please remove changes to "tools/toolchains/mbed_toolchain.py", as it needs to support even older versions of Mbed OS (online) that may still use the __MBED_CMSIS_RTOS_CM define.

OK to remove __MBED_CMSIS_RTOS_CM from CMake, assuming we don't have some out-of-tree libraries that depend on that define.

I have reverted the changes of __MBED_CMSIS_RTOS_CM/A9 in "tools/toolchains/mbed_toolchain.py", but the modifications of the frozen files to support Cortex-A5 are kept (These can be removed at any time if necessary).

@LDong-Arm @0xc0170 @Patater Please review again.

LDong-Arm · 2021-06-23T08:49:33Z

tools/toolchains/mbed_toolchain.py

+    "Cortex-A5":       ["__CORTEX_A5", "ARM_MATH_CA5", "__FPU_PRESENT",
+                        "__CMSIS_RTOS", "__EVAL"],


It's a new core added without changing existing ones, so it should be okay

0xc0170 · 2021-06-24T15:23:28Z

CI started (while waiting for the approval)

mbed-ci · 2021-06-24T16:09:51Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 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-GCC_ARM	✔️
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-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-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_greentea-test	✔️

ciarmcom · 2021-06-30T11:50:52Z

This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-hal, please complete review of the changes to move the PR forward. Thank you for your contributions.

0xc0170 · 2021-06-30T14:46:03Z

@Patater If you approve it, this should be ready for integration

mergify bot added the needs: work label Jun 1, 2021

ciarmcom added the release-type: feature label Jun 1, 2021

ciarmcom requested review from a team June 1, 2021 03:30

ciarmcom added needs: review component: hal component: core component: tools and removed needs: review needs: work labels Jun 1, 2021

ciarmcom added the needs: review label Jun 1, 2021

0xc0170 previously requested changes Jun 1, 2021

View reviewed changes

tools/toolchains/mbed_toolchain.py Show resolved Hide resolved

mergify bot added needs: work and removed needs: review labels Jun 1, 2021

LDong-Arm reviewed Jun 1, 2021

View reviewed changes

Meano force-pushed the develop branch from 85872bc to b752a52 Compare June 1, 2021 09:23

LDong-Arm reviewed Jun 1, 2021

View reviewed changes

0xc0170 reviewed Jun 1, 2021

View reviewed changes

Meano force-pushed the develop branch from b752a52 to 9162f83 Compare June 2, 2021 16:44

Meano force-pushed the develop branch from 9162f83 to 57b1eeb Compare June 3, 2021 10:50

LDong-Arm previously approved these changes Jun 3, 2021

View reviewed changes

Meano force-pushed the develop branch from 57b1eeb to 2b55de1 Compare June 4, 2021 11:00

Meano commented Jun 21, 2021

View reviewed changes

mergify bot added needs: work and removed needs: review labels Jun 22, 2021

Meano added 5 commits June 23, 2021 13:34

Remove unused old symbols: __MBED_CMSIS_RTOS_CM/A9

d8917a4

Unify the __CORTEX_A macro in the files containing cmsis.h

6feca90

Unify the cores/Cortex-A.cmake

a57b892

Make changes for Cortex-A5 support

ff21881

Modify the python tools to support Cortex-A5

2d2d1cf

Meano force-pushed the develop branch from db344b2 to 2d2d1cf Compare June 23, 2021 05:41

LDong-Arm approved these changes Jun 23, 2021

View reviewed changes

0xc0170 approved these changes Jun 24, 2021

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Jun 24, 2021

0xc0170 added needs: review and removed needs: CI labels Jun 25, 2021

ciarmcom added the stale Stale Pull Request label Jun 30, 2021

0xc0170 requested a review from Patater June 30, 2021 14:45

ciarmcom removed the stale Stale Pull Request label Jun 30, 2021

Patater approved these changes Jul 1, 2021

View reviewed changes

Patater merged commit b5c2f70 into ARMmbed:master Jul 1, 2021

mbedmain added release-version: 6.13.0 Release-pending and removed release-type: feature Release-pending labels Jul 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Make changes for Cortex-A5 support #14718

Feature: Make changes for Cortex-A5 support #14718

Meano commented Jun 1, 2021 •

edited

Loading

ciarmcom commented Jun 1, 2021

ciarmcom commented Jun 1, 2021

0xc0170 left a comment

LDong-Arm left a comment •

edited

Loading

LDong-Arm commented Jun 1, 2021 •

edited

Loading

LDong-Arm Jun 1, 2021

0xc0170 Jun 1, 2021

LDong-Arm Jun 1, 2021

0xc0170 Jun 1, 2021

0xc0170 commented Jun 1, 2021

0xc0170 Jun 1, 2021

0xc0170 Jun 1, 2021

0xc0170 Jun 1, 2021

Meano commented Jun 1, 2021 •

edited

Loading

Meano commented Jun 2, 2021

LDong-Arm left a comment •

edited

Loading

Meano Jun 21, 2021

Meano Jun 21, 2021 •

edited

Loading

LDong-Arm Jun 23, 2021

Meano commented Jun 23, 2021

LDong-Arm Jun 23, 2021

0xc0170 commented Jun 24, 2021

mbed-ci commented Jun 24, 2021

ciarmcom commented Jun 30, 2021

0xc0170 commented Jun 30, 2021

		"Cortex-A5": ["__CORTEX_A5", "ARM_MATH_CA5", "__FPU_PRESENT",
		"__CMSIS_RTOS", "__EVAL"],

Feature: Make changes for Cortex-A5 support #14718

Feature: Make changes for Cortex-A5 support #14718

Conversation

Meano commented Jun 1, 2021 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Jun 1, 2021

ciarmcom commented Jun 1, 2021

0xc0170 left a comment

Choose a reason for hiding this comment

LDong-Arm left a comment • edited Loading

Choose a reason for hiding this comment

LDong-Arm commented Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 commented Jun 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Meano commented Jun 1, 2021 • edited Loading

Meano commented Jun 2, 2021

LDong-Arm left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Meano Jun 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Meano commented Jun 23, 2021

Choose a reason for hiding this comment

0xc0170 commented Jun 24, 2021

mbed-ci commented Jun 24, 2021

Jenkins CI Test : ✔️ SUCCESS

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

ciarmcom commented Jun 30, 2021

0xc0170 commented Jun 30, 2021

Meano commented Jun 1, 2021 •

edited

Loading

LDong-Arm left a comment •

edited

Loading

LDong-Arm commented Jun 1, 2021 •

edited

Loading

Meano commented Jun 1, 2021 •

edited

Loading

LDong-Arm left a comment •

edited

Loading

Meano Jun 21, 2021 •

edited

Loading