firmware/examples/zedboard/Cargo.toml

@@ -20,7 +20,7 @@ zedboard-bsp = { path = "../../zedboard-bsp" }
 num_enum = { version = "0.7", default-features = false }
 l3gd20 = { git = "https://github.com/us-irs/l3gd20.git", branch = "add-async-if" }
 embedded-io = "0.7"
-bitbybit = "1.4"
+bitbybit = "2"
 arbitrary-int = "2"
 embedded-io-async = "0.7"
 critical-section = "1"
@@ -30,7 +30,7 @@ embedded-hal = "1"
 embedded-hal-async = "1"
 fugit = "0.3"
 log = "0.4"
-rand = { version = "0.9", default-features = false, features = ["small_rng"] }
+rand = { version = "0.10", default-features = false }
 
 embassy-executor = { git = "https://github.com/us-irs/embassy.git", rev = "fd40f3e2f2efb67434a9e7d90eb35a30e30d1736", features = [
   "arch-cortex-ar",

firmware/examples/zedboard/src/bin/qspi.rs

@@ -30,6 +30,7 @@ fn entry_point() -> ! {
 }
 
 const ERASE_PROGRAM_READ_TEST: bool = false;
+const TEST_QSPI_BASE: u32 = 0x20000;
 
 #[embassy_executor::main]
 async fn main(_spawner: Spawner) -> ! {
@@ -127,17 +128,36 @@ async fn main(_spawner: Spawner) -> ! {
     if ERASE_PROGRAM_READ_TEST {
         info!("performing erase, program, read test");
         spansion_qspi
-            .erase_sector(0x10000)
+            .erase_sector(TEST_QSPI_BASE)
             .expect("erasing sector failed");
-        spansion_qspi.read_fast_read(0x10000, &mut read_buf, true);
+        spansion_qspi.read_fast_read(TEST_QSPI_BASE, &mut read_buf, true);
         for read in read_buf.iter() {
             assert_eq!(*read, 0xFF);
         }
         read_buf.fill(0);
-        spansion_qspi.write_pages(0x10000, &write_buf).unwrap();
-        spansion_qspi.read_fast_read(0x10000, &mut read_buf, true);
-        for (read, written) in read_buf.iter().zip(write_buf.iter()) {
-            assert_eq!(read, written);
+        let mut current_offset = 0_usize;
+        let mut current_qspi_offset = TEST_QSPI_BASE;
+        for (idx, chunk) in write_buf
+            .chunks(qspi_spansion::RECOMMENDED_PROGRAM_PAGE_SIZE)
+            .enumerate()
+        {
+            spansion_qspi
+                .program(current_qspi_offset, chunk)
+                .expect("programming failed");
+            spansion_qspi.read_fast_read(
+                current_qspi_offset,
+                &mut read_buf[current_offset..current_offset + chunk.len()],
+                true,
+            );
+            assert_eq!(
+                chunk,
+                &read_buf[current_offset..current_offset + chunk.len()],
+                "read and write missmatch at chunk index {}, data offset {}",
+                idx,
+                current_offset
+            );
+            current_offset += chunk.len();
+            current_qspi_offset += chunk.len() as u32;
         }
         info!("test successful");
     }
@@ -147,7 +167,7 @@ async fn main(_spawner: Spawner) -> ! {
     let guard = spansion_lqspi.read_guard();
     unsafe {
         core::ptr::copy_nonoverlapping(
-            (qspi::QSPI_START_ADDRESS + 0x10000) as *const u8,
+            (qspi::QSPI_START_ADDRESS + TEST_QSPI_BASE as usize) as *const u8,
             read_buf.as_mut_ptr(),
             256,
         );

firmware/zedboard-bsp/CHANGELOG.md

@@ -10,8 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Fixed
 
-- QSPI robustness fixes. Read and fast-read operations are now chunked according to the 252 byte
-  limit specified in the TRM.
+- QSPI robustness fixes. Read, fast-read and write operations are now chunked according to the 252
+  byte limit specified in the TRM.
 
 ## Added
 

firmware/zedboard-bsp/Cargo.toml

@@ -13,6 +13,7 @@ categories = ["embedded", "no-std", "hardware-support"]
 zynq7000 = { path = "../zynq7000", version = "0.1" }
 zynq7000-hal = { path = "../zynq7000-hal", version = "0.1" }
 bitbybit = "2"
+log = "0.4"
 arbitrary-int = "2"
 num_enum = { version = "0.7", default-features = false }
 thiserror = { version = "2", default-features = false }

firmware/zedboard-bsp/src/qspi_spansion.rs

@@ -2,10 +2,17 @@ use core::cell::RefCell;
 
 use arbitrary_int::{prelude::*, u24};
 use zynq7000_hal::qspi::{
-    FIFO_DEPTH, LinearQspiConfig, MAX_BYTES_PER_TRANSFER_IO_MODE, QspiIoMode, QspiIoTransferGuard,
-    QspiLinearAddressing, QspiLinearReadGuard,
+    FIFO_DEPTH, LinearQspiConfig, MAX_BYTES_PER_TRANSFER_IO_MODE, QspiIoMode, QspiLinearAddressing,
+    QspiLinearReadGuard,
 };
 
+/// 4 bytes are reserved for command byte and address. Rounded down at a 16 byte boundary,
+/// recommended by flash memory datasheet.
+pub const MAX_DATA_BYTES_PER_WRITE: usize = 240;
+/// Probably the most performant chunk/program size to program the chip without crossing page
+/// boundaries without exceeding the FIFO size.
+pub const RECOMMENDED_PROGRAM_PAGE_SIZE: usize = 128;
+
 pub const QSPI_DEV_COMBINATION_REV_F: zynq7000_hal::qspi::QspiDeviceCombination =
     zynq7000_hal::qspi::QspiDeviceCombination {
         vendor: zynq7000_hal::qspi::QspiVendor::WinbondAndSpansion,
@@ -13,7 +20,8 @@ pub const QSPI_DEV_COMBINATION_REV_F: zynq7000_hal::qspi::QspiDeviceCombination
         two_devices: false,
     };
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(u8)]
 pub enum RegisterId {
     /// WRR
     WriteRegisters = 0x01,
@@ -160,7 +168,7 @@ impl ExtendedDeviceId {
     }
 }
 
-#[bitbybit::bitfield(u8, debug)]
+#[bitbybit::bitfield(u8, debug, forbid_overlaps)]
 pub struct StatusRegister1 {
     #[bit(7, rw)]
     status_register_write_disable: bool,
@@ -176,7 +184,7 @@ pub struct StatusRegister1 {
     write_in_progress: bool,
 }
 
-#[bitbybit::bitfield(u8, debug)]
+#[bitbybit::bitfield(u8, debug, forbid_overlaps)]
 pub struct ConfigRegister1 {
     #[bits(6..=7, rw)]
     latency_code: u2,
@@ -216,11 +224,15 @@ pub enum ProgramPageError {
     ProgrammingErrorBitSet,
     #[error("address error: {0}")]
     Addr(#[from] AddrError),
-    #[error("data is larger than page size {PAGE_SIZE}")]
-    DataLargerThanPage,
+    #[error("program data is larger than page size {PAGE_SIZE}")]
+    DataTooLarge,
+    #[error("program data is not aligned to 16 bytes")]
+    NotAligned,
+    #[error("program data crosses page boundary")]
+    CrossesPageBoundary,
 }
 
-#[derive(Default, Debug, PartialEq, Eq)]
+#[derive(Default, Debug, PartialEq, Eq, Copy, Clone)]
 pub struct Config {
     pub set_quad_bit_if_necessary: bool,
     pub latency_config: Option<u2>,
@@ -496,12 +508,9 @@ impl QspiSpansionS25Fl256SIoMode {
         if addr + data.len() as u32 > u24::MAX.as_u32() {
             return Err(AddrError::OutOfRange.into());
         }
-        if !addr.is_multiple_of(PAGE_SIZE as u32) {
-            return Err(AddrError::Alignment.into());
-        }
-        for chunk in data.chunks(PAGE_SIZE) {
+        for chunk in data.chunks(RECOMMENDED_PROGRAM_PAGE_SIZE) {
             self.program(addr, chunk)?;
-            addr += PAGE_SIZE as u32;
+            addr += chunk.len() as u32;
         }
         Ok(())
     }
@@ -509,13 +518,21 @@ impl QspiSpansionS25Fl256SIoMode {
     /// This function also takes care of enabling writes before programming the page.
     /// This function will block until the operation has completed.
     ///
-    /// The data length max not exceed the page size [PAGE_SIZE].
+    /// The data length may not exceed [MAX_DATA_BYTES_PER_WRITE]. Furthermore, the data needs
+    /// to be aligned to 16 bytes and the programming operation is not allowed to cross a page.
+    /// boundary. It is recommended to program in 128 byte chunks.
     pub fn program(&mut self, addr: u32, data: &[u8]) -> Result<(), ProgramPageError> {
         if addr + data.len() as u32 > u24::MAX.as_u32() {
             return Err(AddrError::OutOfRange.into());
         }
-        if data.len() > PAGE_SIZE {
-            return Err(ProgramPageError::DataLargerThanPage);
+        if data.len() > MAX_DATA_BYTES_PER_WRITE {
+            return Err(ProgramPageError::DataTooLarge);
+        }
+        if !data.len().is_multiple_of(16) {
+            return Err(ProgramPageError::NotAligned);
+        }
+        if (addr as usize % PAGE_SIZE) + data.len() > PAGE_SIZE {
+            return Err(ProgramPageError::CrossesPageBoundary);
         }
         self.write_enable();
         let qspi = self.0.get_mut();
@@ -529,7 +546,8 @@ impl QspiSpansionS25Fl256SIoMode {
         transfer.write_word_txd_00(u32::from_ne_bytes(raw_word));
         let mut read_index: u32 = 0;
         let mut current_byte_index = 0;
-        let fifo_writes = data.len().div_ceil(4);
+        // Full four byte writes.
+        let fifo_writes = data.len() / 4;
         // Fill the FIFO until it is full.
         for _ in 0..core::cmp::min(fifo_writes, FIFO_DEPTH - 1) {
             transfer.write_word_txd_00(u32::from_ne_bytes(
@@ -539,51 +557,10 @@ impl QspiSpansionS25Fl256SIoMode {
             ));
             current_byte_index += 4;
         }
+
         transfer.start();
 
-        let mut wait_for_tx_slot = |transfer: &mut QspiIoTransferGuard| loop {
-            let status_read = transfer.read_status();
-            // Double read to avoid RX underflows as specified in TRM.
-            if status_read.rx_above_threshold() && transfer.read_status().rx_above_threshold() {
-                transfer.read_rx_data();
-                read_index = read_index.wrapping_add(4);
-            }
-            if !status_read.tx_full() {
-                break;
-            }
-        };
-
-        while current_byte_index < data.len() {
-            // Immediately fill the FIFO again with the remaining 8 bytes.
-            wait_for_tx_slot(&mut transfer);
-
-            let word = match core::cmp::min(4, data.len() - current_byte_index) {
-                1 => {
-                    let mut bytes = [0; 4];
-                    bytes[0] = data[current_byte_index];
-                    u32::from_ne_bytes(bytes)
-                }
-                2 => {
-                    let mut bytes = [0; 4];
-                    bytes[0..2].copy_from_slice(&data[current_byte_index..current_byte_index + 2]);
-                    u32::from_ne_bytes(bytes)
-                }
-                3 => {
-                    let mut bytes = [0; 4];
-                    bytes[0..3].copy_from_slice(&data[current_byte_index..current_byte_index + 3]);
-                    u32::from_ne_bytes(bytes)
-                }
-                4 => u32::from_ne_bytes(
-                    data[current_byte_index..current_byte_index + 4]
-                        .try_into()
-                        .unwrap(),
-                ),
-                _ => unreachable!(),
-            };
-            transfer.write_word_txd_00(word);
-            current_byte_index += 4;
-        }
-
+        // Wait until the transfer is done by waiting until all RX bytes have been received.
         while read_index < data.len() as u32 {
             // Double read to avoid RX underflows as specified in TRM.
             let status_read = transfer.read_status();

firmware/zedboard-qspi-flasher/src/main.rs

@@ -145,7 +145,7 @@ fn main() -> ! {
                 }
             }
         }
-        let write_size = core::cmp::min(qspi_spansion::PAGE_SIZE, boot_bin_size - current_addr);
+        let write_size = core::cmp::min(qspi_spansion::RECOMMENDED_PROGRAM_PAGE_SIZE, boot_bin_size - current_addr);
         let write_slice = &boot_bin_slice[current_addr..current_addr + write_size];
         log::debug!("Programming address {:#x}", current_addr);
         match spansion_qspi.program(current_addr as u32, write_slice) {
@@ -160,11 +160,7 @@ fn main() -> ! {
             }
         }
         if VERIFY_PROGRAMMING {
-            spansion_qspi.read_fast_read(
-                current_addr as u32,
-                &mut read_buf[0..write_size],
-                true,
-            );
+            spansion_qspi.read_fast_read(current_addr as u32, &mut read_buf[0..write_size], true);
             if &read_buf[0..write_size] != write_slice {
                 error!(
                     "data verification failed at address {:#x}: wrote {:x?}, read {:x?}",

firmware/zynq7000-hal/src/qspi/mod.rs

@@ -40,6 +40,7 @@ pub const FIFO_DEPTH: usize = 63;
 /// > The maximum number of bytes per command sequence in this mode is limited by the depth of the
 /// > TxFIFO of 252 bytes.
 pub const MAX_BYTES_PER_TRANSFER_IO_MODE: usize = FIFO_DEPTH * 4;
+
 /// In linear-addressed mode, the QSPI is memory-mapped, with the address starting here.
 pub const QSPI_START_ADDRESS: usize = 0xFC00_0000;