diff --git a/docs/adr.md b/docs/adr.md index 40bbb58..9cbc301 100644 --- a/docs/adr.md +++ b/docs/adr.md @@ -355,6 +355,18 @@ derived key. atomic-rename writes (see [v0.1 technical → `persistence`](v0.1-mvp-technical.md#module-breakdown) and [file layout](v0.1-mvp-technical.md#file-layout)). +- **FatFS caveat (Spike 3, verified 2026-07-11):** FatFS's `f_rename` returns + `FR_EXIST` on an existing destination — it does **not** replace like POSIX + `rename(2)`. So the atomic save must `f_unlink` the target before renaming the + `*.tmp` over it, and pair that with **boot recovery**: a lingering `*.tmp` at + startup means the last save didn't finish → promote it (it is the newest + complete, fsync'd copy). See the + [Spike 3 postmortem](postmortems/2026-07-05-spike3-sd-cmd59.md#resolution-2026-07-11). +- **SD-card compatibility:** use a genuine card, ideally **≤32 GB (SDHC/FAT32)**. + Large or counterfeit SDXC cards may reject `CMD59` (SPI-mode CRC) and fail to + mount; we keep CRC required rather than run the user's writing over an + unchecked bus. The device reports a swap-the-card message instead of a hex + code. --- diff --git a/docs/postmortems/2026-07-05-spike3-sd-cmd59.md b/docs/postmortems/2026-07-05-spike3-sd-cmd59.md index fa08e03..8a33e03 100644 --- a/docs/postmortems/2026-07-05-spike3-sd-cmd59.md +++ b/docs/postmortems/2026-07-05-spike3-sd-cmd59.md @@ -1,8 +1,10 @@ # Spike 3 (SD) — blocked on a card that rejects CMD59 (SPI-mode CRC) > Date: 2026-07-05 · Build at time of failure: `07-05 16:07Z @f77f669-dirty` -> Status: **paused, not failed** — bench card is incompatible; awaiting a -> compliant microSD. Wiring and firmware are proven good. +> Status: **RESOLVED 2026-07-11** — a genuine 32 GB SDHC card mounts, negotiates +> 10 MHz, and round-trips cleanly on the shared SPI2 bus. Wiring and firmware +> were always good; the 133 GB SDXC card's CMD59 rejection was the sole fault. +> See [Resolution](#resolution-2026-07-11) below. > > Context: Spike 3 in > [`../v0.1-mvp-technical.md`](../v0.1-mvp-technical.md#hardware-bring-up-order), @@ -10,6 +12,39 @@ > firmware notes [`../../firmware/README.md`](../../firmware/README.md). > Spike program: [`../../firmware/src/bin/sd_fat.rs`](../../firmware/src/bin/sd_fat.rs). +## Resolution (2026-07-11) + +Re-ran `sd_fat` with a genuine 32 GB SDHC card (build `07-11 08:53Z +@195311a-dirty`). Everything the paused write predicted held: + +- `sdspi_host: data CRC set=1` — **CMD59 now succeeds**. Init runs clean through + card identification (`sdmmc_card_init: card type is SD`). +- `card mounted at /sd — max 10000 kHz, negotiated 10000 kHz`. +- `FAT usage — 29806 MiB total, 29802 MiB free` (~29.8 GiB of the 32 GB card). +- `round-trip OK — 92 bytes: create …tmp → fsync → rename → read back identical`. + +Wiring, pull-ups, bus sharing, and the hand-rolled FFI mount path all confirmed +correct — the only variable that changed was the card, exactly as diagnosed. The +`cmd=52` / `cmd=5` "command not supported" lines still print and are still +normal (SDIO probes a memory card ignores). + +### New finding: FatFS `f_rename` does not overwrite (atomic-save is not POSIX) + +The first run created `/sd/spike3.md`; the **second** run failed at `rename tmp +-> final` with `File exists (os error 17)`. Root cause is a FatFS semantics +difference, not a fault: **`f_rename` returns `FR_EXIST` when the destination +already exists** — unlike POSIX `rename(2)`, which atomically replaces it. So the +textbook write-`*.tmp` → rename-over-target idiom from ADR-007 does **not** work +as-is on FAT; the destination must be unlinked first. + +That unlink opens a crash window: between `f_unlink(target)` and `f_rename`, the +target is momentarily gone while `*.tmp` holds the complete, fsync'd new content. +The consequence for the real persistence module is a **boot-recovery** rule: a +lingering `*.tmp` at startup means the last save didn't finish → promote it +(it's the newest complete copy). The spike now does unlink-then-rename and +tolerates a missing target, so re-runs are idempotent; recorded in +[ADR-007](../adr.md#adr-007-storage-split--fat-on-sd-for-working-copy-littlefs-on-flash-for-config). + ## Summary Built the Spike 3 bench program (`sd_fat`): bring up the SD card over the EPD's @@ -143,16 +178,22 @@ it is explicitly **not recommended** and not applied. ## Follow-ups -- [ ] Re-run with a genuine ≤32 GB card → expect clean mount + round-trip. -- [ ] Strip the debug logging once green: remove `CONFIG_LOG_MAXIMUM_LEVEL_DEBUG` - from `sdkconfig.defaults` and the `esp_log_level_set` block in `sd_fat.rs`. -- [ ] Write up Spike 3 as verified in - [`../../firmware/README.md`](../../firmware/README.md) (wiring, LFN - requirement, card-compatibility note), matching the other spikes. -- [ ] Record a **recommended-SD-card note** for v0.1 (genuine, ≤32 GB; - large/counterfeit SDXC may fail CMD59) — product doc / ADR-007. -- [ ] Settle the **shared-bus arbitration** decision (EPD lock vs. SPI3 for SD). -- [ ] Enable PSRAM, then build Spike 7 (gitoxide push) for the push leg. +- [x] Re-run with a genuine ≤32 GB card → clean mount + round-trip (2026-07-11). +- [x] Strip the debug logging once green: removed `CONFIG_LOG_MAXIMUM_LEVEL_DEBUG` + from `sdkconfig.defaults` and the `esp_log_level_set` block in `sd_fat.rs` + (2026-07-11). +- [x] Write up Spike 3 as verified in + [`../../firmware/README.md`](../../firmware/README.md) (2026-07-11). +- [x] Record a **recommended-SD-card note** for v0.1 (genuine, ≤32 GB; + large/counterfeit SDXC may fail CMD59) — in + [ADR-007](../adr.md#adr-007-storage-split--fat-on-sd-for-working-copy-littlefs-on-flash-for-config) + (2026-07-11). +- [x] Enable PSRAM (done, `CONFIG_SPIRAM`) and build Spike 7 (git push) — both + complete; see [Spike 7 postmortem](2026-07-05-spike7-gix-https-push.md). +- [ ] **Still open:** settle the **shared-bus arbitration** decision (EPD lock + vs. SPI3 for SD) before wiring persistence into `main.rs`. +- [ ] **Still open:** implement the FatFS atomic-save (unlink-then-rename + + `*.tmp` boot-recovery) in the real `persistence` module. ## Artifacts (this session) diff --git a/firmware/README.md b/firmware/README.md index 4c6e88f..fc5080b 100644 --- a/firmware/README.md +++ b/firmware/README.md @@ -66,6 +66,30 @@ of it. `sdkconfig.defaults` gains the full certificate bundle and a bigger main task stack for the mbedtls handshake — a one-time esp-idf reconfigure on the next build. +**Spike 3 — SD card (FAT) on shared SPI2: verified 2026-07-11.** A separate +binary — [`src/bin/sd_fat.rs`](src/bin/sd_fat.rs), flashed with `just flash-sd` — +brings up the SD card on the EPD's SPI2 bus, mounts FAT at `/sd`, and exercises +the persistence module's atomic save (write `*.tmp` → fsync → rename → +read-back). Wiring: **SCK 12 · MOSI 11** (shared with the EPD) **· MISO 13** +(new; the write-only EPD never used it) **· SD CS 10** (EPD CS is 7). + +Bench result (genuine 32 GB SDHC card): mounts at 10 MHz, `29806 MiB total`, +atomic round-trip byte-identical. Two findings baked into the code: + +- **Card compatibility.** A 133 GB SDXC card failed init at `CMD59` (SPI-mode + CRC); a genuine ≤32 GB card works. We keep CRC required and reject bad cards + with a swap-the-card message rather than run over an unchecked bus. See the + [Spike 3 postmortem](../docs/postmortems/2026-07-05-spike3-sd-cmd59.md). +- **FatFS rename ≠ POSIX rename.** `f_rename` won't overwrite an existing + target (returns `FR_EXIST`), so the atomic save unlinks the destination first; + the real persistence module must add `*.tmp` boot-recovery. Long filenames + (`CONFIG_FATFS_LFN_HEAP`) are required for the two-dot `*.md.tmp` name. + +Still open before persistence lands in `main.rs`: the **shared-bus arbitration** +question — the EPD driver holds an exclusive SPI2 lock for its lifetime, so the +EPD and an arbitrated SD device can't both be live on one host yet (release/ +re-acquire around EPD ops, or give the SD its own SPI3). This spike ran SD-only. + **Spike 5 — partial refresh + typing: verified 2026-07-04.** `main.rs` wires the keyboard to the panel: [`src/usb_kbd.rs`](src/usb_kbd.rs) feeds decoded key-downs (US layout, edge-detected) into a queue, and the main loop keeps a @@ -118,8 +142,8 @@ reseat the jumpers (CS first) before debugging code. Next up per [`docs/v0.1-mvp-technical.md`](../docs/v0.1-mvp-technical.md#hardware-bring-up-order): -Wi-Fi/TLS (Spike 6, implemented above), then gitoxide push (Spike 7); SD is -deferred. +Wi-Fi/TLS (Spike 6, implemented above), then git push (Spike 7), then SD +(Spike 3) — all verified. **Spike 1 — Blink: verified 2026-07-04.** GPIO 2 + on-board WS2812 toggled at 1 Hz with `blink N` on USB-serial, proving toolchain, esp-idf link, and