-
Notifications
You must be signed in to change notification settings - Fork 2k
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
buildsystem: Always expose CPU_RAM_BASE & SIZE flags #19746
Conversation
bors merge |
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
This causes
in the CI :-/ |
It happens for ESP32x because |
Hm, the problem with ESP32x SoC is that they don't have that only one RAM but a number of SRAMs of different sizes, different byte/word access requirements and mapped into different address spaces. Additionally, parts of these SRAMS are used by different hardware components like cache, BT, ... Just for example an ESP32 has 520 kB SRAM:
The
That is, only SRAM2 is used as RAM. So we could use It becomes even more difficult for ESP32-S3. Here the
|
At least for I expect those only to be used in CPU specific drivers such as memory protection units, DMA drivers, etc. Those will likely have a good understanding of the memory layout and only make use of the macros if they make sense in the context of the platform. I think it is also fine for platforms that have a more complex memory layout to just not provide these macros. It is better to have no info, than an oversimplification that is misleading. (But when those macros do match the actual memory layout, I think it is absoutly fine to expose them.) |
19763: cpu/esp32: define RAM_START_ADDR and RAM_LEN r=maribu a=gschorcht ### Contribution description This PR fixes the problem ``` /bin/sh: 1: arithmetic expression: expecting primary: "" ``` for ESP32x SoCs that was introduced with PR #19746. The reason for the error message was that `RAM_LEN` was not defined for ESP32x SoCs. The solution is a bit tricky since ESP32x SoCs use a combination of SRAMs of different sizes and with different byte/word access requirements. Additionally, several hardware components such as the instruction cache or the Bluetooth controller share the RAM so that the start address and the size that is usable may differ depending on the hardware components used and configured parameters like the cache size a.s.o. Therefore, the DRAM region parameters as defined in the memory layout of the linker scripts are used to define `RAM_START_ADDR` and `RAM_LEN` in `cpu/esp32/Makefile.include`. Some checks have been added to the linker scripts to ensure that the same parameters are used in the linker scripts and for `RAM_LEN` and `RAM_START_ADDR`. This is to ensure that none of the parameters are changed without generating an assertion. **Note:** Since I don't know for what other purposes than the `riotboot` module these parameters might be relevant, I'm not sure if the values represent what they are supposed to. ### Testing procedure Green CI with full compilation ### Issues/PRs references Fixes PR #19746 Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Contribution description
Hello 🐧
This moves the definition of
CPU_RAM_BASE/SIZE
from being only available in certain situation to be always available.Reason for change is to simplify common code in the cpu folder.
In cooperation with @benpicco
Testing procedure
Passing CI
Issues/PRs references
First usage will be in the PMP driver. Although there is more code in RIOT that could be refactored to use these defines instead of hacks / hardcoded values.