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.

Sign up for GitHub

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

Jump to bottom

linker: Generate snippets files for dtcm and itcm #66254

Merged
merged 2 commits into from
Jan 25, 2024
Merged

linker: Generate snippets files for dtcm and itcm #66254

merged 2 commits into from
Jan 25, 2024

Conversation

raffi-g
Copy link
Contributor

@raffi-g raffi-g commented Dec 7, 2023

This PR allows to link code and data blocks, e.g. the vector table, into the tightly coupled memory (itcm and dtcm, resp. ilm and dlm) using the cmake function zephyr_linker_sources.

@zephyrbot zephyrbot added area: ARM ARM (32-bit) Architecture area: RISCV RISCV Architecture (32-bit & 64-bit) area: Architectures area: Build System labels Dec 7, 2023
@raffi-g raffi-g mentioned this pull request Dec 14, 2023
Closed
@raffi-g
Copy link
Contributor Author

raffi-g commented Dec 20, 2023

any opinions, @fkokosinski or any of the other reviewers?

tejlmand
tejlmand requested changes Jan 4, 2024
View reviewed changes
include/zephyr/arch/arm/cortex_m/scripts/linker.ld Show resolved Hide resolved
cmake/modules/extensions.cmake Outdated Show resolved Hide resolved
cmake/modules/extensions.cmake Show resolved Hide resolved
@raffi-g raffi-g force-pushed the itcm-dtcm-linker-snippets branch 2 times, most recently from bcf088c to 6cb242c Compare January 9, 2024 15:51
@raffi-g
Copy link
Contributor Author

raffi-g commented Jan 11, 2024

hi @tejlmand
I think I've addressed all your points. Could you take another look?

tejlmand
tejlmand reviewed Jan 12, 2024
View reviewed changes
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

much better.
a small nit.

cmake/modules/extensions.cmake Outdated Show resolved Hide resolved
@raffi-g raffi-g force-pushed the itcm-dtcm-linker-snippets branch from 6cb242c to b016e50 Compare January 12, 2024 16:27
@fkokosinski
Copy link
Member

RISC-V changes LGTM. Not giving an ACK though because there are failed tests reported by the CI, although they are not reproducible for me, so I'm not sure if they're related to these changes.

@raffi-g
linker: Generate snippets files for dtcm and itcm
4fc9214 
This allows to link code and data blocks, e.g. the vector table, into
tightly coupled memory using `zephyr_linker_sources`.

Signed-off-by: Greter Raffael <rgreter@baumer.com>
@raffi-g
linker: Only use itcm and dtcm snippets if regions exist
850abfe 
This tests whether there actually is an itcm or dtcm in the device tree.
Otherwise a FATAL_ERROR is generated.

Signed-off-by: Greter Raffael <rgreter@baumer.com>
@raffi-g raffi-g force-pushed the itcm-dtcm-linker-snippets branch from b016e50 to 850abfe Compare January 15, 2024 14:06
@raffi-g
Copy link
Contributor Author

raffi-g commented Jan 15, 2024

RISC-V changes LGTM. Not giving an ACK though because there are failed tests reported by the CI, although they are not reproducible for me, so I'm not sure if they're related to these changes.

I can't reproduce it either. I guess it's not related to this PR. I've only changed the cmake error message since the last successful CI run.

@raffi-g
Copy link
Contributor Author

raffi-g commented Jan 16, 2024

@fkokosinski I've rebased the branch which triggered the CI again. Now it succeeded. So the failure was not caused by my changes. Could you approve the PR now?

@fkokosinski
Copy link
Member

@fkokosinski I've rebased the branch which triggered the CI again. Now it succeeded. So the failure was not caused by my changes. Could you approve the PR now?

Yup, thanks for the ping.

fkokosinski
fkokosinski approved these changes Jan 16, 2024
View reviewed changes
Copy link
Member

@fkokosinski fkokosinski left a comment

Choose a reason for hiding this comment

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

RISC-V changes LGTM.

@raffi-g raffi-g requested a review from tejlmand January 18, 2024 10:25
@raffi-g
Copy link
Contributor Author

raffi-g commented Jan 23, 2024

much better. a small nit.

@tejlmand could I bother you again? I've addressed your nit

@dleach02 dleach02 merged commit 81022fa into zephyrproject-rtos:main Jan 25, 2024
22 checks passed
@raffi-g raffi-g deleted the itcm-dtcm-linker-snippets branch January 25, 2024 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fkokosinski fkokosinski fkokosinski approved these changes

@tejlmand tejlmand tejlmand approved these changes

@57300 57300 Awaiting requested review from 57300

@carlocaione carlocaione Awaiting requested review from carlocaione

@dcpleung dcpleung Awaiting requested review from dcpleung

@edersondisouza edersondisouza Awaiting requested review from edersondisouza

@jeremybettis jeremybettis Awaiting requested review from jeremybettis

@katsuster katsuster Awaiting requested review from katsuster

@kgugala kgugala Awaiting requested review from kgugala

@mgielda mgielda Awaiting requested review from mgielda

@microbuilder microbuilder Awaiting requested review from microbuilder

@nashif nashif Awaiting requested review from nashif

@nordicjm nordicjm Awaiting requested review from nordicjm

@npitre npitre Awaiting requested review from npitre

@tgorochowik tgorochowik Awaiting requested review from tgorochowik

Assignees

@fkokosinski fkokosinski

Labels
area: Architectures area: ARM ARM (32-bit) Architecture area: Build System area: RISCV RISCV Architecture (32-bit & 64-bit)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@raffi-g @fkokosinski @tejlmand @zephyrbot @dleach02