From 20e078836167e715870a24168e8686457de4ea06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20M=C3=BAdry?= Date: Tue, 15 Oct 2024 22:28:26 +0200 Subject: [PATCH 1/2] fix(sdmmc): Improve SD card state checking after write/read command --- components/driver/include/driver/sdmmc_defs.h | 3 ++- components/sdmmc/sdmmc_cmd.c | 9 +++++++-- components/sdmmc/sdmmc_mmc.c | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/components/driver/include/driver/sdmmc_defs.h b/components/driver/include/driver/sdmmc_defs.h index 56cff2407e..ee396efb0c 100644 --- a/components/driver/include/driver/sdmmc_defs.h +++ b/components/driver/include/driver/sdmmc_defs.h @@ -109,6 +109,7 @@ extern "C" { #define MMC_R1_CURRENT_STATE_POS (9) #define MMC_R1_CURRENT_STATE_MASK (0x1E00)/* card current state */ #define MMC_R1_CURRENT_STATE_TRAN (4) +#define MMC_R1_CURRENT_STATE_STATUS(status) ((status & MMC_R1_CURRENT_STATE_MASK) >> MMC_R1_CURRENT_STATE_POS) /* SPI mode R1 response type bits */ #define SD_SPI_R1_IDLE_STATE (1<<0) @@ -425,7 +426,7 @@ extern "C" { * * 67 45 23 01 ef cd ab 89 * - * MMC_RSP_BITS will extact bits as follows: + * MMC_RSP_BITS will extract bits as follows: * * start=0 len=4 -> result=0x00000007 * start=0 len=12 -> result=0x00000567 diff --git a/components/sdmmc/sdmmc_cmd.c b/components/sdmmc/sdmmc_cmd.c index 7ee5eefe6d..84a34ea623 100644 --- a/components/sdmmc/sdmmc_cmd.c +++ b/components/sdmmc/sdmmc_cmd.c @@ -416,6 +416,11 @@ esp_err_t sdmmc_write_sectors(sdmmc_card_t* card, const void* src, return err; } +static bool mmc_ready_for_data(uint32_t status) +{ + return (status & MMC_R1_READY_FOR_DATA) && (MMC_R1_CURRENT_STATE_STATUS(status) == MMC_R1_CURRENT_STATE_TRAN); +} + esp_err_t sdmmc_write_sectors_dma(sdmmc_card_t* card, const void* src, size_t start_block, size_t block_count) { @@ -448,7 +453,7 @@ esp_err_t sdmmc_write_sectors_dma(sdmmc_card_t* card, const void* src, uint32_t status = 0; size_t count = 0; /* SD mode: wait for the card to become idle based on R1 status */ - while (!host_is_spi(card) && !(status & MMC_R1_READY_FOR_DATA)) { + while (!host_is_spi(card) && !mmc_ready_for_data(status)) { // TODO: add some timeout here err = sdmmc_send_cmd_send_status(card, &status); if (err != ESP_OK) { @@ -543,7 +548,7 @@ esp_err_t sdmmc_read_sectors_dma(sdmmc_card_t* card, void* dst, } uint32_t status = 0; size_t count = 0; - while (!host_is_spi(card) && !(status & MMC_R1_READY_FOR_DATA)) { + while (!host_is_spi(card) && !mmc_ready_for_data(status)) { // TODO: add some timeout here err = sdmmc_send_cmd_send_status(card, &status); if (err != ESP_OK) { diff --git a/components/sdmmc/sdmmc_mmc.c b/components/sdmmc/sdmmc_mmc.c index 4f726ea90a..5d2cefd19e 100644 --- a/components/sdmmc/sdmmc_mmc.c +++ b/components/sdmmc/sdmmc_mmc.c @@ -263,8 +263,8 @@ esp_err_t sdmmc_init_mmc_check_ext_csd(sdmmc_card_t* card) ESP_LOGE(TAG, "%s: send_status returned 0x%x", __func__, err); goto out; } - status = ((status & MMC_R1_CURRENT_STATE_MASK) >> MMC_R1_CURRENT_STATE_POS); - if (status != MMC_R1_CURRENT_STATE_TRAN) { + + if (MMC_R1_CURRENT_STATE_STATUS(status) != MMC_R1_CURRENT_STATE_TRAN) { ESP_LOGE(TAG, "%s: card not in transfer state", __func__); err = ESP_ERR_INVALID_STATE; goto out; From 91b891c259851012db29ab62f5b70c59f878429b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20M=C3=BAdry?= Date: Tue, 15 Oct 2024 23:22:27 +0200 Subject: [PATCH 2/2] fix(sdmmc): Send status (CMD13) even if write/read command fails --- components/driver/include/driver/sdmmc_defs.h | 2 +- components/sdmmc/sdmmc_cmd.c | 30 ++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/components/driver/include/driver/sdmmc_defs.h b/components/driver/include/driver/sdmmc_defs.h index ee396efb0c..373ec15d7d 100644 --- a/components/driver/include/driver/sdmmc_defs.h +++ b/components/driver/include/driver/sdmmc_defs.h @@ -109,7 +109,7 @@ extern "C" { #define MMC_R1_CURRENT_STATE_POS (9) #define MMC_R1_CURRENT_STATE_MASK (0x1E00)/* card current state */ #define MMC_R1_CURRENT_STATE_TRAN (4) -#define MMC_R1_CURRENT_STATE_STATUS(status) ((status & MMC_R1_CURRENT_STATE_MASK) >> MMC_R1_CURRENT_STATE_POS) +#define MMC_R1_CURRENT_STATE_STATUS(status) (((status) & MMC_R1_CURRENT_STATE_MASK) >> MMC_R1_CURRENT_STATE_POS) /* SPI mode R1 response type bits */ #define SD_SPI_R1_IDLE_STATE (1<<0) diff --git a/components/sdmmc/sdmmc_cmd.c b/components/sdmmc/sdmmc_cmd.c index 84a34ea623..0399475d51 100644 --- a/components/sdmmc/sdmmc_cmd.c +++ b/components/sdmmc/sdmmc_cmd.c @@ -416,7 +416,7 @@ esp_err_t sdmmc_write_sectors(sdmmc_card_t* card, const void* src, return err; } -static bool mmc_ready_for_data(uint32_t status) +static bool sdmmc_ready_for_data(uint32_t status) { return (status & MMC_R1_READY_FOR_DATA) && (MMC_R1_CURRENT_STATE_STATUS(status) == MMC_R1_CURRENT_STATE_TRAN); } @@ -445,15 +445,23 @@ esp_err_t sdmmc_write_sectors_dma(sdmmc_card_t* card, const void* src, } else { cmd.arg = start_block * block_size; } + + uint32_t status = 0; esp_err_t err = sdmmc_send_cmd(card, &cmd); + esp_err_t err_cmd13 = sdmmc_send_cmd_send_status(card, &status); + if (err != ESP_OK) { - ESP_LOGE(TAG, "%s: sdmmc_send_cmd returned 0x%x", __func__, err); + if (err_cmd13 == ESP_OK) { + ESP_LOGE(TAG, "%s: sdmmc_send_cmd returned 0x%x, status 0x%" PRIx32, __func__, err, status); + } else { + ESP_LOGE(TAG, "%s: sdmmc_send_cmd returned 0x%x, failed to get status (0x%x)", __func__, err, err_cmd13); + } return err; } - uint32_t status = 0; + size_t count = 0; /* SD mode: wait for the card to become idle based on R1 status */ - while (!host_is_spi(card) && !mmc_ready_for_data(status)) { + while (!host_is_spi(card) && !sdmmc_ready_for_data(status)) { // TODO: add some timeout here err = sdmmc_send_cmd_send_status(card, &status); if (err != ESP_OK) { @@ -541,14 +549,22 @@ esp_err_t sdmmc_read_sectors_dma(sdmmc_card_t* card, void* dst, } else { cmd.arg = start_block * block_size; } + + uint32_t status = 0; esp_err_t err = sdmmc_send_cmd(card, &cmd); + esp_err_t err_cmd13 = sdmmc_send_cmd_send_status(card, &status); + if (err != ESP_OK) { - ESP_LOGE(TAG, "%s: sdmmc_send_cmd returned 0x%x", __func__, err); + if (err_cmd13 == ESP_OK) { + ESP_LOGE(TAG, "%s: sdmmc_send_cmd returned 0x%x, status 0x%" PRIx32, __func__, err, status); + } else { + ESP_LOGE(TAG, "%s: sdmmc_send_cmd returned 0x%x, failed to get status (0x%x)", __func__, err, err_cmd13); + } return err; } - uint32_t status = 0; + size_t count = 0; - while (!host_is_spi(card) && !mmc_ready_for_data(status)) { + while (!host_is_spi(card) && !sdmmc_ready_for_data(status)) { // TODO: add some timeout here err = sdmmc_send_cmd_send_status(card, &status); if (err != ESP_OK) {