Support mbed_start_application for Cortex-M0+ #9791
Conversation
platform/mbed_application.c
Outdated
| @@ -170,7 +177,11 @@ __asm static void start_new_application(void *sp, void *pc) | |||
| void start_new_application(void *sp, void *pc) | |||
| { | |||
| __asm volatile( | |||
| "movw r2, #0 \n" // Fail to compile "mov r2, #0" with ARMC6. Replace with MOVW. | |||
| #if defined(__CORTEX_M0PLUS) | |||
| "mov r2, #0 \n" // No MOVW instruction on Cortex-M0+ | |||
There was a problem hiding this comment.
interesting note that ARMC6 fails, does this compile for m0+ ?
There was a problem hiding this comment.
I don't have armc6 to test. This was a comment from @ccli8. I also thought it was interesting, because this is part of the code path for IAR and GCC, not Arm compiler, so I don't know the significance of the comment. I left it as is in this PR.
There was a problem hiding this comment.
I see, @ccli8 should that comment be removed or ?
There was a problem hiding this comment.
@sarahmarshy ARMC6 (not ARM) toolchain doesn't define __CC_ARM, but define __GNUC__, so ARMC6 will compile this code path.
@0xc0170 I re-compile this code path with ARMC6 (6.10) and GCC_ARM (2018/Q2), mov r2, #0 compiles failed only on ARMC6/M23 combination. So if the comment is to be left, it can change to more accurate like Fail to compile "mov r2, #0" with ARMC6/M23. Replace with MOVW.
#elif defined (__GNUC__) || defined (__ICCARM__)
void start_new_application(void *sp, void *pc)
{
__asm volatile(
"movw r2, #0 \n" // Fail to compile "mov r2, #0" with ARMC6. Replace with MOVW.There was a problem hiding this comment.
Thanks @ccli8 . The latest update took care of it
|
@sarahmarshy, thank you for your changes. |
platform/mbed_application.c
Outdated
| for (i = 0; i < 8; i++) { | ||
| NVIC->IP[i] = 0x00000000; | ||
| } | ||
| #else |
There was a problem hiding this comment.
As an alternative to having this as a separate block could you set isr_groups_32 = 1 for m0+ and reuse the code below?
There was a problem hiding this comment.
Good point. I'll do that.
There was a problem hiding this comment.
Fixed in latest commit 👍
|
CI started |
Test run: FAILEDSummary: 4 of 8 test jobs failed Failed test jobs:
|
|
@sarahmarshy Please review the build failure. Was this changeset tested in IAR and ARM compilers? |
|
@cmonr, no, but it has been now. Should be fixed by latest commit 👍 |
platform/mbed_application.c
Outdated
| @@ -158,7 +161,11 @@ static void powerdown_scb(uint32_t vtor) | |||
|
|
|||
| __asm static void start_new_application(void *sp, void *pc) | |||
| { | |||
| #if defined(__CORTEX_M0PLUS) | |||
There was a problem hiding this comment.
It's best to just use MOVS unconditionally here - it's a 16-bit instruction instead of 32-bit (which is why the M0+ supports it). So you'll save ROM on all Cortex-Ms.
There was a problem hiding this comment.
Good call. I've added this in my most recent commit.
platform/mbed_application.c
Outdated
| @@ -170,7 +173,7 @@ __asm static void start_new_application(void *sp, void *pc) | |||
| void start_new_application(void *sp, void *pc) | |||
| { | |||
| __asm volatile( | |||
| "movw r2, #0 \n" // Fail to compile "mov r2, #0" with ARMC6. Replace with MOVW. | |||
| "movs r2, #0 \n" | |||
There was a problem hiding this comment.
Glad that worked. (It's so fiddly getting inline assembler to go smoothly across all toolchains - especially as GCC refuses to use unified syntax for Thumb-1 devices. Actually a bit surprised it is accepting MOVS...)
The MOVT comment is no longer relevant with the MOVW gone.
sarahmarshy
force-pushed
the
m0-start-app
branch
from
8c2a229 to
acf6158
Compare
sarahmarshy
force-pushed
the
m0-start-app
branch
from
acf6158 to
3b98ebc
Compare
|
started CI |
Test run: SUCCESSSummary: 12 of 12 test jobs passed |
|
@0xc0170 does this need any other reviews before merge? |
Description
Add support for
mbed_start_applicationfor Cortex M0+ targets. This will enable M0+ targets to support bootloader applications.Pull request type
Reviewers
@c1728p9