FLC Rework

hal/src/peripherals/flash_controller.rs

@@ -111,6 +111,52 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
         Ok(())
     }
 
+    /// Busy loop until FLC is ready.
+    ///
+    /// This MUST be called BEFORE any FLC operation EXCEPT clearing interrupts.
+    fn wait_until_ready(&self) {
+        while !self.flc.ctrl().read().pend().bit_is_clear() {}
+    }
+
+    /// Clear any stale errors in the FLC Interrupt Register
+    ///
+    /// This can be called without waiting for the FLC to be ready.
+    fn clear_interrupts(&self) {
+        self.flc.intr().modify(|_, w| w.af().clear_bit());
+    }
+
+    /// Prepares the FLC for a write operation.
+    ///
+    /// Procedure:
+    ///  - Wait until FLC ready
+    ///  - Disable icc0
+    ///  - Clear FLC interrupts
+    ///  - Set clock divisor
+    ///  - Unlock write protection
+    ///  - EXECUTE OPERATION (CLOSURE)
+    ///  - Wait until FLC ready
+    ///  - Lock write protection
+    ///  - Flush ICC
+    ///  - Enable icc0
+    fn write_guard<F: Fn()>(&self, sys_clk: &SystemClock, operation: F) -> Result<(), FlashErr> {
+        // Pre-write
+        self.wait_until_ready();
+        self.disable_icc0();
+        self.clear_interrupts();
+        self.set_clock_divisor(sys_clk)?;
+        self.unlock_write_protection();
+
+        operation();
+
+        // Post-write
+        self.wait_until_ready();
+        self.lock_write_protection();
+        self.flush_icc()?;
+        self.enable_icc0();
+
+        Ok(())
+    }
+
     /// Flushes the flash line buffer and arm instruction cache.
     ///
     /// This MUST be called after any write/erase flash controller operations.
@@ -123,6 +169,7 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
         let mut empty_buffer = [0; 4];
         self.read_bytes(ptr, &mut empty_buffer)?;
         self.read_bytes(ptr + FLASH_PAGE_SIZE, &mut empty_buffer)?;
+
         Ok(())
     }
 
@@ -175,7 +222,7 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
         }
 
         for byte in word_chunk.into_remainder() {
-            // SAFETY:
+            // SAFETY:CLOSURE
             // * src is valid for reads. Because read range is checked at the
             // beginning of function.
             //
@@ -195,7 +242,6 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
         Ok(())
     }
 
-    ///
     /// # Safety
     ///
     /// Writes must not corrupt potentially executable instructions of the program.
@@ -214,10 +260,6 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
     ) -> Result<(), FlashErr> {
         self.check_address_bounds(address..(address + data.len() as u32))?;
 
-        self.set_clock_divisor(sys_clk)?;
-
-        self.disable_icc0();
-
         // Check alignment
         let mut physical_addr = address;
         let bytes_unaligned = if (address & 0xF) > 0 {
@@ -229,7 +271,7 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
         // Write unaligned data
         if bytes_unaligned > 0 {
             let unaligned_data = &data[0..core::cmp::min(bytes_unaligned, data.len())];
-            self.write_lt_128_unaligned(physical_addr, unaligned_data)?;
+            self.write_lt_128_unaligned(physical_addr, unaligned_data, sys_clk)?;
             physical_addr += unaligned_data.len() as u32;
         }
 
@@ -244,25 +286,37 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
             let mut buffer_128_bits: [u32; 4] = [0; 4];
             word.enumerate()
                 .for_each(|(idx, chunk)| buffer_128_bits[idx] = chunk);
-            self.write128(physical_addr, &buffer_128_bits)?;
+            self.write128(physical_addr, &buffer_128_bits, sys_clk)?;
             physical_addr += 16;
         }
 
         // remainder from chunks
         if !chunk_8.remainder().is_empty() {
-            self.write_lt_128_unaligned(physical_addr, chunk_8.remainder())?;
+            self.write_lt_128_unaligned(physical_addr, chunk_8.remainder(), sys_clk)?;
         }
 
-        self.flush_icc()?;
-
-        self.enable_icc0();
-
         Ok(())
     }
 
-    /// Writes less than 128 bits (16 bytes) of data to flash. Data needs to fit
-    /// within one flash word (16 bytes).
-    fn write_lt_128_unaligned(&self, address: u32, data: &[u8]) -> Result<(), FlashErr> {
+    /// Writes less than 128 bits (16 bytes) of data to flash.
+    /// Data needs to fit within one flash word (16 bytes).
+    ///
+    /// # Safety
+    ///
+    /// Writes must not corrupt potentially executable instructions of the program.
+    /// Callers must ensure that the following condition is met:
+    /// * If `address` points to a portion of the program's instructions, `data` must
+    ///   contain valid instructions that does not introduce undefined behavior.
+    ///
+    /// It is very difficult to define what would cause undefined behavior when
+    /// modifying program instructions. This would almost certainly result
+    /// in unwanted and likely undefined behavior. Do so at your own risk.
+    unsafe fn write_lt_128_unaligned(
+        &self,
+        address: u32,
+        data: &[u8],
+        sys_clk: &SystemClock,
+    ) -> Result<(), FlashErr> {
         // Get byte idx within 128-bit word
         let byte_idx = (address & 0xF) as usize;
 
@@ -284,34 +338,45 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
                 new_data[idx] = u32::from_le_bytes(word_chunk.try_into().unwrap())
             });
 
-        self.write128(aligned_addr, &new_data)
+        self.write128(aligned_addr, &new_data, sys_clk)
     }
 
     /// Writes 128 bits (16 bytes) of data to flash.
-    // make sure to disable ICC with ICC_Disable(); before Running this function
-    fn write128(&self, address: u32, data: &[u32; 4]) -> Result<(), FlashErr> {
+    /// Address must be 128-bit aligned.
+    ///
+    /// # Safety
+    ///
+    /// Writes must not corrupt potentially executable instructions of the program.
+    /// Callers must ensure that the following condition is met:
+    /// * If `address` points to a portion of the program's instructions, `data` must
+    ///   contain valid instructions that does not introduce undefined behavior.
+    ///
+    /// It is very difficult to define what would cause undefined behavior when
+    /// modifying program instructions. This would almost certainly result
+    /// in unwanted and likely undefined behavior. Do so at your own risk.
+    unsafe fn write128(
+        &self,
+        address: u32,
+        data: &[u32; 4],
+        sys_clk: &SystemClock,
+    ) -> Result<(), FlashErr> {
         // Check if adddress is 128-bit aligned
         if address & 0xF > 0 {
             return Err(FlashErr::AddressNotAligned128);
         }
 
-        self.unlock_write_protection();
-
-        // Clear stale errors
-        self.flc.intr().modify(|_, w| w.af().clear_bit());
-
-        while !self.flc.ctrl().read().pend().bit_is_clear() {}
-
-        self.flc.addr().modify(|_, w| w.addr().variant(address));
-        self.flc.data(0).modify(|_, w| w.data().variant(data[0]));
-        self.flc.data(1).modify(|_, w| w.data().variant(data[1]));
-        self.flc.data(2).modify(|_, w| w.data().variant(data[2]));
-        self.flc.data(3).modify(|_, w| w.data().variant(data[3]));
+        self.write_guard(sys_clk, || {
+            self.flc.addr().modify(|_, w| w.addr().variant(address));
+            self.flc.data(0).modify(|_, w| w.data().variant(data[0]));
+            self.flc.data(1).modify(|_, w| w.data().variant(data[1]));
+            self.flc.data(2).modify(|_, w| w.data().variant(data[2]));
+            self.flc.data(3).modify(|_, w| w.data().variant(data[3]));
 
-        self.flc.ctrl().modify(|_, w| w.wr().set_bit());
-        while !self.flc.ctrl().read().wr().is_complete() {}
+            self.flc.ctrl().modify(|_, w| w.wr().set_bit());
 
-        self.lock_write_protection();
+            // Wait until write completes
+            while !self.flc.ctrl().read().wr().is_complete() {}
+        })?;
 
         Ok(())
     }
@@ -327,27 +392,12 @@ impl<'gcr, 'icc> FlashController<'gcr, 'icc> {
     pub unsafe fn page_erase(&self, address: u32, sys_clk: &SystemClock) -> Result<(), FlashErr> {
         self.check_address_bounds(address..address)?;
 
-        if self.set_clock_divisor(sys_clk).is_err() {
-            return Err(FlashErr::FlcClkErr);
-        }
+        self.write_guard(sys_clk, || {
+            self.flc.addr().modify(|_, w| w.addr().variant(address));
 
-        self.unlock_write_protection();
-
-        // Clear stale errors
-        self.flc.intr().modify(|_, w| w.af().clear_bit());
-
-        while !self.flc.ctrl().read().pend().bit_is_clear() {}
-
-        self.flc.addr().modify(|_, w| w.addr().variant(address));
-
-        self.flc.ctrl().modify(|_, w| w.erase_code().erase_page());
-        self.flc.ctrl().modify(|_, w| w.pge().set_bit());
-
-        while !self.flc.ctrl().read().pend().bit_is_clear() {}
-
-        self.lock_write_protection();
-
-        self.flush_icc()?;
+            self.flc.ctrl().modify(|_, w| w.erase_code().erase_page());
+            self.flc.ctrl().modify(|_, w| w.pge().set_bit());
+        })?;
 
         Ok(())
     }

tests/build.rs

@@ -13,6 +13,8 @@ fn main() {
         .write_all(include_bytes!("memory.x"))
         .unwrap();
 
+    println!("cargo:rustc-link-arg=--nmagic");
+
     // Only re-run the build script when this file, memory.x, or link.x is changed.
     println!("cargo:rerun-if-changed=build.rs");
     println!("cargo:rerun-if-changed=memory.x");

tests/memory.x

@@ -5,9 +5,5 @@ MEMORY
     RAM        (rw)  :  ORIGIN = ORIGIN(STACK) + LENGTH(STACK),          LENGTH = 128K - LENGTH(STACK)
 }
 
-/*
-Add a block of memory for the stack before the RAM block, so that a stack overflow leaks into
-reserved space and flash memory, instead of .data and .bss.
-*/
-
 _stack_start = ORIGIN(STACK) + LENGTH(STACK);
+_stack_end = ORIGIN(STACK);

tests/src/main.rs

@@ -58,6 +58,26 @@ fn main() -> ! {
     .configure_timer_2(Oscillator::ISO, Prescaler::_4096)
     .build();
 
+    // run FLC tests with semi-hosting
+    flc_tests::run_flc_tests(
+        &mut stdout,
+        manager.flash_controller().unwrap(),
+        manager.system_clock().unwrap(),
+    );
+
+    {
+        let mut uart = manager.build_uart().unwrap().build(115200);
+
+        // run FLC tests with UART
+        flc_tests::run_flc_tests(
+            &mut uart,
+            manager.flash_controller().unwrap(),
+            manager.system_clock().unwrap(),
+        );
+
+        // UART instance is tossed here
+    }
+
     flc_tests::run_flc_tests(
         &mut stdout,
         manager.flash_controller().unwrap(),

tests/src/tests/flc_tests.rs

@@ -1,7 +1,6 @@
 //! Flash controller tests
 
 use core::fmt::Write;
-use cortex_m_semihosting::hio;
 use max78000_hal::peripherals::flash_controller::{FlashController, FlashErr};
 use max78000_hal::peripherals::oscillator::{
     Ibro, IbroDivider, IbroFrequency, Iso, IsoDivider, IsoFrequency, Oscillator, SystemClock,
@@ -14,8 +13,8 @@ use max78000_hal::peripherals::PeripheralHandle;
 /// [`flash_write_full_outbounds`],
 /// [`flash_write_partially_outbound_beginning`],
 /// [`flash_write_full_partially_outbound_end`].
-pub fn run_flc_tests(
-    stdout: &mut hio::HostStream,
+pub fn run_flc_tests<T: Write>(
+    stdout: &mut T,
     flash_controller: PeripheralHandle<'_, FlashController<'_, '_>>,
     mut sys_clk: PeripheralHandle<'_, SystemClock<'_, '_>>,
 ) {