Repository navigation
espressif: release SDMMC cards per slot - #11500
ladyada-eagleclaw wants to merge 4 commits into
Conversation
Select the actual host slot and deinitialize only that slot, preserving other users of the controller. Pin the SDK reinitialization and interrupt-lifetime fixes required for repeated slot teardown. Co-authored-by: Limor Fried <[email protected]>
Use remote Wi-Fi components and preserve the configured SDIO transport across VM reloads. Reserve its slot from sdioio allocation. Board wiring remains in the board definition. Co-authored-by: Limor Fried <[email protected]>
|
Could the two esp-idf commits merge into |
Initialize the required nested runtime after fetching ESP-Hosted, using the same filter or shallow-clone options as mbedtls. Co-authored-by: Limor Fried <[email protected]>
dhalbert
left a comment
dhalbert
left a comment
There was a problem hiding this comment.
@ladyada-eagleclaw Please submit a separate PR to our ESP-IDF fork for the ESP-IDF changes. Also re those changes:
- Why is one change in the legacy driver? Is that just an artifact of how that driver is structured?
- See if the problems these changes fixed are mentioned upstream in an issue or a recent commit.
espressif: add optional ESP-Hosted Wi-Fi on ESP32-P4
|
The separate SDK PR is now open: adafruit/esp-idf#17, targeting The The reinitialization fix already exists upstream as df53e6a2 on release/v6.0 and c9a7a0fd on master. espressif/esp-idf#17889 describes the same SDMMC/Hosted double-initialization problem, with a v6.0.1 confirmation. For the ISR/removal fix, the upstream search found related cleanup, a null-slot guard, and slot add/remove mutex changes, linked in the SDK PR. I did not find an exact counterpart that both serializes ISR access with removal and dispatches SDIO callbacks to the actual interrupting slot. Simply returning when the current transfer slot is gone can skip the surviving Hosted slot's interrupt. The SDK PR's source tree is identical to the previously tested AI-assisted with Codex. |
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
Review effort: Lite
Findings: 5
Open (5)
esp_wifiin the baseCOMPONENTSlist, even whenCIRCUITPY_ESP_HOSTEDis… · NewCIRCUITPY_ESP_HOSTED=1, this still links the IDFesp_wificomponent (`... esp-tls esp_wifi… · Newsdmmc_host_deinit()lifecycle tracking was removed, butsdmmc_host_init()is still… · Newsdmmc_host_deinit()lifecycle tracking was removed, butsdmmc_host_init()is still… · NewCONFIG_ESP_HOSTED_SDIO_*… · New
What changed in this PR
Adds support for using ESP-Hosted (Wi-Fi coprocessor) on the Espressif port, including new submodules and build system integration.
Changes:
- Add
esp-hostedandesp-wifi-remotesubmodules and fetch nested submodules in CI. - Introduce
CIRCUITPY_ESP_HOSTEDbuild flag and wire it through Make/CMake component selection. - Update Wi-Fi and SDIO codepaths to accommodate ESP-Hosted SDIO pin usage / slot conflicts.
| File | Description |
|---|---|
| tools/ci_fetch_deps.py | Initializes esp-hosted nested protobuf-c submodule during CI dependency fetch. |
| ports/espressif/mpconfigport.mk | Adds CIRCUITPY_ESP_HOSTED option for esp32p4 and ties Wi-Fi/SSL to it. |
| ports/espressif/esp-wifi-remote | Adds new submodule pointer for ESP Wi-Fi remote component. |
| ports/espressif/esp-idf | Updates esp-idf submodule revision. |
| ports/espressif/esp-hosted | Adds new submodule pointer for esp-hosted-mcu. |
| ports/espressif/common-hal/wifi/__init__.c | Adds ESP-Hosted init path and SDIO pin claiming; switches to #include "esp_wifi.h". |
| ports/espressif/common-hal/wifi/ScannedNetworks.c | Switches to #include "esp_wifi.h". |
| ports/espressif/common-hal/wifi/Radio.c | Switches to #include "esp_wifi.h". |
| ports/espressif/common-hal/sdioio/SDCard.c | Avoids ESP-Hosted SDIO slot; changes slot init/deinit lifecycle. |
| ports/espressif/Makefile | Adds ESP-Hosted build/link integration and include injection paths. |
| ports/espressif/CMakeLists.txt | Adds ESP-Hosted components/dirs when enabled; sets ESP_IDF_VERSION env var. |
| .gitmodules | Registers new esp-hosted and esp-wifi-remote submodules. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| set(COMPONENTS bt driver esp_driver_dac esp_driver_gpio esp_driver_gptimer esp_driver_i2c esp_driver_i2s esp_driver_ledc esp_driver_pcnt esp_driver_rmt esp_driver_sdmmc esp_driver_spi esp_driver_touch_sens esp_driver_tsens esp_driver_uart esp-tls esp_adc esp_event esp_netif esp_psram esp_security esp_wifi esptool_py freertos log lwip main mbedtls mdns pthread soc ulp wpa_supplicant esp_lcd vfs esp_stdio sdmmc esp-camera) | ||
| set(EXTRA_COMPONENT_DIRS "esp-protocols/components/mdns" "esp-camera") | ||
|
|
||
| if(CIRCUITPY_ESP_HOSTED) | ||
| list(APPEND EXTRA_COMPONENT_DIRS "esp-hosted" "esp-wifi-remote/components/esp_wifi_remote") | ||
| list(APPEND COMPONENTS esp-hosted esp_wifi_remote) |
| ESP_IDF_COMPONENTS_LINK += esp_netif esp_security esp-tls esp_wifi lwip mbedtls mdns | ||
| ifneq ($(CIRCUITPY_ESP_HOSTED),1) | ||
| ESP_IDF_COMPONENTS_LINK += esp_coex esp_phy wpa_supplicant |
| sdmmc_host_deinit_slot(self->slot); | ||
| never_reset_sdio[get_slot_index(self)] = false; | ||
| slot_in_use[get_slot_index(self)] = false; |
| if (slot_in_use[i]) { | ||
| sdmmc_host_deinit_slot(i); | ||
| } | ||
| slot_in_use[i] = false; |
| #ifdef CONFIG_ESP_HOSTED_ENABLED | ||
| const uint8_t pins[] = { | ||
| CONFIG_ESP_HOSTED_SDIO_PIN_CLK, CONFIG_ESP_HOSTED_SDIO_PIN_CMD, | ||
| CONFIG_ESP_HOSTED_SDIO_PIN_D0, CONFIG_ESP_HOSTED_SDIO_PIN_D1, | ||
| CONFIG_ESP_HOSTED_SDIO_PIN_D2, CONFIG_ESP_HOSTED_SDIO_PIN_D3, | ||
| CONFIG_ESP_HOSTED_SDIO_GPIO_RESET_SLAVE, | ||
| }; | ||
| for (size_t i = 0; i < MP_ARRAY_SIZE(pins); i++) { | ||
| claim_pin_number(pins[i]); | ||
| never_reset_pin_number(pins[i]); | ||
| } | ||
| ESP_ERROR_CHECK(esp_hosted_init()); | ||
| #endif |
|
Just merged #11513 instead. |
|
@ladyada-eagleclaw There may still be a need for a smaller number of adafruit-specific ESP-IDF changes. That's what my Claude review said when considering how this PR would change if we updated to ESP-IDF v6.1. So check if you need a new PR. |
AI-assisted with Codex during Metro ESP32-P4 bring-up. This PR extracts the SDMMC slot-lifetime fix from #11497; frequency units and ESP-Hosted integration are separate changes.
Problem and fix
sdioiocould select one slot while leavingsdmmc_host_t.slotat its default. It also released the entire controller when its own cards were closed, instead of releasing each card's slot. This prevents independent users, including a Wi-Fi coprocessor, from sharing the controller safely.Set
host.slotto the selected slot and usesdmmc_host_deinit_slot()for explicit deinitialization and VM cleanup. Remove the now-redundant controller-lifetime flag.SDK fixes
The SDK pin contains two narrowly scoped prerequisites on the existing CircuitPython SDK base:
df53e6a2c312c677d2d2c3d738985b26b04a805d: make initialization idempotent and clear the released controller pointer.sd_host_get_free_descriptors_count(MCAUSE=5,MTVAL=0x94).Both commits are publicly fetchable from
adafruit/esp-idf:metro-p4-sdmmc-reinit.Validation
Metro ESP32-P4 Rev A, silicon v0.1, 16 MB flash/PSRAM, SD card and C6 connected. On the integrated tree: SD-first startup; 8 MiB patterned file write/read checks at 20 and 40 MHz; all 64 byte offsets for a 32 KiB read with garbage collection; and 24 mount/read/unmount/deinit cycles while Wi-Fi scanned all passed. Normal reboot returned with safe-mode reason
NONE.The hardware run included the separately submitted C6 integration; this PR isolates the controller/slot fix. The earlier integrated tree built for all six P4 configurations and an ESP32-S3. The isolated PR also builds with
make -C ports/espressif BOARD=adafruit_feather_esp32s3_4mbflash_2mbpsram -j6(1,847,072 bytes used; 1,036,512 bytes free in its firmware partition). Changed-file pre-commit andgit diff --checkpass. Only Metro was hardware-tested.