sdspi: use response timeout passed from upper layer

Previously SDSPI host driver would rely on retry count when waiting for
the card to read or write data. This caused different timeout times
depending on CPU frequency and card clock frequency. In practice, card
performance does not depend on these two factors.
This change uses timeout_ms field of sdmmc_command_t introduced
previously for SDMMC host.

Fixes https://esp32.com/viewtopic.php?f=2&t=3440&p=16037 and similar
issues related to SDSPI timeouts.

diff --git a/components/driver/sdspi_host.c b/components/driver/sdspi_host.c
index 57cf3f99359d0a9b99f2c53266a9267858875950..74cc24c2440423973fbcfa63807846c4501dc2a4 100644 (file)
--- a/components/driver/sdspi_host.c
+++ b/components/driver/sdspi_host.c
@@ -25,12 +25,12 @@
 #include "driver/sdspi_host.h"\r
 #include "sdspi_private.h"\r
 #include "sdspi_crc.h"\r
+#include "esp_timer.h"\r
+\r
 \r
 /// Max number of transactions in flight (used in start_command_write_blocks)\r
 #define SDSPI_TRANSACTION_COUNT 4\r
 #define SDSPI_MOSI_IDLE_VAL     0xff    //!< Data value which causes MOSI to stay high\r
-/// FIXME: this has to be replaced with a timeout expressed in ms, rather in retries\r
-#define SDSPI_RETRY_COUNT       1000\r
 #define GPIO_UNUSED 0xff                //!< Flag indicating that CD/WP is unused\r
 /// Size of the buffer returned by get_block_buf\r
 #define SDSPI_BLOCK_BUF_SIZE    (SDSPI_MAX_DATA_LEN + 4)\r
@@ -426,7 +426,7 @@ static esp_err_t start_command_default(int slot, int flags, sdspi_hw_cmd_t *cmd)
 }\r
 \r
 // Wait until MISO goes high\r
-static esp_err_t poll_busy(int slot, spi_transaction_t* t)\r
+static esp_err_t poll_busy(int slot, spi_transaction_t* t, int timeout_ms)\r
 {\r
     uint8_t t_rx;\r
     *t = (spi_transaction_t) {\r
@@ -436,7 +436,9 @@ static esp_err_t poll_busy(int slot, spi_transaction_t* t)
     };\r
     esp_err_t ret;\r
 \r
-    for (int i = 0; i < SDSPI_RETRY_COUNT; i++) {\r
+    uint64_t t_end = esp_timer_get_time() + timeout_ms * 1000;\r
+    int nonzero_count = 0;\r
+    do {\r
         t_rx = SDSPI_MOSI_IDLE_VAL;\r
         t->rx_data[0] = 0;\r
         ret = spi_device_transmit(spi_handle(slot), t);\r
@@ -444,16 +446,17 @@ static esp_err_t poll_busy(int slot, spi_transaction_t* t)
             return ret;\r
         }\r
         if (t->rx_data[0] != 0) {\r
-            if (i < SDSPI_RETRY_COUNT - 2) {\r
-                i = SDSPI_RETRY_COUNT - 2;\r
+            if (++nonzero_count == 2) {\r
+                return ESP_OK;\r
             }\r
         }\r
-    }\r
-    return ESP_OK;\r
+    } while(esp_timer_get_time() < t_end);\r
+    ESP_LOGD(TAG, "%s: timeout", __func__);\r
+    return ESP_ERR_TIMEOUT;\r
 }\r
 \r
 // Wait for response token\r
-static esp_err_t poll_response_token(int slot, spi_transaction_t* t)\r
+static esp_err_t poll_response_token(int slot, spi_transaction_t* t, int timeout_ms)\r
 {\r
     uint8_t t_rx;\r
     *t = (spi_transaction_t) {\r
@@ -462,8 +465,8 @@ static esp_err_t poll_response_token(int slot, spi_transaction_t* t)
         .length = 8,\r
     };\r
     esp_err_t ret;\r
-\r
-    for (int retry = 0; retry < SDSPI_RETRY_COUNT; retry++) {\r
+    uint64_t t_end = esp_timer_get_time() + timeout_ms * 1000;\r
+    do {\r
         t_rx = SDSPI_MOSI_IDLE_VAL;\r
         t->rx_data[0] = 0;\r
         ret = spi_device_transmit(spi_handle(slot), t);\r
@@ -471,7 +474,7 @@ static esp_err_t poll_response_token(int slot, spi_transaction_t* t)
             return ret;\r
         }\r
         if ((t->rx_data[0] & TOKEN_RSP_MASK) == TOKEN_RSP_OK) {\r
-            break;\r
+            return ESP_OK;\r
         }\r
         if ((t->rx_data[0] & TOKEN_RSP_MASK) == TOKEN_RSP_CRC_ERR) {\r
             return ESP_ERR_INVALID_CRC;\r
@@ -479,19 +482,17 @@ static esp_err_t poll_response_token(int slot, spi_transaction_t* t)
         if ((t->rx_data[0] & TOKEN_RSP_MASK) == TOKEN_RSP_WRITE_ERR) {\r
             return ESP_ERR_INVALID_RESPONSE;\r
         }\r
-        if (retry == SDSPI_RETRY_COUNT - 1) {\r
-            return ESP_ERR_TIMEOUT;\r
-        }\r
-    }\r
+    } while (esp_timer_get_time() < t_end);\r
 \r
-    return ESP_OK;\r
+    ESP_LOGD(TAG, "%s: timeout", __func__);\r
+    return ESP_ERR_TIMEOUT;\r
 }\r
 \r
 // Wait for data token, reading 8 bytes at a time.\r
 // If the token is found, write all subsequent bytes to extra_ptr,\r
 // and store the number of bytes written to extra_size.\r
 static esp_err_t poll_data_token(int slot, spi_transaction_t* t,\r
-        uint8_t* extra_ptr, size_t* extra_size)\r
+        uint8_t* extra_ptr, size_t* extra_size, int timeout_ms)\r
 {\r
     uint8_t t_rx[8];\r
     *t = (spi_transaction_t) {\r
@@ -500,7 +501,8 @@ static esp_err_t poll_data_token(int slot, spi_transaction_t* t,
         .length = sizeof(t_rx) * 8,\r
     };\r
     esp_err_t ret;\r
-    for (int retry = 0; retry < SDSPI_RETRY_COUNT; retry++) {\r
+    uint64_t t_end = esp_timer_get_time() + timeout_ms * 1000;\r
+    do {\r
         memset(t_rx, SDSPI_MOSI_IDLE_VAL, sizeof(t_rx));\r
         ret = spi_device_transmit(spi_handle(slot), t);\r
         if (ret != ESP_OK) {\r
@@ -522,13 +524,11 @@ static esp_err_t poll_data_token(int slot, spi_transaction_t* t,
             }\r
         }\r
         if (found) {\r
-            break;\r
+            return ESP_OK;\r
         }\r
-        if (retry == SDSPI_RETRY_COUNT - 1) {\r
-            return ESP_ERR_TIMEOUT;\r
-        }\r
-    }\r
-    return ESP_OK;\r
+    } while (esp_timer_get_time() < t_end);\r
+    ESP_LOGD(TAG, "%s: timeout", __func__);\r
+    return ESP_ERR_TIMEOUT;\r
 }\r
 \r
 \r
@@ -608,11 +608,14 @@ static esp_err_t start_command_read_blocks(int slot, sdspi_hw_cmd_t *cmd,
         if (need_poll) {\r
             // Wait for data to be ready\r
             spi_transaction_t* t_poll = get_transaction(slot);\r
-            poll_data_token(slot, t_poll, cmd_u8 + SDSPI_CMD_R1_SIZE, &extra_data_size);\r
+            ret = poll_data_token(slot, t_poll, cmd_u8 + SDSPI_CMD_R1_SIZE, &extra_data_size, cmd->timeout_ms);\r
+            release_transaction(slot);\r
+            if (ret != ESP_OK) {\r
+                return ret;\r
+            }\r
             if (extra_data_size) {\r
                 extra_data_ptr = cmd_u8 + SDSPI_CMD_R1_SIZE;\r
             }\r
-            release_transaction(slot);\r
         }\r
 \r
         // Arrange RX buffer\r
@@ -674,14 +677,17 @@ static esp_err_t start_command_read_blocks(int slot, sdspi_hw_cmd_t *cmd,
         // To end multi block transfer, send stop command and wait for the\r
         // card to process it\r
         sdspi_hw_cmd_t stop_cmd;\r
-        make_hw_cmd(MMC_STOP_TRANSMISSION, 0, &stop_cmd);\r
+        make_hw_cmd(MMC_STOP_TRANSMISSION, 0, cmd->timeout_ms, &stop_cmd);\r
         ret = start_command_default(slot, SDSPI_CMD_FLAG_RSP_R1, &stop_cmd);\r
         if (ret != ESP_OK) {\r
             return ret;\r
         }\r
         spi_transaction_t* t_poll = get_transaction(slot);\r
-        ret = poll_busy(slot, t_poll);\r
+        ret = poll_busy(slot, t_poll, cmd->timeout_ms);\r
         release_transaction(slot);\r
+        if (ret != ESP_OK) {\r
+            return ret;\r
+        }\r
     }\r
     return ESP_OK;\r
 }\r
@@ -768,7 +774,7 @@ static esp_err_t start_command_write_blocks(int slot, sdspi_hw_cmd_t *cmd,
 \r
         // Poll for response\r
         spi_transaction_t* t_poll = get_transaction(slot);\r
-        ret = poll_response_token(slot, t_poll);\r
+        ret = poll_response_token(slot, t_poll, cmd->timeout_ms);\r
         release_transaction(slot);\r
         if (ret != ESP_OK) {\r
             return ret;\r
@@ -776,7 +782,7 @@ static esp_err_t start_command_write_blocks(int slot, sdspi_hw_cmd_t *cmd,
 \r
         // Wait for the card to finish writing data\r
         t_poll = get_transaction(slot);\r
-        ret = poll_busy(slot, t_poll);\r
+        ret = poll_busy(slot, t_poll, cmd->timeout_ms);\r
         release_transaction(slot);\r
         if (ret != ESP_OK) {\r
             return ret;\r
@@ -803,7 +809,7 @@ static esp_err_t start_command_write_blocks(int slot, sdspi_hw_cmd_t *cmd,
         wait_for_transactions(slot);\r
 \r
         spi_transaction_t* t_poll = get_transaction(slot);\r
-        ret = poll_busy(slot, t_poll);\r
+        ret = poll_busy(slot, t_poll, cmd->timeout_ms);\r
         release_transaction(slot);\r
         if (ret != ESP_OK) {\r
             return ret;\r

diff --git a/components/driver/sdspi_private.h b/components/driver/sdspi_private.h
index d28cfcf88c419025a2aede22d41317364d1d551a..e14dd19bb86ce70ca0c060ed870db811a5ee2bea 100644 (file)
--- a/components/driver/sdspi_private.h
+++ b/components/driver/sdspi_private.h
@@ -73,6 +73,8 @@ typedef struct {
     uint8_t r1;\r
     /// Up to 16 bytes of response. Luckily, this is aligned on 4 byte boundary.\r
     uint32_t response[4];\r
+    /// response timeout, in milliseconds\r
+    int timeout_ms;\r
 } sdspi_hw_cmd_t;\r
 \r
 #define SDSPI_CMD_NORESP_SIZE   6   //!< Size of the command without any response\r
@@ -90,7 +92,7 @@ typedef struct {
 \r
 #define SDSPI_MAX_DATA_LEN      512     //!< Max size of single block transfer\r
 \r
-void make_hw_cmd(uint32_t opcode, uint32_t arg, sdspi_hw_cmd_t *hw_cmd);\r
+void make_hw_cmd(uint32_t opcode, uint32_t arg, int timeout_ms, sdspi_hw_cmd_t *hw_cmd);\r
 \r
 esp_err_t sdspi_host_start_command(int slot, sdspi_hw_cmd_t *cmd,\r
                                    void *data, uint32_t data_size, int flags);\r

diff --git a/components/driver/sdspi_transaction.c b/components/driver/sdspi_transaction.c
index 95f98240f98613eef6e4dd7fe6452c71496147a2..64271749ad5047f7dfba84c77b4d6dba2a38f85c 100644 (file)
--- a/components/driver/sdspi_transaction.c
+++ b/components/driver/sdspi_transaction.c
@@ -36,7 +36,7 @@ static uint8_t sdspi_msg_crc7(sdspi_hw_cmd_t* hw_cmd)
     return sdspi_crc7((const uint8_t *)hw_cmd, bytes_to_crc);
 }
 
-void make_hw_cmd(uint32_t opcode, uint32_t arg, sdspi_hw_cmd_t *hw_cmd)
+void make_hw_cmd(uint32_t opcode, uint32_t arg, int timeout_ms, sdspi_hw_cmd_t *hw_cmd)
 {
     hw_cmd->start_bit = 0;
     hw_cmd->transmission_bit = 1;
@@ -48,6 +48,7 @@ void make_hw_cmd(uint32_t opcode, uint32_t arg, sdspi_hw_cmd_t *hw_cmd)
     uint32_t arg_s = __builtin_bswap32(arg);
     memcpy(hw_cmd->arguments, &arg_s, sizeof(arg_s));
     hw_cmd->crc7 = sdspi_msg_crc7(hw_cmd);
+    hw_cmd->timeout_ms = timeout_ms;
 }
 
 esp_err_t sdspi_host_do_transaction(int slot, sdmmc_command_t *cmdinfo)
@@ -55,7 +56,7 @@ esp_err_t sdspi_host_do_transaction(int slot, sdmmc_command_t *cmdinfo)
     _lock_acquire(&s_lock);
     // Convert the command to wire format
     sdspi_hw_cmd_t hw_cmd;
-    make_hw_cmd(cmdinfo->opcode, cmdinfo->arg, &hw_cmd);
+    make_hw_cmd(cmdinfo->opcode, cmdinfo->arg, cmdinfo->timeout_ms, &hw_cmd);
 
     // Flags indicate which of the transfer types should be used
     int flags = 0;