Skip to content

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

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ rusb = "0.8.0"
rand = "0.6.1"

[features]
# Enable support for using this library across multiple threads.
#
# Made optional / opt-in for compatibility with AVR targets
# which are single-core and do not have atomic operations.
sync = []

# Use a 256 byte buffer for control transfers instead of 128.
control-buffer-256 = []

Expand Down
83 changes: 79 additions & 4 deletions src/bus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,83 @@ use crate::{Result, UsbDirection, UsbError};
use core::cell::RefCell;
use core::mem;
use core::ptr;

#[cfg(feature = "sync")]
use core::sync::atomic::{AtomicPtr, Ordering};

#[cfg(not(feature = "sync"))]
use core::cell::Cell;

pub(crate) struct BusPtr<B> {
#[cfg(feature = "sync")]
inner: AtomicPtr<B>,

#[cfg(not(feature = "sync"))]
inner: Cell<*mut B>,
}

impl<B> BusPtr<B> {
fn new() -> Self {
Self {
#[cfg(feature = "sync")]
inner: AtomicPtr::new(ptr::null_mut()),

#[cfg(not(feature = "sync"))]
inner: Cell::new(ptr::null_mut()),
}
}

fn set(&self, ptr: *mut B) {
#[cfg(feature = "sync")]
{
self.inner.store(ptr, Ordering::SeqCst);
}
#[cfg(not(feature = "sync"))]
{
self.inner.set(ptr);
}
}

pub(crate) fn get(&self) -> *mut B {
#[cfg(feature = "sync")]
{
self.inner.load(Ordering::SeqCst)
}

#[cfg(not(feature = "sync"))]
{
self.inner.get()
}
}
}

/// A trait with a `Sync` bound if the `sync` feature is enabled.
///
/// This is necessary because bounds can't be made conditional based on feature flags. Otherwise,
/// the definition of `UsbBus` could be simpler, like:
///
/// ```compile_fail
/// pub trait UsbBus: #[cfg(feature = "sync")] Sync + Sized {
/// /* ... */
/// }
/// ```
//
// Another alternative would be to duplicate the `UsbBus` definition, one with the `Sync` bound and
// one without, and conditionally select them based on the `sync` feature. But that's not feasible
// to maintain by hand, because of how complex the `UsbBus` trait is. However, it may be possible
// to do with a macro to automatically generate both definitions from one body.
#[cfg(feature = "sync")]
pub trait ConditionalSync: Sync {}

#[cfg(feature = "sync")]
impl<T: Sync> ConditionalSync for T {}

#[cfg(not(feature = "sync"))]
pub trait ConditionalSync {}

#[cfg(not(feature = "sync"))]
impl<T> ConditionalSync for T {}

/// A trait for device-specific USB peripherals. Implement this to add support for a new hardware
/// platform.
///
Expand All @@ -14,7 +89,7 @@ use core::sync::atomic::{AtomicPtr, Ordering};
/// take place before [`enable`](UsbBus::enable) is called. After the bus is enabled, in practice
/// most access won't mutate the object itself but only endpoint-specific registers and buffers, the
/// access to which is mostly arbitrated by endpoint handles.
pub trait UsbBus: Sync + Sized {
pub trait UsbBus: ConditionalSync + Sized {
/// Allocates an endpoint and specified endpoint parameters. This method is called by the device
/// and class implementations to allocate endpoints, and can only be called before
/// [`enable`](UsbBus::enable) is called.
Expand Down Expand Up @@ -148,7 +223,7 @@ struct AllocatorState {
/// Helper type used for UsbBus resource allocation and initialization.
pub struct UsbBusAllocator<B: UsbBus> {
bus: RefCell<B>,
bus_ptr: AtomicPtr<B>,
bus_ptr: BusPtr<B>,
state: RefCell<AllocatorState>,
}

Expand All @@ -158,7 +233,7 @@ impl<B: UsbBus> UsbBusAllocator<B> {
pub fn new(bus: B) -> UsbBusAllocator<B> {
UsbBusAllocator {
bus: RefCell::new(bus),
bus_ptr: AtomicPtr::new(ptr::null_mut()),
bus_ptr: BusPtr::new(),
state: RefCell::new(AllocatorState {
next_interface_number: 0,
next_string_index: 4,
Expand All @@ -179,7 +254,7 @@ impl<B: UsbBus> UsbBusAllocator<B> {
// in the RefCell.
let mut bus_ref = self.bus.borrow_mut();
let bus_ptr_v = &mut *bus_ref as *mut B;
self.bus_ptr.store(bus_ptr_v, Ordering::SeqCst);
self.bus_ptr.set(bus_ptr_v);

// And then leave the RefCell borrowed permanently so that it cannot be borrowed mutably
// anymore.
Expand Down
9 changes: 4 additions & 5 deletions src/endpoint.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::bus::UsbBus;
use crate::bus::{BusPtr, UsbBus};
use crate::{Result, UsbDirection};
use core::marker::PhantomData;
use core::ptr;
use core::sync::atomic::{AtomicPtr, Ordering};

/// Trait for endpoint type markers.
pub trait EndpointDirection {
Expand Down Expand Up @@ -49,7 +48,7 @@ pub enum EndpointType {
/// Handle for a USB endpoint. The endpoint direction is constrained by the `D` type argument, which
/// must be either `In` or `Out`.
pub struct Endpoint<'a, B: UsbBus, D: EndpointDirection> {
bus_ptr: &'a AtomicPtr<B>,
bus_ptr: &'a BusPtr<B>,
address: EndpointAddress,
ep_type: EndpointType,
max_packet_size: u16,
Expand All @@ -59,7 +58,7 @@ pub struct Endpoint<'a, B: UsbBus, D: EndpointDirection> {

impl<B: UsbBus, D: EndpointDirection> Endpoint<'_, B, D> {
pub(crate) fn new<'a>(
bus_ptr: &'a AtomicPtr<B>,
bus_ptr: &'a BusPtr<B>,
address: EndpointAddress,
ep_type: EndpointType,
max_packet_size: u16,
Expand All @@ -76,7 +75,7 @@ impl<B: UsbBus, D: EndpointDirection> Endpoint<'_, B, D> {
}

fn bus(&self) -> &B {
let bus_ptr = self.bus_ptr.load(Ordering::SeqCst);
let bus_ptr = self.bus_ptr.get();
if bus_ptr == ptr::null_mut() {
panic!("UsbBus initialization not complete");
}
Expand Down
4 changes: 4 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ pub mod class_prelude {
pub use crate::UsbError;
}

#[cfg(all(feature = "sync", target_arch = "avr"))]
compile_error!("`sync` feature is not supported on AVR targets.");

#[cfg(feature = "sync")]
fn _ensure_sync() {
use crate::bus::{PollResult, UsbBus, UsbBusAllocator};
use crate::class_prelude::*;
Expand Down