Merge branch 'fix/sdio_speed_4bit' into 'master'

fix(sdio): update sdio to better run in 4bit HS mode

See merge request idf/esp-idf!2704

components/driver/include/driver/sdio_slave.h

@@ -47,10 +47,12 @@ typedef enum {
 
 /// Timing of SDIO slave
 typedef enum {
-    SDIO_SLAVE_TIMING_NSEND_PSAMPLE = 0,///< Send at negedge, and sample at posedge. Default value for SD protocol.
-    SDIO_SLAVE_TIMING_NSEND_NSAMPLE,    ///< Send at negedge, and sample at negedge
-    SDIO_SLAVE_TIMING_PSEND_PSAMPLE,    ///< Send at posedge, and sample at posedge
+    SDIO_SLAVE_TIMING_PSEND_PSAMPLE = 0,/**< Send at posedge, and sample at posedge. Default value for HS mode.
+                                         *   Normally there's no problem using this to work in DS mode.
+                                         */
+    SDIO_SLAVE_TIMING_NSEND_PSAMPLE    ,///< Send at negedge, and sample at posedge. Default value for DS mode and below.
     SDIO_SLAVE_TIMING_PSEND_NSAMPLE,    ///< Send at posedge, and sample at negedge
+    SDIO_SLAVE_TIMING_NSEND_NSAMPLE,    ///< Send at negedge, and sample at negedge
 } sdio_slave_timing_t;
 
 /// Configuration of SDIO slave mode
@@ -72,20 +74,20 @@ typedef struct {
                             ///< Buffer size of the slave pre-defined between host and slave before communication. All receive buffer given to the driver should be larger than this.
     sdio_event_cb_t     event_cb;           ///< when the host interrupts slave, this callback will be called with interrupt number (0-7).
     uint32_t            flags; ///< Features to be enabled for the slave, combinations of ``SDIO_SLAVE_FLAG_*``.
-#define SDIO_SLAVE_FLAG_DAT2_DISABLED       BIT(0)      /**< It is required by the SD specification that all 4 data 
-        lines should be used and pulled up even in 1-bit mode or SPI mode. However, as a feature, the user can speicfy 
-        this flag to make use of DAT2 pin in 1-bit mode. Note that the host cannot read CCCR registers to know we don't 
+#define SDIO_SLAVE_FLAG_DAT2_DISABLED       BIT(0)      /**< It is required by the SD specification that all 4 data
+        lines should be used and pulled up even in 1-bit mode or SPI mode. However, as a feature, the user can specify
+        this flag to make use of DAT2 pin in 1-bit mode. Note that the host cannot read CCCR registers to know we don't
         support 4-bit mode anymore, please do this at your own risk.
         */
-#define SDIO_SLAVE_FLAG_HOST_INTR_DISABLED  BIT(1)      /**< The DAT1 line is used as the interrupt line in SDIO        
-        protocol. However, as a feature, the user can speicfy this flag to make use of DAT1 pin of the slave in 1-bit 
+#define SDIO_SLAVE_FLAG_HOST_INTR_DISABLED  BIT(1)      /**< The DAT1 line is used as the interrupt line in SDIO
+        protocol. However, as a feature, the user can specify this flag to make use of DAT1 pin of the slave in 1-bit
         mode. Note that the host has to do polling to the interrupt registers to know whether there are interrupts from
-        the slave. And it cannot read CCCR registers to know we don't support 4-bit mode anymore, please do this at 
+        the slave. And it cannot read CCCR registers to know we don't support 4-bit mode anymore, please do this at
         your own risk.
         */
-#define SDIO_SLAVE_FLAG_INTERNAL_PULLUP     BIT(2)      /**< Enable internal pullups for enabled pins. It is required 
-        by the SD specification that all the 4 data lines should be pulled up even in 1-bit mode or SPI mode. Note that 
-        the internal pull-ups are not sufficient for stable communication, please do connect external pull-ups on the 
+#define SDIO_SLAVE_FLAG_INTERNAL_PULLUP     BIT(2)      /**< Enable internal pullups for enabled pins. It is required
+        by the SD specification that all the 4 data lines should be pulled up even in 1-bit mode or SPI mode. Note that
+        the internal pull-ups are not sufficient for stable communication, please do connect external pull-ups on the
         bus. This is only for example and debug use.
         */
 } sdio_slave_config_t;
@@ -214,12 +216,12 @@ uint8_t* sdio_slave_recv_get_buf(sdio_slave_buf_handle_t handle, size_t *len_o);
 esp_err_t sdio_slave_send_queue(uint8_t* addr, size_t len, void* arg, TickType_t wait);
 
 /** Return the ownership of a finished transaction.
- * @param arg_o Argument of the finished transaction.
+ * @param out_arg Argument of the finished transaction. Set to NULL if unused.
  * @param wait Time to wait if there's no finished sending transaction.
  *
  * @return ESP_ERR_TIMEOUT if no transaction finished, or ESP_OK if succeed.
  */
-esp_err_t sdio_slave_send_get_finished(void** arg_o, TickType_t wait);
+esp_err_t sdio_slave_send_get_finished(void** out_arg, TickType_t wait);
 
 /** Start a new sending transfer, and wait for it (blocked) to be finished.
  *

components/driver/sdio_slave.c

@@ -472,9 +472,9 @@ static void configure_pin(int pin, uint32_t func, bool pullup)
     PIN_INPUT_ENABLE(reg);
     PIN_FUNC_SELECT(reg, sdmmc_func);
     PIN_SET_DRV(reg, drive_strength);
+    gpio_pulldown_dis(pin);
     if (pullup) {
         gpio_pullup_en(pin);
-        gpio_pulldown_dis(pin);
     }
 }
 
@@ -516,28 +516,28 @@ static inline esp_err_t sdio_slave_hw_init(sdio_slave_config_t *config)
 
     switch(config->timing) {
         case SDIO_SLAVE_TIMING_PSEND_PSAMPLE:
-            HOST.conf.frc_sdio20 = 0xf;
+            HOST.conf.frc_sdio20 = 0x1f;
             HOST.conf.frc_sdio11 = 0;
-            HOST.conf.frc_pos_samp = 0xf;
+            HOST.conf.frc_pos_samp = 0x1f;
             HOST.conf.frc_neg_samp = 0;
             break;
         case SDIO_SLAVE_TIMING_PSEND_NSAMPLE:
-            HOST.conf.frc_sdio20 = 0xf;
+            HOST.conf.frc_sdio20 = 0x1f;
             HOST.conf.frc_sdio11 = 0;
             HOST.conf.frc_pos_samp = 0;
-            HOST.conf.frc_neg_samp = 0xf;
+            HOST.conf.frc_neg_samp = 0x1f;
             break;
         case SDIO_SLAVE_TIMING_NSEND_PSAMPLE:
             HOST.conf.frc_sdio20 = 0;
-            HOST.conf.frc_sdio11 = 0xf;
-            HOST.conf.frc_pos_samp = 0xf;
+            HOST.conf.frc_sdio11 = 0x1f;
+            HOST.conf.frc_pos_samp = 0x1f;
             HOST.conf.frc_neg_samp = 0;
             break;
         case SDIO_SLAVE_TIMING_NSEND_NSAMPLE:
             HOST.conf.frc_sdio20 = 0;
-            HOST.conf.frc_sdio11 = 0xf;
+            HOST.conf.frc_sdio11 = 0x1f;
             HOST.conf.frc_pos_samp = 0;
-            HOST.conf.frc_neg_samp = 0xf;
+            HOST.conf.frc_neg_samp = 0x1f;
             break;
     }
 
@@ -929,9 +929,11 @@ esp_err_t sdio_slave_send_queue(uint8_t* addr, size_t len, void* arg, TickType_t
     return ESP_OK;
 }
 
-esp_err_t sdio_slave_send_get_finished(void** arg, TickType_t wait)
+esp_err_t sdio_slave_send_get_finished(void** out_arg, TickType_t wait)
 {
-    portBASE_TYPE err = xQueueReceive(context.ret_queue, arg, wait);
+    void* arg = NULL;
+    portBASE_TYPE err = xQueueReceive(context.ret_queue, &arg, wait);
+    if (out_arg) *out_arg = arg;
     if (err != pdTRUE) return ESP_ERR_TIMEOUT;
     return ESP_OK;
 }

components/driver/sdmmc_host.c

@@ -290,6 +290,8 @@ static void configure_pin(int pin)
     const int sdmmc_func = 3;
     const int drive_strength = 3;
     assert(pin!=-1);
+    gpio_pulldown_dis(pin);
+
     uint32_t reg = GPIO_PIN_MUX_REG[pin];
     assert(reg != UINT32_MAX);
     PIN_INPUT_ENABLE(reg);
@@ -586,26 +588,17 @@ esp_err_t sdmmc_host_pullup_en(int slot, int width)
     }
     //according to the spec, the host control the clk, we don't to pull it up here
     gpio_pullup_en(sdmmc_slot_info[slot].cmd_gpio);
-    gpio_pulldown_dis(sdmmc_slot_info[slot].cmd_gpio);
     gpio_pullup_en(sdmmc_slot_info[slot].d0_gpio);
-    gpio_pulldown_dis(sdmmc_slot_info[slot].d0_gpio);
     if (width >= 4) {
         gpio_pullup_en(sdmmc_slot_info[slot].d1_gpio);
-        gpio_pulldown_dis(sdmmc_slot_info[slot].d1_gpio);
         gpio_pullup_en(sdmmc_slot_info[slot].d2_gpio);
-        gpio_pulldown_dis(sdmmc_slot_info[slot].d2_gpio);
         gpio_pullup_en(sdmmc_slot_info[slot].d3_gpio);
-        gpio_pulldown_dis(sdmmc_slot_info[slot].d3_gpio);
     }
     if (width == 8) {
         gpio_pullup_en(sdmmc_slot_info[slot].d4_gpio);
-        gpio_pulldown_dis(sdmmc_slot_info[slot].d4_gpio);
         gpio_pullup_en(sdmmc_slot_info[slot].d5_gpio);
-        gpio_pulldown_dis(sdmmc_slot_info[slot].d5_gpio);
         gpio_pullup_en(sdmmc_slot_info[slot].d6_gpio);
-        gpio_pulldown_dis(sdmmc_slot_info[slot].d6_gpio);
         gpio_pullup_en(sdmmc_slot_info[slot].d7_gpio);
-        gpio_pulldown_dis(sdmmc_slot_info[slot].d7_gpio);
     }
     return ESP_OK;
 }

examples/peripherals/sdio/host/Kconfig.projbuild

@@ -1,19 +0,0 @@
-menu "Example Configuration"
-
-choice EXAMPLE_SLAVE
-    prompt "Id of Slave used in Espressif master-slave board."
-    default EXAMPLE_SLAVE_NONE
-    help
-        If Espressif master-slave board is used, select which slave is used.
-
-config EXAMPLE_SLAVE_NONE
-    bool "Not using Espressif master-slave board."
-config EXAMPLE_SLAVE_B1
-    bool "Using slave B1"
-config EXAMPLE_SLAVE_B2
-    bool "Using slave B2"
-config EXAMPLE_SLAVE_B3
-    bool "Using slave B3"
-endchoice
-
-endmenu

examples/peripherals/sdio/host/main/app_main.c

@@ -152,7 +152,7 @@ esp_err_t slave_init(esp_slave_context_t *context)
 #ifdef CONFIG_SDIO_EXAMPLE_HIGHSPEED
     config.max_freq_khz = SDMMC_FREQ_HIGHSPEED;
 #else
-    config.max_freq_khz = SDMMC_FREQ_PROBING;
+    config.max_freq_khz = SDMMC_FREQ_DEFAULT;
 #endif
 
     sdmmc_slot_config_t slot_config = SDMMC_SLOT_CONFIG_DEFAULT();
@@ -209,14 +209,15 @@ void slave_power_on()
     };
     gpio_config(&cfg);
     gpio_set_level(SLAVE_PWR_GPIO, 0);  //low active
+    vTaskDelay(100);
 #endif
 }
 
+DMA_ATTR uint8_t rcv_buffer[READ_BUFFER_LEN];
+
 //try to get an interrupt from the slave and handle it, return if none.
 esp_err_t process_event(esp_slave_context_t *context)
 {
-    uint8_t buffer[READ_BUFFER_LEN];
-
     esp_err_t ret = esp_slave_wait_int(context, 0);
     if (ret == ESP_ERR_TIMEOUT) {
         return ret;
@@ -241,7 +242,7 @@ esp_err_t process_event(esp_slave_context_t *context)
         ESP_LOGD(TAG, "new packet coming");
         while (1) {
             size_t size_read = READ_BUFFER_LEN;
-            ret = esp_slave_get_packet(context, buffer, READ_BUFFER_LEN, &size_read, wait_ms);
+            ret = esp_slave_get_packet(context, rcv_buffer, READ_BUFFER_LEN, &size_read, wait_ms);
             if (ret == ESP_ERR_NOT_FOUND) {
                 ESP_LOGE(TAG, "interrupt but no data can be read");
                 break;
@@ -251,7 +252,7 @@ esp_err_t process_event(esp_slave_context_t *context)
             }
 
             ESP_LOGI(TAG, "receive data, size: %d", size_read);
-            ESP_LOG_BUFFER_HEXDUMP(TAG, buffer, size_read, ESP_LOG_INFO);
+            ESP_LOG_BUFFER_HEXDUMP(TAG, rcv_buffer, size_read, ESP_LOG_INFO);
             if (ret == ESP_OK) {
                 break;
             }
@@ -297,13 +298,13 @@ void job_write_reg(esp_slave_context_t *context, int value)
 //use 1+1+1+1+4+4=12 packets, 513 and 517 not sent
 int packet_len[] = {3, 6, 12, 128, 511, 512, 513, 517};
 //the sending buffer should be word aligned
-DMA_ATTR uint8_t buffer[1024];
+DMA_ATTR uint8_t send_buffer[READ_BUFFER_LEN];
 
 //send packets to the slave (they will return and be handled by the interrupt handler)
 void job_fifo(esp_slave_context_t *context)
 {
     for (int i = 0; i < 1024; i++) {
-        buffer[i] = 0x46 + i * 5;
+        send_buffer[i] = 0x46 + i * 5;
     }
 
     esp_err_t ret;
@@ -318,7 +319,7 @@ void job_fifo(esp_slave_context_t *context)
     for (int i = 0; i < sizeof(packet_len) / sizeof(int); i++) {
         const int wait_ms = 50;
         int length = packet_len[i];
-        ret = esp_slave_send_packet(context, buffer + pointer, length, wait_ms);
+        ret = esp_slave_send_packet(context, send_buffer + pointer, length, wait_ms);
         if (ret == ESP_ERR_TIMEOUT) {
             ESP_LOGD(TAG, "several packets are expected to timeout.");
         } else {