aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDario Nieuwenhuis <[email protected]>2022-06-09 21:28:13 +0200
committerGitHub <[email protected]>2022-06-09 21:28:13 +0200
commitdb344c2bda55bd0352a43720788185cc4d3a420e (patch)
treeb93b2d927d5c84b74dce456f9be5e88ec4bbfe18
parent77c7d8f31b89d13117a7294842d60f02950fdd23 (diff)
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
-rw-r--r--embassy-hal-common/src/peripheral.rs26
-rw-r--r--embassy-nrf/src/buffered_uarte.rs30
-rw-r--r--embassy-stm32/src/eth/v1/mod.rs2
-rw-r--r--embassy-stm32/src/eth/v2/mod.rs2
-rw-r--r--embassy-stm32/src/usart/buffered.rs14
-rw-r--r--examples/stm32f4/src/bin/usart_buffered.rs3
6 files changed, 27 insertions, 50 deletions
diff --git a/embassy-hal-common/src/peripheral.rs b/embassy-hal-common/src/peripheral.rs
index 89420a422..db2bc7888 100644
--- a/embassy-hal-common/src/peripheral.rs
+++ b/embassy-hal-common/src/peripheral.rs
@@ -52,33 +52,11 @@ pub(crate) fn can_be_preempted(irq: &impl Interrupt) -> bool {
52impl<'a, S: PeripheralState> PeripheralMutex<'a, S> { 52impl<'a, S: PeripheralState> PeripheralMutex<'a, S> {
53 /// Create a new `PeripheralMutex` wrapping `irq`, with `init` initializing the initial state. 53 /// Create a new `PeripheralMutex` wrapping `irq`, with `init` initializing the initial state.
54 /// 54 ///
55 /// self requires `S` to live for `'static`, because if the `PeripheralMutex` is leaked, the
56 /// interrupt won't be disabled, which may try accessing the state at any time. To use non-`'static`
57 /// state, see [`Self::new_unchecked`].
58 ///
59 /// Registers `on_interrupt` as the `irq`'s handler, and enables it. 55 /// Registers `on_interrupt` as the `irq`'s handler, and enables it.
60 pub fn new( 56 pub fn new(
61 irq: S::Interrupt, 57 irq: S::Interrupt,
62 storage: &'a mut StateStorage<S>, 58 storage: &'a mut StateStorage<S>,
63 init: impl FnOnce() -> S, 59 init: impl FnOnce() -> S,
64 ) -> Self
65 where
66 'a: 'static,
67 {
68 // safety: safe because state is `'static`.
69 unsafe { Self::new_unchecked(irq, storage, init) }
70 }
71
72 /// Create a `PeripheralMutex` without requiring the state is `'static`.
73 ///
74 /// See also [`Self::new`].
75 ///
76 /// # Safety
77 /// The created instance must not be leaked (its `drop` must run).
78 pub unsafe fn new_unchecked(
79 irq: S::Interrupt,
80 storage: &'a mut StateStorage<S>,
81 init: impl FnOnce() -> S,
82 ) -> Self { 60 ) -> Self {
83 if can_be_preempted(&irq) { 61 if can_be_preempted(&irq) {
84 panic!("`PeripheralMutex` cannot be created in an interrupt with higher priority than the interrupt it wraps"); 62 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> {
88 66
89 // Safety: The pointer is valid and not used by anyone else 67 // Safety: The pointer is valid and not used by anyone else
90 // because we have the `&mut StateStorage`. 68 // because we have the `&mut StateStorage`.
91 state_ptr.write(init()); 69 unsafe { state_ptr.write(init()) };
92 70
93 irq.disable(); 71 irq.disable();
94 irq.set_handler(|p| { 72 irq.set_handler(|p| unsafe {
95 // Safety: it's OK to get a &mut to the state, since 73 // Safety: it's OK to get a &mut to the state, since
96 // - We checked that the thread owning the `PeripheralMutex` can't preempt us in `new`. 74 // - We checked that the thread owning the `PeripheralMutex` can't preempt us in `new`.
97 // Interrupts' priorities can only be changed with raw embassy `Interrupts`, 75 // Interrupts' priorities can only be changed with raw embassy `Interrupts`,
diff --git a/embassy-nrf/src/buffered_uarte.rs b/embassy-nrf/src/buffered_uarte.rs
index e1d32a311..6972d625d 100644
--- a/embassy-nrf/src/buffered_uarte.rs
+++ b/embassy-nrf/src/buffered_uarte.rs
@@ -164,22 +164,20 @@ impl<'d, U: UarteInstance, T: TimerInstance> BufferedUarte<'d, U, T> {
164 ppi_ch2.enable(); 164 ppi_ch2.enable();
165 165
166 Self { 166 Self {
167 inner: unsafe { 167 inner: PeripheralMutex::new(irq, &mut state.0, move || StateInner {
168 PeripheralMutex::new_unchecked(irq, &mut state.0, move || StateInner { 168 phantom: PhantomData,
169 phantom: PhantomData, 169 timer,
170 timer, 170 _ppi_ch1: ppi_ch1,
171 _ppi_ch1: ppi_ch1, 171 _ppi_ch2: ppi_ch2,
172 _ppi_ch2: ppi_ch2, 172
173 173 rx: RingBuffer::new(rx_buffer),
174 rx: RingBuffer::new(rx_buffer), 174 rx_state: RxState::Idle,
175 rx_state: RxState::Idle, 175 rx_waker: WakerRegistration::new(),
176 rx_waker: WakerRegistration::new(), 176
177 177 tx: RingBuffer::new(tx_buffer),
178 tx: RingBuffer::new(tx_buffer), 178 tx_state: TxState::Idle,
179 tx_state: TxState::Idle, 179 tx_waker: WakerRegistration::new(),
180 tx_waker: WakerRegistration::new(), 180 }),
181 })
182 },
183 } 181 }
184 } 182 }
185 183
diff --git a/embassy-stm32/src/eth/v1/mod.rs b/embassy-stm32/src/eth/v1/mod.rs
index 465436d06..327deea2a 100644
--- a/embassy-stm32/src/eth/v1/mod.rs
+++ b/embassy-stm32/src/eth/v1/mod.rs
@@ -146,7 +146,7 @@ impl<'d, T: Instance, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, T,
146 config_pins!(ref_clk, mdio, mdc, crs, rx_d0, rx_d1, tx_d0, tx_d1, tx_en); 146 config_pins!(ref_clk, mdio, mdc, crs, rx_d0, rx_d1, tx_d0, tx_d1, tx_en);
147 147
148 // NOTE(unsafe) We are ourselves not leak-safe. 148 // NOTE(unsafe) We are ourselves not leak-safe.
149 let state = PeripheralMutex::new_unchecked(interrupt, &mut state.0, || Inner::new(peri)); 149 let state = PeripheralMutex::new(interrupt, &mut state.0, || Inner::new(peri));
150 150
151 // NOTE(unsafe) We have exclusive access to the registers 151 // NOTE(unsafe) We have exclusive access to the registers
152 let dma = ETH.ethernet_dma(); 152 let dma = ETH.ethernet_dma();
diff --git a/embassy-stm32/src/eth/v2/mod.rs b/embassy-stm32/src/eth/v2/mod.rs
index afde8ea9e..6a49904d1 100644
--- a/embassy-stm32/src/eth/v2/mod.rs
+++ b/embassy-stm32/src/eth/v2/mod.rs
@@ -83,7 +83,7 @@ impl<'d, T: Instance, P: PHY, const TX: usize, const RX: usize> Ethernet<'d, T,
83 config_pins!(ref_clk, mdio, mdc, crs, rx_d0, rx_d1, tx_d0, tx_d1, tx_en); 83 config_pins!(ref_clk, mdio, mdc, crs, rx_d0, rx_d1, tx_d0, tx_d1, tx_en);
84 84
85 // NOTE(unsafe) We are ourselves not leak-safe. 85 // NOTE(unsafe) We are ourselves not leak-safe.
86 let state = PeripheralMutex::new_unchecked(interrupt, &mut state.0, || Inner::new(peri)); 86 let state = PeripheralMutex::new(interrupt, &mut state.0, || Inner::new(peri));
87 87
88 // NOTE(unsafe) We have exclusive access to the registers 88 // NOTE(unsafe) We have exclusive access to the registers
89 let dma = ETH.ethernet_dma(); 89 let dma = ETH.ethernet_dma();
diff --git a/embassy-stm32/src/usart/buffered.rs b/embassy-stm32/src/usart/buffered.rs
index 7b63638a0..36d176b91 100644
--- a/embassy-stm32/src/usart/buffered.rs
+++ b/embassy-stm32/src/usart/buffered.rs
@@ -35,7 +35,7 @@ pub struct BufferedUart<'d, T: Instance> {
35impl<'d, T: Instance> Unpin for BufferedUart<'d, T> {} 35impl<'d, T: Instance> Unpin for BufferedUart<'d, T> {}
36 36
37impl<'d, T: Instance> BufferedUart<'d, T> { 37impl<'d, T: Instance> BufferedUart<'d, T> {
38 pub unsafe fn new( 38 pub fn new(
39 state: &'d mut State<'d, T>, 39 state: &'d mut State<'d, T>,
40 _uart: Uart<'d, T, NoDma, NoDma>, 40 _uart: Uart<'d, T, NoDma, NoDma>,
41 irq: impl Unborrow<Target = T::Interrupt> + 'd, 41 irq: impl Unborrow<Target = T::Interrupt> + 'd,
@@ -45,13 +45,15 @@ impl<'d, T: Instance> BufferedUart<'d, T> {
45 unborrow!(irq); 45 unborrow!(irq);
46 46
47 let r = T::regs(); 47 let r = T::regs();
48 r.cr1().modify(|w| { 48 unsafe {
49 w.set_rxneie(true); 49 r.cr1().modify(|w| {
50 w.set_idleie(true); 50 w.set_rxneie(true);
51 }); 51 w.set_idleie(true);
52 });
53 }
52 54
53 Self { 55 Self {
54 inner: PeripheralMutex::new_unchecked(irq, &mut state.0, move || StateInner { 56 inner: PeripheralMutex::new(irq, &mut state.0, move || StateInner {
55 phantom: PhantomData, 57 phantom: PhantomData,
56 tx: RingBuffer::new(tx_buffer), 58 tx: RingBuffer::new(tx_buffer),
57 tx_waker: WakerRegistration::new(), 59 tx_waker: WakerRegistration::new(),
diff --git a/examples/stm32f4/src/bin/usart_buffered.rs b/examples/stm32f4/src/bin/usart_buffered.rs
index 80b65f0d4..2a613ee4f 100644
--- a/examples/stm32f4/src/bin/usart_buffered.rs
+++ b/examples/stm32f4/src/bin/usart_buffered.rs
@@ -22,8 +22,7 @@ async fn main(_spawner: Spawner, p: Peripherals) {
22 let irq = interrupt::take!(USART3); 22 let irq = interrupt::take!(USART3);
23 let mut tx_buf = [0u8; 32]; 23 let mut tx_buf = [0u8; 32];
24 let mut rx_buf = [0u8; 32]; 24 let mut rx_buf = [0u8; 32];
25 let mut buf_usart = 25 let mut buf_usart = BufferedUart::new(&mut state, usart, irq, &mut tx_buf, &mut rx_buf);
26 unsafe { BufferedUart::new(&mut state, usart, irq, &mut tx_buf, &mut rx_buf) };
27 26
28 loop { 27 loop {
29 let buf = buf_usart.fill_buf().await.unwrap(); 28 let buf = buf_usart.fill_buf().await.unwrap();