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

Use critical-section 1.0 for locking. #28

Merged
merged 1 commit into from Mar 3, 2023
Merged

Conversation

Dirbaio
Copy link
Member

@Dirbaio Dirbaio commented Aug 10, 2022

This allows having no platform-specific code in rtt-target at all,
as long as the user imports the right critical-section implementation.

This is a breaking change.

TODO before merging:

This allows having no platform-specific code in rtt-target at all,
as long as the user imports the right critical-section implementation.
@Dirbaio
Copy link
Member Author

Dirbaio commented Aug 19, 2022

cortex-m 0.7.6 is released with critical-section support, i've removed the [patch.crates.io]. I think this should be ready to go :)

@jannic
Copy link

jannic commented Aug 19, 2022

Note that this is a semver breaking change, as it removes cargo features.
EDIT: Oops you already noted it in the description - I somehow missed it and only noticed it when some build failed.

@mvirkkunen
Copy link
Contributor

Neat, another annoying cortex-m feature bites the dust!

Breaking semver is fine because I've wanted to clean things up for a long time anyways, for example steal defmt's idea of initializing everything before main. With critical-section I can make that happen more easily. I'll see if I can make that happen within a weekend or two, and if I can't figure out a nice way, I'll release just this change as-is as a new version because it's a good addition in and of itself.

@mvirkkunen
Copy link
Contributor

mvirkkunen commented Aug 20, 2022

Oh, the "gotta use the crate or it gets pruned" problem is back again. The example binaries fail to link without a use cortex_m;. This has happened with other crates too, in that even if your own code doesn't actually use the crate, it still needs to be use'd. The examples could be a bit artificial in that they don't actually need anything from cortex-m though. I'll fix them before merging.

Just out of interest, do you know if there is a RISC-V implementation of critical-section? I think some people were using RTT on that too.

@jannic
Copy link

jannic commented Aug 20, 2022

Just out of interest, do you know if there is a RISC-V implementation of critical-section? I think some people were using RTT on that too.

In versions <1.0.0, some implementations were part of the critical-section crate, and one of them was for riscv.
With 1.0.0, the implementations were removed and should now be provided by platform-support crates. However, it seems like this was not done yet. Should not be difficult to add, at least for single-core, where disabling interrupts should be sufficient.

@Dirbaio
Copy link
Member Author

Dirbaio commented Aug 21, 2022

PR for riscv critical-section at rust-embedded/riscv#110

@Yatekii Yatekii enabled auto-merge March 3, 2023 21:18
@Yatekii Yatekii added this pull request to the merge queue Mar 3, 2023
Merged via the queue into probe-rs:master with commit b270980 Mar 3, 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.

None yet

4 participants