Skip to content

Conversation

@bugadani
Copy link
Contributor

@bugadani bugadani commented Aug 17, 2023

Thank you!

Thank you for your contribution.
Please make sure that your submission includes the following:

Must

  • The code compiles without errors or warnings.
  • All examples work.
  • cargo fmt was run.
  • Your changes were added to the CHANGELOG.md in the proper section.
  • You updated existing examples or added examples (if applicable).
  • Added examples are checked in CI

Nice to have

  • You add a description of your work to this PR.
  • You added proper docs for your newly added features and code.

Closes #543

I've taken the liberty of removing some of the documentation that made no sense. There is no fine-grained control here, just start and pause/resume and only of the second core :)

This PR changes the second core's program to a FnOnce closure, and copies it to some arbitrary place (properly aligned address at the physical bottom of the stack) in the second core's stack before starting the core. I've changed the Stack's alignment based on what I've seen in the Xtensa ISA, and the code now asserts at compile time that the top of the stack is properly aligned (too).

I've also changed the Stack to be uninitialized as I don't think stack needs to be zeroed. Please do correct me if I'm wrong!

This PR allows the second core's function to return. In this case, the implementation just parks the core.

@bugadani bugadani force-pushed the fun branch 5 times, most recently from 634fa20 to a9dd9e9 Compare August 17, 2023 18:46
@bugadani bugadani marked this pull request as ready for review August 17, 2023 18:49
@bugadani bugadani force-pushed the fun branch 3 times, most recently from f4654dd to e89b17e Compare August 18, 2023 03:40
@bjoernQ
Copy link
Contributor

bjoernQ commented Aug 18, 2023

Nice! I wonder if we want to remove that AppCoreGuard thing? It's probably more annoying than it helps?

@bugadani
Copy link
Contributor Author

bugadani commented Aug 18, 2023

I was wondering if I can move the core parking method to that struct, anfld maybe give it an "is_runninh" but we can remove it for sure. It has some uses in holding on to lifetimes so not everything had to be 'static

@bugadani
Copy link
Contributor Author

Let me know if you want me to do anything about it, I'm personally fine with either option.

@bjoernQ
Copy link
Contributor

bjoernQ commented Aug 21, 2023

Let's see what other's opinions are on that - it also shouldn't block this PR. Can be done separately

@bjoernQ
Copy link
Contributor

bjoernQ commented Aug 22, 2023

Let's see what other's opinions are on that - it also shouldn't block this PR. Can be done separately

@jessebraham @MabezDev any opinion on removing or keeping the guard? otherwise this seems fine to merge I guess

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

Oops, forgot about this. I think we can leave the guard for now. I don't see a super-pressing reason to remove it right now.

This PR looks good to me though, thanks @bugadani for taking care of this!

@bjoernQ
Copy link
Contributor

bjoernQ commented Aug 22, 2023

Seems no one else complains ..... Will merge this now

@bjoernQ bjoernQ merged commit bf4efcf into esp-rs:main Aug 22, 2023
@bugadani bugadani deleted the fun branch August 22, 2023 14:36
playfulFence pushed a commit to playfulFence/esp-hal that referenced this pull request Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve ergonomics of running code on the second core of dual core chips

3 participants