From bbbdec24d93d1edab2b7b3e6a7c1b6b4667c319c Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 6 Feb 2018 10:53:41 +0800 Subject: [PATCH 1/5] sdspi: handle error flags for R3/R7 responses Previously error flags were only handled for R1 responses. This change moves error handling into a separate function and calls it for R1/R3/R7. --- components/driver/include/driver/sdmmc_defs.h | 6 +++ components/driver/sdspi_transaction.c | 42 +++++++++++++------ components/sdmmc/sdmmc_cmd.c | 4 +- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/components/driver/include/driver/sdmmc_defs.h b/components/driver/include/driver/sdmmc_defs.h index c13df6d1aa..96e0743d5f 100644 --- a/components/driver/include/driver/sdmmc_defs.h +++ b/components/driver/include/driver/sdmmc_defs.h @@ -86,7 +86,13 @@ /* SPI mode R1 response type bits */ #define SD_SPI_R1_IDLE_STATE (1<<0) +#define SD_SPI_R1_ERASE_RST (1<<1) +#define SD_SPI_R1_ILLEGAL_CMD (1<<2) #define SD_SPI_R1_CMD_CRC_ERR (1<<3) +#define SD_SPI_R1_ERASE_SEQ_ERR (1<<4) +#define SD_SPI_R1_ADDR_ERR (1<<5) +#define SD_SPI_R1_PARAM_ERR (1<<6) +#define SD_SPI_R1_NO_RESPONSE (1<<7) /* 48-bit response decoding (32 bits w/o CRC) */ #define MMC_R1(resp) ((resp)[0]) diff --git a/components/driver/sdspi_transaction.c b/components/driver/sdspi_transaction.c index 64271749ad..728979b333 100644 --- a/components/driver/sdspi_transaction.c +++ b/components/driver/sdspi_transaction.c @@ -51,6 +51,34 @@ void make_hw_cmd(uint32_t opcode, uint32_t arg, int timeout_ms, sdspi_hw_cmd_t * hw_cmd->timeout_ms = timeout_ms; } +static void r1_response_to_err(uint8_t r1, esp_err_t *out_err) +{ + if (r1 & SD_SPI_R1_NO_RESPONSE) { + ESP_LOGD(TAG, "R1 response not found"); + *out_err = ESP_ERR_TIMEOUT; + } else if (r1 & SD_SPI_R1_CMD_CRC_ERR) { + ESP_LOGD(TAG, "R1 response: command CRC error"); + *out_err = ESP_ERR_INVALID_CRC; + } else if (r1 & SD_SPI_R1_ILLEGAL_CMD) { + ESP_LOGD(TAG, "R1 response: command not supported"); + *out_err = ESP_ERR_NOT_SUPPORTED; + } else if (r1 & SD_SPI_R1_ADDR_ERR) { + ESP_LOGD(TAG, "R1 response: alignment error"); + *out_err = ESP_ERR_INVALID_ARG; + } else if (r1 & SD_SPI_R1_PARAM_ERR) { + ESP_LOGD(TAG, "R1 response: size error"); + *out_err = ESP_ERR_INVALID_SIZE; + } else if ((r1 & SD_SPI_R1_ERASE_RST) || + (r1 & SD_SPI_R1_ERASE_SEQ_ERR)) { + *out_err = ESP_ERR_INVALID_STATE; + } else if (r1 & SD_SPI_R1_IDLE_STATE) { + // Idle state is handled at command layer + } else if (r1 != 0) { + ESP_LOGD(TAG, "R1 response: unexpected value 0x%02x", r1); + *out_err = ESP_ERR_INVALID_RESPONSE; + } +} + esp_err_t sdspi_host_do_transaction(int slot, sdmmc_command_t *cmdinfo) { _lock_acquire(&s_lock); @@ -93,21 +121,11 @@ esp_err_t sdspi_host_do_transaction(int slot, sdmmc_command_t *cmdinfo) // Some errors should be reported using return code if (flags & SDSPI_CMD_FLAG_RSP_R1) { cmdinfo->response[0] = hw_cmd.r1; - if (hw_cmd.r1 == 0xff) { - // No response received at all - } else if (hw_cmd.r1 & SD_SPI_R1_CMD_CRC_ERR) { - ret = ESP_ERR_INVALID_CRC; - } else if (hw_cmd.r1 & SD_SPI_R1_IDLE_STATE) { - // Idle state is handled at command layer - } else if (hw_cmd.r1 != 0) { - ESP_LOGD(TAG, "Unexpected R1 response: 0x%02x", hw_cmd.r1); - } + r1_response_to_err(hw_cmd.r1, &ret); } else if (flags & SDSPI_CMD_FLAG_RSP_R2) { cmdinfo->response[0] = (((uint32_t)hw_cmd.r1) << 8) | (hw_cmd.response[0] >> 24); } else if (flags & (SDSPI_CMD_FLAG_RSP_R3 | SDSPI_CMD_FLAG_RSP_R7)) { - // Drop r1 response, only copy the other 4 bytes of data - // TODO: can we somehow preserve r1 response and keep upper layer - // same as in SD mode? + r1_response_to_err(hw_cmd.r1, &ret); cmdinfo->response[0] = __builtin_bswap32(hw_cmd.response[0]); } } diff --git a/components/sdmmc/sdmmc_cmd.c b/components/sdmmc/sdmmc_cmd.c index 594e469767..2c58843c2d 100644 --- a/components/sdmmc/sdmmc_cmd.c +++ b/components/sdmmc/sdmmc_cmd.c @@ -112,7 +112,9 @@ esp_err_t sdmmc_card_init(const sdmmc_host_t* config, sdmmc_card_t* card) ESP_LOGD(TAG, "SDHC/SDXC card"); host_ocr |= SD_OCR_SDHC_CAP; } else if (err == ESP_ERR_TIMEOUT) { - ESP_LOGD(TAG, "CMD8 timeout; not an SDHC/SDXC card"); + ESP_LOGD(TAG, "CMD8 timeout; not an SD v2.00 card"); + } else if (is_spi && err == ESP_ERR_NOT_SUPPORTED) { + ESP_LOGD(TAG, "CMD8 rejected; not an SD v2.00 card"); } else { ESP_LOGE(TAG, "%s: send_if_cond (1) returned 0x%x", __func__, err); return err; From 229f67b8162a2feed3c3a5d38b225a59ef98ec7a Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Fri, 9 Feb 2018 01:57:48 +0800 Subject: [PATCH 2/5] sdspi: handle delayed R1 responses for data read commands --- components/driver/sdspi_host.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/components/driver/sdspi_host.c b/components/driver/sdspi_host.c index 74cc24c244..ef8d627703 100644 --- a/components/driver/sdspi_host.c +++ b/components/driver/sdspi_host.c @@ -575,8 +575,9 @@ static esp_err_t start_command_read_blocks(int slot, sdspi_hw_cmd_t *cmd, { bool need_stop_command = rx_length > SDSPI_MAX_DATA_LEN; spi_transaction_t* t_command = get_transaction(slot); + const int cmd_extra_bytes = 8; *t_command = (spi_transaction_t) { - .length = (SDSPI_CMD_R1_SIZE + 8) * 8, + .length = (SDSPI_CMD_R1_SIZE + cmd_extra_bytes) * 8, .tx_buffer = cmd, .rx_buffer = cmd, }; @@ -587,9 +588,21 @@ static esp_err_t start_command_read_blocks(int slot, sdspi_hw_cmd_t *cmd, release_transaction(slot); uint8_t* cmd_u8 = (uint8_t*) cmd; - size_t pre_scan_data_size = 8; + size_t pre_scan_data_size = cmd_extra_bytes; uint8_t* pre_scan_data_ptr = cmd_u8 + SDSPI_CMD_R1_SIZE; + /* R1 response is delayed by 1-8 bytes from the request. + * This loop searches for the response and writes it to cmd->r1. + */ + while ((cmd->r1 & SD_SPI_R1_NO_RESPONSE) != 0 && pre_scan_data_size > 0) { + cmd->r1 = *pre_scan_data_ptr; + ++pre_scan_data_ptr; + --pre_scan_data_size; + } + if (cmd->r1 & SD_SPI_R1_NO_RESPONSE) { + ESP_LOGD(TAG, "no response token found"); + return ESP_ERR_TIMEOUT; + } while (rx_length > 0) { size_t extra_data_size = 0; From 4a2489b99a49ba2801fa4fd6fae7dba0a9c70428 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 12 Mar 2018 15:47:06 +0800 Subject: [PATCH 3/5] sdspi: fix compatibility issue in multi block read SDSPI driver optimized polling of the response tokens by requesting two extra bytes on top of the block size (512) and CRC (2), and checking whether these bytes contained the data response token or not. In case the token was there, further polling would not need to happen, thereby reducing latency between two consecutive blocks transferred. However this caused compatibility issues when these two extra bytes were sent after reading the final block. When STOP_TRANSMISSION command was sent, these extra two bytes were treated as part of the command, causing an invalid command error. This fixes the logic by only requesting extra two bytes if the block being read is not the final block. In addition to that, more strict error checking is implemented for command response tokens. --- components/driver/sdspi_host.c | 39 ++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/components/driver/sdspi_host.c b/components/driver/sdspi_host.c index ef8d627703..6ac8bfbe4d 100644 --- a/components/driver/sdspi_host.c +++ b/components/driver/sdspi_host.c @@ -34,6 +34,8 @@ #define GPIO_UNUSED 0xff //!< Flag indicating that CD/WP is unused /// Size of the buffer returned by get_block_buf #define SDSPI_BLOCK_BUF_SIZE (SDSPI_MAX_DATA_LEN + 4) +/// Maximum number of dummy bytes between the request and response (minimum is 1) +#define SDSPI_RESPONSE_MAX_DELAY 8 /// Structure containing run time configuration for a single SD slot @@ -422,6 +424,30 @@ static esp_err_t start_command_default(int slot, int flags, sdspi_hw_cmd_t *cmd) .rx_buffer = cmd }; esp_err_t ret = spi_device_transmit(spi_handle(slot), &t); + if (cmd->cmd_index == MMC_STOP_TRANSMISSION) { + /* response is a stuff byte from previous transfer, ignore it */ + cmd->r1 = 0xff; + } + int response_delay_bytes = SDSPI_RESPONSE_MAX_DELAY; + while ((cmd->r1 & SD_SPI_R1_NO_RESPONSE) != 0 && response_delay_bytes-- > 0) { + spi_transaction_t* t = get_transaction(slot); + *t = (spi_transaction_t) { + .flags = SPI_TRANS_USE_RXDATA | SPI_TRANS_USE_TXDATA, + .length = 8, + }; + t->tx_data[0] = 0xff; + ret = spi_device_transmit(spi_handle(slot), t); + uint8_t r1 = t->rx_data[0]; + release_transaction(slot); + if (ret != ESP_OK) { + return ret; + } + cmd->r1 = r1; + } + if (cmd->r1 & SD_SPI_R1_NO_RESPONSE) { + ESP_LOGD(TAG, "%s: no response token found", __func__); + return ESP_ERR_TIMEOUT; + } return ret; } @@ -564,6 +590,9 @@ static esp_err_t poll_data_token(int slot, spi_transaction_t* t, * indicating the start of the next block. Actual scanning is done by * setting pre_scan_data_ptr to point to these last 2 bytes, and setting * pre_scan_data_size = 2, then going to step 2 to receive the next block. + * When the final block is being received, the number of extra bytes is 2 + * (only for CRC), because we don't need to wait for start token of the + * next block, and some cards are getting confused by these two extra bytes. * * With this approach the delay between blocks of a multi-block transfer is * ~95 microseconds, out of which 35 microseconds are spend doing the CRC check. @@ -575,9 +604,8 @@ static esp_err_t start_command_read_blocks(int slot, sdspi_hw_cmd_t *cmd, { bool need_stop_command = rx_length > SDSPI_MAX_DATA_LEN; spi_transaction_t* t_command = get_transaction(slot); - const int cmd_extra_bytes = 8; *t_command = (spi_transaction_t) { - .length = (SDSPI_CMD_R1_SIZE + cmd_extra_bytes) * 8, + .length = (SDSPI_CMD_R1_SIZE + SDSPI_RESPONSE_MAX_DELAY) * 8, .tx_buffer = cmd, .rx_buffer = cmd, }; @@ -588,7 +616,7 @@ static esp_err_t start_command_read_blocks(int slot, sdspi_hw_cmd_t *cmd, release_transaction(slot); uint8_t* cmd_u8 = (uint8_t*) cmd; - size_t pre_scan_data_size = cmd_extra_bytes; + size_t pre_scan_data_size = SDSPI_RESPONSE_MAX_DELAY; uint8_t* pre_scan_data_ptr = cmd_u8 + SDSPI_CMD_R1_SIZE; /* R1 response is delayed by 1-8 bytes from the request. @@ -640,7 +668,7 @@ static esp_err_t start_command_read_blocks(int slot, sdspi_hw_cmd_t *cmd, } // receive actual data - const size_t receive_extra_bytes = 4; + const size_t receive_extra_bytes = (rx_length > SDSPI_MAX_DATA_LEN) ? 4 : 2; memset(rx_data, 0xff, will_receive + receive_extra_bytes); spi_transaction_t* t_data = get_transaction(slot); *t_data = (spi_transaction_t) { @@ -695,6 +723,9 @@ static esp_err_t start_command_read_blocks(int slot, sdspi_hw_cmd_t *cmd, if (ret != ESP_OK) { return ret; } + if (stop_cmd.r1 != 0) { + ESP_LOGD(TAG, "%s: STOP_TRANSMISSION response 0x%02x", __func__, stop_cmd.r1); + } spi_transaction_t* t_poll = get_transaction(slot); ret = poll_busy(slot, t_poll, cmd->timeout_ms); release_transaction(slot); From e20e64ace9948189103cd36565bfe79901444c21 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Fri, 30 Mar 2018 18:40:06 +0800 Subject: [PATCH 4/5] fatfs/test: use 16k cluster size to speed up formatting --- components/fatfs/test/test_fatfs_sdmmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/fatfs/test/test_fatfs_sdmmc.c b/components/fatfs/test/test_fatfs_sdmmc.c index ae53009007..bc833e3d63 100644 --- a/components/fatfs/test/test_fatfs_sdmmc.c +++ b/components/fatfs/test/test_fatfs_sdmmc.c @@ -39,7 +39,8 @@ static void test_setup(void) sdmmc_slot_config_t slot_config = SDMMC_SLOT_CONFIG_DEFAULT(); esp_vfs_fat_sdmmc_mount_config_t mount_config = { .format_if_mount_failed = true, - .max_files = 5 + .max_files = 5, + .allocation_unit_size = 16 * 1024 }; TEST_ESP_OK(esp_vfs_fat_sdmmc_mount("/sdcard", &host, &slot_config, &mount_config, NULL)); } From d6b1d0bb3d75e3402acfc8804d4dad0b97743846 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Fri, 30 Mar 2018 18:44:13 +0800 Subject: [PATCH 5/5] fatfs/test: enable tests on SD card --- components/fatfs/test/test_fatfs_sdmmc.c | 44 +++++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/components/fatfs/test/test_fatfs_sdmmc.c b/components/fatfs/test/test_fatfs_sdmmc.c index bc833e3d63..6dc1ec3fc0 100644 --- a/components/fatfs/test/test_fatfs_sdmmc.c +++ b/components/fatfs/test/test_fatfs_sdmmc.c @@ -51,9 +51,8 @@ static void test_teardown(void) } static const char* test_filename = "/sdcard/hello.txt"; -static const char* test_filename_utf_8 = "/sdcard/测试文件.txt"; -TEST_CASE("Mount fails cleanly without card inserted", "[fatfs][ignore]") +TEST_CASE("Mount fails cleanly without card inserted", "[fatfs][sd][ignore]") { size_t heap_size; HEAP_SIZE_CAPTURE(heap_size); @@ -65,22 +64,22 @@ TEST_CASE("Mount fails cleanly without card inserted", "[fatfs][ignore]") }; for (int i = 0; i < 3; ++i) { - printf("Initializing card, attempt %d ", i); + printf("Initializing card, attempt %d\n", i); esp_err_t err = esp_vfs_fat_sdmmc_mount("/sdcard", &host, &slot_config, &mount_config, NULL); - printf(" err=%d\n", err); - TEST_ESP_ERR(ESP_FAIL, err); + printf("err=%d\n", err); + TEST_ESP_ERR(ESP_ERR_TIMEOUT, err); } HEAP_SIZE_CHECK(heap_size, 0); } -TEST_CASE("(SD) can create and write file", "[fatfs][sdcard][ignore]") +TEST_CASE("(SD) can create and write file", "[fatfs][sd][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_create_file_with_text(test_filename, fatfs_test_hello_str); test_teardown(); } -TEST_CASE("(SD) can read file", "[fatfs][ignore]") +TEST_CASE("(SD) can read file", "[fatfs][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_create_file_with_text(test_filename, fatfs_test_hello_str); @@ -89,7 +88,7 @@ TEST_CASE("(SD) can read file", "[fatfs][ignore]") } -TEST_CASE("(SD) overwrite and append file", "[fatfs][sdcard][ignore]") +TEST_CASE("(SD) overwrite and append file", "[fatfs][sd][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_overwrite_append(test_filename); @@ -97,56 +96,56 @@ TEST_CASE("(SD) overwrite and append file", "[fatfs][sdcard][ignore]") } -TEST_CASE("(SD) can lseek", "[fatfs][sdcard][ignore]") +TEST_CASE("(SD) can lseek", "[fatfs][sd][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_lseek("/sdcard/seek.txt"); test_teardown(); } -TEST_CASE("(SD) stat returns correct values", "[fatfs][ignore]") +TEST_CASE("(SD) stat returns correct values", "[fatfs][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_stat("/sdcard/stat.txt", "/sdcard"); test_teardown(); } -TEST_CASE("(SD) unlink removes a file", "[fatfs][ignore]") +TEST_CASE("(SD) unlink removes a file", "[fatfs][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_unlink("/sdcard/unlink.txt"); test_teardown(); } -TEST_CASE("(SD) link copies a file, rename moves a file", "[fatfs][ignore]") +TEST_CASE("(SD) link copies a file, rename moves a file", "[fatfs][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_link_rename("/sdcard/link"); test_teardown(); } -TEST_CASE("(SD) can create and remove directories", "[fatfs][ignore]") +TEST_CASE("(SD) can create and remove directories", "[fatfs][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_mkdir_rmdir("/sdcard/dir"); test_teardown(); } -TEST_CASE("(SD) can opendir root directory of FS", "[fatfs][ignore]") +TEST_CASE("(SD) can opendir root directory of FS", "[fatfs][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_can_opendir("/sdcard"); test_teardown(); } -TEST_CASE("(SD) opendir, readdir, rewinddir, seekdir work as expected", "[fatfs][ignore]") +TEST_CASE("(SD) opendir, readdir, rewinddir, seekdir work as expected", "[fatfs][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_opendir_readdir_rewinddir("/sdcard/dir"); test_teardown(); } -TEST_CASE("(SD) multiple tasks can use same volume", "[fatfs][ignore]") +TEST_CASE("(SD) multiple tasks can use same volume", "[fatfs][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_concurrent("/sdcard/f"); @@ -155,7 +154,7 @@ TEST_CASE("(SD) multiple tasks can use same volume", "[fatfs][ignore]") static void speed_test(void* buf, size_t buf_size, size_t file_size, bool write); -TEST_CASE("(SD) write/read speed test", "[fatfs][sdcard][ignore]") +TEST_CASE("(SD) write/read speed test", "[fatfs][sd][test_env=UT_T1_SDMODE]") { size_t heap_size; HEAP_SIZE_CAPTURE(heap_size); @@ -165,7 +164,7 @@ TEST_CASE("(SD) write/read speed test", "[fatfs][sdcard][ignore]") for (size_t i = 0; i < buf_size / 4; ++i) { buf[i] = esp_random(); } - const size_t file_size = 4 * 1024 * 1024; + const size_t file_size = 1 * 1024 * 1024; speed_test(buf, 4 * 1024, file_size, true); speed_test(buf, 8 * 1024, file_size, true); @@ -197,7 +196,7 @@ static void speed_test(void* buf, size_t buf_size, size_t file_size, bool write) TEST_ESP_OK(esp_vfs_fat_sdmmc_unmount()); } -TEST_CASE("(SD) mount two FAT partitions, SDMMC and WL, at the same time", "[fatfs][sdcard][ignore]") +TEST_CASE("(SD) mount two FAT partitions, SDMMC and WL, at the same time", "[fatfs][sd][test_env=UT_T1_SDMODE]") { esp_vfs_fat_sdmmc_mount_config_t mount_config = { .format_if_mount_failed = true, @@ -248,7 +247,10 @@ TEST_CASE("(SD) mount two FAT partitions, SDMMC and WL, at the same time", "[fat * Ensure that the text editor is UTF-8 compatible when compiling these tests. */ #if defined(CONFIG_FATFS_API_ENCODING_UTF_8) && (CONFIG_FATFS_CODEPAGE == 936) -TEST_CASE("(SD) can read file using UTF-8 encoded strings", "[fatfs][ignore]") + +static const char* test_filename_utf_8 = "/sdcard/测试文件.txt"; + +TEST_CASE("(SD) can read file using UTF-8 encoded strings", "[fatfs][sd][test_env=UT_T1_SDMODE]") { test_setup(); test_fatfs_create_file_with_text(test_filename_utf_8, fatfs_test_hello_str_utf); @@ -262,4 +264,4 @@ TEST_CASE("(SD) opendir, readdir, rewinddir, seekdir work as expected using UTF- test_fatfs_opendir_readdir_rewinddir_utf_8("/sdcard/目录"); test_teardown(); } -#endif +#endif // CONFIG_FATFS_API_ENCODING_UTF_8 && CONFIG_FATFS_CODEPAGE == 936