Set sram0 to represent the physical RAM in nRF5340 and nRF9160 DKs #24637
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ioannisg
force-pushed
the
nrf5340_fix_sram0_node
branch
from
d69c512 to
4422251
Compare
ioannisg
changed the title
ioannisg
changed the title
ioannisg
commented
Apr 23, 2020
Member
Author
There was a problem hiding this comment.
Now the sentence is more "generic" (as long as we're using zephyr,sram for image ram, which we do in all ARM platforms AFAIK)
galak
requested changes
Apr 23, 2020
Contributor
galak
left a comment
galak
left a comment
There was a problem hiding this comment.
Please use reserved memory as per:
ioannisg
force-pushed
the
nrf5340_fix_sram0_node
branch
from
4422251 to
3a73b45
Compare
|
All checks passed. Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages. |
galak
requested changes
Apr 23, 2020
Contributor
galak
left a comment
galak
left a comment
There was a problem hiding this comment.
Looks good, outside of the question if you need to label memory nodes (which I'd ask be removed, unless there's a really good reason for it).
ioannisg
force-pushed
the
nrf5340_fix_sram0_node
branch
from
3a73b45 to
2a1979a
Compare
Member
Author
No I did not need the labels, that's right. What I wanted is some inline descriptive comments. |
galak
reviewed
Apr 23, 2020
galak
requested changes
Apr 23, 2020
Contributor
galak
left a comment
galak
left a comment
There was a problem hiding this comment.
still a few labels in nrf9160dk_nrf9160_common.dts
boards/arm/nrf5340pdk_nrf5340/nrf5340pdk_nrf5340_cpuapp_common.dts
Outdated
Show resolved
Hide resolved
ioannisg
force-pushed
the
nrf5340_fix_sram0_node
branch
from
2a1979a to
c629155
Compare
ioannisg
force-pushed
the
nrf5340_fix_sram0_node
branch
from
c629155 to
626b4a5
Compare
galak
approved these changes
Apr 24, 2020
galak
reviewed
Apr 24, 2020
boards/arm/nrf5340pdk_nrf5340/nrf5340pdk_nrf5340_shared_sram_planning_conf.dts
Outdated
Show resolved
Hide resolved
ioannisg
removed request for
Vudentz,
dcpleung,
finikorg,
jfischer-no,
jhedberg,
joerchan,
nategraff-sifive,
pfalcon,
ruuddw,
sathishkuttan,
thoh-ot,
vanwinkeljan,
vonhust,
wentongwu and
wopu-ot
Member
Author
|
needs #25009 |
We should not be using sram0 for image SRAM in nrf5340pdk. sram0 represents the physical SRAM and that one includes the shared memory between the two M33 CPUs on the SoC. We should not be re-sizing sram0 to account for the shared RAM; instead we would like to have sram0 representing the whole available SRAM. For that, we define a new memory node, sram0_image to represent the 'image' SRAM that is available for Zephyr on the board. sram0_image is the chosen image SRAM for default builds, i.e. when TrustZone is ignored (TRUSTED_EXECUTION_SECURE is not defined). Signed-off-by: Ioannis Glaropoulos <[email protected]>
We do not want sram0_shared to represent physical ram; this is just a portion of sram reserved for shared memory between Application and Network MCU. Therfore, we remove the 'mmio' compatible property and transform this node to a reserved-memory node definition, inside which we define the sram0_shared node along with its reg property. In addition we correct the documentation about the shared memory, stressing that it is placed after the image RAM of nrf5340 Application MCU (not after the secure SRAM). Signed-off-by: Ioannis Glaropoulos <[email protected]>
sram0 node is needed to hold the size of the total, physical SRAM available on nRF9160 SoC. We use sram0_s to represent the Secure image SRAM for nRF9160_dk builds. Signed-off-by: Håkon Øye Amundsen <[email protected]> Signed-off-by: Ioannis Glaropoulos <[email protected]>
We do not want sram0_ns and sram0_bsd to represent physical ram; these are just portions of sram reserved for the non-secure image and the bsd library, respectively. Thus we can remove the compatible property from these nodes. We also make use of 'reserved-memory' to represent the different memory partitions to be used by the nrf9160 builds. Signed-off-by: Ioannis Glaropoulos <[email protected]>
When we build Zephyr as Secure image on nRF340 Application MCU and nRF9160 SoC we would like to pass the information about the reserved memory area allocated to the Non-Secure images. The information may be needed to apply proper security configuration. We add a "chosen" node in board .dts file for this purpose. Signed-off-by: Ioannis Glaropoulos <[email protected]>
ioannisg
force-pushed
the
nrf5340_fix_sram0_node
branch
from
8ff9d22 to
3ffec9a
Compare
carlescufi
approved these changes
May 9, 2020
Member
carlescufi
left a comment
carlescufi
left a comment
There was a problem hiding this comment.
Seems to be using the commonly agreed mechanism now.
nashif
approved these changes
May 10, 2020
MaureenHelm
approved these changes
May 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We would like to have
sram0DT node represent the available SRAM memory on nrf9160 and nRF5340 SoCs. As such, sram0 shall not be resized in Board DT configuration.In DTS we define sram0 node as such, but in boards//nrf5340pdk_ and nrf9160dk_ we have been using sram0 to represent (also) the image RAM allocated to Zephyr builds. The consequence is that sram0 is resized to match the requirements of the specific build (security domain, inter-processor shared memory, etc.), so we do not have a DTS generated macro to represent the total SRAM.
In this PR we fix this problem by having sram0 always represent the physical SRAM.
For nRF5340 we introduce sram0_image, to represent the RAM available for Zephyr image(s), secure and non-secure.
This PR is also adding additional information regarding reserved memory areas in board DT, such as SRAM memory areas explicitly allocated to secure and non-secure Zephyr execution environment.