You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "casaroli (via GitHub)" <gi...@apache.org> on 2026/10/07 07:56:27 UTC

[PR] arch/arm/rp23xx: Align the flash MTD driver with the Pico SDK [nuttx]

casaroli opened a new pull request, #20480:
URL: https://github.com/apache/nuttx/pull/20480

   ## Summary
   
   I compared `rp23xx_flash_mtd.c` with the `hardware_flash` library of the Pico SDK 2.3.1 and with the RP2350 datasheet. This PR fixes what I found, one commit per item.
   
   1. **Save and restore the QSPI state.** The bootrom flash functions reset the QMI window 1 (PSRAM) registers and the QSPI pads, and `flash_flush_cache()` discards dirty XIP cache lines. So after the first erase or program, PSRAM read garbage, and PSRAM writes still in the cache were lost. Now the driver cleans the cache first (through the alias that avoids erratum RP2350-E11), and saves and restores the pads and the M1 registers, as the SDK does. This removes `rp23xx_psram_restore()`, the earlier fix that nothing called.
   2. **Restore XIP with the bootrom XIP setup function.** The driver called `flash_select_xip_read_mode()` and tested its return value, but that ROM function returns `void`, so the test read a random `r0`. Now the driver copies the XIP setup function that the bootrom leaves in boot RAM to SRAM, and calls it, as the datasheet (5.2.7, 5.4.8.10) and the SDK do. It restores the read mode and clock divisor found at boot.
   3. **Erase and program in small steps.** Interrupts were off for a whole request, which can be seconds. Now they are off for one 64K block erase or one 256 byte page program. On SMP the driver no longer sends the pause call to its own CPU, and the isolation data is static.
   4. **Keep the operation off PSRAM and flash.** During the operation an access to flash or PSRAM is a bus fault. The driver now copies a source page that is in flash or PSRAM to SRAM, and switches to a 1 KiB SRAM stack if the caller's stack is in PSRAM (the normal case with `RP23XX_PSRAM_HEAP_USER`).
   5. **Check the region against the flash size.** An address past the end of the flash wraps around to the NuttX image. The driver now reads the JEDEC ID at initialization and refuses a region that does not fit.
   
   The busy waits in this area are not an erratum workaround. They are the QMI direct-mode protocol (datasheet 12.14.5). The PSRAM side is in apache/nuttx#20478. The two PRs are independent and merge in either order without conflict.
   
   ## Impact
   
   RP2350 boards with `CONFIG_RP23XX_FLASH_MTD`. PSRAM and the flash MTD now work together. No upstream configuration enables both yet, so the PSRAM bugs did not show in CI.
   
   Interrupt latency during a flash write drops to one block erase or one page program. A write is slower, because each page is one bootrom call with a cache flush. The driver uses 256 bytes of SRAM for the XIP setup copy, 256 bytes for the page buffer and, with `CONFIG_RP23XX_PSRAM`, 1 KiB for the stack. `rp23xx_psram_restore()` is removed. It had no callers.
   
   ## Testing
   
   Host: macOS arm64, Arm GNU Toolchain 13.2.rel1 (the CI version).
   
   Builds with no new warnings, each commit on its own too: `pimoroni-pico-2-plus:xipfs` with PSRAM on, `pimoroni-pico-2-plus:xipfs` with `RP23XX_FLASH_MTD_SAFE_XIP`, `pimoroni-pico-2-plus:xipfs-nxflat`, `pimoroni-pico-2-plus:smp` with the flash MTD and PSRAM on, and `raspberrypi-pico-2:nsh` with the flash MTD on. Some of these give the linker warning "LOAD segment with RWX permissions", and xipfs-nxflat gives a format warning in `nxflatxip_main.c`. Master gives the same warnings. All functions that run while the QMI is in direct mode link in SRAM, and call only SRAM or ROM code. `tools/checkpatch.sh -c -u -m -g` passes.
   
   Renode (RP2350 model with the real bootrom, `xipfs_test` on `/dev/rpflash`, PSRAM on):
   
   | build | boot RAM | result |
   |---|---|---|
   | master | empty | 89 passed, 1 failed |
   | this PR | empty, so `flash_enter_cmd_xip()` is used | 89 passed, 1 failed |
   | this PR | a `bx lr` stub, so the SRAM copy is called | 89 passed, 1 failed |
   
   The one failure, "a dot-dot component is refused", is the same on master. Renode does not model the effect of the bootrom on the PSRAM registers or the bus fault in direct mode, so it cannot show the bugs that commits 1 and 4 fix. It does run all the new code paths. The JEDEC ID reads as zero in Renode, so the size check is skipped there.
   
   I did not run this on a board yet. It is a draft until I run on a Pimoroni Pico Plus 2: `xipfs_test`, `ramtest` on the PSRAM after flash writes, and a flash write from a task whose stack is in PSRAM.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/arm/rp23xx: Align the flash MTD driver with the Pico SDK [nuttx]

Posted by "casaroli (via GitHub)" <gi...@apache.org>.
casaroli commented on code in PR #20480:
URL: https://github.com/apache/nuttx/pull/20480#discussion_r4208179656


##########
arch/arm/src/rp23xx/rp23xx_flash_mtd.c:
##########
@@ -172,6 +177,7 @@ struct rp23xx_flash_op_s
   CODE void (*func)(FAR struct rp23xx_flash_op_s *op);
   uint32_t addr;
   FAR const uint8_t *data;
+  FAR uint8_t *rx;

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/arm/rp23xx: Align the flash MTD driver with the Pico SDK [nuttx]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20480:
URL: https://github.com/apache/nuttx/pull/20480#issuecomment-6033685373

   <!-- membrowse-pr-comment -->
   ## <img src="https://membrowse.com/membrowse-logo.svg" height="24" align="top"> MemBrowse Memory Report
   
   *No memory changes detected for:*
   - [arduino-mega2560](https://membrowse.com/public/apache/nuttx/target/arduino-mega2560/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   - [esp32-devkitc](https://membrowse.com/public/apache/nuttx/target/esp32-devkitc/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   - [hifive1-revb](https://membrowse.com/public/apache/nuttx/target/hifive1-revb/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   - [mirtoo](https://membrowse.com/public/apache/nuttx/target/mirtoo/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   - [qemu-armv8a](https://membrowse.com/public/apache/nuttx/target/qemu-armv8a/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   - [qemu-intel64](https://membrowse.com/public/apache/nuttx/target/qemu-intel64/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   - [rx65n-rsk2mb](https://membrowse.com/public/apache/nuttx/target/rx65n-rsk2mb/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   - [s698pm-dkit](https://membrowse.com/public/apache/nuttx/target/s698pm-dkit/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   - [stm32-nucleo-f103rb](https://membrowse.com/public/apache/nuttx/target/stm32-nucleo-f103rb/compare?head=bab2ed4fe7aac201f5e8951ede164bd907b51c61&base=20f3b659372c64220a2b97f43a4c1b31de5c9dd3)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/arm/rp23xx: Align the flash MTD driver with the Pico SDK [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #20480:
URL: https://github.com/apache/nuttx/pull/20480


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] arch/arm/rp23xx: Align the flash MTD driver with the Pico SDK [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20480:
URL: https://github.com/apache/nuttx/pull/20480#discussion_r4206096420


##########
arch/arm/src/rp23xx/rp23xx_flash_mtd.c:
##########
@@ -172,6 +177,7 @@ struct rp23xx_flash_op_s
   CODE void (*func)(FAR struct rp23xx_flash_op_s *op);
   uint32_t addr;
   FAR const uint8_t *data;
+  FAR uint8_t *rx;

Review Comment:
   reuse data, by remove the const from data



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org