DRAFT_PR: Add flexspi + SPI Storage support #312

Shaibal-Microsoft · 2025-02-13T15:45:08Z

This PR adds NXP flexspi support

felipebalbi

This needs to be rewritten considerably. Please remember to use the PAC and follow the pattern of the other drivers.

felipebalbi · 2025-02-21T18:17:17Z

Cargo.toml

-rt = [
-    "mimxrt685s-pac?/rt",
-    "mimxrt633s-pac?/rt",
-]
+rt = ["mimxrt685s-pac?/rt", "mimxrt633s-pac?/rt"]


unnecessary change

Not sure how but its happening automatically. when I added my change, and saved the file, some of the lines are automatically getting formatted. Maybe because I am using rust-analyzer plugin?

felipebalbi · 2025-02-21T18:17:36Z

Cargo.toml

-mimxrt685s-pac = { version = "0.2.2", optional = true, features = ["rt", "critical-section", "defmt"] }
-mimxrt633s-pac = { version = "0.2.0", optional = true, features = ["rt", "critical-section", "defmt"] }
+mimxrt685s-pac = { version = "0.2.2", optional = true, features = [
+    "rt",
+    "critical-section",
+    "defmt",
+] }
+mimxrt633s-pac = { version = "0.2.0", optional = true, features = [
+    "rt",
+    "critical-section",
+    "defmt",
+] }


unnecessary change

felipebalbi · 2025-02-21T18:18:57Z

examples/rt685s-evk/memory.x

-
+

Unnecessary change

felipebalbi · 2025-02-21T18:19:12Z

examples/rt685s-evk/memory.x

+
+

nit: one blank line ought to suffice.

felipebalbi · 2025-02-21T18:19:25Z

examples/rt685s-evk/memory.x

-
+

unnecessary change

felipebalbi · 2025-02-21T18:23:03Z

examples/rt685s-evk/src/bin/flexspi.rs

+    // Copy the Ram code to .flexspi_code partition
+    copy_flexspi_ramcode_to_ram();
+
+    unsafe { asm!("cpsid i") }


https://docs.rs/cortex-m/latest/cortex_m/interrupt/fn.disable.html

felipebalbi · 2025-02-21T18:23:22Z

examples/rt685s-evk/src/bin/flexspi.rs

+
+    cortex_m::asm::dsb();
+
+    unsafe { asm!("cpsie i") }


https://docs.rs/cortex-m/latest/cortex_m/interrupt/fn.enable.html

felipebalbi · 2025-02-21T18:24:59Z

src/flexspi.rs

+struct FlexSpi {
+    MCR0: u32,
+    /**< Module Control Register 0, offset: 0x0 */
+    MCR1: u32,
+    /**< Module Control Register 1, offset: 0x4 */
+    MCR2: u32,
+    /**< Module Control Register 2, offset: 0x8 */
+    AHBCR: u32,
+    /**< AHB Bus Control Register, offset: 0xC */
+    INTEN: u32,
+    /**< Interrupt Enable Register, offset: 0x10 */
+    INTR: u32,
+    /**< Interrupt Register, offset: 0x14 */
+    LUTKEY: u32,
+    /**< LUT Key Register, offset: 0x18 */
+    LUTCR: u32,
+    /**< LUT Control Register, offset: 0x1C */
+    AHBRXBUFCR0: [u32; 8],
+    /**< AHB RX Buffer 0 Control Register 0..AHB RX Buffer 7 Control Register 0, array offset: 0x20, array step: 0x4 */
+    RESERVED_0: [u8; 32],
+    FLSHCR0: [u32; 4],
+    /**< Flash Control Register 0, array offset: 0x60, array step: 0x4 */
+    FLSHCR1: [u32; 4],
+    /**< Flash Control Register 1, array offset: 0x70, array step: 0x4 */
+    FLSHCR2: [u32; 4],
+    /**< Flash Control Register 2, array offset: 0x80, array step: 0x4 */
+    RESERVED_1: [u8; 4],
+    FLSHCR4: u32,
+    /**< Flash Control Register 4, offset: 0x94 */
+    RESERVED_2: [u8; 8],
+    pub IPCR0: u32,
+    /**< IP Control Register 0, offset: 0xA0 */
+    IPCR1: u32,
+    /**< IP Control Register 1, offset: 0xA4 */
+    RESERVED_3: [u8; 8],
+    IPCMD: u32,
+    /**< IP Command Register, offset: 0xB0 */
+    DLPR: u32,
+    /**< Data Learn Pattern Register, offset: 0xB4 */
+    IPRXFCR: u32,
+    /**< IP RX FIFO Control Register, offset: 0xB8 */
+    IPTXFCR: u32,
+    /**< IP TX FIFO Control Register, offset: 0xBC */
+    DLLCR: [u32; 2],
+    /**< DLL Control Register 0, array offset: 0xC0, array step: 0x4 */
+    RESERVED_4: [u8; 24],
+    STS0: u32,
+    /**< Status Register 0, offset: 0xE0 */
+    STS1: u32,
+    /**< Status Register 1, offset: 0xE4 */
+    STS2: u32,
+    /**< Status Register 2, offset: 0xE8 */
+    AHBSPNDSTS: u32,
+    /**< AHB Suspend Status Register, offset: 0xEC */
+    IPRXFSTS: u32,
+    /**< IP RX FIFO Status Register, offset: 0xF0 */
+    IPTXFSTS: u32,
+    /**< IP TX FIFO Status Register, offset: 0xF4 */
+    RESERVED_5: [u8; 8],
+    RFDR: [u32; 32],
+    /**< IP RX FIFO Data Register 0..IP RX FIFO Data Register 31, array offset: 0x100, array step: 0x4 */
+    TFDR: [u32; 32],
+    /**< IP TX FIFO Data Register 0..IP TX FIFO Data Register 31, array offset: 0x180, array step: 0x4 */
+    LUT: [u32; 120],
+}


Use the PAC:

https://docs.rs/mimxrt685s-pac/latest/mimxrt685s_pac/flexspi/index.html

felipebalbi · 2025-02-21T18:26:15Z

src/flexspi.rs

+        match config.rx_sample_clock {
+            FlexspiReadSampleClock::FlexspiReadSampleClkLoopbackInternally => {
+                regs.mcr0().modify(|_, w| w.rxclksrc().rxclksrc_0());
+            }
+            FlexspiReadSampleClock::FlexspiReadSampleClkLoopbackFromDqsPad => {
+                regs.mcr0().modify(|_, w| w.rxclksrc().rxclksrc_1());
+            }
+            FlexspiReadSampleClock::FlexspiReadSampleClkLoopbackFromSckPad => {
+                regs.mcr0().modify(|_, w| w.rxclksrc().rxclksrc_3());
+            }
+            FlexspiReadSampleClock::FlexspiReadSampleClkExternalInputFromDqsPad => {
+                regs.mcr0().modify(|_, w| w.rxclksrc().rxclksrc_3());
+            }
+        }


https://docs.rs/mimxrt685s-pac/latest/mimxrt685s_pac/flexspi/mcr0/enum.Rxclksrc.html

felipebalbi · 2025-02-21T18:28:20Z

src/flexspi.rs

+        if config.enable_doze {
+            regs.mcr0().modify(|_, w| w.dozeen().set_bit());
+        } else {
+            regs.mcr0().modify(|_, w| w.dozeen().clear_bit());
+        }


https://docs.rs/mimxrt685s-pac/latest/mimxrt685s_pac/flexspi/mcr0/type.DozeenW.html

Additionally, you can use variant():

regs.mcr0().modify(|_, w| w.dozeen().variant(config.enable_doze);

Finally, all of the modify() to the same register, can be combined into a single block, saving on the amount of read-modify-write cycles needed.

jerrysxie · 2025-02-23T22:13:39Z

examples/rt685s-evk/memory.x

 	RAM      : ORIGIN = 0x20080000, LENGTH = 1536K
+	FLEXSPIRAM: ORIGIN = 0x0, LENGTH = 10K


Why is the origin set to 0x0? What VMA does it generate? Can we double check the output with cargo objdump --bin app -- -d --no-show-raw-insn?

jerrysxie · 2025-02-23T22:18:26Z

examples/rt685s-evk/src/bin/flexspi.rs

+
+    flexspi.cmdport.erase_sector(ADDR);
+
+    let status = flexspi.cmdport.read_status_register();


Can we add some comments to what this is doing? write enable -> erase -> verify that it is erased -> write -> read back to verify?

jerrysxie · 2025-02-23T22:20:13Z

src/flexspi.rs

+
+#[repr(C)]
+#[allow(non_snake_case)]
+struct FlexSpi {


Like we talked about, we should assume that PAC is loaded into RAM so let's not shadow these registers in RAM. This will simplify the code a great deal.

jerrysxie · 2025-02-23T22:25:25Z

src/flexspi.rs

+        panic!("Erase operation is not implemented for Data Port. Please use Command Port for erase operation");
+    }
+
+    fn write(&mut self, offset: u32, bytes: &[u8]) -> Result<(), Self::Error> {


We can chunk this array into u32s.

jerrysxie · 2025-02-23T22:30:48Z

src/flexspi.rs

+    const WRITE_SIZE: usize = 512;
+    const ERASE_SIZE: usize = 4096;
+
+    #[link_section = ".data"]


Why is this single function linked to .data section?

jerrysxie · 2025-02-23T22:32:42Z

src/flexspi.rs

+impl FlexSpiCmdPort<Blocking, Ram> {
+    /// Lock Flash
+    pub fn lock_flash(&mut self) {
+        // Lock the command port


Are these to be done later? Can we add a TODO comment or something to identify features that are missing?

jerrysxie · 2025-02-23T22:45:54Z

src/flexspi.rs

+    }
+}
+
+impl BlockingNorFlash for FlexSpiDataPort<Blocking, Ram> {


If we make the assumption that FlexSpi code will be loaded into RAM and reduce the typestate, this will simplify the code a great deal.

jerrysxie · 2025-02-23T22:46:29Z

src/flexspi.rs

+        loop {
+            let mut data = 0;
+
+            data = ((bytes[i + 3] as u32) << 24)


We should consider using DMA for these long copy operation.

Shaibal-Microsoft · 2025-03-04T14:03:56Z

@felipebalbi @jerrysxie This PR will go through a huge clean up once firmware loading on RAM change is merged

examples/rt685s-evk/src/bin/flexspi.rs

Shaibal-Microsoft · 2025-03-05T18:06:13Z

src/spinorstorage.rs

+
+impl crate::storage::BlockingNorStorageDriver for SpiStorage<Blocking> {
+    fn lock(&self) -> Result<(), Self::Error> {
+        Ok(())


@kakarimi Please copy all the SPI device related code to this file from Macronix Device Driver as per the design proposed

Shaibal-Microsoft · 2025-03-05T18:06:48Z

src/spinorstorage.rs

+
+impl SpiStorage<Blocking> {
+    pub fn new_blocking<T: Instance>(_spiinstance: T) -> Self {
+        let info = T::info();


@kakarimi This is where we are going to instantiate the SPI device from HAL

Shaibal-Microsoft · 2025-03-05T18:07:50Z

examples/rt685s-evk/src/bin/flexspistoragedevicedriver.rs

+
+static ADDR: u32 = 0x2F000;
+
+struct StorageDeviceDriver {


@kakarimi struct StorageDeviceDriver is an example equivalent to Macronix Device Driver.

Shaibal-Microsoft · 2025-03-05T18:08:39Z

examples/rt685s-evk/src/bin/flexspistoragedevicedriver.rs

+    pub fn init(&self) {
+        let bus_ref = self.flexspi_nor_storage_bus.as_ref().unwrap();
+        let cmdarr = NorStorageCmdSeq {
+            fast_read: Some(NorStorageCmd {


@kakarimi I have already filled this example from Macronix Datasheet. I believe you should be able to use this code as is.

Shaibal-Microsoft · 2025-03-05T18:10:42Z

src/flexspistorage.rs

+}
+
+impl<M: Mode> crate::storage::ConfigureCmdSeq for FlexspiStorage<M> {
+    fn configure_cmd_seq(&self, cmd_seq: &NorStorageCmdSeq) {}


@kakarimi I need to complete this implementation tomorrow. This will make sure that our commands are ready to be used by the bus driver. Similar change need to be done in SPI storage bus driver file "spinorstorage.rs"

kakarimi · 2025-03-06T19:10:59Z

examples/rt685s-evk/src/bin/flexspi-storage-service.rs

@Shaibal-Microsoft as discussed offline, please consider adding capacity as part of the configuration structure to pass to the driver as opposed to let Boot ROM determine that.

jerrysxie

I have not looked in depth at flexspi code but from a high-level design perspective. We need to extract the generic traits out of embassy-imxrt first.

jerrysxie · 2025-03-07T20:35:32Z

src/storage.rs

+}
+
+/// Blocking NOR Storage Driver
+pub trait BlockingNorStorageDriver: BlockingNorFlash + BlockingReadNorFlash + ConfigureCmdSeq {


This trait does not belong in embassy-imxrt. We will have other non-NXP MCU and independent flash driver implement these traits so storage service can use them. It does not make sense for client to depend on hardware specific embassy-imxrt to get these traits.

We should extract these traits into its own crate like embedded-hal or put these generic trait in embedded-service for now: https://github.com/OpenDevicePartnership/embedded-services/tree/main/embedded-service/src.

Can we create a crate like "Embedded-Storage-Bus" to have the traits to be used by all storage busses like FlexSPI, SPI, I2C, etc?

Sure, but the trait as defined feels less like a storage bus, but more like a storage command interface. The new crate can be a separate repo or it can be a part of embedded-service for now.

Thanks. I will move the traits to embedded services for now.

Actually I removed all those commands as I found it not fitting a good bus design. Now it contain only 1 API i.e. "send_command" which fits a good bus API and generic enough to be used with any bus like SPI, FlexSPI, etc

jerrysxie · 2025-03-07T20:36:28Z

src/spi_nor_storage_bus.rs

@@ -0,0 +1,140 @@
+use embassy_hal_internal::Peripheral;


This should be implemented by the SPI HAL. No need for this file to exist without the SPI HAL.

Sure. I created it to showcase the design with multiple storage bus drivers. I will remove it.

- Flexspi NOR Storage Bus Driver - Example Storage Service

Shaibal-Microsoft requested review from felipebalbi, jerrysxie, JamesHuard and madeleyneVaca as code owners February 13, 2025 15:45

felipebalbi requested changes Feb 21, 2025

View reviewed changes

jerrysxie added the enhancement New feature or request label Feb 23, 2025

jerrysxie assigned Shaibal-Microsoft Feb 23, 2025

jerrysxie requested changes Feb 23, 2025

View reviewed changes

Shaibal-Microsoft marked this pull request as draft March 4, 2025 14:02

Shaibal-Microsoft force-pushed the main branch from 30ee3c8 to 9a04a10 Compare March 4, 2025 14:03

Shaibal-Microsoft force-pushed the main branch from 9a04a10 to e56746c Compare March 4, 2025 14:05

Shaibal-Microsoft changed the title ~~DRAFT_PR: Add flexspi support~~ DRAFT_PR: Add flexspi + SPI Storage support Mar 4, 2025

kakarimi reviewed Mar 4, 2025

View reviewed changes

examples/rt685s-evk/src/bin/flexspi.rs Outdated Show resolved Hide resolved

kakarimi reviewed Mar 4, 2025

View reviewed changes

examples/rt685s-evk/src/bin/flexspi.rs Outdated Show resolved Hide resolved

kakarimi reviewed Mar 4, 2025

View reviewed changes

examples/rt685s-evk/src/bin/flexspi.rs Outdated Show resolved Hide resolved

Shaibal-Microsoft force-pushed the main branch 5 times, most recently from 5c7e03a to 0b14a86 Compare March 5, 2025 14:39

Shaibal-Microsoft commented Mar 5, 2025

View reviewed changes

Shaibal-Microsoft force-pushed the main branch from 0b14a86 to d70cdd6 Compare March 6, 2025 10:11

jeffglaum requested a review from dymk March 6, 2025 15:39

kakarimi reviewed Mar 6, 2025

View reviewed changes

Shaibal-Microsoft force-pushed the main branch from 6ba2ca9 to 1b4b886 Compare March 6, 2025 19:20

Shaibal-Microsoft force-pushed the main branch 2 times, most recently from 40dd349 to 65a51fa Compare March 7, 2025 18:13

jerrysxie requested changes Mar 7, 2025

View reviewed changes

Shaibal-Microsoft force-pushed the main branch 10 times, most recently from 7e382e0 to 1f45293 Compare March 10, 2025 06:38

Add Flexspi Storage Bus Driver

2fddf70

- Flexspi NOR Storage Bus Driver - Example Storage Service

Shaibal-Microsoft force-pushed the main branch from 1f45293 to 2fddf70 Compare March 10, 2025 11:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAFT_PR: Add flexspi + SPI Storage support #312

DRAFT_PR: Add flexspi + SPI Storage support #312

Shaibal-Microsoft commented Feb 13, 2025

felipebalbi left a comment

felipebalbi Feb 21, 2025

Shaibal-Microsoft Mar 10, 2025

felipebalbi Feb 21, 2025

felipebalbi Feb 21, 2025

felipebalbi Feb 21, 2025

felipebalbi Feb 21, 2025

felipebalbi Feb 21, 2025

felipebalbi Feb 21, 2025

felipebalbi Feb 21, 2025

felipebalbi Feb 21, 2025

felipebalbi Feb 21, 2025

jerrysxie Feb 23, 2025

jerrysxie Feb 23, 2025

jerrysxie Feb 23, 2025

jerrysxie Feb 23, 2025

jerrysxie Feb 23, 2025

jerrysxie Feb 23, 2025

jerrysxie Feb 23, 2025

jerrysxie Feb 23, 2025

Shaibal-Microsoft commented Mar 4, 2025

Shaibal-Microsoft Mar 5, 2025

Shaibal-Microsoft Mar 5, 2025

Shaibal-Microsoft Mar 5, 2025

Shaibal-Microsoft Mar 5, 2025

Shaibal-Microsoft Mar 5, 2025

kakarimi Mar 6, 2025

jerrysxie left a comment

jerrysxie Mar 7, 2025

Shaibal-Microsoft Mar 8, 2025

jerrysxie Mar 9, 2025

Shaibal-Microsoft Mar 10, 2025 •

edited

Loading

jerrysxie Mar 7, 2025

Shaibal-Microsoft Mar 8, 2025

		RAM : ORIGIN = 0x20080000, LENGTH = 1536K
		FLEXSPIRAM: ORIGIN = 0x0, LENGTH = 10K


		flexspi.cmdport.erase_sector(ADDR);

		let status = flexspi.cmdport.read_status_register();

DRAFT_PR: Add flexspi + SPI Storage support #312

Are you sure you want to change the base?

DRAFT_PR: Add flexspi + SPI Storage support #312

Conversation

Shaibal-Microsoft commented Feb 13, 2025

felipebalbi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shaibal-Microsoft commented Mar 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerrysxie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shaibal-Microsoft Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shaibal-Microsoft Mar 10, 2025 •

edited

Loading