Make W25N erase wait for the chip and report what it did

[Raffi1202]

Problem

On a RADIOLINKF405 with a W25N01GV, flash_erase prints Erasing... Done. and flash_info then reports usedSize=0, but after a power cycle flash_info shows the previous usedSize again and the old log is still on the chip. The Configurator "Erase Flash" button behaves the same. Seen on INAV 9.0.1 and confirmed by a second user with the same board on 9.1.0-RC1. Reported in #11376.

Cause

src/main/io/flashfs.c:72-77 on maintenance-10.x: flashfsEraseCompletely() sets the tail address to 0 in RAM without looking at the chip. The value the next boot shows comes from flashfsIdentifyStartOfFreeSpace() (flashfs.c:500, called from flashfsInit() at :583), so a failed erase reads as usedSize=0 until reboot.

src/main/drivers/flash_w25n.c:219-223: w25n_waitForReady() calls w25n_setTimeout() unconditionally. flashPartitionErase() (src/main/drivers/flash.c:313-317) calls flashWaitForReady(0) after every block erase, which replaces the 15 ms deadline armed at flash_w25n.c:306 with one that has already expired, so the wait returns at once. The next w25n_eraseSector() (flash_w25n.c:301-307) discards the result of its own ready-wait and sends write-enable and block-erase into a chip that is still busy. The NOR driver treats 0 as "use the armed deadline" (flash_m25p16.c:216).

That loop only runs when the flash has more than one partition; a single FLASHFS partition takes the full-chip path (flash.c:307-310, w25n_eraseCompletely() at flash_w25n.c:311-316), which already waits per block. The reporter's flash_info lists a single partition, so the timeout defect alone does not account for that report; on that build this change alters what flash_info reports after the erase.

Change

w25n_waitForReady() arms a new deadline only for a non-zero timeout and otherwise waits for the deadline the pending operation armed. w25n_eraseSector() returns without sending commands when the ready-wait fails and invalidates the cached currentPage after issuing the erase; w25n_eraseCompletely() waits for the last block. flashfsEraseCompletely() calls flashFlush() before the erase and afterwards, when flashIsReady(), sets the tail address from flashfsIdentifyStartOfFreeSpace(); a chip still erasing (NOR bulk erase) keeps the previous 0.

Test

Not run on hardware or SITL. Cause verified by reading flash_w25n.c:219-223 and flash.c:313-317 on maintenance-10.x. Not built: the upstream "Build firmware" run for 9d112b9 is waiting for approval (https://github.com/iNavFlight/inav/actions/runs/34619673614) and the fork branch has no workflow runs. Qodo flagged the post-erase scan reading into a dirty W25N program buffer; commit 9d112b9 adds the flashFlush() before the erase for that. Compiled for all targets and the four SITL builds on the fork, green: https://github.com/Raffi1202/inav/actions/runs/34770680514

Flash / RAM

Builds clean on all targets. No size comparison yet: the fork build has no baseline for this branch, and the upstream size report runs once CI is released for this PR.

Docs

No documentation change needed: docs/Cli.md and docs/Blackbox.md describe flash_erase and "erase flash" as erasing the chip; this change adds no setting and no new command.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Make W25N erases blocking and report verified flash usage

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Preserve armed W25N deadlines so partition erases wait for every block.
• Reject erase commands while busy and invalidate stale NAND page-buffer state.
• Re-scan ready flash after erase so reported usage reflects persisted contents.

Diagram

graph TD
  CLI["flash erase"] --> FS["FlashFS erase"] --> PART["Partition erase"] --> W25N["W25N block loop"] --> CHIP["Flash chip"] --> READY{"Flash ready?"}
  READY -->|Ready| SCAN["Scan free space"] --> USAGE["Reported usage"]
  READY -->|Busy| USAGE

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Propagate erase results through the flash API

• ➕ Would provide explicit success or failure instead of inferring results from stored data.
• ➕ Could let callers distinguish timeouts from completed operations.
• ➖ Requires a cross-cutting change from void erase interfaces to structured results.
• ➖ NAND erase-failure status is not actionable without bad-block management and BBLUT support.
• ➖ Treating factory bad blocks as fatal could break erasure on healthy NAND devices.

2. Verify every block immediately after erasing

• ➕ Would identify the exact blocks that retained data.
• ➕ Could support targeted retries where safe.
• ➖ Adds reads and substantial latency to an already multi-second operation.
• ➖ Factory bad blocks complicate interpreting verification failures.
• ➖ Duplicates the existing FlashFS free-space scan for the user-visible requirement.

Recommendation: Keep the PR's scoped approach: repair the W25N deadline contract, prevent commands while busy, invalidate stale page-buffer state, and re-measure completed erases. A result-bearing erase API is a worthwhile future design only when paired with NAND bad-block management; introducing it here would broaden risk without producing reliable failure semantics.

Files changed (2) +43 / -4

Bug fix (2) +43 / -4

flash_w25n.cHonor armed erase deadlines and preserve NAND read consistency +28/-3

Honor armed erase deadlines and preserve NAND read consistency

• A zero wait timeout now reuses the deadline armed by the pending operation, allowing partition erases to block until each NAND block finishes. Sector erase avoids issuing commands to a busy chip, invalidates the cached page, and full-chip emulation waits for the final block.

src/main/drivers/flash_w25n.c

flashfs.cReport flash contents measured after erase +15/-1

Report flash contents measured after erase

• FlashFS now scans ready flash after erasure to derive the actual tail address instead of always reporting zero usage. Devices still busy with asynchronous bulk erasure retain the prior optimistic-zero behavior to avoid misclassifying unreadable flash as full.

src/main/io/flashfs.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Later writes can corrupt erased flash ✓ Resolved 🐞 Bug ≡ Correctness

Description

flashfsEraseCompletely() calls flashfsIdentifyStartOfFreeSpace() without first resolving the
W25N driver's dirty program cache, so the scan's page read replaces the chip buffer while the dirty
flag and old program address remain. When an erase follows an end-of-device partial-page write, a
later close or write executes that scanned page at the pre-erase address, potentially copying
unerased data or consuming a program cycle on freshly erased memory.

Code

src/main/io/flashfs.c[R86-87]

+    if (flashIsReady()) {
+        flashfsSetTailAddress(flashfsIdentifyStartOfFreeSpace());

Evidence

An end-of-device partial write can leave W25N bufferDirty set because only page-boundary writes
execute the buffer. The erase invalidates currentPage but not that dirty state; the added scan
consequently performs a page-data read into the same device buffer, while later close and write
paths execute it using the retained pre-erase address.

src/main/blackbox/blackbox.c[2423-2443]
src/main/drivers/flash_w25n.c[315-330]
src/main/drivers/flash_w25n.c[407-447]
src/main/drivers/flash_w25n.c[474-524]
src/main/io/flashfs.c[93-106]
src/main/io/flashfs.c[548-577]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The post-erase free-space scan can overwrite W25N's internal page buffer while `bufferDirty` still refers to pending pre-erase data. A subsequent flush or write then executes the scanned buffer at the old program address.
## Fix Focus Areas
- src/main/io/flashfs.c[72-87]
- src/main/drivers/flash_w25n.c[407-480]
## Recommended Fix
Resolve the driver's pending program cache before starting the erase, such as by calling `flashFlush()` before `flashPartitionErase()`. Preserve the existing behavior of discarding the separate flashfs write buffer, and ensure no dirty W25N state survives into the post-erase scan.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗