From 4bc9236bd72706e2e35bb7485a0c57182688f2d4 Mon Sep 17 00:00:00 2001 From: Julien Calixte Date: Sat, 11 Jul 2026 11:28:22 +0200 Subject: [PATCH] =?UTF-8?q?docs(adr):=20record=20ADR-012=20=E2=80=94=20SD?= =?UTF-8?q?=20on=20its=20own=20SPI3=20host?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document the shared-bus arbitration decision: rather than rework the proven EPD SPI layer and add a cross-thread mutex on the save path, the SD moves to a dedicated SPI3. Update the boot sequence, risk table, Spike 3 postmortem follow-up, hardware overview, and firmware README. --- docs/adr.md | 57 +++++++++++++++++++ docs/hardware.md | 6 +- .../postmortems/2026-07-05-spike3-sd-cmd59.md | 5 +- docs/v0.1-mvp-technical.md | 8 +-- firmware/README.md | 21 ++++--- 5 files changed, 80 insertions(+), 17 deletions(-) diff --git a/docs/adr.md b/docs/adr.md index 9cbc301..be3c455 100644 --- a/docs/adr.md +++ b/docs/adr.md @@ -549,6 +549,63 @@ into [ADR-007](#adr-007-storage-split--fat-on-sd-for-working-copy-littlefs-on-fl --- +## ADR-012: SD on its own SPI3 host (not shared with the EPD on SPI2) + +**Status:** Accepted — 2026-07-11 +**Scope:** v0.1 hardware; whole project. + +### Context + +The EPD (SSD1683) and the SD card both want SPI. The v0.1 plan (the boot +sequence in [v0.1 technical](v0.1-mvp-technical.md#hardware-bring-up-order) and +the storage context of +[ADR-007](#adr-007-storage-split--fat-on-sd-for-working-copy-littlefs-on-flash-for-config)) +assumed **one shared SPI2 bus** with a per-device chip-select. Spike 3 (verified +2026-07-11, [postmortem](postmortems/2026-07-05-spike3-sd-cmd59.md)) proved the +SD works on the SPI2 wiring, but surfaced the integration blocker: the EPD driver +uses esp-idf-hal's `SpiBusDriver`, whose constructor takes an **exclusive +`spi_device_acquire_bus(BLOCK)` and holds it for the driver's whole lifetime** +(it must keep CS asserted across a cmd→data sequence while toggling DC). While +held, no other device on that host can transact — so an SD on SPI2 is locked out +for as long as the panel driver is alive. Compounding it, persistence/git runs on +a **dedicated thread** (Spike 7) while the EPD refreshes on the main task, so SD +and EPD access are genuinely concurrent. + +### Options considered + +| Option | Pros | Cons | +| --- | --- | --- | +| **Shared SPI2, arbitrate** | One bus; ~2 fewer GPIOs. | Rewrite the proven EPD SPI layer to per-transaction device drivers; add a cross-thread mutex around all SPI2 access; residual "corruption on save during render" risk on the highest-value path. | +| **SD on its own SPI3** | EPD code untouched; no arbitration/mutex; each bus at its own clock; matches the risk-table fallback exactly. | ~2 extra GPIOs + traces. | + +### Decision + +**SD gets its own SPI3 host.** The EPD keeps SPI2 and its exclusive-lock model, +unchanged. This is the mitigation the technical-doc risk table already names +("move SD to a separate SPI peripheral — ESP32-S3 has two"). SPI3 is free (SPI0/1 +are flash + PSRAM; nothing else uses SPI3). + +Pins — **SD on SPI3:** SCK 14, MOSI 15, MISO 13, CS 10 (MISO/CS unchanged from +the spike; only SCK/MOSI move off the EPD-shared 12/11). **EPD stays on SPI2:** +SCK 12, MOSI 11, CS 7, DC 6, RST 5, BUSY 4. + +### Consequences + +- No shared-bus arbitration or mutex — the git thread's SD I/O never contends + with an EPD refresh. Removes the "corruption on save during render" risk for + the device's first value (not losing the user's writing). +- Each bus runs at its own clock (EPD ~4 MHz on jumpers; SD 10 MHz+). +- Costs ~2 extra GPIOs + traces; the pin budget has room (avoids flash 26–32, + octal PSRAM 33–37, strapping 0/3/45/46, USB 19/20, RGB 38/48, EPD 4–7/11/12). +- Supersedes the "shared SPI2, different CS" assumption in the boot sequence and + ADR-007's storage context; the `sd_fat` spike is rewired to SPI3 and its + EPD-CS-deselect step (only meaningful on a shared bus) is removed. +- The `SpiBusDriver`-holds-the-lock mechanism was read from the constructor, not + re-verified on silicon; it doesn't affect this decision (SPI3 sidesteps it), + but is the first thing to confirm if a shared bus is ever revisited. + +--- + ## How to add a new ADR 1. Append a new `## ADR-NNN: ` section to this file. diff --git a/docs/hardware.md b/docs/hardware.md index eda76b1..9a41540 100644 --- a/docs/hardware.md +++ b/docs/hardware.md @@ -29,6 +29,6 @@ The board is on the bench and bring-up is largely done — per-spike results live in [`spikes.md`](spikes.md) and [`v0.1-mvp-technical.md`](v0.1-mvp-technical.md#hardware-bring-up-order), with failure write-ups in [`postmortems/`](postmortems/README.md). Notable: the -keyboard runs bus-powered on the S3's native USB port, and SD wiring is -proven on shared SPI2 but blocked on a compatible ≤32 GB card -([postmortem](postmortems/2026-07-05-spike3-sd-cmd59.md)). +keyboard runs bus-powered on the S3's native USB port, and the SD/FAT stack is +verified on a 32 GB card (2026-07-11), now moving to its own SPI3 host per +ADR-012 ([postmortem](postmortems/2026-07-05-spike3-sd-cmd59.md)). diff --git a/docs/postmortems/2026-07-05-spike3-sd-cmd59.md b/docs/postmortems/2026-07-05-spike3-sd-cmd59.md index 8a33e03..708a793 100644 --- a/docs/postmortems/2026-07-05-spike3-sd-cmd59.md +++ b/docs/postmortems/2026-07-05-spike3-sd-cmd59.md @@ -190,8 +190,9 @@ it is explicitly **not recommended** and not applied. (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`. +- [x] Settle the **shared-bus arbitration** decision → **SD on its own SPI3** + (ADR-012, 2026-07-11); spike rewired to SPI3 (SCK 14 / MOSI 15), pending a + bench re-run on the new wiring. - [ ] **Still open:** implement the FatFS atomic-save (unlink-then-rename + `*.tmp` boot-recovery) in the real `persistence` module. diff --git a/docs/v0.1-mvp-technical.md b/docs/v0.1-mvp-technical.md index ff2d79b..cc794bd 100644 --- a/docs/v0.1-mvp-technical.md +++ b/docs/v0.1-mvp-technical.md @@ -65,7 +65,7 @@ read snapshot (render diff). 4. Mount FAT on SD → verify /sd/repo and /sd/repo/notes.md exist ├─ missing → fatal: "missing /sd/repo — re-mount SD and reboot" └─ present → continue -5. Init SPI bus (shared: EPD + SD on different CS) +5. Init two SPI buses: EPD on SPI2, SD on SPI3 (separate hosts — ADR-012) 6. Init EPD, full refresh: splash + boot log 7. Start tasks: usb, wifi (spawned in `Off` state — no radio bring-up), ui, render 8. ui_task opens /sd/repo/notes.md, places cursor, enqueues full render @@ -89,8 +89,8 @@ spike 4 is the gate for — critically — whether `epd-waveshare` already supports the panel's controller (SSD1683-class) or whether we write a thin custom driver against `embedded-hal`. Either way, ~300 LoC; this spike answers which. -3. **Spike 3 — SD.** Mount FAT, read/write a file. Validates SPI sharing with - EPD (or separate bus if needed). +3. **Spike 3 — SD.** Mount FAT, read/write a file. Verified 2026-07-11; the + shared-bus question resolved to a separate SPI3 for the SD (ADR-012). 4. **Spike 4 — USB host.** Enumerate the Nuphy as a boot-protocol HID keyboard, log keycodes over UART. 5. **Spike 5 — Partial refresh.** Type a string letter-by-letter, partial @@ -342,7 +342,7 @@ Mirrored as live conflicts in | TinyUSB host drops HID reports under load | dropped keystrokes during fast typing | enable larger USB rx buffer; if still bad, fall back to BLE-HID for v0.1 | | EPD partial refresh slower than 200 ms | typing feels laggy | reduce font size to shrink dirty area; or render multi-char bursts | | TLS heap pressure on PSRAM | OOM during push | tune mbedtls to smaller cipher suites; force GC of glyph cache before push | -| SD + EPD on same SPI bus collide | corruption on save during render | move SD to a separate SPI peripheral (ESP32-S3 has two) | +| SD + EPD on same SPI bus collide | corruption on save during render | **ADOPTED (ADR-012):** SD on its own SPI3 host | Every one of these is detected by a spike before integration starts — we are not finding them at the end. diff --git a/firmware/README.md b/firmware/README.md index 8815618..b59b42f 100644 --- a/firmware/README.md +++ b/firmware/README.md @@ -68,10 +68,12 @@ 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). +brings up the SD card, mounts FAT at `/sd`, and exercises the persistence +module's atomic save (write `*.tmp` → fsync → rename → read-back). Per ADR-012 +the SD runs on its **own SPI3 host** — **SCK 14 · MOSI 15 · MISO 13 · SD CS 10** +— leaving the EPD alone on SPI2. (The 2026-07-11 bench proof below ran on the +earlier shared-SPI2 wiring; the code is now rewired to SPI3, pending a re-run +after moving the SCK/MOSI jumpers to 14/15.) 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: @@ -85,10 +87,13 @@ atomic round-trip byte-identical. Two findings baked into the code: 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. +**Arbitration resolved (ADR-012):** the EPD driver holds an exclusive SPI2 lock +for its whole lifetime, and persistence runs on its own thread, so a shared bus +would need an EPD rewrite plus a cross-thread mutex on the save path. Instead the +SD gets its own SPI3 — the EPD stays untouched, no arbitration. Remaining before +persistence lands in `main.rs`: re-run the spike on the SPI3 wiring, then wire +the atomic save (unlink-then-rename + `*.tmp` boot-recovery) into a `persistence` +module. **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