NaxdyOrg/embassy
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Replace PeripheralStateUnchecked with register_interrupt_unchecked

Browse source
This commit is contained in:
Liam Murphy 2021-07-29 15:11:26 +10:00
parent 4d9514cbcb
commit d5ba35424d
5 changed files with 84 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

66
embassy-extras/src/peripheral.rs
View file

@ -6,42 +6,20 @@ use cortex_m::peripheral::scb::{Exception, SystemHandler, VectActive};
use cortex_m::peripheral::{NVIC, SCB};
use embassy::interrupt::{Interrupt, InterruptExt};
/// A version of `PeripheralState` without the `'static` bound,
/// for cases where the compiler can't statically make sure
/// that `on_interrupt` doesn't reference anything which might be invalidated.
///
/// # Safety
/// When types implementing this trait are used with `PeripheralMutex`,
/// no fields referenced by `on_interrupt`'s lifetimes must end without first calling `Drop` on the `PeripheralMutex`.
pub unsafe trait PeripheralStateUnchecked: Send {
type Interrupt: Interrupt;
fn on_interrupt(&mut self);
}
/// A type which can be used as state with `PeripheralMutex`.
///
/// It needs to be `Send` because `&mut` references are sent back and forth between the 'thread' which owns the `PeripheralMutex` and the interrupt,
/// and `&mut T` is only `Send` where `T: Send`.
///
/// It also requires `'static`, because although `Pin` guarantees that the memory of the state won't be invalidated,
/// It also requires `'static` to be used safely with `PeripheralMutex::register_interrupt`,
/// because although `Pin` guarantees that the memory of the state won't be invalidated,
/// it doesn't guarantee that the lifetime will last.
pub trait PeripheralState: Send + 'static {
pub trait PeripheralState: Send {
type Interrupt: Interrupt;
fn on_interrupt(&mut self);
}
// SAFETY: `T` has to live for `'static` to implement `PeripheralState`, thus its lifetime cannot end.
unsafe impl<T> PeripheralStateUnchecked for T
where
T: PeripheralState,
{
type Interrupt = T::Interrupt;
fn on_interrupt(&mut self) {
self.on_interrupt()
}
}
pub struct PeripheralMutex<S: PeripheralStateUnchecked> {
pub struct PeripheralMutex<S: PeripheralState> {
state: UnsafeCell<S>,
irq_setup_done: bool,
@ -98,7 +76,25 @@ pub(crate) fn can_be_preempted(irq: &impl Interrupt) -> bool {
}
}
impl<S: PeripheralStateUnchecked> PeripheralMutex<S> {
impl<S: PeripheralState + 'static> PeripheralMutex<S> {
/// Registers `on_interrupt` as the wrapped interrupt's interrupt handler and enables it.
///
/// This requires this `PeripheralMutex`'s `PeripheralState` to live for `'static`,
/// because `Pin` only guarantees that it's memory won't be repurposed,
/// not that it's lifetime will last.
///
/// To use non-`'static` `PeripheralState`, use the unsafe `register_interrupt_unchecked`.
///
/// Note: `'static` doesn't mean it _has_ to live for the entire program, like an `&'static T`;
/// it just means it _can_ live for the entire program - for example, `u8` lives for `'static`.
pub fn register_interrupt(self: Pin<&mut Self>) {
// SAFETY: `S: 'static`, so there's no way it's lifetime can expire.
unsafe { self.register_interrupt_unchecked() }
}
}
impl<S: PeripheralState> PeripheralMutex<S> {
/// Create a new `PeripheralMutex` wrapping `irq`, with the initial state `state`.
pub fn new(state: S, irq: S::Interrupt) -> Self {
if can_be_preempted(&irq) {
panic!("`PeripheralMutex` cannot be created in an interrupt with higher priority than the interrupt it wraps");
@ -114,8 +110,18 @@ impl<S: PeripheralStateUnchecked> PeripheralMutex<S> {
}
}
pub fn register_interrupt(self: Pin<&mut Self>) {
let this = unsafe { self.get_unchecked_mut() };
/// Registers `on_interrupt` as the wrapped interrupt's interrupt handler and enables it.
///
/// # Safety
/// The lifetime of any data in `PeripheralState` that is accessed by the interrupt handler
/// must not end without `Drop` being called on this `PeripheralMutex`.
///
/// This can be accomplished by either not accessing any data with a lifetime in `on_interrupt`,
/// or making sure that nothing like `mem::forget` is used on the `PeripheralMutex`.
// TODO: this name isn't the best.
pub unsafe fn register_interrupt_unchecked(self: Pin<&mut Self>) {
let this = self.get_unchecked_mut();
if this.irq_setup_done {
return;
}
@ -172,7 +178,7 @@ impl<S: PeripheralStateUnchecked> PeripheralMutex<S> {
}
}
impl<S: PeripheralStateUnchecked> Drop for PeripheralMutex<S> {
impl<S: PeripheralState> Drop for PeripheralMutex<S> {
fn drop(&mut self) {
self.irq.disable();
self.irq.remove_handler();

52
embassy-extras/src/peripheral_shared.rs
View file

@ -5,30 +5,19 @@ use embassy::interrupt::{Interrupt, InterruptExt};
use crate::peripheral::can_be_preempted;
/// A version of `PeripheralState` without the `'static` bound,
/// for cases where the compiler can't statically make sure
/// that `on_interrupt` doesn't reference anything which might be invalidated.
///
/// # Safety
/// When types implementing this trait are used with `Peripheral`,
/// no fields referenced by `on_interrupt`'s lifetimes must end without first calling `Drop` on the `Peripheral`.
pub unsafe trait PeripheralStateUnchecked: Sync {
type Interrupt: Interrupt;
fn on_interrupt(&self);
}
/// A type which can be used as state with `Peripheral`.
///
/// It needs to be `Sync` because references are shared between the 'thread' which owns the `Peripheral` and the interrupt.
///
/// It also requires `'static`, because although `Pin` guarantees that the memory of the state won't be invalidated,
/// It also requires `'static` to be used safely with `Peripheral::register_interrupt`,
/// because although `Pin` guarantees that the memory of the state won't be invalidated,
/// it doesn't guarantee that the lifetime will last.
pub trait PeripheralState: Sync + 'static {
pub trait PeripheralState: Sync {
type Interrupt: Interrupt;
fn on_interrupt(&self);
}
pub struct Peripheral<S: PeripheralStateUnchecked> {
pub struct Peripheral<S: PeripheralState> {
state: S,
irq_setup_done: bool,
@ -38,7 +27,24 @@ pub struct Peripheral<S: PeripheralStateUnchecked> {
_pinned: PhantomPinned,
}
impl<S: PeripheralStateUnchecked> Peripheral<S> {
impl<S: PeripheralState + 'static> Peripheral<S> {
/// Registers `on_interrupt` as the wrapped interrupt's interrupt handler and enables it.
///
/// This requires this `Peripheral`'s `PeripheralState` to live for `'static`,
/// because `Pin` only guarantees that it's memory won't be repurposed,
/// not that it's lifetime will last.
///
/// To use non-`'static` `PeripheralState`, use the unsafe `register_interrupt_unchecked`.
///
/// Note: `'static` doesn't mean it _has_ to live for the entire program, like an `&'static T`;
/// it just means it _can_ live for the entire program - for example, `u8` lives for `'static`.
pub fn register_interrupt(self: Pin<&mut Self>) {
// SAFETY: `S: 'static`, so there's no way it's lifetime can expire.
unsafe { self.register_interrupt_unchecked() }
}
}
impl<S: PeripheralState> Peripheral<S> {
pub fn new(irq: S::Interrupt, state: S) -> Self {
if can_be_preempted(&irq) {
panic!("`Peripheral` cannot be created in an interrupt with higher priority than the interrupt it wraps");
@ -54,8 +60,16 @@ impl<S: PeripheralStateUnchecked> Peripheral<S> {
}
}
pub fn register_interrupt(self: Pin<&mut Self>) {
let this = unsafe { self.get_unchecked_mut() };
/// Registers `on_interrupt` as the wrapped interrupt's interrupt handler and enables it.
///
/// # Safety
/// The lifetime of any data in `PeripheralState` that is accessed by the interrupt handler
/// must not end without `Drop` being called on this `Peripheral`.
///
/// This can be accomplished by either not accessing any data with a lifetime in `on_interrupt`,
/// or making sure that nothing like `mem::forget` is used on the `Peripheral`.
pub unsafe fn register_interrupt_unchecked(self: Pin<&mut Self>) {
let this = self.get_unchecked_mut();
if this.irq_setup_done {
return;
}
@ -100,7 +114,7 @@ impl<S: PeripheralStateUnchecked> Peripheral<S> {
}
}
impl<S: PeripheralStateUnchecked> Drop for Peripheral<S> {
impl<S: PeripheralState> Drop for Peripheral<S> {
fn drop(&mut self) {
self.irq.disable();
self.irq.remove_handler();

9
embassy-extras/src/usb/mod.rs
View file

@ -9,7 +9,7 @@ use usb_device::device::UsbDevice;
mod cdc_acm;
pub mod usb_serial;
use crate::peripheral::{PeripheralMutex, PeripheralStateUnchecked};
use crate::peripheral::{PeripheralMutex, PeripheralState};
use embassy::interrupt::Interrupt;
use usb_serial::{ReadInterface, UsbSerial, WriteInterface};
@ -63,7 +63,9 @@ where
let mutex = Pin::new_unchecked(&mut *mutex);
// Use inner to register the irq
mutex.register_interrupt();
// SAFETY: the safety contract of this function makes sure the `UsbDevice` won't be invalidated
// without the `PeripheralMutex` being dropped.
mutex.register_interrupt_unchecked();
}
}
@ -127,8 +129,7 @@ where
}
}
// SAFETY: The safety contract of `PeripheralStateUnchecked` is forwarded to `Usb::start`.
unsafe impl<'bus, B, T, I> PeripheralStateUnchecked for State<'bus, B, T, I>
impl<'bus, B, T, I> PeripheralState for State<'bus, B, T, I>
where
B: UsbBus,
T: ClassSet<B>,

13
embassy-nrf/src/buffered_uarte.rs
View file

@ -7,7 +7,7 @@ use core::task::{Context, Poll};
use embassy::interrupt::InterruptExt;
use embassy::io::{AsyncBufRead, AsyncWrite, Result};
use embassy::util::{Unborrow, WakerRegistration};
use embassy_extras::peripheral::{PeripheralMutex, PeripheralStateUnchecked};
use embassy_extras::peripheral::{PeripheralMutex, PeripheralState};
use embassy_extras::ring_buffer::RingBuffer;
use embassy_extras::{low_power_wait_until, unborrow};
@ -175,7 +175,7 @@ impl<'d, U: UarteInstance, T: TimerInstance> BufferedUarte<'d, U, T> {
pub fn set_baudrate(self: Pin<&mut Self>, baudrate: Baudrate) {
let mut inner = self.inner();
inner.as_mut().register_interrupt();
unsafe { inner.as_mut().register_interrupt_unchecked() }
inner.with(|state| {
let r = U::regs();
@ -195,7 +195,7 @@ impl<'d, U: UarteInstance, T: TimerInstance> BufferedUarte<'d, U, T> {
impl<'d, U: UarteInstance, T: TimerInstance> AsyncBufRead for BufferedUarte<'d, U, T> {
fn poll_fill_buf(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<&[u8]>> {
let mut inner = self.inner();
inner.as_mut().register_interrupt();
unsafe { inner.as_mut().register_interrupt_unchecked() }
inner.with(|state| {
// Conservative compiler fence to prevent optimizations that do not
// take in to account actions by DMA. The fence has been placed here,
@ -220,7 +220,7 @@ impl<'d, U: UarteInstance, T: TimerInstance> AsyncBufRead for BufferedUarte<'d,
fn consume(self: Pin<&mut Self>, amt: usize) {
let mut inner = self.inner();
inner.as_mut().register_interrupt();
unsafe { inner.as_mut().register_interrupt_unchecked() }
inner.as_mut().with(|state| {
trace!("consume {:?}", amt);
state.rx.pop(amt);
@ -232,7 +232,7 @@ impl<'d, U: UarteInstance, T: TimerInstance> AsyncBufRead for BufferedUarte<'d,
impl<'d, U: UarteInstance, T: TimerInstance> AsyncWrite for BufferedUarte<'d, U, T> {
fn poll_write(self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &[u8]) -> Poll<Result<usize>> {
let mut inner = self.inner();
inner.as_mut().register_interrupt();
unsafe { inner.as_mut().register_interrupt_unchecked() }
let poll = inner.as_mut().with(|state| {
trace!("poll_write: {:?}", buf.len());
@ -285,8 +285,7 @@ impl<'a, U: UarteInstance, T: TimerInstance> Drop for State<'a, U, T> {
}
}
// SAFETY: the safety contract of `PeripheralStateUnchecked` is forwarded to `BufferedUarte::new`.
unsafe impl<'a, U: UarteInstance, T: TimerInstance> PeripheralStateUnchecked for State<'a, U, T> {
impl<'a, U: UarteInstance, T: TimerInstance> PeripheralState for State<'a, U, T> {
type Interrupt = U::Interrupt;
fn on_interrupt(&mut self) {
trace!("irq: start");

8
embassy-stm32/src/eth/v2/mod.rs
View file

@ -4,7 +4,7 @@ use core::sync::atomic::{fence, Ordering};
use core::task::Waker;
use embassy::util::{AtomicWaker, Unborrow};
use embassy_extras::peripheral::{PeripheralMutex, PeripheralStateUnchecked};
use embassy_extras::peripheral::{PeripheralMutex, PeripheralState};
use embassy_extras::unborrow;
use embassy_net::{Device, DeviceCapabilities, LinkState, PacketBuf, MTU};
@ -159,7 +159,8 @@ impl<'d, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, P, TX, RX> {
// NOTE(unsafe) We won't move this
let this = unsafe { self.get_unchecked_mut() };
let mut mutex = unsafe { Pin::new_unchecked(&mut this.state) };
mutex.as_mut().register_interrupt();
// SAFETY: The lifetime of `Inner` is only due to `PhantomData`; it isn't actually referencing any data with that lifetime.
unsafe { mutex.as_mut().register_interrupt_unchecked() }
mutex.with(|s| {
s.desc_ring.init();
@ -343,8 +344,7 @@ impl<'d, const TX: usize, const RX: usize> Inner<'d, TX, RX> {
}
}
// SAFETY: The lifetime of `Inner` is only due to `PhantomData`; it isn't actually referencing any data with that lifetime.
unsafe impl<'d, const TX: usize, const RX: usize> PeripheralStateUnchecked for Inner<'d, TX, RX> {
impl<'d, const TX: usize, const RX: usize> PeripheralState for Inner<'d, TX, RX> {
type Interrupt = crate::interrupt::ETH;
fn on_interrupt(&mut self) {