feat(sdspi): Add an option to modify wait time for MISO before sending next command

Closes https://github.com/espressif/esp-idf/issues/16909

components/esp_driver_sdspi/include/driver/sdspi_host.h

@@ -80,6 +80,10 @@ typedef struct {
                                     0 means "active low", i.e. card is protected when the GPIO is low;
                                     1 means "active high", i.e. card is protected when GPIO is high. */
     uint16_t duty_cycle_pos;  ///< Duty cycle of positive clock, in 1/256th increments (128 = 50%/50% duty). Setting this to 0 (=not setting it) is equivalent to setting this to 128.
+    int8_t wait_for_miso; /*!< Timeout value in the driver will be waiting for MISO to be high before sending commands. Possible values are the following:
+                               0: default value (40ms); -1: no waiting (0ms); 1-127: timeout in ms; else: invalid value, default will be used.
+                               This can be used to speed up transactions in certain scenarios but should not be needed if correct pull-up resistors are used.
+                               Use with care on devices where multiple SPI slaves use the same SPI bus.*/
 } sdspi_device_config_t;
 
 #define SDSPI_SLOT_NO_CS          GPIO_NUM_NC      ///< indicates that card select line is not used
@@ -91,14 +95,15 @@ typedef struct {
 /**
  * Macro defining default configuration of SD SPI device.
  */
-#define SDSPI_DEVICE_CONFIG_DEFAULT() {\
+#define SDSPI_DEVICE_CONFIG_DEFAULT() { \
     .host_id   = SDSPI_DEFAULT_HOST, \
     .gpio_cs   = GPIO_NUM_13, \
     .gpio_cd   = SDSPI_SLOT_NO_CD, \
     .gpio_wp   = SDSPI_SLOT_NO_WP, \
     .gpio_int  = GPIO_NUM_NC, \
     .gpio_wp_polarity = SDSPI_IO_ACTIVE_LOW, \
-    .duty_cycle_pos = 0,\
+    .duty_cycle_pos = 0, \
+    .wait_for_miso = 0, \
 }
 
 /**

components/esp_driver_sdspi/src/sdspi_host.c

@@ -43,10 +43,15 @@
 typedef struct {
     spi_host_device_t   host_id; //!< SPI host id.
     spi_device_handle_t spi_handle; //!< SPI device handle, used for transactions
+    uint8_t* block_buf;
+    /// semaphore of gpio interrupt
+    SemaphoreHandle_t   semphr_int;
+    uint16_t duty_cycle_pos;  ///< Duty cycle of positive clock, in 1/256th increments (128 = 50%/50% duty). Setting this to 0 (=not setting it) is equivalent to setting this to 128.
     uint8_t gpio_cs;            //!< CS GPIO, or GPIO_UNUSED
     uint8_t gpio_cd;            //!< Card detect GPIO, or GPIO_UNUSED
     uint8_t gpio_wp;            //!< Write protect GPIO, or GPIO_UNUSED
     uint8_t gpio_int;           //!< Write protect GPIO, or GPIO_UNUSED
+    uint8_t poll_busy_start_command_timeout;  ///< Timeout value in milliseconds the driver will be waiting for MISO to be high before sending commands.
     /// GPIO write protect polarity.
     /// 0 means "active low", i.e. card is protected when the GPIO is low;
     /// 1 means "active high", i.e. card is protected when GPIO is high.
@@ -55,10 +60,6 @@ typedef struct {
     uint8_t data_crc_enabled : 1;
     /// Intermediate buffer used when application buffer is not in DMA memory;
     /// allocated on demand, SDSPI_BLOCK_BUF_SIZE bytes long. May be zero.
-    uint8_t* block_buf;
-    /// semaphore of gpio interrupt
-    SemaphoreHandle_t   semphr_int;
-    uint16_t duty_cycle_pos;  ///< Duty cycle of positive clock, in 1/256th increments (128 = 50%/50% duty). Setting this to 0 (=not setting it) is equivalent to setting this to 128.
 } slot_info_t;
 
 // Reserved for old API to be back-compatible
@@ -327,6 +328,28 @@ static void gpio_intr(void* arg)
     }
 }
 
+static inline int wait_for_miso_to_poll_busy_timeout_ms(int8_t wait_for_miso)
+{
+    static int default_timeout = 40; // default timeout in ms
+    int ret;
+    switch (wait_for_miso) {
+    case -1:
+        ret = 0; // no waiting
+        break;
+    case 0:
+        ret = default_timeout;
+        break;
+    case 1 ... 127:
+        ret = (int) wait_for_miso; // timeout in ms
+        break;
+    default:
+        ret = default_timeout; // unsupported values (from -128 to -2), use default
+        break;
+    }
+    assert(ret >= 0);
+    return ret;
+}
+
 esp_err_t sdspi_host_init_device(const sdspi_device_config_t* slot_config, sdspi_dev_handle_t* out_handle)
 {
     ESP_LOGD(TAG, "%s: SPI%d cs=%d cd=%d wp=%d wp_polarity:%d",
@@ -341,6 +364,7 @@ esp_err_t sdspi_host_init_device(const sdspi_device_config_t* slot_config, sdspi
         .host_id = slot_config->host_id,
         .gpio_cs = slot_config->gpio_cs,
         .duty_cycle_pos = slot_config->duty_cycle_pos,
+        .poll_busy_start_command_timeout = wait_for_miso_to_poll_busy_timeout_ms(slot_config->wait_for_miso),
     };
 
     // Attach the SD card to the SPI bus
@@ -452,7 +476,6 @@ cleanup:
     }
     free(slot);
     return ret;
-
 }
 
 esp_err_t sdspi_host_start_command(sdspi_dev_handle_t handle, sdspi_hw_cmd_t *cmd, void *data,
@@ -473,8 +496,21 @@ esp_err_t sdspi_host_start_command(sdspi_dev_handle_t handle, sdspi_hw_cmd_t *cm
     ESP_LOGV(TAG, "%s: slot=%i, CMD%d, arg=0x%08"PRIx32" flags=0x%x, data=%p, data_size=%"PRIu32" crc=0x%02x",
              __func__, handle, cmd_index, cmd_arg, flags, data, data_size, cmd->crc7);
 
-    spi_device_acquire_bus(slot->spi_handle, portMAX_DELAY);
+    esp_err_t ret;
-    poll_busy(slot, 40, true);
+    ret = spi_device_acquire_bus(slot->spi_handle, portMAX_DELAY);
+    if (ret != ESP_OK) {
+        ESP_LOGD(TAG, "%s: spi_device_acquire_bus returned 0x%x", __func__, ret);
+        return ret;
+    }
+
+    ret = poll_busy(slot, slot->poll_busy_start_command_timeout, true);
+    if (ret != ESP_OK && ret != ESP_ERR_TIMEOUT) {
+        ESP_LOGD(TAG, "%s: poll_busy error=0x%x", __func__, ret);
+        cs_high(slot);
+        release_bus(slot);
+        spi_device_release_bus(slot->spi_handle);
+        return ret;
+    }
 
     // For CMD0, clock out 80 cycles to help the card enter idle state,
     // *before* CS is asserted.
@@ -482,7 +518,7 @@ esp_err_t sdspi_host_start_command(sdspi_dev_handle_t handle, sdspi_hw_cmd_t *cm
         go_idle_clockout(slot);
     }
     // actual transaction
-    esp_err_t ret = ESP_OK;
+    ret = ESP_OK;
 
     cs_low(slot);
     if (flags & SDSPI_CMD_FLAG_DATA) {
@@ -574,6 +610,11 @@ static esp_err_t start_command_default(slot_info_t *slot, int flags, sdspi_hw_cm
 // Wait until MISO goes high
 static esp_err_t poll_busy(slot_info_t *slot, int timeout_ms, bool polling)
 {
+    if (timeout_ms < 0) {
+        return ESP_ERR_INVALID_ARG;
+    } else if (timeout_ms == 0) {
+        return ESP_OK;
+    }
     uint8_t t_rx;
     spi_transaction_t t = {
         .tx_buffer = &t_rx,

components/esp_driver_sdspi/test_apps/sdspi/components/sdspi_tests/sdmmc_test_begin_end_spi.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -37,13 +37,24 @@ void sdmmc_test_spi_skip_if_board_incompatible(int slot, int freq_khz)
 
 }
 
-void sdmmc_test_spi_begin(int slot, int freq_khz, sdmmc_card_t *out_card)
+void sdmmc_test_spi_begin(int slot, int freq_khz, sdmmc_card_t *out_card,
+                          sdmmc_host_t *_config, sdspi_device_config_t *_dev_config, spi_bus_config_t *_bus_config)
 {
     sdmmc_host_t config = SDSPI_HOST_DEFAULT();
     sdspi_device_config_t dev_config = SDSPI_DEVICE_CONFIG_DEFAULT();
     spi_bus_config_t bus_config = {};
-    sdspi_dev_handle_t handle;
 
+    if (_config != NULL) {
+        config = *_config;
+    }
+    if (_dev_config != NULL) {
+        dev_config = *_dev_config;
+    }
+    if (_bus_config != NULL) {
+        bus_config = *_bus_config;
+    }
+
+    sdspi_dev_handle_t handle;
     /* Similar to the checks in sdmmc_test_spi_skip_if_board_incompatible, but
      * we fail the test if we somehow got to this point with an incompatible board.
      */

components/esp_driver_sdspi/test_apps/sdspi/components/sdspi_tests/sdmmc_test_begin_end_spi.h

@@ -1,11 +1,12 @@
 /*
- * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 #pragma once
 
 #include "sd_protocol_types.h"
+#include "driver/sdspi_host.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -29,7 +30,8 @@ void sdmmc_test_spi_skip_if_board_incompatible(int slot, int freq_khz);
  * @brief Helper function to initialize the SDMMC host and slot for the test using the given settings, for SPI mode
  * @see sdmmc_test_sd_begin
  */
-void sdmmc_test_spi_begin(int slot, int freq_khz, sdmmc_card_t *out_card);
+void sdmmc_test_spi_begin(int slot, int freq_khz, sdmmc_card_t *out_card,
+                          sdmmc_host_t *_config, sdspi_device_config_t *_dev_config, spi_bus_config_t *_bus_config);
 
 /**
  * @brief Helper function to deinitialize the SDMMC host and slot after the test, for SPI mode

components/esp_driver_sdspi/test_apps/sdspi/components/sdspi_tests/sdmmc_test_erase_spi.c

@@ -16,7 +16,7 @@ static void do_one_sdspi_erase(int slot, int freq_khz)
 {
     sdmmc_card_t card;
     sdmmc_test_spi_skip_if_board_incompatible(slot, freq_khz);
-    sdmmc_test_spi_begin(slot, freq_khz, &card);
+    sdmmc_test_spi_begin(slot, freq_khz, &card, NULL, NULL, NULL);
     sdmmc_card_print_info(stdout, &card);
     sdmmc_test_sd_erase_blocks(&card);
     sdmmc_test_spi_end(slot, &card);

components/esp_driver_sdspi/test_apps/sdspi/components/sdspi_tests/sdmmc_test_probe_spi.c

@@ -14,7 +14,7 @@ static void do_one_sdspi_probe(int slot, int freq_khz)
 {
     sdmmc_card_t card;
     sdmmc_test_spi_skip_if_board_incompatible(slot, freq_khz);
-    sdmmc_test_spi_begin(slot, freq_khz, &card);
+    sdmmc_test_spi_begin(slot, freq_khz, &card, NULL, NULL, NULL);
     sdmmc_card_print_info(stdout, &card);
     uint8_t* buffer = heap_caps_calloc(512, 1, MALLOC_CAP_DMA);
     TEST_ESP_OK(sdmmc_read_sectors(&card, buffer, 0, 1));

components/esp_driver_sdspi/test_apps/sdspi/components/sdspi_tests/sdmmc_test_rw_spi.c

@@ -1,11 +1,12 @@
 /*
- * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 #include <stddef.h>
 #include "unity.h"
 #include "sdmmc_cmd.h"
+#include "driver/sdspi_host.h"
 #include "sdmmc_test_begin_end_spi.h"
 #include "sdmmc_test_rw_common.h"
 
@@ -15,7 +16,7 @@ static void do_one_sdspi_perf_test(int slot, int freq_khz)
 {
     sdmmc_card_t card;
     sdmmc_test_spi_skip_if_board_incompatible(slot, freq_khz);
-    sdmmc_test_spi_begin(slot, freq_khz, &card);
+    sdmmc_test_spi_begin(slot, freq_khz, &card, NULL, NULL, NULL);
     sdmmc_card_print_info(stdout, &card);
     sdmmc_test_rw_performance(&card, NULL);
     sdmmc_test_spi_end(slot, &card);
@@ -39,7 +40,7 @@ static void do_one_sdspi_rw_test_with_offset(int slot, int freq_khz)
 {
     sdmmc_card_t card;
     sdmmc_test_spi_skip_if_board_incompatible(slot, freq_khz);
-    sdmmc_test_spi_begin(slot, freq_khz, &card);
+    sdmmc_test_spi_begin(slot, freq_khz, &card, NULL, NULL, NULL);
     sdmmc_card_print_info(stdout, &card);
     sdmmc_test_rw_with_offset(&card);
     sdmmc_test_spi_end(slot, &card);
@@ -63,7 +64,7 @@ static void do_one_sdspi_rw_test_unaligned_buffer(int slot, int freq_khz)
 {
     sdmmc_card_t card;
     sdmmc_test_spi_skip_if_board_incompatible(slot, freq_khz);
-    sdmmc_test_spi_begin(slot, freq_khz, &card);
+    sdmmc_test_spi_begin(slot, freq_khz, &card, NULL, NULL, NULL);
     sdmmc_card_print_info(stdout, &card);
     sdmmc_test_rw_unaligned_buffer(&card);
     sdmmc_test_spi_end(slot, &card);
@@ -78,3 +79,31 @@ TEST_CASE("sdspi read/write using unaligned buffer, slot 1", "[sdspi]")
 {
     do_one_sdspi_rw_test_unaligned_buffer(SLOT_1, SDMMC_FREQ_DEFAULT);
 }
+
+/* ========== Read/write performance tests with wait_for_miso == -1, SPI ========== */
+
+static void do_one_sdspi_perf_test_dont_wait_for_miso(int slot, int freq_khz)
+{
+    sdmmc_card_t card;
+    sdmmc_host_t config = SDSPI_HOST_DEFAULT();
+    sdspi_device_config_t dev_config = SDSPI_DEVICE_CONFIG_DEFAULT();
+    dev_config.wait_for_miso = -1; // no waiting for MISO
+    spi_bus_config_t bus_config = {};
+    sdmmc_test_spi_skip_if_board_incompatible(slot, freq_khz);
+    sdmmc_test_spi_begin(slot, freq_khz, &card, &config, &dev_config, &bus_config);
+    sdmmc_card_print_info(stdout, &card);
+    sdmmc_test_rw_performance(&card, NULL);
+    sdmmc_test_spi_end(slot, &card);
+}
+
+TEST_CASE("sdspi read/write performance - wait_for_miso == -1, slot 0", "[sdspi]")
+{
+    do_one_sdspi_perf_test_dont_wait_for_miso(SLOT_0, SDMMC_FREQ_HIGHSPEED);
+}
+
+TEST_CASE("sdspi read/write performance - wait_for_miso == -1, slot 1", "[sdspi]")
+{
+    //TODO: IDF-8749
+    //here freq should be changed to SDMMC_FREQ_HIGHSPEED after fixing IDF-8749
+    do_one_sdspi_perf_test_dont_wait_for_miso(SLOT_1, SDMMC_FREQ_DEFAULT);
+}

examples/storage/sd_card/sdspi/README.md

@@ -236,3 +236,15 @@ In the absence of connected pullups and having the weak pullups enabled, you can
 It will also provide the voltage levels at the corresponding SD pins. By default, this information is provided for ESP32 chip only, and for other chipsets, verify the availability of ADC pins for the respective GPIO using [this](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/peripherals/gpio.html#gpio-summary) and configure ADC mapped pins using menuconfig. Then test the voltage levels accordingly.
 
 You can monitor the voltage levels of individual pins using `PIN voltage levels` and `PIN voltage levels with weak pullup`. However, if one pin being pulled low and experiencing interference with another pin, you can detect it through `PIN cross-talk` and `PIN cross-talk with weak pullup`. In the absence of pullups, voltage levels at each pin should range from 0 to 0.3V. With 10k pullups connected, the voltage will be between 3.1V to 3.3V, contingent on the connection between ADC pins and SD pins, and with weak pullups connected, it can fluctuate between 0.8V to 1.2V, depending on pullup strength.
+
+### Slow performance / low throughput (no or incorrect pull-up resistor on MISO line)
+
+The current driver implementation waits for the MISO line to be high before sending the next transaction. This is the correct behavior and fixes certain issues especially when there are more SPI devices connected to same SPI bus. However this can slow down SD throughput on boards lacking a sufficiently strong pull-up resistor on the MISO line.
+
+If you experience this slowdown, you can try adding the following line. Modifying this value can cause problems in certain scenarios (e.g. SD card and another device like TFT screen sharing the same SPI bus resulting in failed communication with SD card), so please use it with caution.
+
+```c
+    sdspi_device_config_t slot_config = SDSPI_DEVICE_CONFIG_DEFAULT()
+    slot_config.wait_for_miso = -1; // <--- Add this line
+    // If this causes problems, try to set the value higher (-1: no waiting (0ms); 0: default value (40ms); 1-127: timeout in ms; else: invalid value, default will be used)
+```