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.

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

Use SRAM2 32Kbytes on STM32L475 / L476 and L486 devices #6172

Merged
merged 8 commits into from
Feb 27, 2018

Conversation

adustm
Copy link
Member

@adustm adustm commented Feb 22, 2018

Description

STM32L475 STM32L476 and STM32L486 devices have 32kbytes of SRAM2 that is used only for interrupt vector at the moment.
With this PR, the stack is now located in SRAM2.
This gives more space in SRAM1 for variables and heap.
It is mandatory for heavy applications like IOT connections

This has already been pushed and merged for IAR toolchain. ARM Toolchain will come soon

Note that the mbed-os code for GCC_ARM is made for heap and stack in the same RAM area. I've rewritten __wrap__sbrk (code already available in mbed-os) and used a new define in targets.json : TWO_RAM_REGIONS

@ARMmbed/team-avnetsilica Please pay attention. I have modified your GCC_ARM linker file and added TWO_REGION_RAM in targets.json for SILICA_SENSOR_NODE. It compiles and link, but I can't test it.
If you don't want to benefit from it, you will have to remove this define and copy the previous GCC linker file in your own directory, instead of using ST' one.
(according to the use cases you are playing with this platform, I guess you would be happy to gain 32kbytes... up to you to decide)

@MarceloSalazar I'm sure you would like to use that...

kind regards
Armelle

@MarceloSalazar
Copy link

@0xc0170 please let's get it in asap! ;-)

#if (defined(TARGET_STM32F051R8) ||\
#if (defined(TARGET_STM32L475VG))
/* only GCC_ARM and IAR toolchain have the stack on SRAM2 */
#if (defined(TOOLCHAIN_GCC_ARM) || defined(TOOLCHAIN_GCC_CR) || defined(__IAR_SYSTEMS_ICC__ ))
Copy link
Contributor

Choose a reason for hiding this comment

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

I would expect to have also && defined(TWO_RAM_REGIONS) on this line ?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

******************************************************************************
*/
#if defined(TWO_RAM_REGIONS)
#if defined(TOOLCHAIN_GCC_ARM) || defined(TOOLCHAIN_GCC_CR)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a a remark, in Cube we use __CC_ARM for ARM, __GNUC__ for GCC and __ICCARM__ for IAR. I don't know which macros are the best to use ?

Copy link
Contributor

@0xc0170 0xc0170 Feb 23, 2018

Choose a reason for hiding this comment

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

Just a a remark, in Cube we use __CC_ARM for ARM, GNUC for GCC and ICCARM for IAR. I don't know which macros are the best to use ?

+1, we should use those that come from the toolchains (be careful that GNUC is defined for ARMCC in mbed as we define gnu extension there)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I will (but I took those from platform/mbed_retarget.cpp, so both defines exist in mbed...)
The multiple use of GNUC was a problem for me, that's why I did not use it.

@adustm
Copy link
Member Author

adustm commented Feb 23, 2018

@0xc0170 @bcostm I applied the requested changes. Thanks for the review

@@ -1053,12 +1053,13 @@ extern "C" uint32_t __HeapLimit;
extern "C" int errno;

// Dynamic memory allocation related syscall.
#if defined(TARGET_NUVOTON)
#if (defined(TARGET_NUVOTON) || defined(TWO_RAM_REGIONS))
Copy link
Contributor

Choose a reason for hiding this comment

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

cleaner would be nuvoton would also define two ram regions so we would have just one config and no target here

Copy link
Member Author

Choose a reason for hiding this comment

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

0xc0170 you are right.
I suggest we create an issue in mbed-os so that NUVOTON can use the same define once this PR is merged. What do you think ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, can be done separately, would be nice o t have a tracking issue so we do not forget, thanks

@@ -1552,7 +1552,7 @@
}
},
"detect_code": ["0765"],
"macros_add": ["USBHOST_OTHER"],
"macros_add": ["USBHOST_OTHER", "TWO_RAM_REGIONS"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is fine here but instead of macro, if this is a config that would result in MBED_CONF_MULTIPLE_RAM_REGIONS (if there are 3 , would it matter? if yes, then two is fine in the name) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@0xc0170 the 'TWO' means that heap and stack are located in 2 separate ram regions.
It won't matter if you have 3 ram regions on the MCU. Only 2 will be concerned by this change.

@adustm
Copy link
Member Author

adustm commented Feb 23, 2018

Hello, I can see 'continuous-integration/jenkins/pr-head — This commit cannot be built'
Could you explain me what is failing ?

Kind regards

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 23, 2018

Hello, I can see 'continuous-integration/jenkins/pr-head — This commit cannot be built'

This is the error from jenkins: mbed_retarget.cpp@1069,21: conflicting declaration 'uint32_t __end__'

@adustm
Copy link
Member Author

adustm commented Feb 23, 2018

This is the error from jenkins: mbed_retarget.cpp@1069,21: conflicting declaration 'uint32_t end'

What target and toolchain, please ? Do you have any idea why it does not fail the same for Nuvoton platform (PFM_M453 or PFM_NUC472) ?

@adustm
Copy link
Member Author

adustm commented Feb 23, 2018

Hi @0xc0170
I will fix the jenkins issue, but be aware that Mbed already contains the conflicting declarations

/c/.../mbed-os  (l4_linkergcc)$ git grep __end__ | grep uint32_t
features/FEATURE_UVISOR/source/rtx/unsupported_malloc.c:extern uint32_t __end__[];      /* __heap_start */
features/FEATURE_UVISOR/source/rtx/unsupported_malloc.c:                          (uint32_t) __end__);
platform/mbed_retarget.cpp:extern "C" uint32_t __end__;
rtos/TARGET_CORTEX/mbed_boot.c:        extern uint32_t         __end__[];
targets/TARGET_NUVOTON/mbed_rtx.h:    extern uint32_t               __end__[];
targets/TARGET_RENESAS/mbed_rtx.h:    extern uint32_t               __end__;
targets/TARGET_STM/mbed_rtx.h:    extern uint32_t               __end__[];]; 

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 26, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Feb 26, 2018

Build : SUCCESS

Build number : 1254
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6172/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Feb 26, 2018

@mbed-ci
Copy link

mbed-ci commented Feb 26, 2018

defined(TARGET_STM32L476VG) ||\
defined(TARGET_STM32L486RG))
/* only GCC_ARM and IAR toolchains have the stack on SRAM2 */
#if (((defined(__GNUC__) && !defined(__CC_ARM)) ||\
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not for ARMCC? only those 2 toolchains are here supported?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hello @0xc0170
IAR was done in #5844
This PR is for GCC_ARM (required for Cloud example).
ARM will come soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume will be created as a new PR that will address ARMCC support for this. I would like to avoid having different behavior between toolchains

Copy link
Member Author

Choose a reason for hiding this comment

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

We will do it .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants