Skip to content

async/spi: replace the "give back the Bus" hack with a raw pointer. #380

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 27, 2022
Merged

async/spi: replace the "give back the Bus" hack with a raw pointer. #380

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 30 additions & 42 deletions embedded-hal-async/src/spi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,8 @@ pub trait SpiDevice: ErrorType {
where
Self: 'a,
R: 'a,
F: FnOnce(&'a mut Self::Bus) -> Fut + 'a,
Fut: Future<
Output = (
&'a mut Self::Bus,
Result<R, <Self::Bus as ErrorType>::Error>,
),
> + 'a;
F: FnOnce(*mut Self::Bus) -> Fut + 'a,
Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>> + 'a;

/// Perform a transaction against the device.
///
Expand All @@ -42,15 +37,14 @@ pub trait SpiDevice: ErrorType {
/// The locking mechanism is implementation-defined. The only requirement is it must prevent two
/// transactions from executing concurrently against the same bus. Examples of implementations are:
/// critical sections, blocking mutexes, async mutexes, returning an error or panicking if the bus is already busy.
///
/// The current state of the Rust typechecker doesn't allow expressing the necessary lifetime constraints, so
/// the `f` closure receives a lifetime-less `*mut Bus` raw pointer instead. The pointer is guaranteed
/// to be valid for the entire duration the closure is running, so dereferencing it is safe.
fn transaction<'a, R, F, Fut>(&'a mut self, f: F) -> Self::TransactionFuture<'a, R, F, Fut>
where
F: FnOnce(&'a mut Self::Bus) -> Fut + 'a,
Fut: Future<
Output = (
&'a mut Self::Bus,
Result<R, <Self::Bus as ErrorType>::Error>,
),
> + 'a;
F: FnOnce(*mut Self::Bus) -> Fut + 'a,
Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>> + 'a;
}

/// Helper methods for SpiDevice.
Expand Down Expand Up @@ -147,8 +141,9 @@ impl<T: SpiDevice> SpiDeviceExt for T {
Word: Copy + 'static,
{
self.transaction(move |bus| async move {
let res = bus.read(buf).await;
(bus, res)
// safety: `bus` is a valid pointer we're allowed to use for the duration of the closure.
let bus = unsafe { &mut *bus };
bus.read(buf).await
})
}

Expand All @@ -164,8 +159,9 @@ impl<T: SpiDevice> SpiDeviceExt for T {
Word: Copy + 'static,
{
self.transaction(move |bus| async move {
let res = bus.write(buf).await;
(bus, res)
// safety: `bus` is a valid pointer we're allowed to use for the duration of the closure.
let bus = unsafe { &mut *bus };
bus.write(buf).await
})
}

Expand All @@ -185,8 +181,9 @@ impl<T: SpiDevice> SpiDeviceExt for T {
Word: Copy + 'static,
{
self.transaction(move |bus| async move {
let res = bus.transfer(read, write).await;
(bus, res)
// safety: `bus` is a valid pointer we're allowed to use for the duration of the closure.
let bus = unsafe { &mut *bus };
bus.transfer(read, write).await
})
}

Expand All @@ -205,8 +202,9 @@ impl<T: SpiDevice> SpiDeviceExt for T {
Word: Copy + 'static,
{
self.transaction(move |bus| async move {
let res = bus.transfer_in_place(buf).await;
(bus, res)
// safety: `bus` is a valid pointer we're allowed to use for the duration of the closure.
let bus = unsafe { &mut *bus };
bus.transfer_in_place(buf).await
})
}
}
Expand All @@ -216,18 +214,13 @@ impl<T: SpiDevice> SpiDevice for &mut T {

type TransactionFuture<'a, R, F, Fut> = T::TransactionFuture<'a, R, F, Fut>
where
Self: 'a, R: 'a, F: FnOnce(&'a mut Self::Bus) -> Fut + 'a,
Fut: Future<Output = (&'a mut Self::Bus, Result<R, <Self::Bus as ErrorType>::Error>)> + 'a;
Self: 'a, R: 'a, F: FnOnce(*mut Self::Bus) -> Fut + 'a,
Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>> + 'a;

fn transaction<'a, R, F, Fut>(&'a mut self, f: F) -> Self::TransactionFuture<'a, R, F, Fut>
where
F: FnOnce(&'a mut Self::Bus) -> Fut + 'a,
Fut: Future<
Output = (
&'a mut Self::Bus,
Result<R, <Self::Bus as ErrorType>::Error>,
),
> + 'a,
F: FnOnce(*mut Self::Bus) -> Fut + 'a,
Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>> + 'a,
{
T::transaction(self, f)
}
Expand Down Expand Up @@ -449,27 +442,22 @@ where

type TransactionFuture<'a, R, F, Fut> = impl Future<Output = Result<R, Self::Error>> + 'a
where
Self: 'a, R: 'a, F: FnOnce(&'a mut Self::Bus) -> Fut + 'a,
Fut: Future<Output = (&'a mut Self::Bus, Result<R, <Self::Bus as ErrorType>::Error>)> + 'a;
Self: 'a, R: 'a, F: FnOnce(*mut Self::Bus) -> Fut + 'a,
Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>> + 'a;

fn transaction<'a, R, F, Fut>(&'a mut self, f: F) -> Self::TransactionFuture<'a, R, F, Fut>
where
R: 'a,
F: FnOnce(&'a mut Self::Bus) -> Fut + 'a,
Fut: Future<
Output = (
&'a mut Self::Bus,
Result<R, <Self::Bus as ErrorType>::Error>,
),
> + 'a,
F: FnOnce(*mut Self::Bus) -> Fut + 'a,
Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>> + 'a,
{
async move {
self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;

let (bus, f_res) = f(&mut self.bus).await;
let f_res = f(&mut self.bus).await;

// On failure, it's important to still flush and deassert CS.
let flush_res = bus.flush().await;
let flush_res = self.bus.flush().await;
let cs_res = self.cs.set_high();

let f_res = f_res.map_err(ExclusiveDeviceError::Spi)?;
Expand Down