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

dualcore: Basic dual core support. #37

Merged
merged 5 commits into from
Dec 8, 2019

Conversation

richardeoin
Copy link
Member

Only the Cortex-M7 core is currently supported upstream

Only the Cortex-M7 core is currently supported upstream
@richardeoin richardeoin added the stm32rs-next Waiting on next stm32-rs release label Aug 29, 2019
@richardeoin
Copy link
Member Author

bors try

@bors
Copy link
Contributor

bors bot commented Nov 17, 2019

try

Merge conflict

@richardeoin
Copy link
Member Author

Ready for review

Note that the parts in CI have been trimmed to only those explicitly listed as supported in the README

Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

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

Looks good to me. I didn't check it w.r.t. the ref manual though.

With "upstream" support existing only for the M7, do you mean the PAC?
What's the plan with the M4? Do you ultimately want to support that one from within this crate as well?
Do we want to tie into microamp?
I'm happy to have details/bugs sorted out later.

bors r+

bors bot added a commit that referenced this pull request Nov 22, 2019
37: dualcore: Basic dual core support. r=jordens a=richardeoin

Only the Cortex-M7 core is currently supported upstream

Co-authored-by: Richard Meadows <[email protected]>
@jordens
Copy link
Member

jordens commented Nov 22, 2019

Ah. You wanted more reviews I guess. Feel free to have bors merge it when @hargoniX approves.

bors cancel

@jordens
Copy link
Member

jordens commented Nov 22, 2019

bors r-
bors d+

@bors
Copy link
Contributor

bors bot commented Nov 22, 2019

✌️ richardeoin can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@bors
Copy link
Contributor

bors bot commented Nov 22, 2019

Canceled

@richardeoin
Copy link
Member Author

More eyes is better if possible.

Yes, "upstream" refers to the PAC. M4 support starts there, because it's derived from a different SVD. Eventually it'd be great to try it with microamp - I haven't done that yet though.

@richardeoin
Copy link
Member Author

^^ found in my own review

@richardeoin
Copy link
Member Author

Okay, merging this now. There's no support for the Cortex-M4 core yet, although some feature gates exist for it already. This needs more consideration in the PAC first.

@richardeoin richardeoin merged commit 6056fe5 into stm32-rs:master Dec 8, 2019
@richardeoin richardeoin deleted the dualcore-1 branch December 8, 2019 17:39
mtthw-meyer pushed a commit to mtthw-meyer/stm32h7xx-hal that referenced this pull request Jul 9, 2020
dualcore: Basic dual core support.
@noppej
Copy link
Contributor

noppej commented Nov 12, 2020

@richardeoin , @jordens ... any more thoughts/plans on supporting the second core on the dual-core devices? If there is a specific body of work to be done or problem to be solved, I'd be willing to help.

@richardeoin
Copy link
Member Author

richardeoin commented Nov 12, 2020

Hi @noppej ! Other than exporting the correct feature flag from the PAC, in principle there's nothing preventing the HAL from being used on the CM4 core already. The downside of that approach is that you end up with two separate crates (one for each core) and there's nothing to prevent data races between them. The other approach is using something like the microamp crate, which I've not investigated at all.

So here are some areas that would help:

  • Exporting that feature flag. Have a look at Minimal requirements for targeting the Cortex-M4 core #62, which does this but is very behind the current master branch. Using the cm4 feature flag doesn't actually matter, except from matching some of the interrupt names with the RM.
  • Adding documentation and examples to help new users get started with the CM4 core. That would cover setting the memory regions in memory.x, and whatever needs to be done to the BOOT_CM4 / BOOT_CM4_ADDx bits to make it work.
  • Research using microamp, which already supports homogeneous cores. Likely this would result in a separate example crate, which we could link to in the README.

@noppej
Copy link
Contributor

noppej commented Nov 12, 2020

Hi @richardeoin. Thanks for your quick response. A couple of thoughts ...

  • Minimal requirements for targeting the Cortex-M4 core #62 is probably a good start, and if we can somehow create two bins in the same crate, with a build.rs script to manage the memory.x file, then it might be sufficient for someone (like me) to build a working app, where the developer manually manages the data races - not really in the spirit of what rust stands for, but it might be enough to build some tests, examples, documentation, etc. That said, it feels like we might be re-inventing the wheel that was started by the microamp team.
  • I've spent the last few days looking into microamp and also into rtic. From what I can tell, someone on the rtic made a solid effort at leveraging microamp to provide a framework for safely managing the inter-core apps. There are at least two challenges I can see with this ...
    -- As far as I can tell there hasn't been much activity on the microamp repository these past few months. Not sure what that means.
    -- I also discovered that the rtic team has recently decided to remove all the multi-core work - look at #27

In summary, I think it is probably worth a conversation with someone on the rtic and microamp teams, and shake out where the various parts of a safe multi-core implementation should land. Thoughts?

@richardeoin
Copy link
Member Author

Agreed that it'd be better not to re-invent multi-core build infrastructure here, especially as many of the parts supported by this crate are only single-core. We have a few rtic examples already and it works very well, so more of that would be great.

I'd try asking in the #rust-embedded room, it's usually a good place to ask about rtic especially.

It's probably worth continuing this discussion in a new Issue/PR also, this one is a bit old.

@noppej
Copy link
Contributor

noppej commented Nov 12, 2020

Thanks @richardeoin ... I appreciate the guidance. I will start the conversation in #rust-embedded room, and open a new Issue here as appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stm32rs-next Waiting on next stm32-rs release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants