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

cpu: make use of cortexm.ld #18636

Merged
merged 4 commits into from
Sep 28, 2022
Merged

cpu: make use of cortexm.ld #18636

merged 4 commits into from
Sep 28, 2022

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Sep 23, 2022

Contribution description

Some MCU families carry their own linker script when they can just as well make use of the generic one.

Testing procedure

Binaries should not change.

Issues/PRs references

split off #18608

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Sep 23, 2022
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Sep 23, 2022
@benpicco benpicco requested a review from maribu September 23, 2022 13:57
@maribu
Copy link
Member

maribu commented Sep 23, 2022

looks good to me. I can test for nrf5x and sam, but not for other MCUs. I have the feeling that linker script changes are better tested before merging

@benpicco
Copy link
Contributor Author

I can confirm that cpu/lm4f120 (ek-lm4f120xl) still works - but really it should suffice to compare the binaries for this, there should be no change.

@maribu
Copy link
Member

maribu commented Sep 28, 2022

Indeed, if binaries don't change there is no sense in testing.

For testing I ran

git checkout <PR>
for board in ek-lm4f120xl mbed_lpc1768 nrf51dk nrf52840dk nrf9160dk arduino-due; do make RIOT_CI_BUILD=1 BOARD=$board -C examples/default -j; done
mv examples/default/bin ~/bin-pr
git checkout master
for board in ek-lm4f120xl mbed_lpc1768 nrf51dk nrf52840dk nrf9160dk arduino-due; do make RIOT_CI_BUILD=1 BOARD=$board -C examples/default -j; done
mv examples/default/bin ~/
for board in ek-lm4f120xl mbed_lpc1768 nrf51dk nrf52840dk nrf9160dk arduino-due; do elf_diff --bin_prefix arm-none-eabi- --html_file ~/$board.html ~/bin-master/$board/default.elf ~/bin-pr/$board/default.elf; done
for board in ek-lm4f120xl mbed_lpc1768 nrf51dk nrf52840dk nrf9160dk arduino-due; do xdg-open ~/$board.html; done

The resulted report shows that in any case the exact same set of symbols was linked in and that the size of the symbols didn't change. I cannot rule out that due to different memory layout still issues arise, but I don't think the chance for such issues is high enough to justify the effort of testing them all.

@benpicco benpicco merged commit 09fd98c into RIOT-OS:master Sep 28, 2022
@benpicco benpicco deleted the cpu/cortexm.ld branch September 28, 2022 12:41
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
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: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants