rp: remove take!, add bind_interrupts!

2 files changed, 169 insertions, 141 deletions

diff --git a/embassy-rp/src/uart/buffered.rs b/embassy-rp/src/uart/buffered.rs
index 9d3de1bd8..12d6b8d91 100644
--- a/embassy-rp/src/uart/buffered.rs
+++ b/embassy-rp/src/uart/buffered.rs
@@ -3,7 +3,7 @@ use core::slice;
 use core::task::Poll;
 
 use atomic_polyfill::{AtomicU8, Ordering};
-use embassy_cortex_m::interrupt::{Interrupt, InterruptExt};
+use embassy_cortex_m::interrupt::{self, Binding, Interrupt, InterruptExt};
 use embassy_hal_common::atomic_ring_buffer::RingBuffer;
 use embassy_sync::waitqueue::AtomicWaker;
 use embassy_time::{Duration, Timer};
@@ -52,7 +52,7 @@ pub struct BufferedUartTx<'d, T: Instance> {
 }
 
 pub(crate) fn init_buffers<'d, T: Instance + 'd>(
-    irq: PeripheralRef<'d, T::Interrupt>,
+    _irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
     tx_buffer: &'d mut [u8],
     rx_buffer: &'d mut [u8],
 ) {
@@ -79,24 +79,23 @@ pub(crate) fn init_buffers<'d, T: Instance + 'd>(
             w.set_rtim(true);
             w.set_txim(true);
         });
-    };
 
-    irq.set_handler(on_interrupt::<T>);
-    irq.unpend();
-    irq.enable();
+        T::Interrupt::steal().unpend();
+        T::Interrupt::steal().enable();
+    };
 }
 
 impl<'d, T: Instance> BufferedUart<'d, T> {
     pub fn new(
         _uart: impl Peripheral<P = T> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         tx: impl Peripheral<P = impl TxPin<T>> + 'd,
         rx: impl Peripheral<P = impl RxPin<T>> + 'd,
         tx_buffer: &'d mut [u8],
         rx_buffer: &'d mut [u8],
         config: Config,
     ) -> Self {
-        into_ref!(irq, tx, rx);
+        into_ref!(tx, rx);
 
         super::Uart::<'d, T, Async>::init(Some(tx.map_into()), Some(rx.map_into()), None, None, config);
         init_buffers::<T>(irq, tx_buffer, rx_buffer);
@@ -109,7 +108,7 @@ impl<'d, T: Instance> BufferedUart<'d, T> {
 
     pub fn new_with_rtscts(
         _uart: impl Peripheral<P = T> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         tx: impl Peripheral<P = impl TxPin<T>> + 'd,
         rx: impl Peripheral<P = impl RxPin<T>> + 'd,
         rts: impl Peripheral<P = impl RtsPin<T>> + 'd,
@@ -118,7 +117,7 @@ impl<'d, T: Instance> BufferedUart<'d, T> {
         rx_buffer: &'d mut [u8],
         config: Config,
     ) -> Self {
-        into_ref!(irq, tx, rx, cts, rts);
+        into_ref!(tx, rx, cts, rts);
 
         super::Uart::<'d, T, Async>::init(
             Some(tx.map_into()),
@@ -163,12 +162,12 @@ impl<'d, T: Instance> BufferedUart<'d, T> {
 impl<'d, T: Instance> BufferedUartRx<'d, T> {
     pub fn new(
         _uart: impl Peripheral<P = T> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         rx: impl Peripheral<P = impl RxPin<T>> + 'd,
         rx_buffer: &'d mut [u8],
         config: Config,
     ) -> Self {
-        into_ref!(irq, rx);
+        into_ref!(rx);
 
         super::Uart::<'d, T, Async>::init(None, Some(rx.map_into()), None, None, config);
         init_buffers::<T>(irq, &mut [], rx_buffer);
@@ -178,13 +177,13 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> {
 
     pub fn new_with_rts(
         _uart: impl Peripheral<P = T> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         rx: impl Peripheral<P = impl RxPin<T>> + 'd,
         rts: impl Peripheral<P = impl RtsPin<T>> + 'd,
         rx_buffer: &'d mut [u8],
         config: Config,
     ) -> Self {
-        into_ref!(irq, rx, rts);
+        into_ref!(rx, rts);
 
         super::Uart::<'d, T, Async>::init(None, Some(rx.map_into()), Some(rts.map_into()), None, config);
         init_buffers::<T>(irq, &mut [], rx_buffer);
@@ -312,12 +311,12 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> {
 impl<'d, T: Instance> BufferedUartTx<'d, T> {
     pub fn new(
         _uart: impl Peripheral<P = T> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         tx: impl Peripheral<P = impl TxPin<T>> + 'd,
         tx_buffer: &'d mut [u8],
         config: Config,
     ) -> Self {
-        into_ref!(irq, tx);
+        into_ref!(tx);
 
         super::Uart::<'d, T, Async>::init(Some(tx.map_into()), None, None, None, config);
         init_buffers::<T>(irq, tx_buffer, &mut []);
@@ -327,13 +326,13 @@ impl<'d, T: Instance> BufferedUartTx<'d, T> {
 
     pub fn new_with_cts(
         _uart: impl Peripheral<P = T> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         tx: impl Peripheral<P = impl TxPin<T>> + 'd,
         cts: impl Peripheral<P = impl CtsPin<T>> + 'd,
         tx_buffer: &'d mut [u8],
         config: Config,
     ) -> Self {
-        into_ref!(irq, tx, cts);
+        into_ref!(tx, cts);
 
         super::Uart::<'d, T, Async>::init(Some(tx.map_into()), None, None, Some(cts.map_into()), config);
         init_buffers::<T>(irq, tx_buffer, &mut []);
@@ -482,97 +481,107 @@ impl<'d, T: Instance> Drop for BufferedUartTx<'d, T> {
     }
 }
 
-pub(crate) unsafe fn on_interrupt<T: Instance>(_: *mut ()) {
-    let r = T::regs();
-    let s = T::buffered_state();
+pub struct BufferedInterruptHandler<T: Instance> {
+    _uart: PhantomData<T>,
+}
 
-    unsafe {
-        // Clear TX and error interrupt flags
-        // RX interrupt flags are cleared by reading from the FIFO.
-        let ris = r.uartris().read();
-        r.uarticr().write(|w| {
-            w.set_txic(ris.txris());
-            w.set_feic(ris.feris());
-            w.set_peic(ris.peris());
-            w.set_beic(ris.beris());
-            w.set_oeic(ris.oeris());
-        });
+impl<T: Instance> interrupt::Handler<T::Interrupt> for BufferedInterruptHandler<T> {
+    unsafe fn on_interrupt() {
+        let r = T::regs();
+        if r.uartdmacr().read().rxdmae() {
+            return;
+        }
 
-        trace!("on_interrupt ris={:#X}", ris.0);
+        let s = T::buffered_state();
 
-        // Errors
-        if ris.feris() {
-            warn!("Framing error");
-        }
-        if ris.peris() {
-            warn!("Parity error");
-        }
-        if ris.beris() {
-            warn!("Break error");
-        }
-        if ris.oeris() {
-            warn!("Overrun error");
-        }
+        unsafe {
+            // Clear TX and error interrupt flags
+            // RX interrupt flags are cleared by reading from the FIFO.
+            let ris = r.uartris().read();
+            r.uarticr().write(|w| {
+                w.set_txic(ris.txris());
+                w.set_feic(ris.feris());
+                w.set_peic(ris.peris());
+                w.set_beic(ris.beris());
+                w.set_oeic(ris.oeris());
+            });
 
-        // RX
-        let mut rx_writer = s.rx_buf.writer();
-        let rx_buf = rx_writer.push_slice();
-        let mut n_read = 0;
-        let mut error = false;
-        for rx_byte in rx_buf {
-            if r.uartfr().read().rxfe() {
-                break;
+            trace!("on_interrupt ris={:#X}", ris.0);
+
+            // Errors
+            if ris.feris() {
+                warn!("Framing error");
             }
-            let dr = r.uartdr().read();
-            if (dr.0 >> 8) != 0 {
-                s.rx_error.fetch_or((dr.0 >> 8) as u8, Ordering::Relaxed);
-                error = true;
-                // only fill the buffer with valid characters. the current character is fine
-                // if the error is an overrun, but if we add it to the buffer we'll report
-                // the overrun one character too late. drop it instead and pretend we were
-                // a bit slower at draining the rx fifo than we actually were.
-                // this is consistent with blocking uart error reporting.
-                break;
+            if ris.peris() {
+                warn!("Parity error");
+            }
+            if ris.beris() {
+                warn!("Break error");
+            }
+            if ris.oeris() {
+                warn!("Overrun error");
             }
-            *rx_byte = dr.data();
-            n_read += 1;
-        }
-        if n_read > 0 {
-            rx_writer.push_done(n_read);
-            s.rx_waker.wake();
-        } else if error {
-            s.rx_waker.wake();
-        }
-        // Disable any further RX interrupts when the buffer becomes full or
-        // errors have occurred. This lets us buffer additional errors in the
-        // fifo without needing more error storage locations, and most applications
-        // will want to do a full reset of their uart state anyway once an error
-        // has happened.
-        if s.rx_buf.is_full() || error {
-            r.uartimsc().write_clear(|w| {
-                w.set_rxim(true);
-                w.set_rtim(true);
-            });
-        }
 
-        // TX
-        let mut tx_reader = s.tx_buf.reader();
-        let tx_buf = tx_reader.pop_slice();
-        let mut n_written = 0;
-        for tx_byte in tx_buf.iter_mut() {
-            if r.uartfr().read().txff() {
-                break;
+            // RX
+            let mut rx_writer = s.rx_buf.writer();
+            let rx_buf = rx_writer.push_slice();
+            let mut n_read = 0;
+            let mut error = false;
+            for rx_byte in rx_buf {
+                if r.uartfr().read().rxfe() {
+                    break;
+                }
+                let dr = r.uartdr().read();
+                if (dr.0 >> 8) != 0 {
+                    s.rx_error.fetch_or((dr.0 >> 8) as u8, Ordering::Relaxed);
+                    error = true;
+                    // only fill the buffer with valid characters. the current character is fine
+                    // if the error is an overrun, but if we add it to the buffer we'll report
+                    // the overrun one character too late. drop it instead and pretend we were
+                    // a bit slower at draining the rx fifo than we actually were.
+                    // this is consistent with blocking uart error reporting.
+                    break;
+                }
+                *rx_byte = dr.data();
+                n_read += 1;
             }
-            r.uartdr().write(|w| w.set_data(*tx_byte));
-            n_written += 1;
-        }
-        if n_written > 0 {
-            tx_reader.pop_done(n_written);
-            s.tx_waker.wake();
+            if n_read > 0 {
+                rx_writer.push_done(n_read);
+                s.rx_waker.wake();
+            } else if error {
+                s.rx_waker.wake();
+            }
+            // Disable any further RX interrupts when the buffer becomes full or
+            // errors have occurred. This lets us buffer additional errors in the
+            // fifo without needing more error storage locations, and most applications
+            // will want to do a full reset of their uart state anyway once an error
+            // has happened.
+            if s.rx_buf.is_full() || error {
+                r.uartimsc().write_clear(|w| {
+                    w.set_rxim(true);
+                    w.set_rtim(true);
+                });
+            }
+
+            // TX
+            let mut tx_reader = s.tx_buf.reader();
+            let tx_buf = tx_reader.pop_slice();
+            let mut n_written = 0;
+            for tx_byte in tx_buf.iter_mut() {
+                if r.uartfr().read().txff() {
+                    break;
+                }
+                r.uartdr().write(|w| w.set_data(*tx_byte));
+                n_written += 1;
+            }
+            if n_written > 0 {
+                tx_reader.pop_done(n_written);
+                s.tx_waker.wake();
+            }
+            // The TX interrupt only triggers once when the FIFO threshold is
+            // crossed. No need to disable it when the buffer becomes empty
+            // as it does re-trigger anymore once we have cleared it.
         }
-        // The TX interrupt only triggers once when the FIFO threshold is
-        // crossed. No need to disable it when the buffer becomes empty
-        // as it does re-trigger anymore once we have cleared it.
     }
 }
 
diff --git a/embassy-rp/src/uart/mod.rs b/embassy-rp/src/uart/mod.rs
index a0ee6b4ce..7234336b4 100644
--- a/embassy-rp/src/uart/mod.rs
+++ b/embassy-rp/src/uart/mod.rs
@@ -3,7 +3,7 @@ use core::marker::PhantomData;
 use core::task::Poll;
 
 use atomic_polyfill::{AtomicU16, Ordering};
-use embassy_cortex_m::interrupt::{Interrupt, InterruptExt};
+use embassy_cortex_m::interrupt::{self, Binding, Interrupt, InterruptExt};
 use embassy_futures::select::{select, Either};
 use embassy_hal_common::{into_ref, PeripheralRef};
 use embassy_sync::waitqueue::AtomicWaker;
@@ -20,7 +20,7 @@ use crate::{pac, peripherals, Peripheral, RegExt};
 #[cfg(feature = "nightly")]
 mod buffered;
 #[cfg(feature = "nightly")]
-pub use buffered::{BufferedUart, BufferedUartRx, BufferedUartTx};
+pub use buffered::{BufferedInterruptHandler, BufferedUart, BufferedUartRx, BufferedUartTx};
 
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum DataBits {
@@ -203,11 +203,9 @@ impl<'d, T: Instance> UartTx<'d, T, Blocking> {
     #[cfg(feature = "nightly")]
     pub fn into_buffered(
         self,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         tx_buffer: &'d mut [u8],
     ) -> BufferedUartTx<'d, T> {
-        into_ref!(irq);
-
         buffered::init_buffers::<T>(irq, tx_buffer, &mut []);
 
         BufferedUartTx { phantom: PhantomData }
@@ -235,25 +233,24 @@ impl<'d, T: Instance, M: Mode> UartRx<'d, T, M> {
     pub fn new(
         _uart: impl Peripheral<P = T> + 'd,
         rx: impl Peripheral<P = impl RxPin<T>> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        _irq: impl Binding<T::Interrupt, InterruptHandler<T>>,
         rx_dma: impl Peripheral<P = impl Channel> + 'd,
         config: Config,
     ) -> Self {
-        into_ref!(rx, irq, rx_dma);
+        into_ref!(rx, rx_dma);
         Uart::<T, M>::init(None, Some(rx.map_into()), None, None, config);
-        Self::new_inner(Some(irq), Some(rx_dma.map_into()))
+        Self::new_inner(true, Some(rx_dma.map_into()))
     }
 
-    fn new_inner(irq: Option<PeripheralRef<'d, T::Interrupt>>, rx_dma: Option<PeripheralRef<'d, AnyChannel>>) -> Self {
-        debug_assert_eq!(irq.is_some(), rx_dma.is_some());
-        if let Some(irq) = irq {
+    fn new_inner(has_irq: bool, rx_dma: Option<PeripheralRef<'d, AnyChannel>>) -> Self {
+        debug_assert_eq!(has_irq, rx_dma.is_some());
+        if has_irq {
             unsafe {
                 // disable all error interrupts initially
                 T::regs().uartimsc().write(|w| w.0 = 0);
+                T::Interrupt::steal().unpend();
+                T::Interrupt::steal().enable();
             }
-            irq.set_handler(on_interrupt::<T>);
-            irq.unpend();
-            irq.enable();
         }
         Self {
             rx_dma,
@@ -299,6 +296,12 @@ impl<'d, T: Instance, M: Mode> Drop for UartRx<'d, T, M> {
         if let Some(_) = self.rx_dma {
             unsafe {
                 T::Interrupt::steal().disable();
+                // clear dma flags. irq handlers use these to disambiguate among themselves.
+                T::regs().uartdmacr().write_clear(|reg| {
+                    reg.set_rxdmae(true);
+                    reg.set_txdmae(true);
+                    reg.set_dmaonerr(true);
+                });
             }
         }
     }
@@ -312,33 +315,41 @@ impl<'d, T: Instance> UartRx<'d, T, Blocking> {
     ) -> Self {
         into_ref!(rx);
         Uart::<T, Blocking>::init(None, Some(rx.map_into()), None, None, config);
-        Self::new_inner(None, None)
+        Self::new_inner(false, None)
     }
 
     #[cfg(feature = "nightly")]
     pub fn into_buffered(
         self,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         rx_buffer: &'d mut [u8],
     ) -> BufferedUartRx<'d, T> {
-        into_ref!(irq);
-
         buffered::init_buffers::<T>(irq, &mut [], rx_buffer);
 
         BufferedUartRx { phantom: PhantomData }
     }
 }
 
-unsafe fn on_interrupt<T: Instance>(_: *mut ()) {
-    let uart = T::regs();
-    let state = T::dma_state();
-    let errs = uart.uartris().read();
-    state.rx_errs.store(errs.0 as u16, Ordering::Relaxed);
-    state.rx_err_waker.wake();
-    // disable the error interrupts instead of clearing the flags. clearing the
-    // flags would allow the dma transfer to continue, potentially signaling
-    // completion before we can check for errors that happened *during* the transfer.
-    uart.uartimsc().write_clear(|w| w.0 = errs.0);
+pub struct InterruptHandler<T: Instance> {
+    _uart: PhantomData<T>,
+}
+
+impl<T: Instance> interrupt::Handler<T::Interrupt> for InterruptHandler<T> {
+    unsafe fn on_interrupt() {
+        let uart = T::regs();
+        if !uart.uartdmacr().read().rxdmae() {
+            return;
+        }
+
+        let state = T::dma_state();
+        let errs = uart.uartris().read();
+        state.rx_errs.store(errs.0 as u16, Ordering::Relaxed);
+        state.rx_err_waker.wake();
+        // disable the error interrupts instead of clearing the flags. clearing the
+        // flags would allow the dma transfer to continue, potentially signaling
+        // completion before we can check for errors that happened *during* the transfer.
+        uart.uartimsc().write_clear(|w| w.0 = errs.0);
+    }
 }
 
 impl<'d, T: Instance> UartRx<'d, T, Async> {
@@ -428,7 +439,17 @@ impl<'d, T: Instance> Uart<'d, T, Blocking> {
         config: Config,
     ) -> Self {
         into_ref!(tx, rx);
-        Self::new_inner(uart, tx.map_into(), rx.map_into(), None, None, None, None, None, config)
+        Self::new_inner(
+            uart,
+            tx.map_into(),
+            rx.map_into(),
+            None,
+            None,
+            false,
+            None,
+            None,
+            config,
+        )
     }
 
     /// Create a new UART with hardware flow control (RTS/CTS)
@@ -447,7 +468,7 @@ impl<'d, T: Instance> Uart<'d, T, Blocking> {
             rx.map_into(),
             Some(rts.map_into()),
             Some(cts.map_into()),
-            None,
+            false,
             None,
             None,
             config,
@@ -457,12 +478,10 @@ impl<'d, T: Instance> Uart<'d, T, Blocking> {
     #[cfg(feature = "nightly")]
     pub fn into_buffered(
         self,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        irq: impl Binding<T::Interrupt, BufferedInterruptHandler<T>>,
         tx_buffer: &'d mut [u8],
         rx_buffer: &'d mut [u8],
     ) -> BufferedUart<'d, T> {
-        into_ref!(irq);
-
         buffered::init_buffers::<T>(irq, tx_buffer, rx_buffer);
 
         BufferedUart {
@@ -478,19 +497,19 @@ impl<'d, T: Instance> Uart<'d, T, Async> {
         uart: impl Peripheral<P = T> + 'd,
         tx: impl Peripheral<P = impl TxPin<T>> + 'd,
         rx: impl Peripheral<P = impl RxPin<T>> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        _irq: impl Binding<T::Interrupt, InterruptHandler<T>>,
         tx_dma: impl Peripheral<P = impl Channel> + 'd,
         rx_dma: impl Peripheral<P = impl Channel> + 'd,
         config: Config,
     ) -> Self {
-        into_ref!(tx, rx, irq, tx_dma, rx_dma);
+        into_ref!(tx, rx, tx_dma, rx_dma);
         Self::new_inner(
             uart,
             tx.map_into(),
             rx.map_into(),
             None,
             None,
-            Some(irq),
+            true,
             Some(tx_dma.map_into()),
             Some(rx_dma.map_into()),
             config,
@@ -504,19 +523,19 @@ impl<'d, T: Instance> Uart<'d, T, Async> {
         rx: impl Peripheral<P = impl RxPin<T>> + 'd,
         rts: impl Peripheral<P = impl RtsPin<T>> + 'd,
         cts: impl Peripheral<P = impl CtsPin<T>> + 'd,
-        irq: impl Peripheral<P = T::Interrupt> + 'd,
+        _irq: impl Binding<T::Interrupt, InterruptHandler<T>>,
         tx_dma: impl Peripheral<P = impl Channel> + 'd,
         rx_dma: impl Peripheral<P = impl Channel> + 'd,
         config: Config,
     ) -> Self {
-        into_ref!(tx, rx, cts, rts, irq, tx_dma, rx_dma);
+        into_ref!(tx, rx, cts, rts, tx_dma, rx_dma);
         Self::new_inner(
             uart,
             tx.map_into(),
             rx.map_into(),
             Some(rts.map_into()),
             Some(cts.map_into()),
-            Some(irq),
+            true,
             Some(tx_dma.map_into()),
             Some(rx_dma.map_into()),
             config,
@@ -531,7 +550,7 @@ impl<'d, T: Instance + 'd, M: Mode> Uart<'d, T, M> {
         mut rx: PeripheralRef<'d, AnyPin>,
         mut rts: Option<PeripheralRef<'d, AnyPin>>,
         mut cts: Option<PeripheralRef<'d, AnyPin>>,
-        irq: Option<PeripheralRef<'d, T::Interrupt>>,
+        has_irq: bool,
         tx_dma: Option<PeripheralRef<'d, AnyChannel>>,
         rx_dma: Option<PeripheralRef<'d, AnyChannel>>,
         config: Config,
@@ -546,7 +565,7 @@ impl<'d, T: Instance + 'd, M: Mode> Uart<'d, T, M> {
 
         Self {
             tx: UartTx::new_inner(tx_dma),
-            rx: UartRx::new_inner(irq, rx_dma),
+            rx: UartRx::new_inner(has_irq, rx_dma),
         }
     }