boards: fix mtd_partition() argument units in flash partition loops - #20014
Merged
Conversation
rongbc
requested review from
acassis,
jerpelea and
xiaoxiang781216
as code owners
August 30, 2026 17:21
github-actions
Bot
added
Size: S
|
Contributor
|
@rongbc Please run checkpatch (see more at https://nuttx.apache.org/docs/latest/components/tools/checkpatch.html) Logs: |
github-actions
Bot
added
Size: L
github-actions
Bot
added
Size: M
xiaoxiang781216
approved these changes
Aug 31, 2026
Contributor
|
@rongbc I would propose to test this on multiple fs implementations (especially fatfs). When the block size is smaller than the erase block size two approaches are possible: use 1 block per erase block, use a number of blocks per erase block. Some systems (e.g. fatfs) might consider them equal resulting in a erase before a write, which is only supported when 1 block is used per erase block. There is an option (MTD_SECT512) that allows using more 512B blocks per erase block that takes care of the correct erase and copy when enabled. |
Contributor
|
@rongbc seems like you are using a LLM to help you (even the Summary follow the AI generated pattern), if so please add Assisted-by: The AI Vendor and Model used |
mtd_partition(mtd, firstblock, nblocks) takes the partition offset and size in units of the underlying device "blocks" (geo.blocksize), not erase blocks. Several board drivers accumulated partoffset and computed the partition size in erase-block units and passed them straight to mtd_partition(), so on devices where blocksize != erasesize (W25/SST25: 256B vs 4KB, SAMD5E5 progmem: 512B vs 8KB) every partition came out erasesize/blocksize (16x) too small and misaligned. Convert partoffset and partszbytes to geo.blocksize units at the mtd_partition() call site while keeping the erase-block accumulation. Affected boards: - stm32f103-minimum (W25) - at32f437-mini (W25) - stm32f429i-disco (SST25F064, enabled in the extflash defconfig) - metro-m4 (SAMD5E5 progmem) Also fix pre-existing nxstyle violations in the touched files so the change passes checkpatch (see CONTRIBUTING.md). Assisted-by: DeepSeek Harness:deepseek-v4-flash Signed-off-by: rongbaichuan <rongbaichuan1027@163.com>
acassis
approved these changes
Aug 31, 2026
Contributor
@rongbc I thought this was a more generic change, it seems to be limited to specific boards with their own MTD setup. There is no need to do any further testing as the fatfs is excluded anyhow. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mtd_partition(mtd, firstblock, nblocks)takes the partition offset and size in units of the underlying device "blocks" (geo.blocksize), not erase blocks. The implementation is authoritative:drivers/mtd/mtd_partition.c:875computesblkpererase = erasesize / blocksize;drivers/mtd/mtd_partition.c:885-886divides the incomingfirstblock/nblocksbyblkpereraseto derive erase-block boundaries (erasestart/eraseend), i.e. the inputs are treated asblocksizeunits;part_bread/part_bwrite(drivers/mtd/mtd_partition.c:274/:296) usepriv->firstblockdirectly as a block index into the parent device;include/nuttx/mtd/mtd.h:286("The offset in bytes to the first block") is stale — the implementation is what callers must match.Four board drivers instead accumulated
partoffsetand computed the partition size in erase-block units and passed them straight tomtd_partition(). On devices whereblocksize != erasesize— W25/SST25 SPI NOR (256B vs 4KB) and SAMD5E5 program memory (512B vs 8KB cluster) — every partition came outerasesize/blocksize(16x) too small, and partitions after the first were misaligned/overlapping. On the W25 this is hardware-confirmed: a 512KB partition is reported as 32KB (see Testing).This change converts
partoffsetandpartszbytestogeo.blocksizeunits at themtd_partition()call site while keeping the loop's erase-block accumulation:Affected boards:
stm32f103-minimum(W25Q32FV on SPI1)at32f437-mini(W25, same code as upstreamstm32f103-minimum)stm32f429i-disco(SST25F064; the buggy path is enabled in the shippedextflashdefconfig)metro-m4(SAMD5E5 program memory viamtd_progmem)The remaining
mtd_partition()call sites were audited and are correct:b-l475e-iot01a/stm32l476vg-discoalready usegeo.blocksize;mikroe-stm32f4hardcodes the 256B conversion; the progmem OTA boards (imxrt/nrf5x/samv7/stm32h7) pass page units that equalgeo.blocksize; the ESP32/ESP32C3/ESP32S3, BL602, TLSR82, RTL8720C andfs/partitionpaths are internally consistent.Impact
blocksize/erasesizeof the intended region and should be re-created/reformatted after this change.mtd_partition()signature untouched). Pure board-level code change.blocksize != erasesizeare affected; devices withblocksize == erasesize(e.g.CONFIG_W25_SECT512) behave unchanged.include/nuttx/mtd/mtd.h:286could be fixed separately; out of scope here).Testing
Hardware (bug reproduction, before fix) — environment:
15678acfstm32f103-minimumcode, reproduced on its derived boardstm32f103-mini-v2(STM32F103RCT6)CONFIG_STM32F103MINIMUM_FLASH=y,CONFIG_STM32F103MINIMUM_FLASH_PART=y,CONFIG_MTD_SMART=y,CONFIG_FS_SMARTFS=y; partition list512,512,512,512(4×512KB)./tools/configure.sh -l stm32f103-minimum:xxx && makeTest steps and result:
mksmartfs /dev/smart0p1and mount it.After the fix (this workspace, compile-level):
15678acf+ this change.stm32f103-minimum/src/stm32_w25.candat32f437-mini/src/at32_w25.c:-fsyntax-onlywith the partition path emulated (CONFIG_STM32_SPI1/CONFIG_AT32_SPI1,CONFIG_MTD_W25,CONFIG_*_FLASH_PART,CONFIG_FS_SMARTFS,CONFIG_MTD_SMART, ...) → exit 0, no errors.stm32f429i-disco/src/stm32_bringup.candsamd5e5/metro-m4/src/sam_smartfs.c: full-file syntax check is not possible in this workspace (generatedinclude/nuttx/config.h/include/arch/chippoint at a stale, different board build); the exact edited statements were extracted from disk into a type-stub harness reproducing the real signatures and compiled with-Wall -Wextra→ exit 0, no errors.mtd_partition(..., partszbytes / erasesize)call sites.Expected post-fix hardware result (to be confirmed on board): repeating the same steps above,
/dev/smart0p1should report 512KB, and/dev/mtd0p*offsets should be contiguous and non-overlapping.