Merge branch 'bugfix/esp_flash_revert_qe_clear' into 'master'

esp_flash: fix the regression of non-quad mode by default chip driver, bugs in add_device and deprecate cs_id

See merge request espressif/esp-idf!8260
This commit is contained in:
Michael (XIAO Xufeng) 2020-04-21 17:52:42 +08:00
commit e2fc591a20
6 changed files with 72 additions and 23 deletions

View file

@ -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

View file

@ -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;
/**

View file

@ -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)

View file

@ -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,
};

View file

@ -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) {

View file

@ -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)