Skip to content

arch/arm/rp23xx: add on-chip flash LittleFS (MTD) support#19255

Open
ricardgb wants to merge 1 commit into
apache:masterfrom
ricardgb:rp23xx-flash-mtd
Open

arch/arm/rp23xx: add on-chip flash LittleFS (MTD) support#19255
ricardgb wants to merge 1 commit into
apache:masterfrom
ricardgb:rp23xx-flash-mtd

Conversation

@ricardgb

@ricardgb ricardgb commented Jul 1, 2026

Copy link
Copy Markdown

⚠️ Disclosure: developed with Claude Code (an AI coding agent) and validated on real Pico 2 W hardware. Please review accordingly.

Summary

Adds an on-chip flash LittleFS for the RP2350 (rp23xx). A new MTD driver (rp23xx_flash_mtd.c) is created over the region of the QSPI flash that lies beyond the NuttX image, and the common board bringup mounts a LittleFS on it for persistent local storage. It uses the RP2350 bootrom flash programming functions.

Design notes:

  • The erase/program routine (do_flash_op) runs from RAM (.ram_code) with interrupts disabled and XIP exited during the operation; the QMI window‑1 (CS1) registers that flash_exit_xip() disturbs on the RP2350 are saved/restored. Erase is chunked one 4 KiB block per critical section to bound the interrupts‑off window.
  • Reads go through the cached XIP window (0x10000000) — the same alias the CPU executes from; freshness after a program/erase is guaranteed because do_flash_op() calls the bootrom flash_flush_cache().
  • Off by default (CONFIG_RP23XX_FLASH_FILE_SYSTEM). Region offset/size and mount point are Kconfig‑configurable.
  • A blank/corrupt optional filesystem is non‑fatal: it is logged and bringup continues (previously a failed mount could abort the rest of bringup). Format once from NSH: mount -t littlefs -o autoformat /dev/rpflash /data.

Impact

  • New: arch/arm/src/rp23xx/rp23xx_flash_mtd.{c,h}.
  • Wiring: arch Make.defs/CMakeLists.txt (compiled only under CONFIG_RP23XX_FLASH_FILE_SYSTEM), arch Kconfig (new options), common bringup (mount block, guarded). No behaviour change when the option is off (the default).

Testing

  • Build: raspberrypi-pico-2:nsh + CONFIG_RP23XX_FLASH_FILE_SYSTEM=y — the driver and bringup compile cleanly; nxstyle clean on all changed files.
  • Hardware (Raspberry Pi Pico 2 W): formatted the FS, wrote/read files, confirmed persistence across a power-cycle (auto-mount of the formatted FS), and copied a code image onto the LittleFS and loaded + executed it from flash:
    nsh> mount -t littlefs -o autoformat /dev/rpflash /data
    nsh> echo hello-littlefs > /data/test.txt
    nsh> cat /data/test.txt
    hello-littlefs
    After a reboot, /data auto-mounts and the contents persist.

🤖 Generated with Claude Code

@ricardgb ricardgb requested a review from jerpelea as a code owner July 1, 2026 17:28
@github-actions github-actions Bot added Arch: arm Issues related to ARM (32-bit) architecture Size: L The size of the change in this PR is large Board: arm labels Jul 2, 2026
Comment thread boards/arm/rp23xx/common/src/rp23xx_common_bringup.c Outdated
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

@Laczen

Laczen commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

@ricardgb I am doubting this is the correct way to setup the mtd on flash for the rp2350. Normally I would expect the mtd to be setup for the whole flash and then to do some partitioning on top of that, not set it up for only a part. If you later want to do some firmware upgrade you also want to have the nuttx part avalable (at e.g. /ota0). Also I think you should always specify the length of the data partition.

So I think there should be 2 parts in the config: 1. Enable mtd_flash support, 2. Enable a mtd_flash data partition (depending on mtd_flash support) with specified mountpoint, offset and size.

@ricardgb

ricardgb commented Jul 4, 2026

Copy link
Copy Markdown
Author

@ricardgb I am doubting this is the correct way to setup the mtd on flash for the rp2350. Normally I would expect the mtd to be setup for the whole flash and then to do some partitioning on top of that, not set it up for only a part. If you later want to do some firmware upgrade you also want to have the nuttx part avalable (at e.g. /ota0). Also I think you should always specify the length of the data partition.

So I think there should be 2 parts in the config: 1. Enable mtd_flash support, 2. Enable a mtd_flash data partition (depending on mtd_flash support) with specified mountpoint, offset and size.

You're right — this should be a whole-flash MTD with mtd_partition() on top, not a pre-offset partial device, so the NuttX/OTA region stays addressable (e.g. /dev/rpflash, /ota0).
I'll restructure the Kconfig into (1) enable flash MTD, (2) a data partition depending on it with explicit mountpoint/offset/size, refactor the driver to be offset-agnostic, and carve the data region with mtd_partition() in bringup. Thanks for the review.

@linguini1 linguini1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for consensus about fully generated PRs.

@Laczen

Laczen commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Waiting for consensus about fully generated PRs.

@linguini1 whether this is fully generated or not doesn't matter. The PR is clean, and the author is willing to do changes to improve it. Yes the PR description is too verbose, but at least it gives a good background for reviewing.

@linguini1

Copy link
Copy Markdown
Contributor

@linguini1 whether this is fully generated or not doesn't matter. The PR is clean, and the author is willing to do changes to improve it. Yes the PR description is too verbose, but at least it gives a good background for reviewing.

I beg to differ. It's not that the PR description itself is the issue, it's that we would be setting a precedent as a project that we accept entirely agent written PRs into the kernel. I am against that, and I know there are others who are against it, which is why I opened a discussion on the mailing list so we could gather feedback. The major issue here is that there is no guarantee that largely unaltered LLM output is compatible with our open source licensing.

@ricardgb ricardgb force-pushed the rp23xx-flash-mtd branch from dfe513a to 58642bb Compare July 6, 2026 06:59
Comment thread arch/arm/src/rp23xx/rp23xx_flash_mtd.c Outdated
@ricardgb ricardgb force-pushed the rp23xx-flash-mtd branch from 58642bb to 8922959 Compare July 6, 2026 17:16
@ricardgb

ricardgb commented Jul 6, 2026

Copy link
Copy Markdown
Author

@ricardgb I am doubting this is the correct way to setup the mtd on flash for the rp2350. Normally I would expect the mtd to be setup for the whole flash and then to do some partitioning on top of that, not set it up for only a part. If you later want to do some firmware upgrade you also want to have the nuttx part avalable (at e.g. /ota0). Also I think you should always specify the length of the data partition.
So I think there should be 2 parts in the config: 1. Enable mtd_flash support, 2. Enable a mtd_flash data partition (depending on mtd_flash support) with specified mountpoint, offset and size.

You're right — this should be a whole-flash MTD with mtd_partition() on top, not a pre-offset partial device, so the NuttX/OTA region stays addressable (e.g. /dev/rpflash, /ota0). I'll restructure the Kconfig into (1) enable flash MTD, (2) a data partition depending on it with explicit mountpoint/offset/size, refactor the driver to be offset-agnostic, and carve the data region with mtd_partition() in bringup. Thanks for the review.

This is now done, thanks

Comment thread arch/arm/src/rp23xx/Kconfig Outdated
@ricardgb ricardgb force-pushed the rp23xx-flash-mtd branch from 8922959 to bc1583e Compare July 6, 2026 23:10
Comment thread arch/arm/src/rp23xx/rp23xx_flash_mtd.c Outdated
Comment thread arch/arm/src/rp23xx/rp23xx_flash_mtd.c Outdated
Comment thread arch/arm/src/rp23xx/rp23xx_flash_mtd.c Outdated
Comment thread arch/arm/src/rp23xx/rp23xx_flash_mtd.c Outdated
Comment thread arch/arm/src/rp23xx/rp23xx_flash_mtd.c Outdated
Comment thread arch/arm/src/rp23xx/rp23xx_flash_mtd.c Outdated
Comment thread arch/arm/src/rp23xx/rp23xx_flash_mtd.c Outdated

if (erase)
{
g_rom.flash_range_erase(addr, count, FLASH_LARGE_BLOCK,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is correct? It seems strange to use FLASH_LARGE_BLOCK.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's correct — this mirrors the Pico SDK's own flash_range_erase(), which calls the bootrom with exactly these arguments: flash_range_erase_func(flash_offs, count, FLASH_BLOCK_SIZE, FLASH_BLOCK_ERASE_CMD), where FLASH_BLOCK_SIZE is (1u << 16) = 65536 and FLASH_BLOCK_ERASE_CMD is 0xD8 (pico-sdk hardware_flash/flash.c).

The 3rd/4th args are the geometry of the optional large block-erase: the bootrom issues the 0xD8 64 KiB block erase only for 64 KiB-aligned 64 KiB runs and falls back to the 4 KiB sector erase (0x20) for anything smaller or unaligned — so it's always safe, including here, where each call erases a single 4 KiB block and therefore always takes the 4 KiB path.

The only oddity is naming: this driver already uses FLASH_BLOCK_SIZE for its 4 KiB MTD erase unit, so the 64 KiB value is named FLASH_LARGE_BLOCK to avoid the clash. I can add a short comment noting the Pico SDK correspondence if that's clearer.

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

please squash into one patch @ricardgb

Add an MTD driver over the RP2350's on-chip QSPI flash, registered as a
whole-chip device (/dev/rpflash) so the NuttX image / OTA region stays
addressable; a bounded data partition (/dev/rpdata) is carved out with
mtd_partition() and a persistent read/write LittleFS is mounted on it via
the common board bringup.

The driver uses the RP2350 bootrom flash programming functions.  The
erase/program routine (do_flash_op) must not be fetched from flash while XIP
is disabled, so it is placed in ".time_critical.flash_mtd", which the RP2350
board linker scripts relocate into RAM.  It runs with interrupts disabled,
one 4 KiB block per critical section, so a large erase (e.g. a LittleFS
format) never holds interrupts off for long or faults on an XIP-cache miss.
Reads go through the cached XIP window -- freshness is guaranteed because
do_flash_op() calls the bootrom flash_flush_cache() after every operation.

The per-operation critical section provides all the hardware atomicity the
driver needs; serializing whole multi-block operations against concurrent
callers is the consumer/filesystem's responsibility under the usual MTD
contract, so the driver holds no mutex (reads are a plain XIP memcpy).

Enabled with CONFIG_RP23XX_FLASH_MTD (off by default); the data partition
offset/size and mount point are configurable.  A blank or corrupt filesystem
is a non-fatal, logged condition (format once from NSH with
'mount -t littlefs -o autoformat /dev/rpdata /data') so it never aborts the
rest of board bringup.

Developed with Claude Code (Anthropic's agentic coding tool) and validated on
a Raspberry Pi Pico 2 W: format, read/write, persistence across an
unmount/remount and a power-cycle, and loading and executing a code image
from the LittleFS.

Signed-off-by: Ricard Rosson <ricard@groundbits.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ricardgb

Copy link
Copy Markdown
Author

Squashed into a single commit (d8c8805), thanks @xiaoxiang781216. The combined message keeps the driver, the .time_critical RAM relocation, and the locking simplification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm Issues related to ARM (32-bit) architecture Board: arm Size: L The size of the change in this PR is large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants