-
Notifications
You must be signed in to change notification settings - Fork 110
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
bump all deps except smoltcp #426
Conversation
While I'm at it, we should probably update the CI script as well:
Edit: The warning comes from |
@@ -30,7 +30,7 @@ rustdoc-args = ["--cfg", "docsrs"] | |||
fugit = "0.3.5" | |||
embedded-hal = { version = "0.2.6", features = ["unproven"] } | |||
embedded-dma = "0.2.0" | |||
cortex-m = "^0.7.4" | |||
cortex-m = { version = "^0.7.7", features = ["critical-section-single-core"] } |
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.
Should the feature only be enabled on the single core variants?
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.
I might be in over my head here...
From what I've managed to figure out is:
cortex_m::interrupt::free
is unsound(?)critical_section::with
is not unsound but needs to be implemented by either this crate or use the "default" provided by the critical-section-single-core feature in cortex-m
https://github.com/rust-embedded/cortex-m/blob/master/CHANGELOG.md states that
- Added critical-section-single-core feature which provides an implementation for the critical_section crate for single-core systems, based on disabling all interrupts. (Change incorrect link from I2C to I2S in mod.rs #447)
- interrupt::free no longer hands out a CriticalSection token because it is unsound on multi-core. Use critical_section::with instead. (Change incorrect link from I2C to I2S in mod.rs #447)
So to answer your question: Probably, in the future when this crate supports multiple cores. As long as we don't provide any way to run code on multiple cores in this crate, using the feature critical-section-single-core
should be fine(?).
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.
Thanks for investigating this @olback ! I'm ok with this solution for now since there's no explicit support for multiple cores yet. It's even possible a HAL isn't actually the right place to implement multicore support, but I think that is still uncertain.
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.
Overall looks good to me 👍 It looks like this actually gets us some extra duplicates (for example bitflags v2.0, whilst our dependencies are still using v1.x) but that they will presumably upgrade too in future.
bors r+ |
cargo outdated
cargo duplicates