Fix various typos in comments and use Reg enum in read_core_register

drv/lpc55-swd/src/armv7debug.rs

@@ -8,6 +8,7 @@
 // https://developer.arm.com/documentation/ddi0403/d/Debug-Architecture?lang=en
 
 use bitflags::bitflags;
+use userlib::FromPrimitive;
 
 pub trait DpAddressable {
     /// Address for accessing a DP Register
@@ -157,7 +158,8 @@ impl Dfsr {
     }
 }
 
-#[derive(PartialEq, Copy, Clone)]
+// See Armv7-M Architecture Reference Manual - C1.6.3 - REGSEL
+#[derive(PartialEq, Copy, Clone, FromPrimitive)]
 #[repr(u16)]
 pub enum Reg {
     R0 = 0b0000000,
@@ -177,16 +179,42 @@ pub enum Reg {
     Lr = 0b0001110,
     Dr = 0b0001111, // DebugReturnAddress, see C1-704
     Xpsr = 0b0010000,
-    Msp = 0b0010001,
-    Psp = 0b0010010,
+    Msp = 0b0010001,   // Main stack pointer
+    Psp = 0b0010010,   // Process stack pointer
     Cfbp = 0b0010100, // [31:24] CONTROL, [23:15] FAULTMASK, [15:8] BASEPRI, [7:0] PRIMASK
-    Fpscr = 0b0100001,
-}
-
-impl From<Reg> for u16 {
-    fn from(r: Reg) -> u16 {
-        r as u16
-    }
+    Fpscr = 0b0100001, // Floating Point Status and Control Register
+    S0 = 0b1000000,
+    S1 = 0b1000001,
+    S2 = 0b1000010,
+    S3 = 0b1000011,
+    S4 = 0b1000100,
+    S5 = 0b1000101,
+    S6 = 0b1000110,
+    S7 = 0b1000111,
+    S8 = 0b1001000,
+    S9 = 0b1001001,
+    S10 = 0b1001010,
+    S11 = 0b1001011,
+    S12 = 0b1001100,
+    S13 = 0b1001101,
+    S14 = 0b1001110,
+    S15 = 0b1001111,
+    S16 = 0b1010000,
+    S17 = 0b1010001,
+    S18 = 0b1010010,
+    S19 = 0b1010011,
+    S20 = 0b1010100,
+    S21 = 0b1010101,
+    S22 = 0b1010110,
+    S23 = 0b1010111,
+    S24 = 0b1011000,
+    S25 = 0b1011001,
+    S26 = 0b1011010,
+    S27 = 0b1011011,
+    S28 = 0b1011100,
+    S29 = 0b1011101,
+    S30 = 0b1011110,
+    S31 = 0b1011111,
 }
 
 pub const _ICSR: u32 = 0xE000ED04;

drv/lpc55-swd/src/main.rs

@@ -49,7 +49,7 @@
 //      up into 9 bit + 8 bit + 8 bit + 9 bit writes
 //   -- 33-bit read (32 bits data, one bit parity) broken up into 8 bit +
 //      8 bit + 8 bit + 9 bit reads. There is also one bit of turnaround after
-//      the read but this is aborbed into idle cycles.
+//      the read but this is absorbed into idle cycles.
 //
 // - The SWD protocol is LSB first. This works very well when bit-banging but
 //   somewhat less well with a register based hardware block such as SPI. The
@@ -70,7 +70,7 @@
 // handler. Note that the SP_RESET handler will change the SP_RESET pin
 // from input to output and back again. By design, there is no code path
 // out of that handler that leaves the SP_RESET pin as an output. If the
-// RoT was to iteself reset, or the swd task to restart, during SP_RESET
+// RoT was to itself reset, or the swd task to restart, during SP_RESET
 // handling, the `setup_pins` call in main would return SP_RESET to its
 // proper configuration.
 //
@@ -105,6 +105,7 @@ use idol_runtime::{
 use lpc55_pac as device;
 use ringbuf::*;
 use static_assertions::const_assert;
+use userlib::FromPrimitive;
 use userlib::{
     hl, set_timer_relative, sys_get_timer, sys_irq_control,
     sys_irq_control_clear_pending, sys_set_timer, task_slot, RecvMessage,
@@ -550,29 +551,12 @@ impl idl::InOrderSpCtrlImpl for ServerImpl {
         _: &RecvMessage,
         register: u16,
     ) -> Result<u32, RequestError<SpCtrlError>> {
-        // C1.6 Debug system registers
-        let r = match register {
-            // R0-R12
-            0b0000000..=0b0001100
-
-            // LR - PSP
-            | 0b0001101..=0b0010010
-
-            // CONTROL/FAULTMASK/BASEPRI/PRIMASK
-            | 0b0010100
-
-            // FPCSR
-            | 0b0100001
-
-            // S0-S31
-            | 0b1000000..=0b1011111 => Ok::<u16, SpCtrlError>(register),
-            _ => Err(SpCtrlError::InvalidCoreRegister)
-        }?;
-
-        if self.write_single_target_addr(DCRSR, r as u32).is_err() {
-            return Err(SpCtrlError::Fault.into());
-        }
-
+        Reg::from_u16(register)
+            .ok_or(SpCtrlError::InvalidCoreRegister)
+            .and_then(|r| {
+                self.write_single_target_addr(DCRSR, r as u32)
+                    .map_err(|_| SpCtrlError::Fault)
+            })?;
         loop {
             match self.dp_read_bitflags::<Dhcsr>() {
                 Ok(dhcsr) => {
@@ -619,7 +603,7 @@ impl idl::InOrderSpCtrlImpl for ServerImpl {
         //
         // At the time of writing this comment, the watchdog timer value used
         // in omicron is 2000ms. With different startup times for the various
-        // SP Hubris applications, it is important to test for watchdog failuew
+        // SP Hubris applications, it is important to test for watchdog failure
         // cases.
         self.watchdog_ms = Some(time_ms);
         Ok(())
@@ -740,7 +724,7 @@ impl NotificationHandler for ServerImpl {
                     } else {
                         // Attempt to do the swap
                         let r = self.swap_sp_slot();
-                        // r.is_err() is potentiall bad. See comment above.
+                        // r.is_err() is potential bad. See comment above.
                         ringbuf_entry!(Trace::WatchdogSwap(r));
                         // Force next user to re-initialize the SWD interface.
                         self.next_use_must_setup_swd();
@@ -767,7 +751,7 @@ impl NotificationHandler for ServerImpl {
             //  when there is a glitch on the JTAG_DETECT signal.
             //
             //  e.g. JTAG_DETECT fired but before the handler was called, it
-            //  deasserted so that the SP_RESET that also fired could be
+            //  de-asserted so that the SP_RESET that also fired could be
             //  handled successfully.
             //}
 
@@ -1728,7 +1712,7 @@ impl ServerImpl {
         {
             if !detected {
                 // Use of sys_irq_control_clear_pending(...) should avoid
-                // appearance of a "spurious" intrerrupt.
+                // appearance of a "spurious" interrupt.
                 // Otherwise, cases where we assert SP_RESET then clean-up the PINT
                 // condition will have a pending notification.
                 ringbuf_entry!(Trace::SpResetNotAsserted);
@@ -1915,7 +1899,7 @@ impl ServerImpl {
 
         // Record a successful measurement before releasing the SP from reset.
         let success = if let Ok(digest) = digest {
-            // SP resets the attestation log and record the new measuremen.
+            // SP resets the attestation log and record the new measurement.
             if !error
                 && self
                     .attest

lib/endoscope-abi/src/lib.rs

@@ -11,7 +11,7 @@
 //! into the SP RAM and runs it. The endoscope program will measure the entire
 //! active flash bank and deposit the resulting Sha3-256 hash into the Shared
 //! structure. The RoT polls the STM32 debug module waiting for a halt or timeout.
-//! The valid measurement is retrieved and recorded if availalble.
+//! The valid measurement is retrieved and recorded if available.
 //!
 
 #![no_std]

lib/endoscope/README.md

@@ -25,7 +25,7 @@ The steps for execution are along these lines:
   - Assert the SP RESET signal to keep the SP
   - Initialize the SWD interface
   - Set VC_CORERESET to trap on the STM32 reset condition.
-  - Deassert the SP RESET signal
+  - De-assert the SP RESET signal
   - ensure that the STM32 halted due to VC_CORERESET
   - Inject the program into the STM32's RAM at the address `__vector_table`.
   - Set The STM32 core register PC/DR to the value in `__vector_table[1]`.
@@ -38,7 +38,7 @@ The steps for execution are along these lines:
       - then read out the `digest` field.
   - Clean up any STM32 debug state.
   - Tear down the SWD session.
-  - Assert and deassert the SP RESET signal to boot the SP from FLASH.
+  - Assert and de-assert the SP RESET signal to boot the SP from FLASH.
 
 On failure, including the presence of an active ST-LINK dongle, the RoT will
 invalidate any previous measurements that have been recorded.
@@ -77,7 +77,7 @@ $ arm-none-eabi-size target/thumbv7em-none-eabihf/release/endoscope
 
 ## Testing
 
-The program can be tested in isolation using gdb. But, it is simpler to
+The program can be tested in isolation using `gdb`. But, it is simpler to
 use the probe-rs-tools.
 
 ### Setup
@@ -122,7 +122,7 @@ VTABLE=0x$(arm-none-eabi-nm -C "${ELF}" | awk '/__vector_table/ {print $1}' -)
 SHARED=0x$(arm-none-eabi-nm -C "${ELF}" | awk '/SHARED/ {print $1}' -)
 ```
 
-### Runing
+### Running
 
 ```sh
 $ time probe-rs run --probe ${PROBE} --chip ${CHIP} "${ELF}"

lib/endoscope/scripts/endoscope.x

@@ -232,7 +232,9 @@ BUG(cortex-m-rt): the reset vector is missing");
 ASSERT(__eexceptions == ADDR(.vector_table) + 0x40, "
 BUG(cortex-m-rt): the exception vectors are missing");
 
-/* XXX We want interrupt vectors to be missing. We are not going to enable any of them. */
+/* Note: We save some more space by removing the unused interrupt vectors.
+ * We are not going to enable any of them.
+ */
 /*
 ASSERT(SIZEOF(.vector_table) > 0x40, "
 ERROR(cortex-m-rt): The interrupt vectors are missing.
@@ -271,7 +273,7 @@ the -fPIC flag. See the documentation of the `cc::Build.pic` method for details.
    *  - Greater than or equal to (Number of Exceptions supported x4)
    * ARMv7m section B1.5.3
    * The Vector table must be naturally aligned to a power of two whose alignment value is greater than or equal to
-   * (Number of Exceptions supported x 4), with a minimum alignmentof 128 bytes.
+   * (Number of Exceptions supported x 4), with a minimum alignment of 128 bytes.
    *
    * Annoyingly this means that vector table alignment is device specific.
    * This is also a nice catch-22: we need to know the size of the vector

lib/endoscope/scripts/stm32h753.x

@@ -3,7 +3,7 @@
 MEMORY
 {
 FLASH (rwx) : ORIGIN = 0x08000000, LENGTH = 0x00100000
-/* RAM is artifically reduced to catch program becoming too large */
+/* RAM is artificially reduced to catch program becoming too large */
 /* RAM (rwx) : ORIGIN   = 0x24000000, LENGTH = 0x00004000 */
 /* STACK (rw) : ORIGIN  = 0x24004000, LENGTH = 0x00001000 */
 /* Use Instruction Tightly Coupled Memory (ITCM) for RAM */