From a2d1efde46ae67ae3ee856fe728ecfc3424807df Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 Mar 2026 14:54:56 +0100 Subject: [PATCH] another tweak for the QSPI spansion driver --- firmware/examples/zedboard/Cargo.toml | 4 +- firmware/examples/zedboard/src/bin/qspi.rs | 34 ++++++-- firmware/zedboard-bsp/CHANGELOG.md | 4 +- firmware/zedboard-bsp/Cargo.toml | 1 + firmware/zedboard-bsp/src/qspi_spansion.rs | 97 +++++++++------------- firmware/zedboard-qspi-flasher/src/main.rs | 8 +- firmware/zynq7000-hal/src/qspi/mod.rs | 1 + 7 files changed, 72 insertions(+), 77 deletions(-) diff --git a/firmware/examples/zedboard/Cargo.toml b/firmware/examples/zedboard/Cargo.toml index 3039dbf..5290944 100644 --- a/firmware/examples/zedboard/Cargo.toml +++ b/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", diff --git a/firmware/examples/zedboard/src/bin/qspi.rs b/firmware/examples/zedboard/src/bin/qspi.rs index dd7159a..4ed8253 100644 --- a/firmware/examples/zedboard/src/bin/qspi.rs +++ b/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, ); diff --git a/firmware/zedboard-bsp/CHANGELOG.md b/firmware/zedboard-bsp/CHANGELOG.md index 5a84e9d..7d42edb 100644 --- a/firmware/zedboard-bsp/CHANGELOG.md +++ b/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 diff --git a/firmware/zedboard-bsp/Cargo.toml b/firmware/zedboard-bsp/Cargo.toml index b0b6164..206d2c7 100644 --- a/firmware/zedboard-bsp/Cargo.toml +++ b/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 } diff --git a/firmware/zedboard-bsp/src/qspi_spansion.rs b/firmware/zedboard-bsp/src/qspi_spansion.rs index d3dd194..3858c6f 100644 --- a/firmware/zedboard-bsp/src/qspi_spansion.rs +++ b/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, @@ -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(); diff --git a/firmware/zedboard-qspi-flasher/src/main.rs b/firmware/zedboard-qspi-flasher/src/main.rs index 7f0454a..b6c9da3 100644 --- a/firmware/zedboard-qspi-flasher/src/main.rs +++ b/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?}", diff --git a/firmware/zynq7000-hal/src/qspi/mod.rs b/firmware/zynq7000-hal/src/qspi/mod.rs index f7a0c62..5046441 100644 --- a/firmware/zynq7000-hal/src/qspi/mod.rs +++ b/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; -- 2.43.0