diff --git a/components/driver/include/driver/sdmmc_host.h b/components/driver/include/driver/sdmmc_host.h index aa3357a736..2c610ad365 100644 --- a/components/driver/include/driver/sdmmc_host.h +++ b/components/driver/include/driver/sdmmc_host.h @@ -43,9 +43,9 @@ extern "C" { .set_card_clk = &sdmmc_host_set_card_clk, \ .do_transaction = &sdmmc_host_do_transaction, \ .deinit = &sdmmc_host_deinit, \ - .command_timeout_ms = 0, \ .io_int_enable = sdmmc_host_io_int_enable, \ .io_int_wait = sdmmc_host_io_int_wait, \ + .command_timeout_ms = 0, \ } /** diff --git a/components/driver/include/driver/sdspi_host.h b/components/driver/include/driver/sdspi_host.h index 9dad6cf7a7..9f72cb0dc4 100644 --- a/components/driver/include/driver/sdspi_host.h +++ b/components/driver/include/driver/sdspi_host.h @@ -40,6 +40,7 @@ extern "C" { .io_voltage = 3.3f, \ .init = &sdspi_host_init, \ .set_bus_width = NULL, \ + .get_bus_width = NULL, \ .set_card_clk = &sdspi_host_set_card_clk, \ .do_transaction = &sdspi_host_do_transaction, \ .deinit = &sdspi_host_deinit, \ @@ -72,8 +73,8 @@ typedef struct { .gpio_mosi = GPIO_NUM_15, \ .gpio_sck = GPIO_NUM_14, \ .gpio_cs = GPIO_NUM_13, \ - .gpio_cd = SDMMC_SLOT_NO_CD, \ - .gpio_wp = SDMMC_SLOT_NO_WP, \ + .gpio_cd = SDSPI_SLOT_NO_CD, \ + .gpio_wp = SDSPI_SLOT_NO_WP, \ .dma_channel = 1 \ } diff --git a/components/driver/sdmmc_host.c b/components/driver/sdmmc_host.c index f09d94d585..95cb81fdcf 100644 --- a/components/driver/sdmmc_host.c +++ b/components/driver/sdmmc_host.c @@ -255,6 +255,12 @@ esp_err_t sdmmc_host_start_command(int slot, sdmmc_hw_cmd_t cmd, uint32_t arg) { if (!(slot == 0 || slot == 1)) { return ESP_ERR_INVALID_ARG; } + if ((SDMMC.cdetect.cards & BIT(slot)) != 0) { + return ESP_ERR_NOT_FOUND; + } + if (cmd.data_expected && cmd.rw && (SDMMC.wrtprt.cards & BIT(slot)) != 0) { + return ESP_ERR_INVALID_STATE; + } while (SDMMC.cmd.start_command == 1) { ; } @@ -397,18 +403,36 @@ esp_err_t sdmmc_host_init_slot(int slot, const sdmmc_slot_config_t* slot_config) // SDIO slave interrupt is edge sensitive to ~(int_n | card_int | card_detect) // set this and card_detect to high to enable sdio interrupt - gpio_matrix_in(GPIO_FUNC_IN_HIGH, pslot->card_int, 0); - if (gpio_cd != -1) { - gpio_set_direction(gpio_cd, GPIO_MODE_INPUT); - gpio_matrix_in(gpio_cd, pslot->card_detect, 0); - } else { - gpio_matrix_in(GPIO_FUNC_IN_HIGH, pslot->card_detect, 0); - } + gpio_matrix_in(GPIO_FUNC_IN_HIGH, pslot->card_int, false); - if (gpio_wp != -1) { - gpio_set_direction(gpio_wp, GPIO_MODE_INPUT); - gpio_matrix_in(gpio_wp, pslot->write_protect, 0); + // Set up Card Detect input + int matrix_in_cd; + if (gpio_cd != SDMMC_SLOT_NO_CD) { + ESP_LOGD(TAG, "using GPIO%d as CD pin", gpio_cd); + gpio_pad_select_gpio(gpio_cd); + gpio_set_direction(gpio_cd, GPIO_MODE_INPUT); + matrix_in_cd = gpio_cd; + } else { + // if not set, default to CD low (card present) + matrix_in_cd = GPIO_FUNC_IN_LOW; } + gpio_matrix_in(matrix_in_cd, pslot->card_detect, false); + + // Set up Write Protect input + int matrix_in_wp; + if (gpio_wp != SDMMC_SLOT_NO_WP) { + ESP_LOGD(TAG, "using GPIO%d as WP pin", gpio_wp); + gpio_pad_select_gpio(gpio_wp); + gpio_set_direction(gpio_wp, GPIO_MODE_INPUT); + matrix_in_wp = gpio_wp; + } else { + // if not set, default to WP high (not write protected) + matrix_in_wp = GPIO_FUNC_IN_HIGH; + } + // WP signal is normally active low, but hardware expects + // an active-high signal, so invert it in GPIO matrix + gpio_matrix_in(matrix_in_wp, pslot->write_protect, true); + // By default, set probing frequency (400kHz) and 1-bit bus esp_err_t ret = sdmmc_host_set_card_clk(slot, 400); if (ret != ESP_OK) { diff --git a/components/driver/sdmmc_transaction.c b/components/driver/sdmmc_transaction.c index 8d87847282..52dec2991c 100644 --- a/components/driver/sdmmc_transaction.c +++ b/components/driver/sdmmc_transaction.c @@ -112,6 +112,7 @@ void sdmmc_host_transaction_handler_deinit() esp_err_t sdmmc_host_do_transaction(int slot, sdmmc_command_t* cmdinfo) { + esp_err_t ret; xSemaphoreTake(s_request_mutex, portMAX_DELAY); #ifdef CONFIG_PM_ENABLE esp_pm_lock_acquire(s_pm_lock); @@ -124,12 +125,14 @@ esp_err_t sdmmc_host_do_transaction(int slot, sdmmc_command_t* cmdinfo) if (cmdinfo->datalen < 4 || cmdinfo->blklen % 4 != 0) { ESP_LOGD(TAG, "%s: invalid size: total=%d block=%d", __func__, cmdinfo->datalen, cmdinfo->blklen); - return ESP_ERR_INVALID_SIZE; + ret = ESP_ERR_INVALID_SIZE; + goto out; } if ((intptr_t) cmdinfo->data % 4 != 0 || !esp_ptr_dma_capable(cmdinfo->data)) { ESP_LOGD(TAG, "%s: buffer %p can not be used for DMA", __func__, cmdinfo->data); - return ESP_ERR_INVALID_ARG; + ret = ESP_ERR_INVALID_ARG; + goto out; } // this clears "owned by IDMAC" bits memset(s_dma_desc, 0, sizeof(s_dma_desc)); @@ -146,10 +149,9 @@ esp_err_t sdmmc_host_do_transaction(int slot, sdmmc_command_t* cmdinfo) sdmmc_host_dma_prepare(&s_dma_desc[0], cmdinfo->blklen, cmdinfo->datalen); } // write command into hardware, this also sends the command to the card - esp_err_t ret = sdmmc_host_start_command(slot, hw_cmd, cmdinfo->arg); + ret = sdmmc_host_start_command(slot, hw_cmd, cmdinfo->arg); if (ret != ESP_OK) { - xSemaphoreGive(s_request_mutex); - return ret; + goto out; } // process events until transfer is complete cmdinfo->error = ESP_OK; @@ -161,6 +163,8 @@ esp_err_t sdmmc_host_do_transaction(int slot, sdmmc_command_t* cmdinfo) } } s_is_app_cmd = (ret == ESP_OK && cmdinfo->opcode == MMC_APP_CMD); + +out: #ifdef CONFIG_PM_ENABLE esp_pm_lock_release(s_pm_lock); #endif diff --git a/components/driver/sdspi_host.c b/components/driver/sdspi_host.c index 5c9db7e46a..e0faa73a68 100644 --- a/components/driver/sdspi_host.c +++ b/components/driver/sdspi_host.c @@ -310,22 +310,22 @@ esp_err_t sdspi_host_init_slot(int slot, const sdspi_slot_config_t* slot_config) // Configure CD and WP pins io_conf = (gpio_config_t) { .intr_type = GPIO_PIN_INTR_DISABLE, - .mode = GPIO_MODE_OUTPUT, + .mode = GPIO_MODE_INPUT, .pin_bit_mask = 0, .pull_up_en = true }; if (slot_config->gpio_cd != SDSPI_SLOT_NO_CD) { io_conf.pin_bit_mask |= (1 << slot_config->gpio_cd); - s_slots[slot].gpio_wp = slot_config->gpio_wp; + s_slots[slot].gpio_cd = slot_config->gpio_cd; } else { - s_slots[slot].gpio_wp = GPIO_UNUSED; + s_slots[slot].gpio_cd = GPIO_UNUSED; } if (slot_config->gpio_wp != SDSPI_SLOT_NO_WP) { io_conf.pin_bit_mask |= (1 << slot_config->gpio_wp); - s_slots[slot].gpio_cd = slot_config->gpio_cd; + s_slots[slot].gpio_wp = slot_config->gpio_wp; } else { - s_slots[slot].gpio_cd = GPIO_UNUSED; + s_slots[slot].gpio_wp = GPIO_UNUSED; } if (io_conf.pin_bit_mask != 0) { diff --git a/components/driver/test/test_sdmmc_sdspi_init.cpp b/components/driver/test/test_sdmmc_sdspi_init.cpp new file mode 100644 index 0000000000..d2406f582c --- /dev/null +++ b/components/driver/test/test_sdmmc_sdspi_init.cpp @@ -0,0 +1,20 @@ +#include "driver/sdmmc_host.h" +#include "driver/sdspi_host.h" + + +/** + * Check that C-style designated initializers are valid in C++ file. + */ +static void test_initializers() __attribute__((unused)); + +static void test_initializers() +{ + sdmmc_host_t sdmmc_host = SDMMC_HOST_DEFAULT(); + (void) sdmmc_host; + sdmmc_slot_config_t sdmmc_slot = SDMMC_SLOT_CONFIG_DEFAULT(); + (void) sdmmc_slot; + sdmmc_host_t sdspi_host = SDSPI_HOST_DEFAULT(); + (void) sdspi_host; + sdspi_slot_config_t sdspi_slot = SDSPI_SLOT_CONFIG_DEFAULT(); + (void) sdspi_slot; +} diff --git a/components/sdmmc/sdmmc_cmd.c b/components/sdmmc/sdmmc_cmd.c index 9b09ab8ce2..7279dec44c 100644 --- a/components/sdmmc/sdmmc_cmd.c +++ b/components/sdmmc/sdmmc_cmd.c @@ -102,14 +102,17 @@ esp_err_t sdmmc_card_init(const sdmmc_host_t* config, sdmmc_card_t* card) /* ----------- standard initialization process starts here ---------- */ - /* Reset SDIO (CMD52, RES) before re-initializing IO (CMD5). - * Non-IO cards are allowed to time out (in SD mode) or - * return "invalid command" error (in SPI mode). - */ + /* Reset SDIO (CMD52, RES) before re-initializing IO (CMD5). */ uint8_t sdio_reset = CCCR_CTL_RES; err = sdmmc_io_rw_direct(card, 0, SD_IO_CCCR_CTL, SD_ARG_CMD52_WRITE, &sdio_reset); - if (err != ESP_OK && err != ESP_ERR_TIMEOUT - && !(is_spi && err == ESP_ERR_NOT_SUPPORTED)) { + if (err == ESP_ERR_TIMEOUT || (is_spi && err == ESP_ERR_NOT_SUPPORTED)) { + /* Non-IO cards are allowed to time out (in SD mode) or + * return "invalid command" error (in SPI mode). + */ + } else if (err == ESP_ERR_NOT_FOUND) { + ESP_LOGD(TAG, "%s: card not present", __func__); + return err; + } else if (err != ESP_OK) { ESP_LOGE(TAG, "%s: sdio_reset: unexpected return: 0x%x", __func__, err ); return err; } diff --git a/components/sdmmc/test/test_sd.c b/components/sdmmc/test/test_sd.c index 0fbe6c5aee..33b3890ccd 100644 --- a/components/sdmmc/test/test_sd.c +++ b/components/sdmmc/test/test_sd.c @@ -20,11 +20,13 @@ #include "driver/sdmmc_host.h" #include "driver/sdspi_host.h" #include "driver/sdmmc_defs.h" +#include "soc/gpio_reg.h" #include "sdmmc_cmd.h" #include "esp_log.h" #include "esp_heap_caps.h" #include #include +#include TEST_CASE("MMC_RSP_BITS", "[sd]") { @@ -239,3 +241,116 @@ TEST_CASE("reads and writes with an unaligned buffer", "[sd][test_env=UT_T1_SDMO free(card); TEST_ESP_OK(sdmmc_host_deinit()); } + +static void test_cd_input(int gpio_cd_num, const sdmmc_host_t* config) +{ + sdmmc_card_t* card = malloc(sizeof(sdmmc_card_t)); + TEST_ASSERT_NOT_NULL(card); + + // SDMMC host should have configured CD as input. + // Enable output as well (not using the driver, to avoid touching input + // enable bits). + gpio_matrix_out(gpio_cd_num, SIG_GPIO_OUT_IDX, false, false); + REG_WRITE(GPIO_ENABLE_W1TS_REG, BIT(gpio_cd_num)); + + // Check that card initialization fails if CD is high + REG_WRITE(GPIO_OUT_W1TS_REG, BIT(gpio_cd_num)); + usleep(100); + TEST_ESP_ERR(ESP_ERR_NOT_FOUND, sdmmc_card_init(config, card)); + + // Check that card initialization succeeds if CD is low + REG_WRITE(GPIO_OUT_W1TC_REG, BIT(gpio_cd_num)); + usleep(100); + TEST_ESP_OK(sdmmc_card_init(config, card)); + + free(card); +} + +TEST_CASE("CD input works in SD mode", "[sd][test_env=UT_T1_SDMODE]") +{ + sdmmc_host_t config = SDMMC_HOST_DEFAULT(); + sdmmc_slot_config_t slot_config = SDMMC_SLOT_CONFIG_DEFAULT(); + const int gpio_cd_num = 5; + slot_config.gpio_cd = gpio_cd_num; + TEST_ESP_OK(sdmmc_host_init()); + TEST_ESP_OK(sdmmc_host_init_slot(SDMMC_HOST_SLOT_1, &slot_config)); + + test_cd_input(gpio_cd_num, &config); + + TEST_ESP_OK(sdmmc_host_deinit()); +} + +TEST_CASE("CD input works in SPI mode", "[sd][test_env=UT_T1_SPIMODE]") +{ + sdmmc_host_t config = SDSPI_HOST_DEFAULT(); + sdspi_slot_config_t slot_config = SDSPI_SLOT_CONFIG_DEFAULT(); + const int gpio_cd_num = 5; + slot_config.gpio_cd = gpio_cd_num; + TEST_ESP_OK(sdspi_host_init()); + TEST_ESP_OK(sdspi_host_init_slot(config.slot, &slot_config)); + + test_cd_input(gpio_cd_num, &config); + + TEST_ESP_OK(sdspi_host_deinit()); +} + +static void test_wp_input(int gpio_wp_num, const sdmmc_host_t* config) +{ + sdmmc_card_t* card = malloc(sizeof(sdmmc_card_t)); + TEST_ASSERT_NOT_NULL(card); + + // SDMMC host should have configured WP as input. + // Enable output as well (not using the driver, to avoid touching input + // enable bits). + gpio_matrix_out(gpio_wp_num, SIG_GPIO_OUT_IDX, false, false); + REG_WRITE(GPIO_ENABLE_W1TS_REG, BIT(gpio_wp_num)); + + // Check that the card can be initialized with WP low + REG_WRITE(GPIO_OUT_W1TC_REG, BIT(gpio_wp_num)); + TEST_ESP_OK(sdmmc_card_init(config, card)); + + uint32_t* data = heap_caps_calloc(1, 512, MALLOC_CAP_DMA); + + // Check that card write succeeds if WP is high + REG_WRITE(GPIO_OUT_W1TS_REG, BIT(gpio_wp_num)); + usleep(100); + TEST_ESP_OK(sdmmc_write_sectors(card, &data, 0, 1)); + + // Check that write fails if WP is low + REG_WRITE(GPIO_OUT_W1TC_REG, BIT(gpio_wp_num)); + usleep(100); + TEST_ESP_ERR(ESP_ERR_INVALID_STATE, sdmmc_write_sectors(card, &data, 0, 1)); + // ...but reads still work + TEST_ESP_OK(sdmmc_read_sectors(card, &data, 0, 1)); + + free(data); + free(card); +} + +TEST_CASE("WP input works in SD mode", "[sd][test_env=UT_T1_SDMODE]") +{ + sdmmc_host_t config = SDMMC_HOST_DEFAULT(); + sdmmc_slot_config_t slot_config = SDMMC_SLOT_CONFIG_DEFAULT(); + const int gpio_wp_num = 5; + slot_config.gpio_wp = gpio_wp_num; + TEST_ESP_OK(sdmmc_host_init()); + TEST_ESP_OK(sdmmc_host_init_slot(SDMMC_HOST_SLOT_1, &slot_config)); + + test_wp_input(gpio_wp_num, &config); + + TEST_ESP_OK(sdmmc_host_deinit()); +} + +TEST_CASE("WP input works in SPI mode", "[sd][test_env=UT_T1_SPIMODE]") +{ + sdmmc_host_t config = SDSPI_HOST_DEFAULT(); + sdspi_slot_config_t slot_config = SDSPI_SLOT_CONFIG_DEFAULT(); + const int gpio_wp_num = 5; + slot_config.gpio_wp = gpio_wp_num; + TEST_ESP_OK(sdspi_host_init()); + TEST_ESP_OK(sdspi_host_init_slot(config.slot, &slot_config)); + + test_wp_input(gpio_wp_num, &config); + + TEST_ESP_OK(sdspi_host_deinit()); +} diff --git a/components/soc/esp32/include/soc/sdmmc_struct.h b/components/soc/esp32/include/soc/sdmmc_struct.h index 9f3625a3d9..7e3c6912eb 100644 --- a/components/soc/esp32/include/soc/sdmmc_struct.h +++ b/components/soc/esp32/include/soc/sdmmc_struct.h @@ -255,7 +255,7 @@ typedef volatile struct { union { struct { - uint32_t cards: 2; ///< bit N reads 1 if card N is present + uint32_t cards: 2; ///< bit N reads 0 if card N is present uint32_t reserved: 30; }; uint32_t val; @@ -263,7 +263,7 @@ typedef volatile struct { union { struct { - uint32_t card0: 2; ///< bit N reads 1 if card N is write protected + uint32_t cards: 2; ///< bit N reads 1 if card N is write protected uint32_t reserved: 30; }; uint32_t val;