Skip to content

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jun 25, 2023

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

@github-actions github-actions bot added Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Jun 25, 2023
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Jun 25, 2023
@gschorcht gschorcht requested a review from maribu June 25, 2023 15:50
@gschorcht gschorcht added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: no fast fail don't abort PR build after first error labels Jun 25, 2023
@riot-ci
Copy link

riot-ci commented Jun 25, 2023

Murdock results

✔️ PASSED

87a9d63 cpu/esp32: ensure correct RAM_START_ADDR and RAM_LEN

Success Failures Total Runtime
6934 0 6934 10m:25s

Artifacts

@gschorcht
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 25, 2023
@bors
Copy link
Contributor

bors bot commented Jun 25, 2023

try

Build failed:

@gschorcht gschorcht force-pushed the cpu/esp32/define_ram_length branch from 65528ce to 87a9d63 Compare June 25, 2023 16:13
@gschorcht
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 25, 2023
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Looks good and I really like the assert for checking the defines in C match the linked firmware

@bors
Copy link
Contributor

bors bot commented Jun 25, 2023

try

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.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@maribu
Copy link
Member

maribu commented Jun 26, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 26, 2023

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.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit ca72d8b into RIOT-OS:master Jun 26, 2023
@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
@gschorcht gschorcht deleted the cpu/esp32/define_ram_length branch September 10, 2023 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: no fast fail don't abort PR build after first error CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants