common/PeripheralMutex: remove unsafe API. (#802)

Following the project's decision that "leak unsafe" APIs are not marked as "unsafe",
update PeripheralMutex to accept non-'static state without unsafe.

Fixes #801
This commit is contained in:
Dario Nieuwenhuis 2022-06-09 21:28:13 +02:00 committed by GitHub
parent 77c7d8f31b
commit db344c2bda
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 25 additions and 48 deletions

View File

@ -52,33 +52,11 @@ pub(crate) fn can_be_preempted(irq: &impl Interrupt) -> bool {
impl<'a, S: PeripheralState> PeripheralMutex<'a, S> { impl<'a, S: PeripheralState> PeripheralMutex<'a, S> {
/// Create a new `PeripheralMutex` wrapping `irq`, with `init` initializing the initial state. /// Create a new `PeripheralMutex` wrapping `irq`, with `init` initializing the initial state.
/// ///
/// self requires `S` to live for `'static`, because if the `PeripheralMutex` is leaked, the
/// interrupt won't be disabled, which may try accessing the state at any time. To use non-`'static`
/// state, see [`Self::new_unchecked`].
///
/// Registers `on_interrupt` as the `irq`'s handler, and enables it. /// Registers `on_interrupt` as the `irq`'s handler, and enables it.
pub fn new( pub fn new(
irq: S::Interrupt, irq: S::Interrupt,
storage: &'a mut StateStorage<S>, storage: &'a mut StateStorage<S>,
init: impl FnOnce() -> S, init: impl FnOnce() -> S,
) -> Self
where
'a: 'static,
{
// safety: safe because state is `'static`.
unsafe { Self::new_unchecked(irq, storage, init) }
}
/// Create a `PeripheralMutex` without requiring the state is `'static`.
///
/// See also [`Self::new`].
///
/// # Safety
/// The created instance must not be leaked (its `drop` must run).
pub unsafe fn new_unchecked(
irq: S::Interrupt,
storage: &'a mut StateStorage<S>,
init: impl FnOnce() -> S,
) -> Self { ) -> Self {
if can_be_preempted(&irq) { if can_be_preempted(&irq) {
panic!("`PeripheralMutex` cannot be created in an interrupt with higher priority than the interrupt it wraps"); panic!("`PeripheralMutex` cannot be created in an interrupt with higher priority than the interrupt it wraps");
@ -88,10 +66,10 @@ impl<'a, S: PeripheralState> PeripheralMutex<'a, S> {
// Safety: The pointer is valid and not used by anyone else // Safety: The pointer is valid and not used by anyone else
// because we have the `&mut StateStorage`. // because we have the `&mut StateStorage`.
state_ptr.write(init()); unsafe { state_ptr.write(init()) };
irq.disable(); irq.disable();
irq.set_handler(|p| { irq.set_handler(|p| unsafe {
// Safety: it's OK to get a &mut to the state, since // Safety: it's OK to get a &mut to the state, since
// - We checked that the thread owning the `PeripheralMutex` can't preempt us in `new`. // - We checked that the thread owning the `PeripheralMutex` can't preempt us in `new`.
// Interrupts' priorities can only be changed with raw embassy `Interrupts`, // Interrupts' priorities can only be changed with raw embassy `Interrupts`,

View File

@ -164,22 +164,20 @@ impl<'d, U: UarteInstance, T: TimerInstance> BufferedUarte<'d, U, T> {
ppi_ch2.enable(); ppi_ch2.enable();
Self { Self {
inner: unsafe { inner: PeripheralMutex::new(irq, &mut state.0, move || StateInner {
PeripheralMutex::new_unchecked(irq, &mut state.0, move || StateInner { phantom: PhantomData,
phantom: PhantomData, timer,
timer, _ppi_ch1: ppi_ch1,
_ppi_ch1: ppi_ch1, _ppi_ch2: ppi_ch2,
_ppi_ch2: ppi_ch2,
rx: RingBuffer::new(rx_buffer), rx: RingBuffer::new(rx_buffer),
rx_state: RxState::Idle, rx_state: RxState::Idle,
rx_waker: WakerRegistration::new(), rx_waker: WakerRegistration::new(),
tx: RingBuffer::new(tx_buffer), tx: RingBuffer::new(tx_buffer),
tx_state: TxState::Idle, tx_state: TxState::Idle,
tx_waker: WakerRegistration::new(), tx_waker: WakerRegistration::new(),
}) }),
},
} }
} }

View File

@ -146,7 +146,7 @@ impl<'d, T: Instance, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, T,
config_pins!(ref_clk, mdio, mdc, crs, rx_d0, rx_d1, tx_d0, tx_d1, tx_en); config_pins!(ref_clk, mdio, mdc, crs, rx_d0, rx_d1, tx_d0, tx_d1, tx_en);
// NOTE(unsafe) We are ourselves not leak-safe. // NOTE(unsafe) We are ourselves not leak-safe.
let state = PeripheralMutex::new_unchecked(interrupt, &mut state.0, || Inner::new(peri)); let state = PeripheralMutex::new(interrupt, &mut state.0, || Inner::new(peri));
// NOTE(unsafe) We have exclusive access to the registers // NOTE(unsafe) We have exclusive access to the registers
let dma = ETH.ethernet_dma(); let dma = ETH.ethernet_dma();

View File

@ -83,7 +83,7 @@ impl<'d, T: Instance, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, T,
config_pins!(ref_clk, mdio, mdc, crs, rx_d0, rx_d1, tx_d0, tx_d1, tx_en); config_pins!(ref_clk, mdio, mdc, crs, rx_d0, rx_d1, tx_d0, tx_d1, tx_en);
// NOTE(unsafe) We are ourselves not leak-safe. // NOTE(unsafe) We are ourselves not leak-safe.
let state = PeripheralMutex::new_unchecked(interrupt, &mut state.0, || Inner::new(peri)); let state = PeripheralMutex::new(interrupt, &mut state.0, || Inner::new(peri));
// NOTE(unsafe) We have exclusive access to the registers // NOTE(unsafe) We have exclusive access to the registers
let dma = ETH.ethernet_dma(); let dma = ETH.ethernet_dma();

View File

@ -35,7 +35,7 @@ pub struct BufferedUart<'d, T: Instance> {
impl<'d, T: Instance> Unpin for BufferedUart<'d, T> {} impl<'d, T: Instance> Unpin for BufferedUart<'d, T> {}
impl<'d, T: Instance> BufferedUart<'d, T> { impl<'d, T: Instance> BufferedUart<'d, T> {
pub unsafe fn new( pub fn new(
state: &'d mut State<'d, T>, state: &'d mut State<'d, T>,
_uart: Uart<'d, T, NoDma, NoDma>, _uart: Uart<'d, T, NoDma, NoDma>,
irq: impl Unborrow<Target = T::Interrupt> + 'd, irq: impl Unborrow<Target = T::Interrupt> + 'd,
@ -45,13 +45,15 @@ impl<'d, T: Instance> BufferedUart<'d, T> {
unborrow!(irq); unborrow!(irq);
let r = T::regs(); let r = T::regs();
r.cr1().modify(|w| { unsafe {
w.set_rxneie(true); r.cr1().modify(|w| {
w.set_idleie(true); w.set_rxneie(true);
}); w.set_idleie(true);
});
}
Self { Self {
inner: PeripheralMutex::new_unchecked(irq, &mut state.0, move || StateInner { inner: PeripheralMutex::new(irq, &mut state.0, move || StateInner {
phantom: PhantomData, phantom: PhantomData,
tx: RingBuffer::new(tx_buffer), tx: RingBuffer::new(tx_buffer),
tx_waker: WakerRegistration::new(), tx_waker: WakerRegistration::new(),

View File

@ -22,8 +22,7 @@ async fn main(_spawner: Spawner, p: Peripherals) {
let irq = interrupt::take!(USART3); let irq = interrupt::take!(USART3);
let mut tx_buf = [0u8; 32]; let mut tx_buf = [0u8; 32];
let mut rx_buf = [0u8; 32]; let mut rx_buf = [0u8; 32];
let mut buf_usart = let mut buf_usart = BufferedUart::new(&mut state, usart, irq, &mut tx_buf, &mut rx_buf);
unsafe { BufferedUart::new(&mut state, usart, irq, &mut tx_buf, &mut rx_buf) };
loop { loop {
let buf = buf_usart.fill_buf().await.unwrap(); let buf = buf_usart.fill_buf().await.unwrap();