zephyr: device: uart: Implement a blocking transmit

There are a lot of comments here trying to explain how this
implementation works.  This is complicated by the fact that the
interface described in zephyr/drivers/uart.h doesn't actually allow data
to ever be transmitted.

I make a few assumptions here:

- The part about not being able to call `fifo_fill` outside of the
  interrupt is just plain wrong.  Since the irq isn't called until a
  transmit has finished, keeping this constaint would mean no data could
  ever be transmitted.

- Rather than come up with come complex way of sharing the transmitted
  data with the irq handler (which we do with receive), we assume that
  it is permissible to just disable the tx irq from the handler, and can
  turn it back on when writes to the fifo fill it up completely.

I've tested this with an CDC/ACM endpoint, and probably should come up
with a better test for at least some kind of real UART.

It looks like the pl011 uart driver actually tries to implement a
workaround for this nonsensically described interface, and given that,
it maybe actually be the case that this current implementation will just
get spammed with these software irqs.

There is also an async interface, but I don't seem to have any uarts
that actually implement it.

All in all, uart.h in Zephyr needs major work, as these interfaces are
ill-defined.

Signed-off-by: David Brown <[email protected]>

Author: d3zd3z

zephyr/src/device/uart.rs

@@ -11,7 +11,7 @@ use crate::error::{Error, Result, to_result_void, to_result};
 use crate::printkln;
 use crate::sys::sync::Semaphore;
 use crate::sync::{Arc, SpinMutex};
-use crate::time::{NoWait, Timeout};
+use crate::time::{Forever, NoWait, Timeout};
 
 use core::ffi::{c_int, c_uchar, c_void};
 use core::ptr;
@@ -164,13 +164,16 @@ const BUFFER_SIZE: usize = 256;
 /// mutex because it can only be waited on when the Mutex is not locked.
 struct IrqOuterData {
     read_sem: Semaphore,
+    write_sem: Semaphore,
     inner: SpinMutex<IrqInnerData>,
 }
 
 /// Data for communication with the UART IRQ.
 struct IrqInnerData {
     /// The Ring buffer holding incoming and read data.
     buffer: ArrayDeque<u8, BUFFER_SIZE>,
+    /// An indicator to the irq if we are waiting for data.
+    write_waiting: bool,
 }
 
 /// This is the irq-driven interface.
@@ -189,8 +192,10 @@ impl UartIrq {
     pub unsafe fn new(uart: Uart) -> Result<UartIrq> {
         let data = Arc::new(IrqOuterData {
             read_sem: Semaphore::new(0, 1)?,
+            write_sem: Semaphore::new(0, 1)?,
             inner: SpinMutex::new(IrqInnerData {
                 buffer: ArrayDeque::new(),
+                write_waiting: false,
             }),
         });
 
@@ -205,7 +210,7 @@ impl UartIrq {
         to_result_void(ret)?;
         // Should this be settable?
         unsafe {
-            raw::uart_irq_tx_enable(uart.device);
+            // raw::uart_irq_tx_enable(uart.device);
             raw::uart_irq_rx_enable(uart.device);
         }
         Ok(UartIrq {
@@ -240,6 +245,28 @@ impl UartIrq {
 
         self.data.try_read(buf)
     }
+
+    // How the irq handler, and the fifo are documented in uart.h, they are unusable.
+    //
+    /// Write data to the UART.  At this point, this is blocking.  How this interacts with DSR is
+    /// unclear.
+    ///
+    /// TODO: Implement a timeout.  This will need to be able to update the time of the timeout,
+    /// which is not yet implemented in Rust.
+    pub unsafe fn write(&mut self, mut buf: &[u8]) {
+        while buf.len() > 0 {
+            let count = self.data.try_write(self.device, buf);
+
+            if count == buf.len() {
+                // The write has completed.
+                break
+            }
+
+            buf = &buf[count..];
+            // Wait for the write semaphore.
+            let _ = self.data.write_sem.take(Forever);
+        }
+    }
 }
 
 impl IrqOuterData {
@@ -263,6 +290,51 @@ impl IrqOuterData {
         }
         pos
     }
+
+    /// Write as much data from the buffer as we can to the device fifo.  When this fails to write
+    /// any more, return how much was written.
+    ///
+    /// The docs for uart_fifo_fill describe a function that actually can't ever be called, since it
+    /// claims it can only be called from irq context, but the irq isn't called until a transmit is
+    /// done. It is unclear what values are used for watermarks, or anything like that.
+    ///
+    /// We're going to make an assumption here that is _is_ permissible to call this outside of the
+    /// irq context, and that it is safe for the irq to just give a semaphore, and then turn off the
+    /// interupt.
+    ///
+    /// This use of the irq enable is undefined, possibly racy, so to make is possibly more robust,
+    /// we will enable the irq before we try writing, and then turn it off if we think we have
+    /// written everything.  The hope is that it is safer to wake up more than we need to, rather
+    /// than less.
+    ///
+    /// As such, we turn on the irq in this function, and then disable it if the amount written is
+    /// equal to the entire buffer.
+    fn try_write(&self, device: *const raw::device, buf: &[u8]) -> usize {
+        let mut inner = self.inner.lock().unwrap();
+
+        let count = unsafe {
+            raw::uart_fifo_fill(device, buf.as_ptr(), buf.len() as i32)
+        };
+        if count < 0 {
+            panic!("Incorrect use of device fifo");
+        }
+
+        let count = count as usize;
+
+        // It is not clear whether it is safe to turn on the irq at the end, or if that needs to
+        // happen at the begining.  For a conventional device, since this code is running with
+        // interrupts masked, it is likely to finish before a character could finish transmitting,
+        // so it won't ever be seen.
+        if count < buf.len() {
+            // If we have more to write, indicate that we want an irq.
+            unsafe {
+                raw::uart_irq_tx_enable(device);
+                inner.write_waiting = true;
+            }
+        }
+
+        count
+    }
 }
 
 extern "C" fn irq_callback(
@@ -292,4 +364,24 @@ extern "C" fn irq_callback(
     if did_read {
         outer.read_sem.give();
     }
+
+    // We get the same IRQ for the write, but have no way of knowing if that is why we are called.
+    // To be safe, if there is someone waiting, we'll turn off the irq, and give the semaphore.
+    // This means what with full-duplex operation, the writer will wake for every read interrupt,
+    // even if there is still no room in the fifo.  We'll only do this if there is data available in
+    // the fifo.
+    if !inner.write_waiting {
+        // Just finish early, the write irq shouldn't be on.
+        return;
+    }
+
+    let tx_ready = unsafe { raw::uart_irq_tx_ready(dev) };
+    if tx_ready < 0 {
+        panic!("tx ready call returned an error");
+    }
+
+    if tx_ready > 0 {
+        outer.write_sem.give();
+        inner.write_waiting = false;
+    }
 }