Skip to content

Support mbed_start_application for Cortex-M23 #6949

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

Merged
merged 4 commits into from
Jun 7, 2018
Merged

Support mbed_start_application for Cortex-M23 #6949

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b3f17a9
Support mbed_start_application for Cortex-M23
ccli8 May 17, 2018
646f614
Fix start_new_application compile error on Cortex-M23
ccli8 May 21, 2018
3c14cb6
Fix mbed_start_application on Cortex-M23
ccli8 May 28, 2018
23dcd82
Remove superfluous MOVT in mbed_start_application/start_new_application
ccli8 May 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion platform/mbed_application.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,20 @@ static void powerdown_nvic()
int i;
int j;

#if defined(__CORTEX_M23)
isr_groups_32 = 16;
Copy link
Contributor

Choose a reason for hiding this comment

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

16 is not a good number... and more than even a M33 could support. The max number of external interrupts a M23 can support is 240. It's sad that M23 doesn't have the ICTR so the max value we could use here is 7 (or 8... depending on how it's used elsewhere). Rule SGCR in v8-M ARM states: "When a particular NVIC interrupt line is not implemented, the registers that are associated with it are reserved." Yet Rule GLNG states "Privileged accesses to unimplemented registers are RES0" so accessing non-existing registers should be harmless but are wasteful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TacoGrandeTX Thanks for your detailed information. I fix it in 3c14cb6.

#else
isr_groups_32 = ((SCnSCB->ICTR & SCnSCB_ICTR_INTLINESNUM_Msk) >> SCnSCB_ICTR_INTLINESNUM_Pos) + 1;
#endif
for (i = 0; i < isr_groups_32; i++) {
NVIC->ICER[i] = 0xFFFFFFFF;
NVIC->ICPR[i] = 0xFFFFFFFF;
for (j = 0; j < 8; j++) {
#if defined(__CORTEX_M23)
NVIC->IPR[i * 8 + j] = 0x00000000;
#else
NVIC->IP[i * 8 + j] = 0x00000000;
#endif
}
}
}
Expand All @@ -69,17 +77,20 @@ static void powerdown_scb(uint32_t vtor)
SCB->SCR = 0x00000000;
// SCB->CCR - Implementation defined value
for (i = 0; i < 12; i++) {
#if defined(__CORTEX_M7)
#if defined(__CORTEX_M7) || defined(__CORTEX_M23)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not good - M23 has only 2 SHPR registers (the first is reserved). Looks like we have mixed up the size of our register definitions... core_cm23.h is using uint32_t (x2) while core_cm7.h is using uint8_t (x12). We need two for loops or should change SCB_Type in core_cm23.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.

@TacoGrandeTX I fix it in 3c14cb6.

SCB->SHPR[i] = 0x00;
#else
SCB->SHP[i] = 0x00;
#endif
}
SCB->SHCSR = 0x00000000;
#if defined(__CORTEX_M23)
#else
SCB->CFSR = 0xFFFFFFFF;
SCB->HFSR = SCB_HFSR_DEBUGEVT_Msk | SCB_HFSR_FORCED_Msk | SCB_HFSR_VECTTBL_Msk;
SCB->DFSR = SCB_DFSR_EXTERNAL_Msk | SCB_DFSR_VCATCH_Msk |
SCB_DFSR_DWTTRAP_Msk | SCB_DFSR_BKPT_Msk | SCB_DFSR_HALTED_Msk;
#endif
// SCB->MMFAR - Implementation defined value
// SCB->BFAR - Implementation defined value
// SCB->AFSR - Implementation defined value
Expand All @@ -106,6 +117,18 @@ __asm static void start_new_application(void *sp, void *pc)

void start_new_application(void *sp, void *pc)
{
#if defined(__CORTEX_M23)
Copy link
Contributor

Choose a reason for hiding this comment

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

This two-instruction sequence is not required... "mov r2, #0" will put 0 (zero-extended) into r2 on the M23. We can remove the entire #if block. MOVW/MOVT is useful for loading a full 32-value into a register without a literal pool... not the case here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry - missed the commit comments! Clearly AC6 is not accepting the inline assembly properly (I ran a local test) so perhaps we could use "movw r2, #0" for both conditions and still remove the #if block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TacoGrandeTX I replace with movw/movt and remove #if block in 3c14cb6.

__asm volatile (
"ldr r2, =0 \n"
"msr control, r2 \n" // Switch to main stack
"mov sp, %0 \n"
"msr primask, r2 \n" // Enable interrupts
"bx %1 \n"
:
: "l" (sp), "l" (pc)
: "r2", "cc", "memory"
);
#else
__asm volatile (
"mov r2, #0 \n"
"msr control, r2 \n" // Switch to main stack
Expand All @@ -116,6 +139,7 @@ void start_new_application(void *sp, void *pc)
: "l" (sp), "l" (pc)
: "r2", "cc", "memory"
);
#endif
}

#else
Expand Down
3 changes: 2 additions & 1 deletion platform/mbed_application.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@

#include<stdint.h>

#if defined(__CORTEX_M3) || defined(__CORTEX_M4) || defined(__CORTEX_M7)
#if defined(__CORTEX_M3) || defined(__CORTEX_M4) || defined(__CORTEX_M7)\
|| defined(__CORTEX_M23)
#define MBED_APPLICATION_SUPPORT 1
#else
#define MBED_APPLICATION_SUPPORT 0
Expand Down