Skip to content

fix(firmware): route SPI2 MISO to GPIO35 so the SD card mounts#575

Merged
andymai merged 1 commit into
mainfrom
fix/sd-spi-gpio35-miso-routing
May 29, 2026
Merged

fix(firmware): route SPI2 MISO to GPIO35 so the SD card mounts#575
andymai merged 1 commit into
mainfrom
fix/sd-spi-gpio35-miso-routing

Conversation

@andymai
Copy link
Copy Markdown
Owner

@andymai andymai commented May 29, 2026

Summary

  • feat(firmware): dynamic SD-SPI clock rate (400 kHz init -> 25 MHz data) #552 added the per-transaction output-enable flip on GPIO35 (the pin CoreS3 shares between LCD DC and SD MISO) but never connected SPI2's FSPIQ (MISO) input signal to the pin in the GPIO matrix. The peripheral therefore read an unconnected input (0x00), embedded-sdmmc spun in its CMD0 retry loop, and the card init failed with CardInit — the SD never mounted.
  • Make GPIO35 a Flex with its input buffer kept live, and route FSPIQ to it via the supported InputSignal::FSPIQ.connect_to(&dc). sd_spi's OE flip still hands the line to the card during reads. Both halves (OE flip + MISO route) are now present; updated the sd_spi module doc and the LcdDisplay type alias accordingly.

Test plan

  • just check-firmware — clean (exit 0).
  • just clippy-firmware (-D warnings) — clean (exit 0).
  • On-device (M5Stack CoreS3, brand-new SD card inserted): flashed --release and watched the defmt boot log:
    • Got response: 0 retries: 51 → 1 (one normal init retry, then success).
    • SD: mount failed (CardInit)gone.
    • Now: SD: read /sd/STACKCHAN.RON failed (FileNotFound); using defaults (card mounts; config file simply absent on the blank card), runtime store: ... seeding from boot config, ble: bonds: 0 persisted bond(s) registered (was "no SD mounted; bonds disabled").
    • ILI9342C ready + render task: 33 ms tick + boot complete — LCD still renders fine through the Flex DC pin; no panic.

Greptile: please review.

#552 added the per-transaction output-enable flip on GPIO35 (shared LCD
DC / SD MISO pin) but never connected SPI2's FSPIQ input signal to the
pin, so the peripheral read an unconnected input (0x00) and embedded-sdmmc
spun in its CMD0 loop until CardInit failed. Make GPIO35 a Flex with its
input buffer live and route FSPIQ to it via InputSignal::connect_to; the
sd_spi OE flip still hands the line to the card during reads.
@andymai andymai merged commit 791341e into main May 29, 2026
12 of 13 checks passed
@andymai andymai deleted the fix/sd-spi-gpio35-miso-routing branch May 29, 2026 18:09
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR fixes a missing half of the GPIO35 dual-purpose pin setup that caused SD card initialisation to fail. #552 added the per-transaction output-enable flip so the SD card could drive MISO, but never wired SPI2's FSPIQ input signal to the pin in the GPIO matrix, so the peripheral always read 0x00 and embedded-sdmmc spun in its CMD0 retry loop.

  • Upgrades GPIO35 from Output to Flex, enables its input buffer, and calls InputSignal::FSPIQ.connect_to(&dc) so SPI2's MISO is properly routed to the physical pin — the missing half that caused CardInit failures.
  • Updates the LcdDisplay type alias to reflect the Flex<'static> DC pin, and rewrites the sd_spi module doc to explain that both the FSPIQ route and the OE flip are required together.

Confidence Score: 5/5

Safe to merge — the change is a focused, well-understood hardware fix with on-device confirmation that both the LCD and SD card work correctly after it.

The fix is minimal and surgical: one pin type change, one InputSignal routing call, a type alias update, and a doc rewrite. The root cause is clearly explained, the on-device test plan covers both affected peripherals (LCD still renders, SD now mounts), and the Flex / InputSignal::FSPIQ approach is the supported esp-hal surface. No logic in the OE-flip path changed.

No files require special attention.

Important Files Changed

Filename Overview
crates/stackchan-firmware/src/main.rs Replaces Output with Flex for GPIO35 (DC/MISO shared pin), enables input buffer, and routes InputSignal::FSPIQ to the pin so SPI2 can sample SD card MISO; updates LcdDisplay type alias accordingly.
crates/stackchan-firmware/src/sd_spi.rs Doc comment updated to describe the full two-part mechanism: the FSPIQ GPIO matrix route (new, in main.rs) and the per-transaction OE flip (existing here); no logic changes.

Reviews (1): Last reviewed commit: "fix(firmware): route SPI2 MISO to GPIO35..." | Re-trigger Greptile

@release-kun release-kun Bot mentioned this pull request May 29, 2026
release-kun Bot added a commit that referenced this pull request May 29, 2026
🤖 I have created a release *beep* *boop*
---


<details><summary>stackchan-firmware: 0.113.2</summary>

##
[0.113.2](stackchan-firmware-v0.113.1...stackchan-firmware-v0.113.2)
(2026-05-29)


### Bug Fixes

* **firmware:** route SPI2 MISO to GPIO35 so the SD card mounts
([#575](#575))
([791341e](791341e))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-kun[bot] <276042328+release-kun[bot]@users.noreply.github.com>
andymai added a commit that referenced this pull request May 29, 2026
…rite (#579)

## Summary

Completes the SD config feature: it could mount (#575) and read (#577)
but not save. `PUT /settings` writeback was broken the same way the read
path was — the staging file `STACKCHAN.NEW` (9-char base) couldn't be
*created* by embedded-sdmmc, and the final copy targeted the
un-creatable `STACKCHAN.RON`.

embedded-sdmmc cannot create long-named files at all, but it **can
overwrite an existing file opened by its short name**. So:
- Stage to a valid 8.3 name (`STKCFG.NEW`) the firmware can create.
- Resolve the existing `STACKCHAN.RON` short name via the shared
`lfn_short_name` helper (factored out of the read path).
- Truncate-write the rendered config onto it — which leaves the
long-name directory entry intact, so the file stays readable as
`STACKCHAN.RON` next boot.

Writeback updates an **existing** config (the realistic lifecycle — the
file must already be present for Wi-Fi/auth to come up). If
`STACKCHAN.RON` is absent, `write_config` returns `FileNotFound` with a
clear log rather than silently writing a differently-named file.

## Test plan

- `just clippy-firmware` (`-D warnings`) — clean.
- **On-device (CoreS3, real `STACKCHAN.RON` on the card):** a temporary
idempotent self-test (rewrite the just-read config, then re-read) —
removed before commit — logged:
  - `dbg writeback: write_config OK`
- `dbg writeback: re-read OK ssid=Interweb hostname=stackchan` —
round-trip intact, long name preserved.

## Surfaced by config now loading (separate, not addressed here)

- `sidecar session: persist failed (Write)` — a different persisted
file; needs its own look.
- `wifi: start_async failed (InternalError(NoMem))` — Wi-Fi now actually
starts (there's an SSID); a pre-existing init path that was never
exercised while config never loaded.

Greptile: please review.
andymai added a commit that referenced this pull request May 29, 2026
… pool sizing (#581)

## Summary

Once a real `STACKCHAN.RON` loaded (after #575/#577), Wi-Fi never came
up — `start_async` failed with `InternalError(NoMem)`. This fixes the
bring-up end to end.

**Root cause (measured on-device):** only **~1.5 KiB** of internal SRAM
was free when the Wi-Fi MAC started. PSRAM (8 MiB free) can't back Wi-Fi
DMA buffers; the bootloader-reclaimed heap is hard-capped at **~72 KiB**
(already full); and the only other internal heap shares the DRAM segment
with the **stack** (growing it overflowed the stack in the BLE+Wi-Fi
coex path).

**Fix 1 — BLE/Wi-Fi mutual exclusion.** With a Wi-Fi SSID configured,
skip BLE bring-up entirely. Frees **~36 KiB** internal (1.5 KiB → 37
KiB) for the Wi-Fi stack and removes coex stack pressure. BLE (Hardware
Buddy) still runs when no SSID is set, so provisioning over BLE is
unaffected.

**Fix 2 — socket pool.** Wi-Fi connecting then exposed a second latent
bug: `STACK_SOCKETS` was 6, predating the 4-worker HTTP pool, so
`smoltcp` panicked (`adding a socket to a full SocketSet`) the instant
the link came up. Sized to real demand (`HTTP_WORKER_COUNT` + mDNS +
SNTP + DHCP/DNS + optional outbound clients).

Also reflows one `write_config` line that merged slightly un-formatted
in #579.

## Test plan

- `just clippy-firmware` (`-D warnings`) — clean.
- **On-device (CoreS3, `STACKCHAN.RON` with `ssid=Interweb`):** flashed
`--release`; defmt log shows:
- `ble: skipped — Wi-Fi SSID configured` and free internal **37 KiB**
before start (was 1.5 KiB).
- `wifi: connecting → wifi: connected`, HTTP workers listening, **DHCP
lease acquired**, and `sntp: synced via pool.ntp.org` — i.e. full
internet reachability.
- **Boots once, no reboot loop;** zero `NoMem`, stack-guard, or
`SocketSet` panics (all three were reproduced before the fix).

## Follow-ups (noted, out of scope)

- `coex` feature is now dead weight when an SSID is set (BLE never
co-runs); dropping it would reclaim more internal RAM and could be
revisited.
- `sidecar session: persist failed (Write)` (separate file) and the
BLE-only path's behavior are unchanged here.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant