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: centralized startup code for cortexm CPUs #3155

Merged
merged 16 commits into from
Jun 16, 2015

Conversation

haukepetersen
Copy link
Contributor

The startup code and exception handlers for cortexm based CPUs were pretty much identical, so centralizing them makes much sense.

@gebart, @jfischer-phytec-iot: I was not blunt enough to touch the kinetis code (as it still differs quite a bit from how we implemented all other platforms). Would you care to have a look at this as a follow-up?

Note: github does a shitty job on showing the actual diffs here. The changes are not as bad as it seems...

@haukepetersen haukepetersen added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation 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 labels Jun 3, 2015
@haukepetersen haukepetersen added this to the Release 2015.06 milestone Jun 3, 2015
#endif

/**
* @brief Use this macro to make interrupt functions over writable with the
Copy link
Member

Choose a reason for hiding this comment

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

s/over writable/overridable/

@jnohlgard
Copy link
Member

@haukepetersen I will try to port this to k60, and possibly kw2x during next week, if time permits.

@haukepetersen
Copy link
Contributor Author

fixed spelling

@jfischer-no
Copy link
Contributor

@haukepetersen @gebart I was so free and have adapted it to kinetis. @haukepetersen Can I open a PR against your Branch?

@jfischer-no
Copy link
Contributor

@haukepetersen I was impatient and did it haukepetersen#11. @gebart Can you review and test it on mulle it please?

@haukepetersen
Copy link
Contributor Author

@jfischer-phytec-iot: looking great, merged!

@jnohlgard
Copy link
Member

I get a hard fault at the end of pre_startup on k60. The return pointer gets overwritten by the canary value and popped. The original implementation inlined the call so that there was no pop $pc

Edit: The old implementation was a noreturn and jumped to the reset handler after pre_reset

@jnohlgard
Copy link
Member

You could delete the stack canary filling, or move it to Cortex M common.

The purpose of it is to fill the ISR stack with a known value so that we can measure ISR stack usage by looking through the memory.

@jfischer-no
Copy link
Contributor

Damn, I miss it, of course it raises hardfault.

@jfischer-no
Copy link
Contributor

@gebart @haukepetersen corrected: haukepetersen#12

@jnohlgard
Copy link
Member

jnohlgard commented Jun 4, 2015 via email

@haukepetersen
Copy link
Contributor Author

@jfischer-phytec-iot: I merged your fix without really looking into it too much. I will look over this complete PR in detail once more on Monday.

@haukepetersen
Copy link
Contributor Author

Ok, as far as I see it, the changes look good and this PR should be ready for merge. Should I squash and start Travis?

@jfischer-no
Copy link
Contributor

@haukepetersen do it :-)

@haukepetersen haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 12, 2015
@haukepetersen
Copy link
Contributor Author

did, ready for Travis :-)

@jnohlgard
Copy link
Member

cpu/cortexm_common/vectors_cortexm.c:74: trailing whitespace.
ERROR: This change introduces new whitespace errors

@haukepetersen
Copy link
Contributor Author

fixed whitespace issue and rebased.

@jfischer-no
Copy link
Contributor

I guess we can merge it ...

@haukepetersen
Copy link
Contributor Author

just needs a little rebase as #3095 was finally merged. Doing this now.

@haukepetersen
Copy link
Contributor Author

rebased, hopefully I didn't break anything... So lets wait for Travis again and merge once he is happy?

- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common startup code
- make use of common exception handlers
- renamed startup.c to vectors.c
- make use of common cortexm isr vectors
- use common cortexm startup code
- renamed startup.c to vectors.c
@jfischer-no
Copy link
Contributor

Travis happy, go

@haukepetersen
Copy link
Contributor Author

yeah!

haukepetersen added a commit that referenced this pull request Jun 16, 2015
cpu: centralized startup code for cortexm CPUs
@haukepetersen haukepetersen merged commit 765c013 into RIOT-OS:master Jun 16, 2015
@haukepetersen haukepetersen deleted the opt_cortex_startup branch June 16, 2015 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 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