Bus_SPI: zero-initialize spi_bus_config_t instead of memset(~0u) - #281
Merged
Conversation
ESP-IDF 6.1 added dma_burst_size to spi_bus_config_t. The memset(~0u) that seeds the *_io_num fields with -1 leaves it at 0xFFFFFFFF, which the GDMA driver rejects on chips with an adjustable burst size (ESP32-S3, C5, C61, P4). spi_bus_initialize() then panics in its error path and the board boot-loops in init(). Set it to 0 (driver default) on 6.1 and later. Reproduced and verified on ESP32-S3 with ESP-IDF v6.1.
memset(~0u) seeded the *_io_num fields with -1 in one stroke but also left every non-pin field at 0xFFFFFFFF, which is what broke on ESP-IDF v6.1 (dma_burst_size). Follow the aggregate-initializer convention used by ESP-IDF itself instead: zero-initialize the struct, then set only the unused pin fields to -1 (quadwp/quadhd always, data4..7 on v4.4 and later where they exist), because 0 would denote GPIO0. With this, every non-pin field present in IDF 3.2 through 6.1 receives a valid value: dma_burst_size = 0 is the driver default on v6.1, and data_io_default_level = 0 is the same Low level that was assigned explicitly before. The v6.1-specific dma_burst_size assignment is therefore no longer needed. Verified A/B on ESP32-S3 (StickS3, CoreS3), ESP32-C5 and ESP32-P4 with ESP-IDF v6.1; builds on ESP-IDF v6.0 (ESP32), Arduino-ESP32 2.0.3 / 2.0.17 (IDF 4.4) and 3.x (IDF 5.5).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #280 (stacked on it; the first commit here is that PR's commit). Please merge #280 first, or merge this one alone since it contains both.
#280 patched the one field that ESP-IDF v6.1 added. This replaces the underlying
memset(&buscfg, ~0u, ...)inspi::init()so the same class of problem cannot come back with the next field ESP-IDF adds:memset(~0u)seeded every*_io_numwith -1 in one stroke, but also left every non-pin field at0xFFFFFFFF, which is exactly what broke on v6.1 (dma_burst_size).spi_bus_config_t(unspecified members become 0). Following that convention, the struct is now zero-initialized and only the unused pin fields are set to -1 explicitly (quadwp/quadhdalways;data4..7under a v4.4+ guard, where they exist), because 0 would denote GPIO0.dma_burst_size = 0is the driver default on v6.1, anddata_io_default_level = 0is the same Low level that was assigned explicitly before. The v6.1-specific assignment from Bus_SPI: initialize spi_bus_config_t::dma_burst_size for ESP-IDF 6.1 #280 is therefore removed again, andinitQuad()(already a designated initializer) andinit()now follow the same pattern.#if defined (ESP_IDF_VERSION_VAL)/#if (ESP_IDF_VERSION >= ...)form so toolchains withoutesp_idf_version.hstill preprocess.Verification
develop). Smoke test on ESP32 (Core BASIC, no GDMA) with ESP-IDF v6.0: initialises and draws normally.data4..7guard boundary), Arduino-ESP32 3.x (IDF 5.5).