Skip to content

Add framework for configuring boot stack size #8039

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 2 commits into from
Oct 8, 2018
Merged

Add framework for configuring boot stack size #8039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ead033
Add framework for configuring boot stack size
c1728p9 Sep 7, 2018
66cce67
Update the K64F to use boot stack size config
c1728p9 Sep 7, 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
19 changes: 18 additions & 1 deletion rtos/mbed_lib.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,22 @@
"config": {
"present": 1
},
"macros": ["_RTE_"]
"macros": ["_RTE_"],
"target_overrides": {
Copy link
Contributor

Choose a reason for hiding this comment

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

In targets/targets.json file boot-stack-size is set to 4kB and here is modified and set to 1kB with some exceptions (like for NRF boards).
Is this done to distinguish stack size for builds with and without RTOS (4kB for non RTOS and typically 1kB for builds with RTOS)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, by default stack sizes are 4K and they are overridden to 1K when the RTOS is present. Targets with special requirements on stack size, such as the nrf51, can further override for both the RTOS and non-RTOS configuration.

"*": {
"target.boot-stack-size": "0x400"
},
"MCU_NRF51": {
"target.boot-stack-size": "0x800"
Copy link

Choose a reason for hiding this comment

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

Will it be good to move these overrides in targets.json instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The size in targets.json is the size for mbed 2 - 0x1000, while rtos/mbed_lib.json is for mbed 5 - 0x400. In the case of the nrf5x the interrupt stack needs to be customized for mbed 5 only so it is set to 0x800 here.

},
"MCU_NRF52840": {
"target.boot-stack-size": "0x800"
},
"MCU_NRF52832": {
"target.boot-stack-size": "0x800"
},
"MCU_NRF51_UNIFIED": {
"target.boot-stack-size": "0x800"
}
}
}
8 changes: 7 additions & 1 deletion ...escale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/device/TOOLCHAIN_ARM_STD/MK64FN1M0xxx12.sct
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@
#define MBED_APP_SIZE 0x100000
#endif

#if !defined(MBED_BOOT_STACK_SIZE)
#define MBED_BOOT_STACK_SIZE 0x400
#endif

#define m_interrupts_start MBED_APP_START
#define m_interrupts_size 0x00000400

Expand All @@ -86,7 +90,7 @@
#if (defined(__stack_size__))
#define Stack_Size __stack_size__
#else
#define Stack_Size 0x0400
#define Stack_Size MBED_BOOT_STACK_SIZE
#endif

#if (defined(__heap_size__))
Expand Down Expand Up @@ -122,4 +126,6 @@ LR_IROM1 m_interrupts_start m_text_start+m_text_size-m_interrupts_start { ; lo
}
RW_IRAM1 ImageLimit(RW_m_data_2) { ; Heap region growing up
}
ARM_LIB_STACK m_data_2_start+m_data_2_size EMPTY -Stack_Size { ; Stack region growing down
}
}
18 changes: 11 additions & 7 deletions ...eescale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/device/TOOLCHAIN_GCC_ARM/MK64FN1M0xxx12.ld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ ENTRY(Reset_Handler)

__ram_vector_table__ = 1;

/* With the RTOS in use, this does not affect the main stack size. The size of
* the stack where main runs is determined via the RTOS. */
__stack_size__ = 0x400;

__heap_size__ = 0x6000;

#if !defined(MBED_APP_START)
#define MBED_APP_START 0
#endif
Expand All @@ -67,6 +61,16 @@ __heap_size__ = 0x6000;
#define MBED_APP_SIZE 0x100000
#endif

#if !defined(MBED_BOOT_STACK_SIZE)
#define MBED_BOOT_STACK_SIZE 0x400
#endif

/* With the RTOS in use, this does not affect the main stack size. The size of
* the stack where main runs is determined via the RTOS. */
__stack_size__ = MBED_BOOT_STACK_SIZE;

__heap_size__ = 0x6000;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like __heap_size__ and HEAP_SIZE symbols now are unused and can be removed.
It would be enough to define STACK_SIZE = MBED_BOOT_STACK_SIZE or even remove also STACK_SIZE and use MBED_BOOT_STACK_SIZE instead.


HEAP_SIZE = DEFINED(__heap_size__) ? __heap_size__ : 0x0400;
STACK_SIZE = DEFINED(__stack_size__) ? __stack_size__ : 0x0400;
M_VECTOR_RAM_SIZE = DEFINED(__ram_vector_table__) ? 0x0400 : 0x0;
Expand Down Expand Up @@ -259,7 +263,7 @@ SECTIONS
__end__ = .;
PROVIDE(end = .);
__HeapBase = .;
. += HEAP_SIZE;
. = ORIGIN(m_data_2) + LENGTH(m_data_2) - STACK_SIZE;
__HeapLimit = .;
__heap_limit = .; /* Add for _sbrk */
} > m_data_2
Expand Down
11 changes: 7 additions & 4 deletions ..._Freescale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/device/TOOLCHAIN_IAR/MK64FN1M0xxx12.icf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@
*/
define symbol __ram_vector_table__ = 1;

/* Heap 1/4 of ram and stack 1/8 */
define symbol __stack_size__=0x8000;
define symbol __heap_size__=0x10000;

if (!isdefinedsymbol(MBED_APP_START)) {
define symbol MBED_APP_START = 0;
}
Expand All @@ -61,6 +57,13 @@ if (!isdefinedsymbol(MBED_APP_SIZE)) {
define symbol MBED_APP_SIZE = 0x100000;
}

if (!isdefinedsymbol(MBED_BOOT_STACK_SIZE)) {
define symbol MBED_BOOT_STACK_SIZE = 0x400;
}

define symbol __stack_size__=MBED_BOOT_STACK_SIZE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theotherjimmy does the name MBED_BOOT_STACK_SIZE seems reasonable to you? Alternatively I could set this to the target define that is set - MBED_CONF_TARGET_BOOT_STACK_SIZE. Let me know what you think.

define symbol __heap_size__=0x10000;

define symbol __ram_vector_table_size__ = isdefinedsymbol(__ram_vector_table__) ? 0x00000400 : 0;
define symbol __ram_vector_table_offset__ = isdefinedsymbol(__ram_vector_table__) ? 0x000003FF : 0;

Expand Down
4 changes: 4 additions & 0 deletions targets/targets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
"network-default-interface-type": {
"help": "Default network interface type. Typical options: null, ETHERNET, WIFI, CELLULAR, MESH",
"value": null
},
"boot-stack-size": {
"help": "Define the boot stack size in bytes. This value must be a multiple of 8",
"value": "0x1000"
}
}
},
Expand Down
10 changes: 10 additions & 0 deletions tools/toolchains/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,12 +742,22 @@ def add_regions(self):
except ConfigException:
pass

def add_linker_defines(self):
stack_param = "target.boot-stack-size"
params, _ = self.config_data

if stack_param in params:
define_string = self.make_ld_define("MBED_BOOT_STACK_SIZE", int(params[stack_param].value, 0))
self.ld.append(define_string)
self.flags["ld"].append(define_string)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theotherjimmy this is how the target define is getting passed to the linker script. Is this the right way to do it or would you like any changes made here?

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks fine.


# Set the configuration data
def set_config_data(self, config_data):
self.config_data = config_data
# new configuration data can change labels, so clear the cache
self.labels = None
self.add_regions()
self.add_linker_defines()

# Creates the configuration header if needed:
# - if there is no configuration data, "mbed_config.h" is not create (or deleted if it exists).
Expand Down