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

config: Add RAM1 memory region parameters to mbed_config.cmake #307

Closed
wants to merge 2 commits into from

Conversation

rajkan01
Copy link
Contributor

@rajkan01 rajkan01 commented Aug 3, 2021

Description

Fixes: #306
Add RAM1 memory region defines to mbed_config.tmpl as most of the targets
in targets.json has both RAM and RAM1 configurations

Test Coverage

  • This change is covered by existing or additional automated tests.
  • Manual testing has been performed (and evidence provided) as automated testing was not feasible.
  • Additional tests are not required for this change (e.g. documentation update).

@codecov
Copy link

codecov bot commented Aug 3, 2021

Codecov Report

Merging #307 (ab4a1bc) into master (ec83fe0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #307   +/-   ##
=======================================
  Coverage   97.08%   97.08%           
=======================================
  Files          92       92           
  Lines        2776     2776           
=======================================
  Hits         2695     2695           
  Misses         81       81           

@rajkan01 rajkan01 force-pushed the update_mem_region_config_tmpl branch from a7e26cd to 06537db Compare August 3, 2021 13:26
Add ARM1 memory region defines to mbed_config.tmpl as most of the
targets in targets.json has both RAM and RAM1 configurations
@rajkan01 rajkan01 force-pushed the update_mem_region_config_tmpl branch from 06537db to ab4a1bc Compare August 3, 2021 16:48
@@ -66,6 +66,12 @@ set(MBED_TARGET_DEFINITIONS{% for component in components %}
{%- endif %}
{% if mbed_ram_size is defined %}
MBED_RAM_SIZE={{ mbed_ram_size | to_hex }}
{%- endif %}
{% if mbed_ram1_start is defined %}
MBED_RAM1_START={{ mbed_ram1_start | to_hex }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to also support MBED_RAM_2..MBED_RAM_N? https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L443xC/TOOLCHAIN_ARM/stm32l443xc.sct#L50 seems to want MBED_RAM2_* at least.

Copy link
Contributor

@LDong-Arm LDong-Arm Aug 11, 2021

Choose a reason for hiding this comment

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

Related question: I always wonder whether the naming/numbering rules are documented or everyone happens to use the same convention. How is Mbed CLI 1 handling this? (ignore this)

Copy link
Contributor

@LDong-Arm LDong-Arm Aug 11, 2021

Choose a reason for hiding this comment

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

@rwalton-arm Maybe it depends on whether we ever define mbed_ram2_start in targets.json?

Searching Mbed CLI 1 scripts (in mbed-os/tools/), there's only mbed_ram_start, not even ram1. So I wonder how (or if at all) MBED_RAM1_START is set by Mbed CLI 1. Maybe it's not set, and linker scripts fall back to #if !defined( and use hardcoded values for linking? (Worth checking)

@Patater Patater closed this Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add RAM1 configuration in mbed_config.tmpl
4 participants