SPI initialization error on ESP-IDF v5.4 (prerelease)

[rosmo]

Environment ( 実行環境 )

• MCU or Board name: M5Paper
• LovyanGFX version: latest develop]
• FrameWork version: ESP-IDF v5.4 branch
• Operating System: Windows

Problem Description ( 問題の内容 )

With latest ESP-IDF v5.4 (works fine on v5.3 though), I was getting these errors:

E (8765) spi: spi_bus_initialize(815): no support changing io default level
W (8765) LGFX: Failed to spi_bus_initialize. 
E (8775) spi_master: spi_master_init_driver(274): host_id not initialized
W (8785) LGFX: Failed to spi_bus_add_device.

It seems like buscfg.data_io_default_level = 0; is needed in src/lgfx/v1/platforms/esp32/common.cpp on line 519 (though I am not sure why since it's all getting memset to zero anyway):

#if defined (ESP_IDF_VERSION_VAL)
  #if (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 0))
        buscfg.isr_cpu_id = ESP_INTR_CPU_AFFINITY_AUTO;
        buscfg.data_io_default_level = 0;
  #elif (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 2, 0))
        buscfg.isr_cpu_id = ESP_INTR_CPU_AFFINITY_AUTO;
  #elif (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 1, 0))
        buscfg.isr_cpu_id = INTR_CPU_ID_AUTO;
  #endif
#endif

[swarren]

A more future-proof fix would be to change the memset() in spi::init() from:

        memset(&buscfg, ~0u, sizeof(spi_bus_config_t));

to:

        memset(&buscfg, 0u, sizeof(spi_bus_config_t));

That way, any future field that gets added to ESP-IDF will be initialized to 0, which is likely what IDF expects for backwards-compatibility.

[rosmo]

Ah right, I missed the ~ character. Surely there must be a reason to init everything to 0xFF?

[lovyan03]

The reason is simple: we want to set unused pin settings to -1 (not used).
If we initialize it to 0, unnecessary settings will be made to GPIO0.

[swarren]

The code should memset to 0 and then explicitly initialize any field that needs to be non-zero; this is standard practice. It's a pity the SDK doesn't provide a macro to initialize all the fields to default values, and that GPIO 0 is a valid value...

[swarren]

Note that not every single pin field needs to be set to -1; only those that were present in the initial version of the struct. That driver explicitly checks whether pins added later are all 0 do distinguish bus width <= 4 vs. bus width > 4 for example. See the code snippet below. Still, it looks like mosi_io_num, data0_io_num, miso_io_num, data1_io_num, sclk_io_num, quadwp_io_num, data2_io_num, quadhd_io_num, data3_io_num do all need to be -1 not 0.

esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, const spi_bus_config_t *bus_config, uint32_t flags, uint32_t* flags_o)
{
#if SOC_SPI_SUPPORT_OCT
    // In the driver of previous version, spi data4 ~ spi data7 are not in spi_bus_config_t struct. So the new-added pins come as 0
    // if they are not really set. Add this boolean variable to check if the user has set spi data4 ~spi data7 pins .
    bool io4_7_is_blank = !bus_config->data4_io_num && !bus_config->data5_io_num && !bus_config->data6_io_num && !bus_config->data7_io_num;
    // This boolean variable specifies if user sets pins used for octal mode (users can set spi data4 ~ spi data7 to -1).
    bool io4_7_enabled = !io4_7_is_blank && bus_config->data4_io_num >= 0 && bus_config->data5_io_num >= 0 &&
                         bus_config->data6_io_num >= 0 && bus_config->data7_io_num >= 0;