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

Make Send+Sync impls optional for compatibility with AVR #70

Closed
wants to merge 6 commits into from

Conversation

agausmann
Copy link

@agausmann agausmann commented May 18, 2021

The AVR architecture doesn't have atomic operations, but the chips are generally single-core anyway so thread safety isn't necessary.

This crate can't be used on AVR in official Rust yet, since this crate uses dyn / trait objects (rust-lang/rust#79889). But there is a pending fix, and I'm testing it out with my own patched version of Rust/LLVM.

agausmann added 6 commits May 17, 2021 21:42
In practice, making `sync` a default feature makes AVR compatibility a
pain, as all the other crates extending this core crate (such as `usbd-hid`)
would have to explicitly opt out of the default feature.
@ianrrees
Copy link
Contributor

Hi @agausmann , it's cool to see Rust coming to AVR! The thread safety actually is still necessary on single-core chips, because interrupts provide additional execution contexts. The ATSAMD21 for instance also lacks a compare-and-swap (CAS) instruction, the usb-device implementation in the HAL uses an interrupt-free block (here) to implement a "mutex".

However, that approach can be problematic, as I ramble on about in this issue.

@Rahix
Copy link

Rahix commented Jul 11, 2021

@agausmann, you can make "atomics" work on AVR by setting

	"max-atomic-width": 16,

in your rustc-target JSON. This works because there are intrinsics that implement the atomic operations by disabling interrupts for their duration. However, I am not certain that they are always compiled correctly at this time.

@agausmann
Copy link
Author

Sorry, I meant to respond to this a long time ago. Just want to clear some things up:

The thread safety actually is still necessary on single-core chips, because interrupts provide additional execution contexts.

Okay, so my comment about "chips are single-core anyway" wasn't accurate in hindsight. Don't even know why I wrote that.

What I should have said: The motivation for this is to use (or at least try to use) usb-device only on the main execution thread without accessing it in ISRs; I've had success with that approach on other MCUs, and I wanted to try that on AVR. This PR was a potential solution for that, I wanted to opt-in to !Send + !Sync to work around the lack of atomics in AVR. (also thanks @Rahix for helping with that)

@agausmann
Copy link
Author

No longer interested in this

@agausmann agausmann closed this Aug 1, 2022
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.

3 participants