diff --git a/components/driver/sdmmc_transaction.c b/components/driver/sdmmc_transaction.c index 52dec2991..8abf9bb92 100644 --- a/components/driver/sdmmc_transaction.c +++ b/components/driver/sdmmc_transaction.c @@ -74,8 +74,10 @@ static esp_pm_lock_handle_t s_pm_lock; static esp_err_t handle_idle_state_events(); static sdmmc_hw_cmd_t make_hw_cmd(sdmmc_command_t* cmd); -static esp_err_t handle_event(sdmmc_command_t* cmd, sdmmc_req_state_t* pstate); -static esp_err_t process_events(sdmmc_event_t evt, sdmmc_command_t* cmd, sdmmc_req_state_t* pstate); +static esp_err_t handle_event(sdmmc_command_t* cmd, sdmmc_req_state_t* state, + sdmmc_event_t* unhandled_events); +static esp_err_t process_events(sdmmc_event_t evt, sdmmc_command_t* cmd, + sdmmc_req_state_t* pstate, sdmmc_event_t* unhandled_events); static void process_command_response(uint32_t status, sdmmc_command_t* cmd); static void fill_dma_descriptors(size_t num_desc); static size_t get_free_descriptors_count(); @@ -122,9 +124,10 @@ esp_err_t sdmmc_host_do_transaction(int slot, sdmmc_command_t* cmdinfo) // convert cmdinfo to hardware register value sdmmc_hw_cmd_t hw_cmd = make_hw_cmd(cmdinfo); if (cmdinfo->data) { - if (cmdinfo->datalen < 4 || cmdinfo->blklen % 4 != 0) { - ESP_LOGD(TAG, "%s: invalid size: total=%d block=%d", - __func__, cmdinfo->datalen, cmdinfo->blklen); + // Length should be either <4 or >=4 and =0 (mod 4). + if (cmdinfo->datalen >= 4 && cmdinfo->datalen % 4 != 0) { + ESP_LOGD(TAG, "%s: invalid size: total=%d", + __func__, cmdinfo->datalen); ret = ESP_ERR_INVALID_SIZE; goto out; } @@ -156,8 +159,9 @@ esp_err_t sdmmc_host_do_transaction(int slot, sdmmc_command_t* cmdinfo) // process events until transfer is complete cmdinfo->error = ESP_OK; sdmmc_req_state_t state = SDMMC_SENDING_CMD; + sdmmc_event_t unhandled_events = { 0 }; while (state != SDMMC_IDLE) { - ret = handle_event(cmdinfo, &state); + ret = handle_event(cmdinfo, &state, &unhandled_events); if (ret != ESP_OK) { break; } @@ -212,7 +216,8 @@ static void fill_dma_descriptors(size_t num_desc) desc->owned_by_idmac = 1; desc->buffer1_ptr = s_cur_transfer.ptr; desc->next_desc_ptr = (last) ? NULL : &s_dma_desc[(next + 1) % SDMMC_DMA_DESC_CNT]; - desc->buffer1_size = size_to_fill; + assert(size_to_fill < 4 || size_to_fill % 4 == 0); + desc->buffer1_size = (size_to_fill + 3) & (~3); s_cur_transfer.size_remaining -= size_to_fill; s_cur_transfer.ptr += size_to_fill; @@ -245,10 +250,11 @@ static esp_err_t handle_idle_state_events() } -static esp_err_t handle_event(sdmmc_command_t* cmd, sdmmc_req_state_t* state) +static esp_err_t handle_event(sdmmc_command_t* cmd, sdmmc_req_state_t* state, + sdmmc_event_t* unhandled_events) { - sdmmc_event_t evt; - esp_err_t err = sdmmc_host_wait_for_event(cmd->timeout_ms / portTICK_PERIOD_MS, &evt); + sdmmc_event_t event; + esp_err_t err = sdmmc_host_wait_for_event(cmd->timeout_ms / portTICK_PERIOD_MS, &event); if (err != ESP_OK) { ESP_LOGE(TAG, "sdmmc_host_wait_for_event returned 0x%x", err); if (err == ESP_ERR_TIMEOUT) { @@ -256,8 +262,13 @@ static esp_err_t handle_event(sdmmc_command_t* cmd, sdmmc_req_state_t* state) } return err; } - ESP_LOGV(TAG, "sdmmc_handle_event: evt %08x %08x", evt.sdmmc_status, evt.dma_status); - process_events(evt, cmd, state); + ESP_LOGV(TAG, "sdmmc_handle_event: event %08x %08x, unhandled %08x %08x", + event.sdmmc_status, event.dma_status, + unhandled_events->sdmmc_status, unhandled_events->dma_status); + event.sdmmc_status |= unhandled_events->sdmmc_status; + event.dma_status |= unhandled_events->dma_status; + process_events(event, cmd, state, unhandled_events); + ESP_LOGV(TAG, "sdmmc_handle_event: events unhandled: %08x %08x", unhandled_events->sdmmc_status, unhandled_events->dma_status); return ESP_OK; } @@ -363,7 +374,8 @@ static inline bool mask_check_and_clear(uint32_t* state, uint32_t mask) { return ret; } -static esp_err_t process_events(sdmmc_event_t evt, sdmmc_command_t* cmd, sdmmc_req_state_t* pstate) +static esp_err_t process_events(sdmmc_event_t evt, sdmmc_command_t* cmd, + sdmmc_req_state_t* pstate, sdmmc_event_t* unhandled_events) { const char* const s_state_names[] __attribute__((unused)) = { "IDLE", @@ -372,7 +384,8 @@ static esp_err_t process_events(sdmmc_event_t evt, sdmmc_command_t* cmd, sdmmc_r "BUSY" }; sdmmc_event_t orig_evt = evt; - ESP_LOGV(TAG, "%s: state=%s", __func__, s_state_names[*pstate]); + ESP_LOGV(TAG, "%s: state=%s evt=%x dma=%x", __func__, s_state_names[*pstate], + evt.sdmmc_status, evt.dma_status); sdmmc_req_state_t next_state = *pstate; sdmmc_req_state_t state = (sdmmc_req_state_t) -1; while (next_state != state) { @@ -386,12 +399,19 @@ static esp_err_t process_events(sdmmc_event_t evt, sdmmc_command_t* cmd, sdmmc_r process_command_response(orig_evt.sdmmc_status, cmd); break; // Need to wait for the CMD_DONE interrupt } - process_command_response(orig_evt.sdmmc_status, cmd); - if (cmd->error != ESP_OK || cmd->data == NULL) { - next_state = SDMMC_IDLE; - break; + if (mask_check_and_clear(&evt.sdmmc_status, SDMMC_INTMASK_CMD_DONE)) { + process_command_response(orig_evt.sdmmc_status, cmd); + if (cmd->error != ESP_OK) { + next_state = SDMMC_IDLE; + break; + } + + if (cmd->data == NULL) { + next_state = SDMMC_IDLE; + } else { + next_state = SDMMC_SENDING_DATA; + } } - next_state = SDMMC_SENDING_DATA; break; @@ -429,6 +449,7 @@ static esp_err_t process_events(sdmmc_event_t evt, sdmmc_command_t* cmd, sdmmc_r ESP_LOGV(TAG, "%s state=%s next_state=%s", __func__, s_state_names[state], s_state_names[next_state]); } *pstate = state; + *unhandled_events = evt; return ESP_OK; } diff --git a/components/sdmmc/sdmmc_cmd.c b/components/sdmmc/sdmmc_cmd.c index f46dbf324..f315a1246 100644 --- a/components/sdmmc/sdmmc_cmd.c +++ b/components/sdmmc/sdmmc_cmd.c @@ -1269,22 +1269,57 @@ static esp_err_t sdmmc_io_rw_extended(sdmmc_card_t* card, int func, esp_err_t sdmmc_io_read_bytes(sdmmc_card_t* card, uint32_t function, uint32_t addr, void* dst, size_t size) { - return sdmmc_io_rw_extended(card, function, addr, - SD_ARG_CMD53_READ | SD_ARG_CMD53_INCREMENT, - dst, size); + /* host quirk: SDIO transfer with length not divisible by 4 bytes + * has to be split into two transfers: one with aligned length, + * the other one for the remaining 1-3 bytes. + */ + uint8_t *pc_dst = dst; + while (size > 0) { + size_t size_aligned = size & (~3); + size_t will_transfer = size_aligned > 0 ? size_aligned : size; + + esp_err_t err = sdmmc_io_rw_extended(card, function, addr, + SD_ARG_CMD53_READ | SD_ARG_CMD53_INCREMENT, + pc_dst, will_transfer); + if (err != ESP_OK) { + return err; + } + pc_dst += will_transfer; + size -= will_transfer; + addr += will_transfer; + } + return ESP_OK; } esp_err_t sdmmc_io_write_bytes(sdmmc_card_t* card, uint32_t function, uint32_t addr, const void* src, size_t size) { - return sdmmc_io_rw_extended(card, function, addr, - SD_ARG_CMD53_WRITE | SD_ARG_CMD53_INCREMENT, - (void*) src, size); + /* same host quirk as in sdmmc_io_read_bytes */ + const uint8_t *pc_src = (const uint8_t*) src; + + while (size > 0) { + size_t size_aligned = size & (~3); + size_t will_transfer = size_aligned > 0 ? size_aligned : size; + + esp_err_t err = sdmmc_io_rw_extended(card, function, addr, + SD_ARG_CMD53_WRITE | SD_ARG_CMD53_INCREMENT, + (void*) pc_src, will_transfer); + if (err != ESP_OK) { + return err; + } + pc_src += will_transfer; + size -= will_transfer; + addr += will_transfer; + } + return ESP_OK; } esp_err_t sdmmc_io_read_blocks(sdmmc_card_t* card, uint32_t function, uint32_t addr, void* dst, size_t size) { + if (size % 4 != 0) { + return ESP_ERR_INVALID_SIZE; + } return sdmmc_io_rw_extended(card, function, addr, SD_ARG_CMD53_READ | SD_ARG_CMD53_INCREMENT | SD_ARG_CMD53_BLOCK_MODE, dst, size); @@ -1293,6 +1328,9 @@ esp_err_t sdmmc_io_read_blocks(sdmmc_card_t* card, uint32_t function, esp_err_t sdmmc_io_write_blocks(sdmmc_card_t* card, uint32_t function, uint32_t addr, const void* src, size_t size) { + if (size % 4 != 0) { + return ESP_ERR_INVALID_SIZE; + } return sdmmc_io_rw_extended(card, function, addr, SD_ARG_CMD53_WRITE | SD_ARG_CMD53_INCREMENT | SD_ARG_CMD53_BLOCK_MODE, (void*) src, size); diff --git a/components/sdmmc/test/test_sdio.c b/components/sdmmc/test/test_sdio.c index 8b885baf5..f4eae0598 100644 --- a/components/sdmmc/test/test_sdio.c +++ b/components/sdmmc/test/test_sdio.c @@ -309,35 +309,33 @@ static void test_cmd52_read_write_single_byte(sdmmc_card_t* card) TEST_ASSERT_EQUAL_UINT8(test_byte_2, val); } -static void test_cmd53_read_write_multiple_bytes(sdmmc_card_t* card) +static void test_cmd53_read_write_multiple_bytes(sdmmc_card_t* card, size_t n_bytes) { printf("Write multiple bytes using CMD53\n"); const size_t scratch_area_reg = SLCHOST_CONF_W0 - DR_REG_SLCHOST_BASE; uint8_t* src = heap_caps_malloc(512, MALLOC_CAP_DMA); uint32_t* src_32 = (uint32_t*) src; - const size_t n_words = 6; - srand(0); - for (size_t i = 0; i < n_words; ++i) { + + for (size_t i = 0; i < (n_bytes + 3) / 4; ++i) { src_32[i] = rand(); } - size_t len = n_words * sizeof(uint32_t); - TEST_ESP_OK(sdmmc_io_write_bytes(card, 1, scratch_area_reg, src, len)); - ESP_LOG_BUFFER_HEX(TAG, src, len); + TEST_ESP_OK(sdmmc_io_write_bytes(card, 1, scratch_area_reg, src, n_bytes)); + ESP_LOG_BUFFER_HEX(TAG, src, n_bytes); printf("Read back using CMD52\n"); uint8_t* dst = heap_caps_malloc(512, MALLOC_CAP_DMA); - for (size_t i = 0; i < len; ++i) { + for (size_t i = 0; i < n_bytes; ++i) { TEST_ESP_OK(sdmmc_io_read_byte(card, 1, scratch_area_reg + i, &dst[i])); } - ESP_LOG_BUFFER_HEX(TAG, dst, len); - TEST_ASSERT_EQUAL_UINT8_ARRAY(src, dst, len); + ESP_LOG_BUFFER_HEX(TAG, dst, n_bytes); + TEST_ASSERT_EQUAL_UINT8_ARRAY(src, dst, n_bytes); printf("Read back using CMD53\n"); - TEST_ESP_OK(sdmmc_io_read_bytes(card, 1, scratch_area_reg, dst, len)); - ESP_LOG_BUFFER_HEX(TAG, dst, len); - TEST_ASSERT_EQUAL_UINT8_ARRAY(src, dst, len); + TEST_ESP_OK(sdmmc_io_read_bytes(card, 1, scratch_area_reg, dst, n_bytes)); + ESP_LOG_BUFFER_HEX(TAG, dst, n_bytes); + TEST_ASSERT_EQUAL_UINT8_ARRAY(src, dst, n_bytes); free(src); free(dst); @@ -364,9 +362,16 @@ TEST_CASE("can probe and talk to ESP32 SDIO slave", "[sdio][ignore]") /* Set up standard SDIO registers */ sdio_slave_common_init(card); - for (int repeat = 0; repeat < 10; ++repeat) { + srand(0); + for (int repeat = 0; repeat < 4; ++repeat) { test_cmd52_read_write_single_byte(card); - test_cmd53_read_write_multiple_bytes(card); + test_cmd53_read_write_multiple_bytes(card, 1); + test_cmd53_read_write_multiple_bytes(card, 2); + test_cmd53_read_write_multiple_bytes(card, 3); + test_cmd53_read_write_multiple_bytes(card, 4); + test_cmd53_read_write_multiple_bytes(card, 5); + test_cmd53_read_write_multiple_bytes(card, 23); + test_cmd53_read_write_multiple_bytes(card, 24); } sdio_slave_set_blocksize(card, 0, 512);