Add HIL tests of DMA & UART, and correctly set DREQ for uart DMA

5 files changed, 184 insertions, 23 deletions

diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs
index 694823742..75d7492e0 100644
--- a/embassy-rp/src/dma.rs
+++ b/embassy-rp/src/dma.rs
@@ -41,18 +41,38 @@ pub unsafe fn read<'a, C: Channel, W: Word>(
     ch: impl Peripheral<P = C> + 'a,
     from: *const W,
     to: &mut [W],
+    dreq: u8,
 ) -> Transfer<'a, C> {
-    let (ptr, len) = crate::dma::slice_ptr_parts_mut(to);
-    copy_inner(ch, from as *const u32, ptr as *mut u32, len, W::size(), false, true)
+    let (to_ptr, len) = crate::dma::slice_ptr_parts_mut(to);
+    copy_inner(
+        ch,
+        from as *const u32,
+        to_ptr as *mut u32,
+        len,
+        W::size(),
+        false,
+        true,
+        dreq,
+    )
 }
 
 pub unsafe fn write<'a, C: Channel, W: Word>(
     ch: impl Peripheral<P = C> + 'a,
     from: &[W],
     to: *mut W,
+    dreq: u8,
 ) -> Transfer<'a, C> {
     let (from_ptr, len) = crate::dma::slice_ptr_parts(from);
-    copy_inner(ch, from_ptr as *const u32, to as *mut u32, len, W::size(), true, false)
+    copy_inner(
+        ch,
+        from_ptr as *const u32,
+        to as *mut u32,
+        len,
+        W::size(),
+        true,
+        false,
+        dreq,
+    )
 }
 
 pub unsafe fn copy<'a, C: Channel, W: Word>(
@@ -71,6 +91,7 @@ pub unsafe fn copy<'a, C: Channel, W: Word>(
         W::size(),
         true,
         true,
+        vals::TreqSel::PERMANENT.0,
     )
 }
 
@@ -82,6 +103,7 @@ fn copy_inner<'a, C: Channel>(
     data_size: DataSize,
     incr_read: bool,
     incr_write: bool,
+    dreq: u8,
 ) -> Transfer<'a, C> {
     into_ref!(ch);
 
@@ -95,6 +117,9 @@ fn copy_inner<'a, C: Channel>(
         compiler_fence(Ordering::SeqCst);
 
         p.ctrl_trig().write(|w| {
+            // TODO: Add all DREQ options to pac vals::TreqSel, and use
+            // `set_treq:sel`
+            w.0 = ((dreq as u32) & 0x3f) << 15usize;
             w.set_data_size(data_size);
             w.set_incr_read(incr_read);
             w.set_incr_write(incr_write);
diff --git a/embassy-rp/src/uart.rs b/embassy-rp/src/uart.rs
index 9d94dcf21..4a3c7a0ce 100644
--- a/embassy-rp/src/uart.rs
+++ b/embassy-rp/src/uart.rs
@@ -113,7 +113,7 @@ impl<'d, T: Instance, M: Mode> UartTx<'d, T, M> {
 
     pub fn blocking_flush(&mut self) -> Result<(), Error> {
         let r = T::regs();
-        unsafe { while r.uartfr().read().txff() {} }
+        unsafe { while !r.uartfr().read().txfe() {} }
         Ok(())
     }
 }
@@ -127,7 +127,7 @@ impl<'d, T: Instance> UartTx<'d, T, Async> {
             });
             // If we don't assign future to a variable, the data register pointer
             // is held across an await and makes the future non-Send.
-            crate::dma::write(ch, buffer, T::regs().uartdr().ptr() as *mut _)
+            crate::dma::write(ch, buffer, T::regs().uartdr().ptr() as *mut _, T::TX_DREQ)
         };
         transfer.await;
         Ok(())
@@ -147,6 +147,10 @@ impl<'d, T: Instance, M: Mode> UartRx<'d, T, M> {
         unsafe {
             for b in buffer {
                 *b = loop {
+                    if r.uartfr().read().rxfe() {
+                        continue;
+                    }
+
                     let dr = r.uartdr().read();
 
                     if dr.oe() {
@@ -157,7 +161,7 @@ impl<'d, T: Instance, M: Mode> UartRx<'d, T, M> {
                         return Err(Error::Parity);
                     } else if dr.fe() {
                         return Err(Error::Framing);
-                    } else if dr.fe() {
+                    } else {
                         break dr.data();
                     }
                 };
@@ -176,7 +180,7 @@ impl<'d, T: Instance> UartRx<'d, T, Async> {
             });
             // If we don't assign future to a variable, the data register pointer
             // is held across an await and makes the future non-Send.
-            crate::dma::read(ch, T::regs().uartdr().ptr() as *const _, buffer)
+            crate::dma::read(ch, T::regs().uartdr().ptr() as *const _, buffer, T::RX_DREQ)
         };
         transfer.await;
         Ok(())
@@ -282,6 +286,30 @@ impl<'d, T: Instance, M: Mode> Uart<'d, T, M> {
         unsafe {
             let r = T::regs();
 
+            tx.io().ctrl().write(|w| w.set_funcsel(2));
+            rx.io().ctrl().write(|w| w.set_funcsel(2));
+
+            tx.pad_ctrl().write(|w| {
+                w.set_ie(true);
+            });
+
+            rx.pad_ctrl().write(|w| {
+                w.set_ie(true);
+            });
+
+            if let Some(pin) = &cts {
+                pin.io().ctrl().write(|w| w.set_funcsel(2));
+                pin.pad_ctrl().write(|w| {
+                    w.set_ie(true);
+                });
+            }
+            if let Some(pin) = &rts {
+                pin.io().ctrl().write(|w| w.set_funcsel(2));
+                pin.pad_ctrl().write(|w| {
+                    w.set_ie(true);
+                });
+            }
+
             let clk_base = crate::clocks::clk_peri_freq();
 
             let baud_rate_div = (8 * clk_base) / config.baudrate;
@@ -302,10 +330,14 @@ impl<'d, T: Instance, M: Mode> Uart<'d, T, M> {
 
             let (pen, eps) = match config.parity {
                 Parity::ParityNone => (false, false),
-                Parity::ParityEven => (true, true),
                 Parity::ParityOdd => (true, false),
+                Parity::ParityEven => (true, true),
             };
 
+            // PL011 needs a (dummy) line control register write to latch in the
+            // divisors. We don't want to actually change LCR contents here.
+            r.uartlcr_h().modify(|_| {});
+
             r.uartlcr_h().write(|w| {
                 w.set_wlen(config.data_bits.bits());
                 w.set_stp2(config.stop_bits == StopBits::STOP2);
@@ -321,15 +353,6 @@ impl<'d, T: Instance, M: Mode> Uart<'d, T, M> {
                 w.set_ctsen(cts.is_some());
                 w.set_rtsen(rts.is_some());
             });
-
-            tx.io().ctrl().write(|w| w.set_funcsel(2));
-            rx.io().ctrl().write(|w| w.set_funcsel(2));
-            if let Some(pin) = &cts {
-                pin.io().ctrl().write(|w| w.set_funcsel(2));
-            }
-            if let Some(pin) = &rts {
-                pin.io().ctrl().write(|w| w.set_funcsel(2));
-            }
         }
 
         Self {
@@ -377,6 +400,10 @@ mod eh02 {
         fn read(&mut self) -> Result<u8, nb::Error<Self::Error>> {
             let r = T::regs();
             unsafe {
+                if r.uartfr().read().rxfe() {
+                    return Err(nb::Error::WouldBlock);
+                }
+
                 let dr = r.uartdr().read();
 
                 if dr.oe() {
@@ -387,10 +414,8 @@ mod eh02 {
                     Err(nb::Error::Other(Error::Parity))
                 } else if dr.fe() {
                     Err(nb::Error::Other(Error::Framing))
-                } else if dr.fe() {
-                    Ok(dr.data())
                 } else {
-                    Err(nb::Error::WouldBlock)
+                    Ok(dr.data())
                 }
             }
         }
@@ -512,6 +537,9 @@ mod sealed {
     pub trait Mode {}
 
     pub trait Instance {
+        const TX_DREQ: u8;
+        const RX_DREQ: u8;
+
         fn regs() -> pac::uart::Uart;
     }
     pub trait TxPin<T: Instance> {}
@@ -538,8 +566,11 @@ impl_mode!(Async);
 pub trait Instance: sealed::Instance {}
 
 macro_rules! impl_instance {
-    ($inst:ident, $irq:ident) => {
+    ($inst:ident, $irq:ident, $tx_dreq:expr, $rx_dreq:expr) => {
         impl sealed::Instance for peripherals::$inst {
+            const TX_DREQ: u8 = $tx_dreq;
+            const RX_DREQ: u8 = $rx_dreq;
+
             fn regs() -> pac::uart::Uart {
                 pac::$inst
             }
@@ -548,8 +579,8 @@ macro_rules! impl_instance {
     };
 }
 
-impl_instance!(UART0, UART0);
-impl_instance!(UART1, UART1);
+impl_instance!(UART0, UART0, 20, 21);
+impl_instance!(UART1, UART1, 22, 23);
 
 pub trait TxPin<T: Instance>: sealed::TxPin<T> + crate::gpio::Pin {}
 pub trait RxPin<T: Instance>: sealed::RxPin<T> + crate::gpio::Pin {}
diff --git a/tests/rp/src/bin/dma_copy_async.rs b/tests/rp/src/bin/dma_copy_async.rs
new file mode 100644
index 000000000..c53f644bd
--- /dev/null
+++ b/tests/rp/src/bin/dma_copy_async.rs
@@ -0,0 +1,41 @@
+#![no_std]
+#![no_main]
+#![feature(type_alias_impl_trait)]
+
+use defmt::{assert_eq, *};
+use embassy_executor::Spawner;
+use embassy_rp::dma::copy;
+use {defmt_rtt as _, panic_probe as _};
+
+#[embassy_executor::main]
+async fn main(_spawner: Spawner) {
+    let p = embassy_rp::init(Default::default());
+    info!("Hello World!");
+
+    // Check `u8` copy
+    {
+        let data: [u8; 2] = [0xC0, 0xDE];
+        let mut buf = [0; 2];
+        unsafe { copy(p.DMA_CH0, &data, &mut buf).await };
+        assert_eq!(buf, data);
+    }
+
+    // Check `u16` copy
+    {
+        let data: [u16; 2] = [0xC0BE, 0xDEAD];
+        let mut buf = [0; 2];
+        unsafe { copy(p.DMA_CH1, &data, &mut buf).await };
+        assert_eq!(buf, data);
+    }
+
+    // Check `u32` copy
+    {
+        let data: [u32; 2] = [0xC0BEDEAD, 0xDEADAAFF];
+        let mut buf = [0; 2];
+        unsafe { copy(p.DMA_CH2, &data, &mut buf).await };
+        assert_eq!(buf, data);
+    }
+
+    info!("Test OK");
+    cortex_m::asm::bkpt();
+}
diff --git a/tests/rp/src/bin/uart.rs b/tests/rp/src/bin/uart.rs
new file mode 100644
index 000000000..92f61464e
--- /dev/null
+++ b/tests/rp/src/bin/uart.rs
@@ -0,0 +1,32 @@
+#![no_std]
+#![no_main]
+#![feature(type_alias_impl_trait)]
+
+use defmt::{assert_eq, *};
+use embassy_executor::Spawner;
+use embassy_rp::uart::{Config, Uart};
+use {defmt_rtt as _, panic_probe as _};
+
+#[embassy_executor::main]
+async fn main(_spawner: Spawner) {
+    let p = embassy_rp::init(Default::default());
+    info!("Hello World!");
+
+    let (tx, rx, uart) = (p.PIN_0, p.PIN_1, p.UART0);
+
+    let config = Config::default();
+    let mut uart = Uart::new_blocking(uart, tx, rx, config);
+
+    // We can't send too many bytes, they have to fit in the FIFO.
+    // This is because we aren't sending+receiving at the same time.
+
+    let data = [0xC0, 0xDE];
+    uart.blocking_write(&data).unwrap();
+
+    let mut buf = [0; 2];
+    uart.blocking_read(&mut buf).unwrap();
+    assert_eq!(buf, data);
+
+    info!("Test OK");
+    cortex_m::asm::bkpt();
+}
diff --git a/tests/rp/src/bin/uart_dma.rs b/tests/rp/src/bin/uart_dma.rs
new file mode 100644
index 000000000..963c22707
--- /dev/null
+++ b/tests/rp/src/bin/uart_dma.rs
@@ -0,0 +1,32 @@
+#![no_std]
+#![no_main]
+#![feature(type_alias_impl_trait)]
+
+use defmt::{assert_eq, *};
+use embassy_executor::Spawner;
+use embassy_rp::uart::{Config, Uart};
+use {defmt_rtt as _, panic_probe as _};
+
+#[embassy_executor::main]
+async fn main(_spawner: Spawner) {
+    let p = embassy_rp::init(Default::default());
+    info!("Hello World!");
+
+    let (tx, rx, uart) = (p.PIN_0, p.PIN_1, p.UART0);
+
+    let config = Config::default();
+    let mut uart = Uart::new(uart, tx, rx, p.DMA_CH0, p.DMA_CH1, config);
+
+    // We can't send too many bytes, they have to fit in the FIFO.
+    // This is because we aren't sending+receiving at the same time.
+
+    let data = [0xC0, 0xDE];
+    uart.write(&data).await.unwrap();
+
+    let mut buf = [0; 2];
+    uart.read(&mut buf).await.unwrap();
+    assert_eq!(buf, data);
+
+    info!("Test OK");
+    cortex_m::asm::bkpt();
+}