From a24037edf9d04087111c1d1dc71c92cc0ad83709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Kr=C3=B6ger?= Date: Sun, 1 Jan 2023 22:02:45 +0100 Subject: [PATCH] rp: Fix BufferedUart drop code Only unregister the interrupt handler when both parts are inactive --- embassy-hal-common/src/atomic_ring_buffer.rs | 4 ++ embassy-rp/src/uart/buffered.rs | 51 ++++++++++---------- examples/rp/src/bin/uart_buffered_split.rs | 2 +- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/embassy-hal-common/src/atomic_ring_buffer.rs b/embassy-hal-common/src/atomic_ring_buffer.rs index a8a6a216..4c944d76 100644 --- a/embassy-hal-common/src/atomic_ring_buffer.rs +++ b/embassy-hal-common/src/atomic_ring_buffer.rs @@ -81,6 +81,10 @@ impl RingBuffer { Writer(self) } + pub fn len(&self) -> usize { + self.len.load(Ordering::Relaxed) + } + pub fn is_full(&self) -> bool { let len = self.len.load(Ordering::Relaxed); let start = self.start.load(Ordering::Relaxed); diff --git a/embassy-rp/src/uart/buffered.rs b/embassy-rp/src/uart/buffered.rs index d853618a..9164794b 100644 --- a/embassy-rp/src/uart/buffered.rs +++ b/embassy-rp/src/uart/buffered.rs @@ -27,7 +27,8 @@ impl State { } pub struct BufferedUart<'d, T: Instance> { - phantom: PhantomData<&'d mut T>, + rx: BufferedUartRx<'d, T>, + tx: BufferedUartTx<'d, T>, } pub struct BufferedUartRx<'d, T: Instance> { @@ -91,7 +92,10 @@ impl<'d, T: Instance> BufferedUart<'d, T> { rx_buffer, config, ); - Self { phantom: PhantomData } + Self { + rx: BufferedUartRx { phantom: PhantomData }, + tx: BufferedUartTx { phantom: PhantomData }, + } } pub fn new_with_rtscts( @@ -116,14 +120,14 @@ impl<'d, T: Instance> BufferedUart<'d, T> { rx_buffer, config, ); - Self { phantom: PhantomData } + Self { + rx: BufferedUartRx { phantom: PhantomData }, + tx: BufferedUartTx { phantom: PhantomData }, + } } - pub fn split(&mut self) -> (BufferedUartRx<'d, T>, BufferedUartTx<'d, T>) { - ( - BufferedUartRx { phantom: PhantomData }, - BufferedUartTx { phantom: PhantomData }, - ) + pub fn split(self) -> (BufferedUartRx<'d, T>, BufferedUartTx<'d, T>) { + (self.rx, self.tx) } } @@ -269,35 +273,32 @@ impl<'d, T: Instance> BufferedUartTx<'d, T> { } } -impl<'d, T: Instance> Drop for BufferedUart<'d, T> { - fn drop(&mut self) { - unsafe { - T::Interrupt::steal().disable(); - let state = T::state(); - state.tx_buf.deinit(); - state.rx_buf.deinit(); - } - } -} - impl<'d, T: Instance> Drop for BufferedUartRx<'d, T> { fn drop(&mut self) { + let state = T::state(); unsafe { - T::Interrupt::steal().disable(); - let state = T::state(); - state.tx_buf.deinit(); state.rx_buf.deinit(); + + // TX is inactive if the the buffer is not available. + // We can now unregister the interrupt handler + if state.tx_buf.len() == 0 { + T::Interrupt::steal().disable(); + } } } } impl<'d, T: Instance> Drop for BufferedUartTx<'d, T> { fn drop(&mut self) { + let state = T::state(); unsafe { - T::Interrupt::steal().disable(); - let state = T::state(); state.tx_buf.deinit(); - state.rx_buf.deinit(); + + // RX is inactive if the the buffer is not available. + // We can now unregister the interrupt handler + if state.rx_buf.len() == 0 { + T::Interrupt::steal().disable(); + } } } } diff --git a/examples/rp/src/bin/uart_buffered_split.rs b/examples/rp/src/bin/uart_buffered_split.rs index 36f31c90..a8a68227 100644 --- a/examples/rp/src/bin/uart_buffered_split.rs +++ b/examples/rp/src/bin/uart_buffered_split.rs @@ -29,7 +29,7 @@ async fn main(spawner: Spawner) { let irq = interrupt::take!(UART0_IRQ); let tx_buf = &mut singleton!([0u8; 16])[..]; let rx_buf = &mut singleton!([0u8; 16])[..]; - let mut uart = BufferedUart::new(uart, irq, tx_pin, rx_pin, tx_buf, rx_buf, Config::default()); + let uart = BufferedUart::new(uart, irq, tx_pin, rx_pin, tx_buf, rx_buf, Config::default()); let (rx, mut tx) = uart.split(); unwrap!(spawner.spawn(reader(rx)));