diff --git a/components/spi_flash/esp_flash_spi_init.c b/components/spi_flash/esp_flash_spi_init.c index 11cf6a22ab..9713a24ae7 100644 --- a/components/spi_flash/esp_flash_spi_init.c +++ b/components/spi_flash/esp_flash_spi_init.c @@ -73,14 +73,14 @@ __attribute__((unused)) static const char TAG[] = "spi_flash"; esp_flash_t *esp_flash_default_chip = NULL; -static IRAM_ATTR NOINLINE_ATTR void cs_initialize(esp_flash_t *chip, const esp_flash_spi_device_config_t *config, bool use_iomux) +static IRAM_ATTR NOINLINE_ATTR void cs_initialize(esp_flash_t *chip, const esp_flash_spi_device_config_t *config, bool use_iomux, int cs_id) { //Not using spicommon_cs_initialize since we don't want to put the whole //spi_periph_signal into the DRAM. Copy these data from flash before the //cache disabling int cs_io_num = config->cs_io_num; int spics_in = spi_periph_signal[config->host_id].spics_in; - int spics_out = spi_periph_signal[config->host_id].spics_out[config->cs_id]; + int spics_out = spi_periph_signal[config->host_id].spics_out[cs_id]; int spics_func = spi_periph_signal[config->host_id].func; uint32_t iomux_reg = GPIO_PIN_MUX_REG[cs_io_num]; @@ -88,6 +88,8 @@ static IRAM_ATTR NOINLINE_ATTR void cs_initialize(esp_flash_t *chip, const esp_f //initialization, disable the cache temporarily chip->os_func->start(chip->os_func_data); if (use_iomux) { + // This requires `gpio_iomux_in` and `gpio_iomux_out` to be in the IRAM. + // `linker.lf` is used fulfill this requirement. gpio_iomux_in(cs_io_num, spics_in); gpio_iomux_out(cs_io_num, spics_func, false); } else { @@ -99,7 +101,7 @@ static IRAM_ATTR NOINLINE_ATTR void cs_initialize(esp_flash_t *chip, const esp_f } GPIO.pin[cs_io_num].pad_driver = 0; gpio_matrix_out(cs_io_num, spics_out, false, false); - if (config->cs_id == 0) { + if (cs_id == 0) { gpio_matrix_in(cs_io_num, spics_in, false); } PIN_FUNC_SELECT(iomux_reg, PIN_FUNC_GPIO); @@ -121,25 +123,35 @@ esp_err_t spi_bus_add_flash_device(esp_flash_t **out_chip, const esp_flash_spi_d if (config->host_id == SPI_HOST) caps = MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT; chip = (esp_flash_t*)heap_caps_malloc(sizeof(esp_flash_t), caps); - host = (spi_flash_host_driver_t*)heap_caps_malloc(sizeof(spi_flash_host_driver_t), caps); - host_data = (memspi_host_data_t*)heap_caps_malloc(sizeof(memspi_host_data_t), caps); - if (!chip || !host || !host_data) { + if (!chip) { ret = ESP_ERR_NO_MEM; goto fail; } + host = (spi_flash_host_driver_t*)heap_caps_malloc(sizeof(spi_flash_host_driver_t), caps); *chip = (esp_flash_t) { .read_mode = config->io_mode, .host = host, }; + if (!host) { + ret = ESP_ERR_NO_MEM; + goto fail; + } + + host_data = (memspi_host_data_t*)heap_caps_malloc(sizeof(memspi_host_data_t), caps); + host->driver_data = host_data; + if (!host_data) { + ret = ESP_ERR_NO_MEM; + goto fail; + } int dev_id; esp_err_t err = esp_flash_init_os_functions(chip, config->host_id, &dev_id); - assert(dev_id < SOC_SPI_PERIPH_CS_NUM(config->host_id) && dev_id >= 0); if (err != ESP_OK) { ret = err; goto fail; } + assert(dev_id < SOC_SPI_PERIPH_CS_NUM(config->host_id) && dev_id >= 0); bool use_iomux = spicommon_bus_using_iomux(config->host_id); memspi_host_config_t host_cfg = { @@ -155,10 +167,12 @@ esp_err_t spi_bus_add_flash_device(esp_flash_t **out_chip, const esp_flash_spi_d goto fail; } - cs_initialize(chip, config, use_iomux); + // The cs_id inside `config` is deprecated, use the `dev_id` provided by the bus lock instead. + cs_initialize(chip, config, use_iomux, dev_id); *out_chip = chip; return ret; fail: + // The memory allocated are free'd in the `spi_bus_remove_flash_device`. spi_bus_remove_flash_device(chip); return ret; } @@ -197,7 +211,7 @@ esp_err_t esp_flash_init_default_chip(void) { memspi_host_config_t cfg = ESP_FLASH_HOST_CONFIG_DEFAULT(); - #ifdef CONFIG_IDF_TARGET_ESP32S2 + #ifdef CONFIG_IDF_TARGET_ESP32S2 // For esp32s2 spi IOs are configured as from IO MUX by default cfg.iomux = ets_efuse_get_spiconfig() == 0 ? true : false; #endif diff --git a/components/spi_flash/include/esp_flash_spi_init.h b/components/spi_flash/include/esp_flash_spi_init.h index 0e0e72d832..f3ac315eeb 100644 --- a/components/spi_flash/include/esp_flash_spi_init.h +++ b/components/spi_flash/include/esp_flash_spi_init.h @@ -24,11 +24,12 @@ extern "C" { /// Configurations for the SPI Flash to init typedef struct { spi_host_device_t host_id; ///< Bus to use - int cs_id; ///< CS pin (signal) to use int cs_io_num; ///< GPIO pin to output the CS signal - esp_flash_io_mode_t io_mode; ///< IO mode to read from the Flash + esp_flash_io_mode_t io_mode; ///< IO mode to read from the Flash esp_flash_speed_t speed; ///< Speed of the Flash clock int input_delay_ns; ///< Input delay of the data pins, in ns. Set to 0 if unknown. + + int cs_id; ///< @deprecated CS pin (signal) to use } esp_flash_spi_device_config_t; /** diff --git a/components/spi_flash/linker.lf b/components/spi_flash/linker.lf index efd5875d57..a31658712f 100644 --- a/components/spi_flash/linker.lf +++ b/components/spi_flash/linker.lf @@ -8,3 +8,9 @@ entries: spi_flash_chip_gd(noflash) memspi_host_driver (noflash) +# `spi_bus_add_flash_device` uses these functions when the cache is disabled +[mapping:driver_spiflash] +archive: libdriver.a +entries: + gpio:gpio_iomux_out (noflash) + gpio:gpio_iomux_in (noflash) diff --git a/components/spi_flash/spi_flash_chip_drivers.c b/components/spi_flash/spi_flash_chip_drivers.c index 3b46b13e7d..6e06d963e0 100644 --- a/components/spi_flash/spi_flash_chip_drivers.c +++ b/components/spi_flash/spi_flash_chip_drivers.c @@ -39,6 +39,8 @@ static const spi_flash_chip_t *default_registered_chips[] = { #ifdef CONFIG_SPI_FLASH_SUPPORT_MXIC_CHIP &esp_flash_chip_mxic, #endif + // Default chip drivers that will accept all chip ID. + // FM, Winbond and XMC chips are supposed to be supported by this chip driver. &esp_flash_chip_generic, NULL, }; diff --git a/components/spi_flash/spi_flash_chip_generic.c b/components/spi_flash/spi_flash_chip_generic.c index 1b2995e29f..539e116655 100644 --- a/components/spi_flash/spi_flash_chip_generic.c +++ b/components/spi_flash/spi_flash_chip_generic.c @@ -456,12 +456,15 @@ esp_err_t spi_flash_common_set_io_mode(esp_flash_t *chip, esp_flash_wrsr_func_t esp_err_t ret = ESP_OK; const bool is_quad_mode = esp_flash_is_quad_mode(chip); bool update_config = false; - const bool force_check = true; //in case some chips doesn't support erase QE + /* + * By default, we don't clear the QE bit even the flash mode is not QIO or QOUT. Force clearing + * QE bit by the generic chip driver (command 01H with 2 bytes) may cause the output of some + * chips (MXIC) no longer valid. + * Enable this option when testing a new flash chip for clearing of QE. + */ + const bool force_check = false; - bool need_check = is_quad_mode; - if (force_check) { - need_check = true; - } + bool need_check = is_quad_mode || force_check; uint32_t sr_update; if (need_check) { diff --git a/components/spi_flash/test/test_esp_flash.c b/components/spi_flash/test/test_esp_flash.c index d029ac8f29..f431362a8b 100644 --- a/components/spi_flash/test/test_esp_flash.c +++ b/components/spi_flash/test/test_esp_flash.c @@ -77,6 +77,9 @@ typedef void (*flash_test_func_t)(esp_flash_t* chip); #define FLASH_TEST_CASE(STR, FUNC_TO_RUN) \ TEST_CASE(STR, "[esp_flash]") {flash_test_func(FUNC_TO_RUN, 1 /* first index reserved for main flash */ );} +#define FLASH_TEST_CASE_IGNORE(STR, FUNC_TO_RUN) \ + TEST_CASE(STR, "[esp_flash][ignore]") {flash_test_func(FUNC_TO_RUN, 1 /* first index reserved for main flash */ );} + /* Use FLASH_TEST_CASE_3 for tests which also run on external flash, which sits in the place of PSRAM (these tests are incompatible with PSRAM) @@ -84,10 +87,14 @@ typedef void (*flash_test_func_t)(esp_flash_t* chip); */ #if defined(CONFIG_SPIRAM_SUPPORT) || TEMPORARY_DISABLED_FOR_TARGETS(ESP32S2) #define FLASH_TEST_CASE_3(STR, FUNCT_TO_RUN) +#define FLASH_TEST_CASE_3_IGNORE(STR, FUNCT_TO_RUN) #else // Disabled for ESP32-S2 due to lack of runners #define FLASH_TEST_CASE_3(STR, FUNC_TO_RUN) \ TEST_CASE(STR", 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH]") {flash_test_func(FUNC_TO_RUN, TEST_CONFIG_NUM);} + +#define FLASH_TEST_CASE_3_IGNORE(STR, FUNC_TO_RUN) \ + TEST_CASE(STR", 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH][ignore]") {flash_test_func(FUNC_TO_RUN, TEST_CONFIG_NUM);} #endif //currently all the configs are the same with esp_flash_spi_device_config_t, no more information required @@ -499,8 +506,9 @@ static void read_and_check(esp_flash_t *chip, const esp_partition_t *part, const esp_err_t esp_flash_set_io_mode(esp_flash_t* chip, bool qe); esp_err_t esp_flash_get_io_mode(esp_flash_t* chip, bool* qe); esp_err_t esp_flash_read_chip_id(esp_flash_t* chip, uint32_t* flash_id); +esp_err_t spi_flash_chip_mxic_probe(esp_flash_t *chip, uint32_t flash_id); -static bool check_winbond_chip(esp_flash_t* chip) +static bool is_winbond_chip(esp_flash_t* chip) { uint32_t flash_id; esp_err_t ret = esp_flash_read_chip_id(chip, &flash_id); @@ -512,6 +520,14 @@ static bool check_winbond_chip(esp_flash_t* chip) } } +static bool is_mxic_chip(esp_flash_t* chip) +{ + uint32_t flash_id; + esp_err_t ret = esp_flash_read_chip_id(chip, &flash_id); + TEST_ESP_OK(ret); + return (spi_flash_chip_mxic_probe(chip, flash_id)==ESP_OK); +} + static void test_toggle_qe(esp_flash_t* chip) { bool qe; @@ -522,14 +538,14 @@ static void test_toggle_qe(esp_flash_t* chip) esp_err_t ret = esp_flash_get_io_mode(chip, &qe); TEST_ESP_OK(ret); - bool is_winbond_chip = check_winbond_chip(chip); + bool allow_failure = is_winbond_chip(chip) || is_mxic_chip(chip); for (int i = 0; i < 4; i ++) { ESP_LOGI(TAG, "write qe: %d->%d", qe, !qe); qe = !qe; chip->read_mode = qe? SPI_FLASH_QOUT: SPI_FLASH_SLOWRD; ret = esp_flash_set_io_mode(chip, qe); - if (is_winbond_chip && !qe && ret == ESP_ERR_FLASH_NO_RESPONSE) { + if (allow_failure && !qe && ret == ESP_ERR_FLASH_NO_RESPONSE) { //allows clear qe failure for Winbond chips ret = ESP_OK; } @@ -539,8 +555,12 @@ static void test_toggle_qe(esp_flash_t* chip) ret = esp_flash_get_io_mode(chip, &qe_read); TEST_ESP_OK(ret); ESP_LOGD(TAG, "qe read: %d", qe_read); - if (qe != qe_read && !qe && is_winbond_chip) { - ESP_LOGE(TAG, "cannot clear QE bit, this may be normal for Winbond chips."); + if (!qe && qe_read) { + if (allow_failure) { + ESP_LOGW(TAG, "cannot clear QE bit for known permanent QE (Winbond or MXIC) chips."); + } else { + ESP_LOGE(TAG, "cannot clear QE bit, please make sure force clearing QE option is enabled in `spi_flash_common_set_io_mode`, and this chip is not a permanent QE one."); + } chip->read_mode = io_mode_before; return; } @@ -550,8 +570,11 @@ static void test_toggle_qe(esp_flash_t* chip) chip->read_mode = io_mode_before; } -FLASH_TEST_CASE("Test esp_flash_write can toggle QE bit", test_toggle_qe); -FLASH_TEST_CASE_3("Test esp_flash_write can toggle QE bit", test_toggle_qe); +// These tests show whether the QE is permanent or not for the chip tested. +// To test the behaviour of a new SPI flash chip, enable force_check flag in generic driver +// `spi_flash_common_set_io_mode` and then run this test. +FLASH_TEST_CASE_IGNORE("Test esp_flash_write can toggle QE bit", test_toggle_qe); +FLASH_TEST_CASE_3_IGNORE("Test esp_flash_write can toggle QE bit", test_toggle_qe); void test_permutations(flashtest_config_t* config)