-
Notifications
You must be signed in to change notification settings - Fork 177
Add pre-default-start-trap feature #337
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
Add pre-default-start-trap feature #337
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Could you include a sample example in the docs to illustrate how to implement the new routine?
I tried to come up with a more or less interesting and realistic example - but it might not be brief enough, let me know if I should reduce it to a minimum |
Usually, we just include an empty implementation with a comment like |
Makes sense - will change it 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! A few comments:
-
should the docs explicitly say that the new symbol must be in the .trap.start section?
-
currently, the new symbol is only used in direct mode and in exceptions with vectored mode. Do you need this to be included in the _continue_trap symbol for vectored interrupts?
Thanks for the quick reply!
It's probably worth to add a recommendation to avoid people running into linker errors later. I will add that 👍
For me at least it's only needed for exceptions (in vectored mode). I guess having it for everything in direct mode is fine but I can't think of a case where I need it for vectored interrupts. Should I note this in the docs? (i.e. that it won't get called for vectored interrupts) |
Sounds good to me. Let's document everything and leave it as it is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks
In esp-hal we offer the "flip-link" feature.
When SP moves below RAM an exception is raised but we need a way to check that situation and move SP into RAM again before further processing.
One way would be to define our own
_default_start_trap
- another way would be to have a way to execute custom code before_default_start_trap
which this PR implements.Not sure if this is the best way to do it but it's simple and should work good enough.
This should be the last missing step to use riscv-rt in esp-hal. (see esp-rs/esp-hal#2390)
Let me know if you want me to change anything