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

cpu/cc2538: eliminate cc2538.ld and use cortexm_base.ld instead #5425

Merged
merged 1 commit into from May 19, 2016
Merged

cpu/cc2538: eliminate cc2538.ld and use cortexm_base.ld instead #5425

merged 1 commit into from May 19, 2016

Conversation

hexluthor
Copy link
Contributor

Pretty self-explanatory. These linker scripts were identical except for the one additional flashcca section.

@PeterKietzmann PeterKietzmann added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation State: duplicate State: The issue/PR is a duplicate of another issue/PR Platform: ARM Platform: This PR/issue effects ARM-based platforms labels May 12, 2016
A-Paul
A-Paul reviewed May 12, 2016
View reviewed changes
cpu/cc2538/ldscripts/cc2538.ld
{
. = ALIGN(8);
_sstack = .;
. = . + STACK_SIZE;
Copy link
Member

Choose a reason for hiding this comment

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

Just to be safe. cortexm_base.ld uses KEEP (*(.isr_stack)) here. Are these two expressions really equivalent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because vectors_cortexm.c uses _sstack and _estack to determine the boundaries of the main stack. isr_stack is not used directly.

As an additional benefit, cc2538 will start using ISR_STACKSIZE like all the other Cortex-M CPUs instead of its own STACK_SIZE value set within the linker script. Presently they both default to 512 bytes.

@A-Paul
Copy link
Member

A-Paul commented May 18, 2016

Successful build and run tests/unittests on the board cc2538dk (cpu: cc2538nf53).
As no CPU specific code was changed I expect no problems for the rest of the "family".

@A-Paul A-Paul added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 18, 2016
@A-Paul
Copy link
Member

A-Paul commented May 18, 2016

@PeterKietzmann you did set the duplicate label. I somehow couldn't examine the other PR.
Any reasons not to merge this?

@PeterKietzmann PeterKietzmann removed the State: duplicate State: The issue/PR is a duplicate of another issue/PR label May 19, 2016
@PeterKietzmann
Copy link
Member

Sorry. Guess it was by accident...

@A-Paul
Copy link
Member

A-Paul commented May 19, 2016

@hexluthor, thank you for explanations. ACK and Murdock is green. Merging ...

@A-Paul A-Paul merged commit 9693c3c into RIOT-OS:master May 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@A-Paul A-Paul

Labels
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hexluthor @A-Paul @PeterKietzmann