Write a more complete UART driver.

src/SUMMARY.md

@@ -259,6 +259,11 @@
   - [MMIO](bare-metal/aps/mmio.md)
   - [Let's write a UART driver](bare-metal/aps/uart.md)
     - [More traits](bare-metal/aps/uart/traits.md)
+  - [A better UART driver](bare-metal/aps/better-uart.md)
+    - [Bitflags](bare-metal/aps/better-uart/bitflags.md)
+    - [Multiple registers](bare-metal/aps/better-uart/registers.md)
+    - [Driver](bare-metal/aps/better-uart/driver.md)
+    - [Using it](bare-metal/aps/better-uart/using.md)
   - [Logging]()
   - [Other projects](bare-metal/aps/other-projects.md)
 - [Useful crates]()

src/bare-metal/aps/better-uart.md

@@ -0,0 +1,30 @@
+# A better UART driver
+
+The PL011 actually has [a bunch more registers][1], and adding offsets to construct pointers to access
+them is error-prone and hard to read. Plus, some of them are bit fields which would be nice to
+access in a structured way.
+
+| Offset | Register name | Width |
+| ------ | ------------- | ----- |
+| 0x00   | DR            | 12    |
+| 0x04   | RSR           | 4     |
+| 0x18   | FR            | 9     |
+| 0x20   | ILPR          | 8     |
+| 0x24   | IBRD          | 16    |
+| 0x28   | FBRD          | 6     |
+| 0x2c   | LCR_H         | 8     |
+| 0x30   | CR            | 16    |
+| 0x34   | IFLS          | 6     |
+| 0x38   | IMSC          | 11    |
+| 0x3c   | RIS           | 11    |
+| 0x40   | MIS           | 11    |
+| 0x44   | ICR           | 11    |
+| 0x48   | DMACR         | 3     |
+
+<details>
+
+- There are also some ID registers which have been omitted for brevity.
+
+</details>
+
+[1]: https://developer.arm.com/documentation/ddi0183/g/programmers-model/summary-of-registers

src/bare-metal/aps/better-uart/bitflags.md

@@ -0,0 +1,14 @@
+# Bitflags
+
+The [`bitflags`](https://crates.io/crates/bitflags) crate is useful for working with bitflags.
+
+```rust,editable,compile_fail
+{{#include ../examples/src/pl011.rs:Flags}}
+```
+
+<details>
+
+* The `bitflags!` macro creates a newtype something like `Flags(u16)`, along with a bunch of method
+  implementations to get and set flags.
+
+</details>

src/bare-metal/aps/better-uart/driver.md

@@ -0,0 +1,14 @@
+# Driver
+
+Now let's use the new `Registers` struct in our driver.
+
+```rust,editable,compile_fail
+{{#include ../examples/src/pl011.rs:Uart}}
+```
+
+<details>
+
+* Note the use of `addr_of!` / `addr_of_mut!` to get pointers to individual fields without creating
+  an intermediate reference, which would be unsound.
+
+</details>

src/bare-metal/aps/better-uart/registers.md

@@ -0,0 +1,16 @@
+# Multiple registers
+
+We can use a struct to represent the memory layout of the UART's registers.
+
+```rust,editable,compile_fail
+{{#include ../examples/src/pl011.rs:Registers}}
+```
+
+<details>
+
+* [`#[repr(C)]`](https://doc.rust-lang.org/reference/type-layout.html#the-c-representation) tells
+  the compiler to lay the struct fields out in order, following the same rules as C. This is
+  necessary for our struct to have a predictable layout, as default Rust representation allows the
+  compiler to (among other things) reorder fields however it sees fit.
+
+</details>

src/bare-metal/aps/examples/Cargo.lock

@@ -6,6 +6,7 @@ version = 3
 name = "ap-examples"
 version = "0.1.0"
 dependencies = [
+ "bitflags",
  "cc",
  "log",
  "psci",
@@ -18,6 +19,12 @@ version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa"
 
+[[package]]
+name = "bitflags"
+version = "2.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f4f6e5df9abedba5099a01a6567c6086a6fbcff57af07c360d356737f9e0c644"
+
 [[package]]
 name = "cc"
 version = "1.0.79"

src/bare-metal/aps/examples/Cargo.toml

@@ -6,6 +6,7 @@ version = "0.1.0"
 edition = "2021"
 
 [dependencies]
+bitflags = "2.0.0"
 log = "0.4.17"
 psci = "0.1.1"
 spin = "0.9.4"

src/bare-metal/aps/examples/src/main.rs

@@ -17,6 +17,7 @@
 
 mod exceptions;
 mod pl011;
+mod pl011_minimal;
 
 use crate::pl011::Uart;
 use core::{fmt::Write, panic::PanicInfo};
@@ -30,7 +31,8 @@ pub const PL011_BASE_ADDRESS: usize = 0x900_0000;
 extern "C" fn main(x0: u64, x1: u64, x2: u64, x3: u64) {
     // Safe because `PL011_BASE_ADDRESS` is the base address of a PL011 device,
     // and nothing else accesses that address range.
-    let mut uart = unsafe { Uart::new(PL011_BASE_ADDRESS) };
+    let mut uart = unsafe { Uart::new(PL011_BASE_ADDRESS as *mut u32) };
+
     writeln!(uart, "main({:#x}, {:#x}, {:#x}, {:#x})", x0, x1, x2, x3).unwrap();
     system_off().unwrap();
 }

src/bare-metal/aps/examples/src/pl011.rs

@@ -12,15 +12,96 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// ANCHOR: Example
-const FLAG_REGISTER_OFFSET: usize = 0x18;
-const FR_BUSY: u8 = 1 << 3;
-const FR_TXFF: u8 = 1 << 5;
+use core::{
+    fmt::{self, Write},
+    ptr::{addr_of, addr_of_mut},
+};
 
-/// Minimal driver for a PL011 UART.
+// ANCHOR: Flags
+use bitflags::bitflags;
+
+bitflags! {
+    /// Flags from the UART flag register.
+    #[repr(transparent)]
+    #[derive(Copy, Clone, Debug, Eq, PartialEq)]
+    struct Flags: u16 {
+        /// Clear to send.
+        const CTS = 1 << 0;
+        /// Data set ready.
+        const DSR = 1 << 1;
+        /// Data carrier detect.
+        const DCD = 1 << 2;
+        /// UART busy transmitting data.
+        const BUSY = 1 << 3;
+        /// Receive FIFO is empty.
+        const RXFE = 1 << 4;
+        /// Transmit FIFO is full.
+        const TXFF = 1 << 5;
+        /// Receive FIFO is full.
+        const RXFF = 1 << 6;
+        /// Transmit FIFO is empty.
+        const TXFE = 1 << 7;
+        /// Ring indicator.
+        const RI = 1 << 8;
+    }
+}
+// ANCHOR_END: Flags
+
+bitflags! {
+    /// Flags from the UART Receive Status Register / Error Clear Register.
+    #[repr(transparent)]
+    #[derive(Copy, Clone, Debug, Eq, PartialEq)]
+    struct ReceiveStatus: u16 {
+        /// Framing error.
+        const FE = 1 << 0;
+        /// Parity error.
+        const PE = 1 << 1;
+        /// Break error.
+        const BE = 1 << 2;
+        /// Overrun error.
+        const OE = 1 << 3;
+    }
+}
+
+// ANCHOR: Registers
+#[repr(C, align(4))]
+struct Registers {
+    dr: u16,
+    _reserved0: [u8; 2],
+    rsr: ReceiveStatus,
+    _reserved1: [u8; 19],
+    fr: Flags,
+    _reserved2: [u8; 6],
+    ilpr: u8,
+    _reserved3: [u8; 3],
+    ibrd: u16,
+    _reserved4: [u8; 2],
+    fbrd: u8,
+    _reserved5: [u8; 3],
+    lcr_h: u8,
+    _reserved6: [u8; 3],
+    cr: u16,
+    _reserved7: [u8; 3],
+    ifls: u8,
+    _reserved8: [u8; 3],
+    imsc: u16,
+    _reserved9: [u8; 2],
+    ris: u16,
+    _reserved10: [u8; 2],
+    mis: u16,
+    _reserved11: [u8; 2],
+    icr: u16,
+    _reserved12: [u8; 2],
+    dmacr: u8,
+    _reserved13: [u8; 3],
+}
+// ANCHOR_END: Registers
+
+// ANCHOR: Uart
+/// Driver for a PL011 UART.
 #[derive(Debug)]
 pub struct Uart {
-    base_address: *mut u8,
+    registers: *mut Registers,
 }
 
 impl Uart {
@@ -32,38 +113,46 @@ impl Uart {
     /// The given base address must point to the 8 MMIO control registers of a
     /// PL011 device, which must be mapped into the address space of the process
     /// as device memory and not have any other aliases.
-    pub unsafe fn new(base_address: usize) -> Self {
+    pub unsafe fn new(base_address: *mut u32) -> Self {
         Self {
-            base_address: base_address as *mut u8,
+            registers: base_address as *mut Registers,
         }
     }
 
     /// Writes a single byte to the UART.
     pub fn write_byte(&self, byte: u8) {
         // Wait until there is room in the TX buffer.
-        while self.read_flag_register() & FR_TXFF != 0 {}
+        while self.read_flag_register().contains(Flags::TXFF) {}
 
-        // Safe because we know that the base address points to the control
+        // Safe because we know that self.registers points to the control
         // registers of a PL011 device which is appropriately mapped.
         unsafe {
             // Write to the TX buffer.
-            self.base_address.write_volatile(byte);
+            addr_of_mut!((*self.registers).dr).write_volatile(byte.into());
         }
 
         // Wait until the UART is no longer busy.
-        while self.read_flag_register() & FR_BUSY != 0 {}
+        while self.read_flag_register().contains(Flags::BUSY) {}
     }
 
-    fn read_flag_register(&self) -> u8 {
-        // Safe because we know that the base address points to the control
+    /// Reads and returns a pending byte, or `None` if nothing has been received.
+    pub fn read_byte(&self) -> Option<u8> {
+        if self.read_flag_register().contains(Flags::RXFE) {
+            None
+        } else {
+            let data = unsafe { addr_of!((*self.registers).dr).read_volatile() };
+            // TODO: Check for error conditions in bits 8-11.
+            Some(data as u8)
+        }
+    }
+
+    fn read_flag_register(&self) -> Flags {
+        // Safe because we know that self.registers points to the control
         // registers of a PL011 device which is appropriately mapped.
-        unsafe { self.base_address.add(FLAG_REGISTER_OFFSET).read_volatile() }
+        unsafe { addr_of!((*self.registers).fr).read_volatile() }
     }
 }
-// ANCHOR_END: Example
-
-// ANCHOR: Traits
-use core::fmt::{self, Write};
+// ANCHOR_END: Uart
 
 impl Write for Uart {
     fn write_str(&mut self, s: &str) -> fmt::Result {
@@ -77,4 +166,3 @@ impl Write for Uart {
 // Safe because it just contains a pointer to device memory, which can be
 // accessed from any context.
 unsafe impl Send for Uart {}
-// ANCHOR_END: Traits

src/bare-metal/aps/examples/src/pl011_minimal.rs

@@ -0,0 +1,80 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// ANCHOR: Example
+const FLAG_REGISTER_OFFSET: usize = 0x18;
+const FR_BUSY: u8 = 1 << 3;
+const FR_TXFF: u8 = 1 << 5;
+
+/// Minimal driver for a PL011 UART.
+#[derive(Debug)]
+pub struct Uart {
+    base_address: *mut u8,
+}
+
+impl Uart {
+    /// Constructs a new instance of the UART driver for a PL011 device at the
+    /// given base address.
+    ///
+    /// # Safety
+    ///
+    /// The given base address must point to the 8 MMIO control registers of a
+    /// PL011 device, which must be mapped into the address space of the process
+    /// as device memory and not have any other aliases.
+    pub unsafe fn new(base_address: usize) -> Self {
+        Self {
+            base_address: base_address as *mut u8,
+        }
+    }
+
+    /// Writes a single byte to the UART.
+    pub fn write_byte(&self, byte: u8) {
+        // Wait until there is room in the TX buffer.
+        while self.read_flag_register() & FR_TXFF != 0 {}
+
+        // Safe because we know that the base address points to the control
+        // registers of a PL011 device which is appropriately mapped.
+        unsafe {
+            // Write to the TX buffer.
+            self.base_address.write_volatile(byte);
+        }
+
+        // Wait until the UART is no longer busy.
+        while self.read_flag_register() & FR_BUSY != 0 {}
+    }
+
+    fn read_flag_register(&self) -> u8 {
+        // Safe because we know that the base address points to the control
+        // registers of a PL011 device which is appropriately mapped.
+        unsafe { self.base_address.add(FLAG_REGISTER_OFFSET).read_volatile() }
+    }
+}
+// ANCHOR_END: Example
+
+// ANCHOR: Traits
+use core::fmt::{self, Write};
+
+impl Write for Uart {
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        for c in s.as_bytes() {
+            self.write_byte(*c);
+        }
+        Ok(())
+    }
+}
+
+// Safe because it just contains a pointer to device memory, which can be
+// accessed from any context.
+unsafe impl Send for Uart {}
+// ANCHOR_END: Traits

src/bare-metal/aps/uart.md

@@ -3,7 +3,7 @@
 The QEMU 'virt' machine has a [PL011][1] UART, so let's write a driver for that.
 
 ```rust,editable,compile_fail
-{{#include examples/src/pl011.rs:Example}}
+{{#include examples/src/pl011_minimal.rs:Example}}
 ```
 
 [1]: https://developer.arm.com/documentation/ddi0183/g

src/bare-metal/aps/uart/traits.md

@@ -1,5 +1,5 @@
 # More traits
 
 ```rust,editable,compile_fail
-{{#include ../examples/src/pl011.rs:Traits}}
+{{#include ../examples/src/pl011_minimal.rs:Traits}}
 ```