From 571864e8aeba85b941133766601543e0decd0faf Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Thu, 5 Sep 2019 13:11:36 +0800 Subject: [PATCH 1/4] esp_flash: fix set qe bit and write command issues There used to be dummy phase before out phase in common command transactions. This corrupts the data. The code before never actually operate (clear) the QE bit, once it finds the QE bit is set. It's hard to check whether the QE set/disable functions work well. This commit: 1. Cancel the dummy phase 2. Set and clear the QE bit according to chip settings, allowing tests for QE bits. However for some chips (Winbond for example), it's not forced to clear the QE bit if not able to. 3. Also refactor to allow chip_generic and other chips to share the same code to read and write qe bit; let common command and read command share configure_host_io_mode. 4. Rename read mode to io mode since maybe we will write data with quad mode one day. --- .../soc/esp32/include/hal/spi_flash_ll.h | 17 +- components/soc/include/hal/spi_flash_hal.h | 29 ++- components/soc/include/hal/spi_flash_types.h | 8 +- components/soc/src/hal/spi_flash_hal.c | 5 - components/soc/src/hal/spi_flash_hal_iram.c | 29 ++- components/spi_flash/CMakeLists.txt | 20 +- components/spi_flash/esp_flash_api.c | 85 ++++++-- components/spi_flash/esp_flash_spi_init.c | 2 +- components/spi_flash/include/esp_flash.h | 18 +- .../spi_flash/include/esp_flash_spi_init.h | 2 +- .../spi_flash/include/memspi_host_driver.h | 2 +- .../spi_flash/include/spi_flash_chip_driver.h | 8 +- .../include/spi_flash_chip_generic.h | 118 ++++++++-- components/spi_flash/spi_flash_chip_generic.c | 204 +++++++++++++----- components/spi_flash/spi_flash_chip_issi.c | 23 +- components/spi_flash/test/test_esp_flash.c | 2 +- 16 files changed, 423 insertions(+), 149 deletions(-) diff --git a/components/soc/esp32/include/hal/spi_flash_ll.h b/components/soc/esp32/include/hal/spi_flash_ll.h index 5522a1721..a30284fa6 100644 --- a/components/soc/esp32/include/hal/spi_flash_ll.h +++ b/components/soc/esp32/include/hal/spi_flash_ll.h @@ -202,21 +202,6 @@ static inline bool spi_flash_ll_host_idle(const spi_dev_t *dev) return dev->ext2.st != 0; } -/** - * Set phases for user-defined transaction to read - * - * @param dev Beginning address of the peripheral registers. - */ -static inline void spi_flash_ll_read_phase(spi_dev_t *dev) -{ - typeof (dev->user) user = { - .usr_command = 1, - .usr_mosi = 0, - .usr_miso = 1, - .usr_addr = 1, - }; - dev->user = user; -} /*------------------------------------------------------------------------------ * Configs *----------------------------------------------------------------------------*/ @@ -239,7 +224,7 @@ static inline void spi_flash_ll_set_cs_pin(spi_dev_t *dev, int pin) * @param dev Beginning address of the peripheral registers. * @param read_mode I/O mode to use in the following transactions. */ -static inline void spi_flash_ll_set_read_mode(spi_dev_t *dev, esp_flash_read_mode_t read_mode) +static inline void spi_flash_ll_set_read_mode(spi_dev_t *dev, esp_flash_io_mode_t read_mode) { typeof (dev->ctrl) ctrl = dev->ctrl; ctrl.val &= ~(SPI_FREAD_QIO_M | SPI_FREAD_QUAD_M | SPI_FREAD_DIO_M | SPI_FREAD_DUAL_M); diff --git a/components/soc/include/hal/spi_flash_hal.h b/components/soc/include/hal/spi_flash_hal.h index bf89c4148..ec1b36151 100644 --- a/components/soc/include/hal/spi_flash_hal.h +++ b/components/soc/include/hal/spi_flash_hal.h @@ -155,23 +155,38 @@ esp_err_t spi_flash_hal_set_write_protect(spi_flash_host_driver_t *chip_drv, boo bool spi_flash_hal_host_idle(spi_flash_host_driver_t *driver); /** - * Configure the SPI host hardware registers for the specified read mode. + * @brief Configure the SPI host hardware registers for the specified io mode. * * Note that calling this configures SPI host registers, so if running any - * other commands as part of set_read_mode() then these must be run before + * other commands as part of set_io_mode() then these must be run before * calling this function. * + * The command value, address length and dummy cycles are configured according + * to the format of read commands: + * + * - command: 8 bits, value set. + * - address: 24 bits + * - dummy: cycles to compensate the input delay + * - out & in data: 0 bits. + * + * The following commands still need to: + * + * - Read data: set address value and data (length and contents), no need + * to touch command and dummy phases. + * - Common read: set command value, address value (or length to 0 if not used) + * - Common write: set command value, address value (or length to 0 if not + * used), disable dummy phase, and set output data. + * * @param driver The driver context - * @param read_mode The HW read mode to use + * @param io_mode The HW read mode to use * @param addr_bitlen Length of the address phase, in bits * @param dummy_cyclelen_base Base cycles of the dummy phase, some extra dummy cycles may be appended to compensate the timing. - * @param read_command Actual reading command to send to flash chip on the bus. + * @param command Actual reading command to send to flash chip on the bus. * * @return always return ESP_OK. */ -esp_err_t spi_flash_hal_configure_host_read_mode(spi_flash_host_driver_t *driver, esp_flash_read_mode_t read_mode, - uint32_t addr_bitlen, uint32_t dummy_cyclelen_base, - uint32_t read_command); +esp_err_t spi_flash_hal_configure_host_io_mode(spi_flash_host_driver_t *driver, uint32_t command, uint32_t addr_bitlen, + int dummy_cyclelen_base, esp_flash_io_mode_t io_mode); /** * Poll until the last operation is done. diff --git a/components/soc/include/hal/spi_flash_types.h b/components/soc/include/hal/spi_flash_types.h index c19eee1ed..3e825c2aa 100644 --- a/components/soc/include/hal/spi_flash_types.h +++ b/components/soc/include/hal/spi_flash_types.h @@ -61,7 +61,7 @@ typedef enum { SPI_FLASH_QIO, ///< Both address & data transferred using quad I/O SPI_FLASH_READ_MODE_MAX, ///< The fastest io mode supported by the host is ``ESP_FLASH_READ_MODE_MAX-1``. -} esp_flash_read_mode_t; +} esp_flash_io_mode_t; ///Slowest io mode supported by ESP32, currently SlowRd #define SPI_FLASH_READ_MODE_MIN SPI_FLASH_SLOWRD @@ -130,9 +130,11 @@ struct spi_flash_host_driver_t { */ bool (*host_idle)(spi_flash_host_driver_t *driver); /** - * Configure the host to work at different read mode. + * Configure the host to work at different read mode. Responsible to compensate the timing and set IO mode. */ - esp_err_t (*configure_host_read_mode)(spi_flash_host_driver_t *driver, esp_flash_read_mode_t read_mode, uint32_t addr_bitlen, uint32_t dummy_bitlen_base, uint32_t read_command); + esp_err_t (*configure_host_io_mode)(spi_flash_host_driver_t *driver, uint32_t command, + uint32_t addr_bitlen, int dummy_bitlen_base, + esp_flash_io_mode_t io_mode); /** * Internal use, poll the HW until the last operation is done. */ diff --git a/components/soc/src/hal/spi_flash_hal.c b/components/soc/src/hal/spi_flash_hal.c index 6a88d35a1..ee787fd91 100644 --- a/components/soc/src/hal/spi_flash_hal.c +++ b/components/soc/src/hal/spi_flash_hal.c @@ -67,8 +67,3 @@ esp_err_t spi_flash_hal_init(spi_flash_memspi_data_t *data_out, const spi_flash_ ESP_EARLY_LOGD(TAG, "extra_dummy: %d", data_out->extra_dummy); return ESP_OK; } - -static inline spi_dev_t *get_spi_dev(spi_flash_host_driver_t *chip_drv) -{ - return ((spi_flash_memspi_data_t *)chip_drv->driver_data)->spi; -} diff --git a/components/soc/src/hal/spi_flash_hal_iram.c b/components/soc/src/hal/spi_flash_hal_iram.c index 56a525baa..ea8e2bfbc 100644 --- a/components/soc/src/hal/spi_flash_hal_iram.c +++ b/components/soc/src/hal/spi_flash_hal_iram.c @@ -52,28 +52,37 @@ esp_err_t spi_flash_hal_device_config(spi_flash_host_driver_t *driver) return ESP_OK; } -esp_err_t spi_flash_hal_configure_host_read_mode(spi_flash_host_driver_t *driver, esp_flash_read_mode_t read_mode, - uint32_t addr_bitlen, uint32_t dummy_cyclelen_base, - uint32_t read_command) +esp_err_t spi_flash_hal_configure_host_io_mode( + spi_flash_host_driver_t *host, + uint32_t command, + uint32_t addr_bitlen, + int dummy_cyclelen_base, + esp_flash_io_mode_t io_mode) { // Add dummy cycles to compensate for latency of GPIO matrix and external delay, if necessary... int dummy_cyclelen = dummy_cyclelen_base + ((spi_flash_memspi_data_t *)driver->driver_data)->extra_dummy; spi_dev_t *dev = get_spi_dev(driver); + spi_flash_ll_set_command8(dev, command); spi_flash_ll_set_addr_bitlen(dev, addr_bitlen); - spi_flash_ll_set_command8(dev, read_command); - spi_flash_ll_read_phase(dev); spi_flash_ll_set_dummy(dev, dummy_cyclelen); - spi_flash_ll_set_read_mode(dev, read_mode); + //disable all data phases, enable them later if needed + spi_flash_ll_set_miso_bitlen(dev, 0); + spi_flash_ll_set_mosi_bitlen(dev, 0); + spi_flash_ll_set_read_mode(dev, io_mode); return ESP_OK; } esp_err_t spi_flash_hal_common_command(spi_flash_host_driver_t *chip_drv, spi_flash_trans_t *trans) { - chip_drv->configure_host_read_mode(chip_drv, SPI_FLASH_FASTRD, 0, 0, 0); + host->configure_host_io_mode(host, trans->command, 0, 0, SPI_FLASH_FASTRD); + spi_dev_t *dev = get_spi_dev(chip_drv); - spi_flash_ll_set_command8(dev, trans->command); - spi_flash_ll_set_addr_bitlen(dev, 0); + //disable dummy if no input phase + if (trans->miso_len == 0) { + spi_flash_ll_set_dummy(dev, 0); + } + spi_flash_ll_set_miso_bitlen(dev, trans->miso_len); spi_flash_ll_set_mosi_bitlen(dev, trans->mosi_len); @@ -122,7 +131,7 @@ void spi_flash_hal_program_page(spi_flash_host_driver_t *chip_drv, const void *b esp_err_t spi_flash_hal_read(spi_flash_host_driver_t *chip_drv, void *buffer, uint32_t address, uint32_t read_len) { spi_dev_t *dev = get_spi_dev(chip_drv); - //the command is already set by ``spi_flash_hal_configure_host_read_mode`` before. + //the command is already set by ``spi_flash_hal_configure_host_io_mode`` before. spi_flash_ll_set_address(dev, address << 8); spi_flash_ll_set_miso_bitlen(dev, read_len * 8); spi_flash_ll_user_start(dev); diff --git a/components/spi_flash/CMakeLists.txt b/components/spi_flash/CMakeLists.txt index 878b3c8cf..497125e47 100644 --- a/components/spi_flash/CMakeLists.txt +++ b/components/spi_flash/CMakeLists.txt @@ -3,25 +3,31 @@ if(BOOTLOADER_BUILD) # Bootloader needs SPIUnlock from this file, but doesn't # need other parts of this component set(srcs "spi_flash_rom_patch.c") + set(cache_srcs "") else() - set(srcs + set(cache_srcs "cache_utils.c" "flash_mmap.c" "flash_ops.c" + ) + set(srcs "partition.c" "spi_flash_rom_patch.c" ) # New implementation + list(APPEND cache_srcs + "esp_flash_api.c" + "esp_flash_spi_init.c" + "spi_flash_os_func_app.c" + "spi_flash_os_func_noos.c" + ) list(APPEND srcs "spi_flash_chip_drivers.c" "spi_flash_chip_generic.c" "spi_flash_chip_issi.c" - "spi_flash_os_func_app.c" - "spi_flash_os_func_noos.c" "memspi_host_driver.c" - "esp_flash_api.c" - "esp_flash_spi_init.c" ) + list(APPEND srcs ${cache_srcs}) set(priv_requires bootloader_support app_update soc) endif() @@ -30,3 +36,7 @@ idf_component_register(SRCS "${srcs}" INCLUDE_DIRS include PRIV_INCLUDE_DIRS private_include LDFRAGMENTS linker.lf) + +# Avoid cache miss by unexpected inlineing when built by -Os +set_source_files_properties(${cache_srcs} PROPERTIES COMPILE_FLAGS + "-fno-inline-functions -fno-inline-small-functions -fno-inline-functions-called-once") diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index bb5be1bb8..7cb0e0d13 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -60,7 +60,7 @@ static const char io_mode_str[][IO_STR_LEN] = { "qio", }; -_Static_assert(sizeof(io_mode_str)/IO_STR_LEN == SPI_FLASH_READ_MODE_MAX, "the io_mode_str should be consistent with the esp_flash_read_mode_t defined in spi_flash_ll.h"); +_Static_assert(sizeof(io_mode_str)/IO_STR_LEN == SPI_FLASH_READ_MODE_MAX, "the io_mode_str should be consistent with the esp_flash_io_mode_t defined in spi_flash_ll.h"); /* Static function to notify OS of a new SPI flash operation. @@ -139,38 +139,51 @@ esp_err_t IRAM_ATTR esp_flash_init(esp_flash_t *chip) if (err == ESP_OK) { // Try to set the flash mode to whatever default mode was chosen - err = chip->chip_drv->set_read_mode(chip); + err = chip->chip_drv->set_io_mode(chip); + if (err == ESP_ERR_FLASH_NO_RESPONSE && !esp_flash_is_quad_mode(chip)) { + //some chips (e.g. Winbond) don't support to clear QE, treat as success + err = ESP_OK; + } } // Done: all fields on 'chip' are initialised return spiflash_end(chip, err); } +//this is not public, but useful in unit tests +esp_err_t IRAM_ATTR esp_flash_read_chip_id(esp_flash_t* chip, uint32_t* flash_id) +{ + esp_err_t err = spiflash_start(chip); + if (err != ESP_OK) { + return err; + } + + // Send generic RDID command twice, check for a matching result and retry in case we just powered on (inner + // function fails if it sees all-ones or all-zeroes.) + err = chip->host->read_id(chip->host, flash_id); + + if (err == ESP_OK) { // check we see the same ID twice, in case of transient power-on errors + uint32_t new_id; + err = chip->host->read_id(chip->host, &new_id); + if (err == ESP_OK && (new_id != *flash_id)) { + err = ESP_ERR_FLASH_NOT_INITIALISED; + } + } + + return spiflash_end(chip, err); +} + static esp_err_t IRAM_ATTR detect_spi_flash_chip(esp_flash_t *chip) { esp_err_t err; uint32_t flash_id; int retries = 10; do { - err = spiflash_start(chip); - if (err != ESP_OK) { - return err; - } - - // Send generic RDID command twice, check for a matching result and retry in case we just powered on (inner - // function fails if it sees all-ones or all-zeroes.) - err = chip->host->read_id(chip->host, &flash_id); - - if (err == ESP_OK) { // check we see the same ID twice, in case of transient power-on errors - uint32_t new_id; - err = chip->host->read_id(chip->host, &new_id); - if (err == ESP_OK && (new_id != flash_id)) { - err = ESP_ERR_FLASH_NOT_INITIALISED; - } - } - - err = spiflash_end(chip, err); - } while (err != ESP_OK && retries-- > 0); + err = esp_flash_read_chip_id(chip, &flash_id); + } while (err == ESP_ERR_FLASH_NOT_INITIALISED && retries-- > 0); + if (err != ESP_OK) { + return err; + } // Detect the chip and set the chip_drv structure for it const spi_flash_chip_t **drivers = esp_flash_registered_chips; @@ -617,6 +630,36 @@ esp_err_t IRAM_ATTR esp_flash_read_encrypted(esp_flash_t *chip, uint32_t address return spi_flash_read_encrypted(address, out_buffer, length); } +// test only, non-public +IRAM_ATTR esp_err_t esp_flash_get_io_mode(esp_flash_t* chip, bool* qe) +{ + VERIFY_OP(get_io_mode); + esp_flash_io_mode_t io_mode; + + esp_err_t err = spiflash_start(chip); + if (err != ESP_OK) { + return err; + } + err = chip->chip_drv->get_io_mode(chip, &io_mode); + err = spiflash_end(chip, err); + if (err == ESP_OK) { + *qe = (io_mode == SPI_FLASH_QOUT); + } + return err; +} + +IRAM_ATTR esp_err_t esp_flash_set_io_mode(esp_flash_t* chip, bool qe) +{ + VERIFY_OP(set_io_mode); + chip->read_mode = (qe? SPI_FLASH_QOUT: SPI_FLASH_SLOWRD); + esp_err_t err = spiflash_start(chip); + if (err != ESP_OK) { + return err; + } + err = chip->chip_drv->set_io_mode(chip); + return spiflash_end(chip, err); +} + #ifndef CONFIG_SPI_FLASH_USE_LEGACY_IMPL esp_err_t esp_flash_app_disable_protect(bool disable) { diff --git a/components/spi_flash/esp_flash_spi_init.c b/components/spi_flash/esp_flash_spi_init.c index 01b6ec717..67e7d2064 100644 --- a/components/spi_flash/esp_flash_spi_init.c +++ b/components/spi_flash/esp_flash_spi_init.c @@ -62,7 +62,7 @@ __attribute__((unused)) static const char TAG[] = "spi_flash"; esp_flash_t *esp_flash_default_chip = NULL; -static IRAM_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) { //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 diff --git a/components/spi_flash/include/esp_flash.h b/components/spi_flash/include/esp_flash.h index caca07596..b2b38806d 100644 --- a/components/spi_flash/include/esp_flash.h +++ b/components/spi_flash/include/esp_flash.h @@ -63,7 +63,7 @@ struct esp_flash_t { const esp_flash_os_functions_t *os_func; ///< Pointer to os-specific hook structure. Call ``esp_flash_init_os_functions()`` to setup this field, after the host is properly initialized. void *os_func_data; ///< Pointer to argument for os-specific hooks. Left NULL and will be initialized with ``os_func``. - esp_flash_read_mode_t read_mode; ///< Configured SPI flash read mode. Set before ``esp_flash_init`` is called. + esp_flash_io_mode_t read_mode; ///< Configured SPI flash read mode. Set before ``esp_flash_init`` is called. uint32_t size; ///< Size of SPI flash in bytes. If 0, size will be detected during initialisation. }; @@ -286,6 +286,22 @@ esp_err_t esp_flash_read_encrypted(esp_flash_t *chip, uint32_t address, void *ou extern esp_flash_t *esp_flash_default_chip; +/******************************************************************************* + * Utility Functions + ******************************************************************************/ + +/** + * @brief Returns true if chip is configured for Quad I/O or Quad Fast Read. + * + * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. + * + * @return true if flash works in quad mode, otherwise false + */ +static inline bool esp_flash_is_quad_mode(const esp_flash_t *chip) +{ + return (chip->read_mode == SPI_FLASH_QIO) || (chip->read_mode == SPI_FLASH_QOUT); +} + #ifdef __cplusplus } #endif diff --git a/components/spi_flash/include/esp_flash_spi_init.h b/components/spi_flash/include/esp_flash_spi_init.h index 4a26a4fd1..f7e981618 100644 --- a/components/spi_flash/include/esp_flash_spi_init.h +++ b/components/spi_flash/include/esp_flash_spi_init.h @@ -22,7 +22,7 @@ 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_read_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. } esp_flash_spi_device_config_t; diff --git a/components/spi_flash/include/memspi_host_driver.h b/components/spi_flash/include/memspi_host_driver.h index cb0f3e9d9..a200f20ed 100644 --- a/components/spi_flash/include/memspi_host_driver.h +++ b/components/spi_flash/include/memspi_host_driver.h @@ -32,7 +32,7 @@ .read = spi_flash_hal_read, \ .max_read_bytes = SPI_FLASH_HAL_MAX_READ_BYTES, \ .host_idle = spi_flash_hal_host_idle, \ - .configure_host_read_mode = spi_flash_hal_configure_host_read_mode, \ + .configure_host_io_mode = spi_flash_hal_configure_host_io_mode, \ .poll_cmd_done = spi_flash_hal_poll_cmd_done, \ .flush_cache = memspi_host_flush_cache, \ } diff --git a/components/spi_flash/include/spi_flash_chip_driver.h b/components/spi_flash/include/spi_flash_chip_driver.h index 9dd8ffa97..46667350d 100644 --- a/components/spi_flash/include/spi_flash_chip_driver.h +++ b/components/spi_flash/include/spi_flash_chip_driver.h @@ -149,7 +149,13 @@ struct spi_flash_chip_t { * * Can return ESP_ERR_FLASH_UNSUPPORTED_HOST or ESP_ERR_FLASH_UNSUPPORTED_CHIP if the specified mode is unsupported. */ - esp_err_t (*set_read_mode)(esp_flash_t *chip); + esp_err_t (*set_io_mode)(esp_flash_t *chip); + + /* + * Get whether the Quad Enable (QE) is set. (*out_io_mode)=SPI_FLASH_QOUT if + * enabled, otherwise disabled + */ + esp_err_t (*get_io_mode)(esp_flash_t *chip, esp_flash_io_mode_t* out_io_mode); }; /* Pointer to an array of pointers to all known drivers for flash chips. This array is used diff --git a/components/spi_flash/include/spi_flash_chip_generic.h b/components/spi_flash/include/spi_flash_chip_generic.h index 672ca6550..36dd3cb89 100644 --- a/components/spi_flash/include/spi_flash_chip_generic.h +++ b/components/spi_flash/include/spi_flash_chip_generic.h @@ -213,7 +213,21 @@ esp_err_t spi_flash_chip_generic_wait_idle(esp_flash_t *chip, uint32_t timeout_m * - ESP_ERR_TIMEOUT if not idle before timeout * - or other error passed from the ``set_write_protect`` or ``common_command`` function of host driver */ -esp_err_t spi_flash_chip_generic_set_read_mode(esp_flash_t *chip); +esp_err_t spi_flash_chip_generic_set_io_mode(esp_flash_t *chip); + +/** + * Get whether the Quad Enable (QE) is set. + * + * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. + * @param out_quad_mode Pointer to store the output mode. + * - SPI_FLASH_QOUT: QE is enabled + * - otherwise: QE is disabled + * + * @return + * - ESP_OK if success + * - or other error passed from the ``common_command`` function of host driver + */ +esp_err_t spi_flash_chip_generic_get_io_mode(esp_flash_t *chip, esp_flash_io_mode_t* out_quad_mode); /** * Generic SPI flash chip_drv, uses all the above functions for its operations. @@ -244,6 +258,78 @@ extern const spi_flash_chip_t esp_flash_chip_generic; */ esp_err_t spi_flash_generic_wait_host_idle(esp_flash_t *chip, uint32_t *timeout_ms); +/// Function pointer type for reading status register with QE bit. +typedef esp_err_t (*esp_flash_rdsr_func_t)(esp_flash_t* chip, uint32_t* out_sr); + +/** + * Use RDSR2 (35H) to read bit 15-8 of the SR, and RDSR (05H) to read bit 7-0. + * + * @param chip Pointer to SPI flash chip to use. + * @param out_sr Pointer to buffer to hold the status register, 16 bits. + * + * @return ESP_OK if success, otherwise error code passed from the + * `common_command` function of the host driver. + */ +esp_err_t spi_flash_common_read_status_16b_rdsr_rdsr2(esp_flash_t* chip, uint32_t* out_sr); + +/** + * Use RDSR2 (35H) to read bit 15-8 of the SR. + * + * @param chip Pointer to SPI flash chip to use. + * @param out_sr Pointer to buffer to hold the status register, 8 bits. + * + * @return ESP_OK if success, otherwise error code passed from the + * `common_command` function of the host driver. + */ +esp_err_t spi_flash_common_read_status_8b_rdsr2(esp_flash_t* chip, uint32_t* out_sr); + +/** + * Use RDSR (05H) to read bit 7-0 of the SR. + * + * @param chip Pointer to SPI flash chip to use. + * @param out_sr Pointer to buffer to hold the status register, 8 bits. + * + * @return ESP_OK if success, otherwise error code passed from the + * `common_command` function of the host driver. + */ +esp_err_t spi_flash_common_read_status_8b_rdsr(esp_flash_t* chip, uint32_t* out_sr); + +/// Function pointer type for writing status register with QE bit. +typedef esp_err_t (*esp_flash_wrsr_func_t)(esp_flash_t* chip, uint32_t sr); + +/** + * Use WRSR (01H) to write bit 7-0 of the SR. + * + * @param chip Pointer to SPI flash chip to use. + * @param sr Value of the status register to write, 8 bits. + * + * @return ESP_OK if success, otherwise error code passed from the + * `common_command` function of the host driver. + */ +esp_err_t spi_flash_common_write_status_8b_wrsr(esp_flash_t* chip, uint32_t sr); + +/** + * Use WRSR (01H) to write bit 15-0 of the SR. + * + * @param chip Pointer to SPI flash chip to use. + * @param sr Value of the status register to write, 16 bits. + * + * @return ESP_OK if success, otherwise error code passed from the + * `common_command` function of the host driver. + */ +esp_err_t spi_flash_common_write_status_16b_wrsr(esp_flash_t* chip, uint32_t sr); + +/** + * Use WRSR2 (31H) to write bit 15-8 of the SR. + * + * @param chip Pointer to SPI flash chip to use. + * @param sr Value of the status register to write, 8 bits. + * + * @return ESP_OK if success, otherwise error code passed from the + * `common_command` function of the host driver. + */ +esp_err_t spi_flash_common_write_status_8b_wrsr2(esp_flash_t* chip, uint32_t sr); + /** * @brief Utility function for set_read_mode chip_drv function. If required, * set and check the QE bit in the flash chip to enable the QIO/QOUT mode. @@ -253,16 +339,19 @@ esp_err_t spi_flash_generic_wait_host_idle(esp_flash_t *chip, uint32_t *timeout_ * * Registers to actually do Quad transtions and command to be sent in reading * should also be configured via - * spi_flash_chip_generic_config_host_read_mode(). + * spi_flash_chip_generic_config_host_io_mode(). * - * @param qe_rdsr_command SPI flash command to read status register - * @param qe_wrsr_command SPI flash command to write status register - * @param qe_sr_bitwidth Width of the status register these commands operate on, in bits. - * @param qe_sr_bit Bit mask for enabling Quad Enable functions on this chip. + * Note that the bit length and qe position of wrsr_func, rdsr_func and + * qe_sr_bit should be consistent. + * + * @param chip Pointer to SPI flash chip to use. + * @param wrsr_func Function pointer for writing the status register + * @param rdsr_func Function pointer for reading the status register + * @param qe_sr_bit status with the qe bit only. * * @return always ESP_OK (currently). */ -esp_err_t spi_flash_common_set_read_mode(esp_flash_t *chip, uint8_t qe_rdsr_command, uint8_t qe_wrsr_command, uint8_t qe_sr_bitwidth, unsigned qe_sr_bit); +esp_err_t spi_flash_common_set_io_mode(esp_flash_t *chip, esp_flash_wrsr_func_t wrsr_func, esp_flash_rdsr_func_t rdsr_func, uint32_t qe_sr_bit); /** * @brief Configure the host registers to use the specified read mode set in @@ -278,17 +367,4 @@ esp_err_t spi_flash_common_set_read_mode(esp_flash_t *chip, uint8_t qe_rdsr_comm * - ESP_ERR_FLASH_NOT_INITIALISED if chip not initialized properly * - or other error passed from the ``configure_host_mode`` function of host driver */ -esp_err_t spi_flash_chip_generic_config_host_read_mode(esp_flash_t *chip); - -/** - * @brief Returns true if chip is configured for Quad I/O or Quad Fast Read. - * - * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. - * - * @return true if flash works in quad mode, otherwise false - */ -static inline bool spi_flash_is_quad_mode(const esp_flash_t *chip) -{ - return (chip->read_mode == SPI_FLASH_QIO) || (chip->read_mode == SPI_FLASH_QOUT); -} - +esp_err_t spi_flash_chip_generic_config_host_io_mode(esp_flash_t *chip); diff --git a/components/spi_flash/spi_flash_chip_generic.c b/components/spi_flash/spi_flash_chip_generic.c index 8bbb9d137..16d6b85e1 100644 --- a/components/spi_flash/spi_flash_chip_generic.c +++ b/components/spi_flash/spi_flash_chip_generic.c @@ -144,7 +144,7 @@ esp_err_t spi_flash_chip_generic_read(esp_flash_t *chip, void *buffer, uint32_t { esp_err_t err = ESP_OK; // Configure the host, and return - spi_flash_chip_generic_config_host_read_mode(chip); + spi_flash_chip_generic_config_host_io_mode(chip); while (err == ESP_OK && length > 0) { uint32_t read_len = MIN(length, chip->host->max_read_bytes); @@ -277,7 +277,7 @@ esp_err_t spi_flash_chip_generic_wait_idle(esp_flash_t *chip, uint32_t timeout_m return (timeout_ms > 0) ? ESP_OK : ESP_ERR_TIMEOUT; } -esp_err_t spi_flash_chip_generic_config_host_read_mode(esp_flash_t *chip) +esp_err_t spi_flash_chip_generic_config_host_io_mode(esp_flash_t *chip) { uint32_t dummy_cyclelen_base; uint32_t addr_bitlen; @@ -320,62 +320,33 @@ esp_err_t spi_flash_chip_generic_config_host_read_mode(esp_flash_t *chip) return ESP_ERR_FLASH_NOT_INITIALISED; } - return chip->host->configure_host_read_mode(chip->host, chip->read_mode, addr_bitlen, dummy_cyclelen_base, read_command); + return chip->host->configure_host_io_mode(chip->host, read_command, addr_bitlen, dummy_cyclelen_base, + chip->read_mode); } -esp_err_t spi_flash_common_set_read_mode(esp_flash_t *chip, uint8_t qe_rdsr_command, uint8_t qe_wrsr_command, uint8_t qe_sr_bitwidth, unsigned qe_sr_bit) -{ - if (spi_flash_is_quad_mode(chip)) { - // Ensure quad modes are enabled, using the Quad Enable parameters supplied. - spi_flash_trans_t t = { - .command = qe_rdsr_command, - .mosi_data = 0, - .mosi_len = 0, - .miso_len = qe_sr_bitwidth, - }; - chip->host->common_command(chip->host, &t); - unsigned sr = t.miso_data[0]; - ESP_EARLY_LOGV(TAG, "set_read_mode: status before 0x%x", sr); - if ((sr & qe_sr_bit) == 0) { - //some chips needs the write protect to be disabled before writing to Status Register - chip->chip_drv->set_chip_write_protect(chip, false); - - sr |= qe_sr_bit; - spi_flash_trans_t t = { - .command = qe_wrsr_command, - .mosi_data = sr, - .mosi_len = qe_sr_bitwidth, - .miso_len = 0, - }; - chip->host->common_command(chip->host, &t); - - /* Check the new QE bit has stayed set */ - spi_flash_trans_t t_rdsr = { - .command = qe_rdsr_command, - .mosi_data = 0, - .mosi_len = 0, - .miso_len = qe_sr_bitwidth - }; - chip->host->common_command(chip->host, &t_rdsr); - sr = t_rdsr.miso_data[0]; - ESP_EARLY_LOGV(TAG, "set_read_mode: status after 0x%x", sr); - if ((sr & qe_sr_bit) == 0) { - return ESP_ERR_FLASH_NO_RESPONSE; - } - - chip->chip_drv->set_chip_write_protect(chip, true); - } - } - return ESP_OK; -} - -esp_err_t spi_flash_chip_generic_set_read_mode(esp_flash_t *chip) +esp_err_t spi_flash_chip_generic_get_io_mode(esp_flash_t *chip, esp_flash_io_mode_t* out_io_mode) { // On "generic" chips, this involves checking // bit 1 (QE) of RDSR2 (35h) result // (it works this way on GigaDevice & Fudan Micro chips, probably others...) const uint8_t BIT_QE = 1 << 1; - return spi_flash_common_set_read_mode(chip, CMD_RDSR2, CMD_WRSR2, 8, BIT_QE); + uint32_t sr; + esp_err_t ret = spi_flash_common_read_status_8b_rdsr2(chip, &sr); + if (ret == ESP_OK) { + *out_io_mode = ((sr & BIT_QE)? SPI_FLASH_QOUT: 0); + } + return ret; +} + +esp_err_t spi_flash_chip_generic_set_io_mode(esp_flash_t *chip) +{ + // On "generic" chips, this involves checking + // bit 9 (QE) of RDSR (05h) result + const uint32_t BIT_QE = 1 << 9; + return spi_flash_common_set_io_mode(chip, + spi_flash_common_write_status_16b_wrsr, + spi_flash_common_read_status_16b_rdsr_rdsr2, + BIT_QE); } static const char chip_name[] = "generic"; @@ -409,5 +380,134 @@ const spi_flash_chip_t esp_flash_chip_generic = { .write_encrypted = spi_flash_chip_generic_write_encrypted, .wait_idle = spi_flash_chip_generic_wait_idle, - .set_read_mode = spi_flash_chip_generic_set_read_mode, + .set_io_mode = spi_flash_chip_generic_set_io_mode, + .get_io_mode = spi_flash_chip_generic_get_io_mode, }; + +/******************************************************************************* + * Utility functions + ******************************************************************************/ + +static esp_err_t spi_flash_common_read_qe_sr(esp_flash_t *chip, uint8_t qe_rdsr_command, uint8_t qe_sr_bitwidth, uint32_t *sr) +{ + spi_flash_trans_t t = { + .command = qe_rdsr_command, + .mosi_data = 0, + .mosi_len = 0, + .miso_len = qe_sr_bitwidth, + }; + esp_err_t ret = chip->host->common_command(chip->host, &t); + *sr = t.miso_data[0]; + return ret; +} + +static esp_err_t spi_flash_common_write_qe_sr(esp_flash_t *chip, uint8_t qe_wrsr_command, uint8_t qe_sr_bitwidth, uint32_t qe) +{ + spi_flash_trans_t t = { + .command = qe_wrsr_command, + .mosi_data = qe, + .mosi_len = qe_sr_bitwidth, + .miso_len = 0, + }; + return chip->host->common_command(chip->host, &t); +} + +esp_err_t spi_flash_common_read_status_16b_rdsr_rdsr2(esp_flash_t* chip, uint32_t* out_sr) +{ + uint32_t sr, sr2; + esp_err_t ret = spi_flash_common_read_qe_sr(chip, CMD_RDSR2, 8, &sr2); + if (ret == ESP_OK) { + ret = spi_flash_common_read_qe_sr(chip, CMD_RDSR, 8, &sr); + } + if (ret == ESP_OK) { + *out_sr = (sr & 0xff) | ((sr2 & 0xff) << 8); + } + return ret; +} + +esp_err_t spi_flash_common_read_status_8b_rdsr2(esp_flash_t* chip, uint32_t* out_sr) +{ + return spi_flash_common_read_qe_sr(chip, CMD_RDSR2, 8, out_sr); +} + +esp_err_t spi_flash_common_read_status_8b_rdsr(esp_flash_t* chip, uint32_t* out_sr) +{ + return spi_flash_common_read_qe_sr(chip, CMD_RDSR, 8, out_sr); +} + +esp_err_t spi_flash_common_write_status_16b_wrsr(esp_flash_t* chip, uint32_t sr) +{ + return spi_flash_common_write_qe_sr(chip, CMD_WRSR, 16, sr); +} + +esp_err_t spi_flash_common_write_status_8b_wrsr(esp_flash_t* chip, uint32_t sr) +{ + return spi_flash_common_write_qe_sr(chip, CMD_WRSR, 8, sr); +} + +esp_err_t spi_flash_common_write_status_8b_wrsr2(esp_flash_t* chip, uint32_t sr) +{ + return spi_flash_common_write_qe_sr(chip, CMD_WRSR2, 8, sr); +} + +esp_err_t spi_flash_common_set_io_mode(esp_flash_t *chip, esp_flash_wrsr_func_t wrsr_func, esp_flash_rdsr_func_t rdsr_func, uint32_t qe_sr_bit) +{ + 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 + + bool need_check = is_quad_mode; + if (force_check) { + need_check = true; + } + + uint32_t sr_update; + if (need_check) { + // Ensure quad modes are enabled, using the Quad Enable parameters supplied. + uint32_t sr; + ret = (*rdsr_func)(chip, &sr); + if (ret != ESP_OK) { + return ret; + } + ESP_EARLY_LOGD(TAG, "set_io_mode: status before 0x%x", sr); + if (is_quad_mode) { + sr_update = sr | qe_sr_bit; + } else { + sr_update = sr & (~qe_sr_bit); + } + ESP_EARLY_LOGV(TAG, "set_io_mode: status update 0x%x", sr_update); + if (sr != sr_update) { + update_config = true; + } + } + + if (update_config) { + //some chips needs the write protect to be disabled before writing to Status Register + chip->chip_drv->set_chip_write_protect(chip, false); + + ret = (*wrsr_func)(chip, sr_update); + if (ret != ESP_OK) { + return ret; + } + + ret = chip->chip_drv->wait_idle(chip, DEFAULT_IDLE_TIMEOUT); + if (ret != ESP_OK) { + return ret; + } + + /* Check the new QE bit has stayed set */ + uint32_t sr; + ret = (*rdsr_func)(chip, &sr); + if (ret != ESP_OK) { + return ret; + } + ESP_EARLY_LOGD(TAG, "set_io_mode: status after 0x%x", sr); + if (sr != sr_update) { + ret = ESP_ERR_FLASH_NO_RESPONSE; + } + + chip->chip_drv->set_chip_write_protect(chip, true); + } + return ret; +} diff --git a/components/spi_flash/spi_flash_chip_issi.c b/components/spi_flash/spi_flash_chip_issi.c index 71684ec19..d5ecd4c04 100644 --- a/components/spi_flash/spi_flash_chip_issi.c +++ b/components/spi_flash/spi_flash_chip_issi.c @@ -35,13 +35,29 @@ esp_err_t spi_flash_chip_issi_probe(esp_flash_t *chip, uint32_t flash_id) return ESP_OK; } -esp_err_t spi_flash_chip_issi_set_read_mode(esp_flash_t *chip) +esp_err_t spi_flash_chip_issi_set_io_mode(esp_flash_t *chip) { /* ISSI uses bit 6 of "basic" SR as Quad Enable */ const uint8_t BIT_QE = 1 << 6; - return spi_flash_common_set_read_mode(chip, CMD_RDSR, CMD_WRSR, 8, BIT_QE); + return spi_flash_common_set_io_mode(chip, + spi_flash_common_write_status_8b_wrsr, + spi_flash_common_read_status_8b_rdsr, + BIT_QE); } +esp_err_t spi_flash_chip_issi_get_io_mode(esp_flash_t *chip, esp_flash_io_mode_t* out_io_mode) +{ + /* ISSI uses bit 6 of "basic" SR as Quad Enable */ + const uint8_t BIT_QE = 1 << 6; + uint32_t sr; + esp_err_t ret = spi_flash_common_read_status_8b_rdsr(chip, &sr); + if (ret == ESP_OK) { + *out_io_mode = ((sr & BIT_QE)? SPI_FLASH_QOUT: 0); + } + return ret; +} + + static const char chip_name[] = "issi"; // The issi chip can use the functions for generic chips except from set read mode and probe, @@ -73,5 +89,6 @@ const spi_flash_chip_t esp_flash_chip_issi = { .write_encrypted = spi_flash_chip_generic_write_encrypted, .wait_idle = spi_flash_chip_generic_wait_idle, - .set_read_mode = spi_flash_chip_issi_set_read_mode, + .set_io_mode = spi_flash_chip_issi_set_io_mode, + .get_io_mode = spi_flash_chip_issi_get_io_mode, }; diff --git a/components/spi_flash/test/test_esp_flash.c b/components/spi_flash/test/test_esp_flash.c index f6eaf94f1..338553ff1 100644 --- a/components/spi_flash/test/test_esp_flash.c +++ b/components/spi_flash/test/test_esp_flash.c @@ -127,7 +127,7 @@ static void release_bus(int host_id) } } -static void setup_new_chip(esp_flash_read_mode_t io_mode, esp_flash_speed_t speed) +static void setup_new_chip(esp_flash_io_mode_t io_mode, esp_flash_speed_t speed) { //the bus should be initialized before the flash is attached to the bus setup_bus(TEST_HOST); From 15d311bb80ecd9191fcabd4a846711b433c4448c Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Thu, 5 Sep 2019 13:17:11 +0800 Subject: [PATCH 2/4] esp_flash: rename internal variables for better readability chip_drv in HAL are renamed as host --- components/soc/src/hal/spi_flash_hal_iram.c | 64 ++++++++++----------- components/spi_flash/memspi_host_driver.c | 4 +- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/components/soc/src/hal/spi_flash_hal_iram.c b/components/soc/src/hal/spi_flash_hal_iram.c index ea8e2bfbc..a6fb23fe5 100644 --- a/components/soc/src/hal/spi_flash_hal_iram.c +++ b/components/soc/src/hal/spi_flash_hal_iram.c @@ -19,22 +19,22 @@ #define ADDRESS_MASK_24BIT 0xFFFFFF -static inline spi_dev_t *get_spi_dev(spi_flash_host_driver_t *chip_drv) +static inline spi_dev_t *get_spi_dev(spi_flash_host_driver_t *host) { - return ((spi_flash_memspi_data_t *)chip_drv->driver_data)->spi; + return ((spi_flash_memspi_data_t *)host->driver_data)->spi; } -void spi_flash_hal_poll_cmd_done(spi_flash_host_driver_t *driver) +void spi_flash_hal_poll_cmd_done(spi_flash_host_driver_t *host) { - while (!spi_flash_ll_cmd_is_done(get_spi_dev(driver))) { + while (!spi_flash_ll_cmd_is_done(get_spi_dev(host))) { //nop } } -esp_err_t spi_flash_hal_device_config(spi_flash_host_driver_t *driver) +esp_err_t spi_flash_hal_device_config(spi_flash_host_driver_t *host) { - spi_flash_memspi_data_t *drv_data = (spi_flash_memspi_data_t *)driver->driver_data; - spi_dev_t *dev = get_spi_dev(driver); + spi_flash_memspi_data_t *drv_data = (spi_flash_memspi_data_t *)host->driver_data; + spi_dev_t *dev = get_spi_dev(host); spi_flash_ll_reset(dev); spi_flash_ll_set_cs_pin(dev, drv_data->cs_num); spi_flash_ll_set_clock(dev, &drv_data->clock_conf); @@ -60,9 +60,9 @@ esp_err_t spi_flash_hal_configure_host_io_mode( esp_flash_io_mode_t io_mode) { // Add dummy cycles to compensate for latency of GPIO matrix and external delay, if necessary... - int dummy_cyclelen = dummy_cyclelen_base + ((spi_flash_memspi_data_t *)driver->driver_data)->extra_dummy; + int dummy_cyclelen = dummy_cyclelen_base + ((spi_flash_memspi_data_t *)host->driver_data)->extra_dummy; - spi_dev_t *dev = get_spi_dev(driver); + spi_dev_t *dev = get_spi_dev(host); spi_flash_ll_set_command8(dev, command); spi_flash_ll_set_addr_bitlen(dev, addr_bitlen); spi_flash_ll_set_dummy(dev, dummy_cyclelen); @@ -73,11 +73,11 @@ esp_err_t spi_flash_hal_configure_host_io_mode( return ESP_OK; } -esp_err_t spi_flash_hal_common_command(spi_flash_host_driver_t *chip_drv, spi_flash_trans_t *trans) +esp_err_t spi_flash_hal_common_command(spi_flash_host_driver_t *host, spi_flash_trans_t *trans) { host->configure_host_io_mode(host, trans->command, 0, 0, SPI_FLASH_FASTRD); - spi_dev_t *dev = get_spi_dev(chip_drv); + spi_dev_t *dev = get_spi_dev(host); //disable dummy if no input phase if (trans->miso_len == 0) { spi_flash_ll_set_dummy(dev, 0); @@ -89,61 +89,61 @@ esp_err_t spi_flash_hal_common_command(spi_flash_host_driver_t *chip_drv, spi_fl spi_flash_ll_write_word(dev, trans->mosi_data); spi_flash_ll_user_start(dev); - chip_drv->poll_cmd_done(chip_drv); + host->poll_cmd_done(host); spi_flash_ll_get_buffer_data(dev, trans->miso_data, 8); return ESP_OK; } -void spi_flash_hal_erase_chip(spi_flash_host_driver_t *chip_drv) +void spi_flash_hal_erase_chip(spi_flash_host_driver_t *host) { - spi_dev_t *dev = get_spi_dev(chip_drv); + spi_dev_t *dev = get_spi_dev(host); spi_flash_ll_erase_chip(dev); - chip_drv->poll_cmd_done(chip_drv); + host->poll_cmd_done(host); } -void spi_flash_hal_erase_sector(spi_flash_host_driver_t *chip_drv, uint32_t start_address) +void spi_flash_hal_erase_sector(spi_flash_host_driver_t *host, uint32_t start_address) { - spi_dev_t *dev = get_spi_dev(chip_drv); + spi_dev_t *dev = get_spi_dev(host); spi_flash_ll_set_addr_bitlen(dev, 24); spi_flash_ll_set_address(dev, start_address & ADDRESS_MASK_24BIT); spi_flash_ll_erase_sector(dev); - chip_drv->poll_cmd_done(chip_drv); + host->poll_cmd_done(host); } -void spi_flash_hal_erase_block(spi_flash_host_driver_t *chip_drv, uint32_t start_address) +void spi_flash_hal_erase_block(spi_flash_host_driver_t *host, uint32_t start_address) { - spi_dev_t *dev = get_spi_dev(chip_drv); + spi_dev_t *dev = get_spi_dev(host); spi_flash_ll_set_addr_bitlen(dev, 24); spi_flash_ll_set_address(dev, start_address & ADDRESS_MASK_24BIT); spi_flash_ll_erase_block(dev); - chip_drv->poll_cmd_done(chip_drv); + host->poll_cmd_done(host); } -void spi_flash_hal_program_page(spi_flash_host_driver_t *chip_drv, const void *buffer, uint32_t address, uint32_t length) +void spi_flash_hal_program_page(spi_flash_host_driver_t *host, const void *buffer, uint32_t address, uint32_t length) { - spi_dev_t *dev = get_spi_dev(chip_drv); + spi_dev_t *dev = get_spi_dev(host); spi_flash_ll_set_addr_bitlen(dev, 24); spi_flash_ll_set_address(dev, (address & ADDRESS_MASK_24BIT) | (length << 24)); spi_flash_ll_program_page(dev, buffer, length); - chip_drv->poll_cmd_done(chip_drv); + host->poll_cmd_done(host); } -esp_err_t spi_flash_hal_read(spi_flash_host_driver_t *chip_drv, void *buffer, uint32_t address, uint32_t read_len) +esp_err_t spi_flash_hal_read(spi_flash_host_driver_t *host, void *buffer, uint32_t address, uint32_t read_len) { - spi_dev_t *dev = get_spi_dev(chip_drv); + spi_dev_t *dev = get_spi_dev(host); //the command is already set by ``spi_flash_hal_configure_host_io_mode`` before. spi_flash_ll_set_address(dev, address << 8); spi_flash_ll_set_miso_bitlen(dev, read_len * 8); spi_flash_ll_user_start(dev); - chip_drv->poll_cmd_done(chip_drv); + host->poll_cmd_done(host); spi_flash_ll_get_buffer_data(dev, buffer, read_len); return ESP_OK; } -bool spi_flash_hal_host_idle(spi_flash_host_driver_t *chip_drv) +bool spi_flash_hal_host_idle(spi_flash_host_driver_t *host) { - spi_dev_t *dev = get_spi_dev(chip_drv); + spi_dev_t *dev = get_spi_dev(host); bool idle = spi_flash_ll_host_idle(dev); // Not clear if this is necessary, or only necessary if @@ -155,10 +155,10 @@ bool spi_flash_hal_host_idle(spi_flash_host_driver_t *chip_drv) return idle; } -esp_err_t spi_flash_hal_set_write_protect(spi_flash_host_driver_t *chip_drv, bool wp) +esp_err_t spi_flash_hal_set_write_protect(spi_flash_host_driver_t *host, bool wp) { - spi_dev_t *dev = get_spi_dev(chip_drv); + spi_dev_t *dev = get_spi_dev(host); spi_flash_ll_set_write_protect(dev, wp); - chip_drv->poll_cmd_done(chip_drv); + host->poll_cmd_done(host); return ESP_OK; } diff --git a/components/spi_flash/memspi_host_driver.c b/components/spi_flash/memspi_host_driver.c index 85b31998a..adc900cd7 100644 --- a/components/spi_flash/memspi_host_driver.c +++ b/components/spi_flash/memspi_host_driver.c @@ -38,7 +38,7 @@ esp_err_t memspi_host_init_pointers(spi_flash_host_driver_t *host, memspi_host_d return ESP_OK; } -esp_err_t memspi_host_read_id_hs(spi_flash_host_driver_t *chip_drv, uint32_t *id) +esp_err_t memspi_host_read_id_hs(spi_flash_host_driver_t *host, uint32_t *id) { //NOTE: we do have a read id function, however it doesn't work in high freq spi_flash_trans_t t = { @@ -47,7 +47,7 @@ esp_err_t memspi_host_read_id_hs(spi_flash_host_driver_t *chip_drv, uint32_t *id .mosi_len = 0, .miso_len = 24 }; - chip_drv->common_command(chip_drv, &t); + host->common_command(host, &t); uint32_t raw_flash_id = t.miso_data[0]; ESP_EARLY_LOGV(TAG, "raw_chip_id: %X\n", raw_flash_id); if (raw_flash_id == 0xFFFFFF || raw_flash_id == 0) { From 1e1d50376b04e7ce66e0a2ef9fd0c1a3fe8e56a0 Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Wed, 4 Sep 2019 21:09:30 +0800 Subject: [PATCH 3/4] esp_flash: add unit test for external flash and QE toggling Tests for external flash chips used to controlled by macros, one bin for one chip. And tests are done manually. This commit refactored the test so that all 3 chips can all run in single test. --- components/spi_flash/test/test_esp_flash.c | 359 ++++++++++++--------- tools/ci/config/target-test.yml | 8 + 2 files changed, 219 insertions(+), 148 deletions(-) diff --git a/components/spi_flash/test/test_esp_flash.c b/components/spi_flash/test/test_esp_flash.c index 338553ff1..0e5679f73 100644 --- a/components/spi_flash/test/test_esp_flash.c +++ b/components/spi_flash/test/test_esp_flash.c @@ -16,19 +16,18 @@ #include "unity.h" #include "driver/gpio.h" #include "soc/io_mux_reg.h" - +#include "sdkconfig.h" #define FUNC_SPI 1 static uint8_t sector_buf[4096]; -// #define TEST_SPI1_CS1 -// #define TEST_SPI2_CS0 -// #define TEST_SPI3_CS0 #define TEST_SPI_SPEED ESP_FLASH_10MHZ #define TEST_SPI_READ_MODE SPI_FLASH_FASTRD //#define FORCE_GPIO_MATRIX +#define EXTRA_SPI1_CLK_IO 17 //the pin which is usually used by the PSRAM clk + #define HSPI_PIN_NUM_MOSI HSPI_IOMUX_PIN_NUM_MOSI #define HSPI_PIN_NUM_MISO HSPI_IOMUX_PIN_NUM_MISO #define HSPI_PIN_NUM_CLK HSPI_IOMUX_PIN_NUM_CLK @@ -41,40 +40,56 @@ static uint8_t sector_buf[4096]; #define VSPI_PIN_NUM_HD VSPI_IOMUX_PIN_NUM_HD #define VSPI_PIN_NUM_WP VSPI_IOMUX_PIN_NUM_WP -#if defined TEST_SPI1_CS1 -# define TEST_HOST SPI_HOST -# define TEST_CS 1 -// #define TEST_CS_PIN 14 -# define TEST_CS_PIN 16 //the pin which is usually used by the PSRAM -// #define TEST_CS_PIN 27 -# define TEST_INPUT_DELAY 0 -# define EXTRA_SPI1_CLK_IO 17 //the pin which is usually used by the PSRAM clk - -#elif defined TEST_SPI2_CS0 - -# define TEST_HOST HSPI_HOST -# define TEST_CS 0 -# define TEST_CS_PIN HSPI_IOMUX_PIN_NUM_CS -# define TEST_INPUT_DELAY 20 - -#elif defined TEST_SPI3_CS0 - -# define TEST_HOST VSPI_HOST -# define TEST_CS 0 -# define TEST_CS_PIN VSPI_IOMUX_PIN_NUM_CS -# define TEST_INPUT_DELAY 0 +#define ALL_TEST_NUM (sizeof(config_list)/sizeof(flashtest_config_t)) +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);} +#ifdef CONFIG_ESP32_SPIRAM_SUPPORT +// These tests needs external flash, right on the place of psram +#define FLASH_TEST_CASE_3(STR, FUNCT_TO_RUN) #else -# define SKIP_EXTENDED_CHIP_TEST +#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, ALL_TEST_NUM);} #endif +//currently all the configs are the same with esp_flash_spi_device_config_t, no more information required +typedef esp_flash_spi_device_config_t flashtest_config_t; static const char TAG[] = "test_esp_flash"; - -#ifndef SKIP_EXTENDED_CHIP_TEST - -static esp_flash_t *test_chip = NULL; +flashtest_config_t config_list[] = { + // 0 always reserved for main flash + { + .host_id = -1, // no need to init + }, + { + .io_mode = TEST_SPI_READ_MODE, + .speed = TEST_SPI_SPEED, + .host_id = SPI_HOST, + .cs_id = 1, + .cs_io_num = 16, //the pin which is usually used by the PSRAM + .input_delay_ns = 0, + }, + /* current runner doesn't have a flash on HSPI + { + .io_mode = TEST_SPI_READ_MODE, + .speed = TEST_SPI_SPEED, + .host = HSPI_HOST, + .cs_id = 0, + .cs_io_num = HSPI_IOMUX_PIN_NUM_CS, + .input_delay_ns = 20, + }, + */ + { + .io_mode = TEST_SPI_READ_MODE, + .speed = TEST_SPI_SPEED, + .host_id = VSPI_HOST, + .cs_id = 0, + .cs_io_num = VSPI_IOMUX_PIN_NUM_CS, + .input_delay_ns = 0, + }, +}; static void setup_bus(spi_host_device_t host_id) { @@ -127,33 +142,50 @@ static void release_bus(int host_id) } } -static void setup_new_chip(esp_flash_io_mode_t io_mode, esp_flash_speed_t speed) +static void setup_new_chip(const flashtest_config_t* test_cfg, esp_flash_t** out_chip) { //the bus should be initialized before the flash is attached to the bus - setup_bus(TEST_HOST); + if (test_cfg->host_id == -1) { + *out_chip = NULL; + return; + } + setup_bus(test_cfg->host_id); esp_flash_spi_device_config_t dev_cfg = { - .host_id = TEST_HOST, - .io_mode = io_mode, - .speed = speed, - .cs_id = TEST_CS, - .cs_io_num = TEST_CS_PIN, - .input_delay_ns = TEST_INPUT_DELAY, + .host_id = test_cfg->host_id, + .io_mode = test_cfg->io_mode, + .speed = test_cfg->speed, + .cs_id = test_cfg->cs_id, + .cs_io_num = test_cfg->cs_io_num, + .input_delay_ns = test_cfg->input_delay_ns, }; - esp_err_t err = spi_bus_add_flash_device(&test_chip, &dev_cfg); + esp_flash_t* init_chip; + esp_err_t err = spi_bus_add_flash_device(&init_chip, &dev_cfg); TEST_ESP_OK(err); - err = esp_flash_init(test_chip); + err = esp_flash_init(init_chip); TEST_ESP_OK(err); + *out_chip = init_chip; } -void teardown_test_chip(void) +void teardown_test_chip(esp_flash_t* chip, spi_host_device_t host) { - spi_bus_remove_flash_device(test_chip); - test_chip = NULL; - release_bus(TEST_HOST); + //happen to work when chip==NULL + spi_bus_remove_flash_device(chip); + release_bus(host); } -#endif +static void flash_test_func(flash_test_func_t func, int test_num) +{ + for (int i = 0; i < test_num; i++) { + flashtest_config_t* config = &config_list[i]; + esp_flash_t* chip; + setup_new_chip(config, &chip); + (*func)(chip); + teardown_test_chip(chip, config->host_id); + } +} + +/* ---------- Test code start ------------*/ static void test_metadata(esp_flash_t *chip) { @@ -164,15 +196,8 @@ static void test_metadata(esp_flash_t *chip) printf("Flash ID %08x detected size %d bytes\n", id, size); } -TEST_CASE("SPI flash metadata functions", "[esp_flash]") -{ -#ifndef SKIP_EXTENDED_CHIP_TEST - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_metadata(test_chip); - teardown_test_chip(); -#endif - test_metadata(NULL); -} +FLASH_TEST_CASE("SPI flash metadata functions", test_metadata); +FLASH_TEST_CASE_3("SPI flash metadata functions", test_metadata); static uint32_t erase_test_region(esp_flash_t *chip, int num_sectors) { @@ -203,7 +228,7 @@ static uint32_t erase_test_region(esp_flash_t *chip, int num_sectors) return offs; } -void test_simple_read_write(void *chip) +void test_simple_read_write(esp_flash_t *chip) { ESP_LOGI(TAG, "Testing chip %p...", chip); uint32_t offs = erase_test_region(chip, 1); @@ -230,17 +255,10 @@ void test_simple_read_write(void *chip) } } -TEST_CASE("SPI flash simple read/write", "[esp_flash]") -{ - test_simple_read_write(NULL); -#ifndef SKIP_EXTENDED_CHIP_TEST - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_simple_read_write(test_chip); - teardown_test_chip(); -#endif -} +FLASH_TEST_CASE("SPI flash simple read/write", test_simple_read_write); +FLASH_TEST_CASE_3("SPI flash simple read/write", test_simple_read_write); -void test_unaligned_read_write(void *chip) +void test_unaligned_read_write(esp_flash_t *chip) { ESP_LOGI(TAG, "Testing chip %p...", chip); uint32_t offs = erase_test_region(chip, 2); @@ -258,17 +276,10 @@ void test_unaligned_read_write(void *chip) TEST_ASSERT(memcmp(buf, msg, strlen(msg) + 1) == 0); } -TEST_CASE("SPI flash unaligned read/write", "[esp_flash]") -{ -#ifndef SKIP_EXTENDED_CHIP_TEST - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_unaligned_read_write(test_chip); - teardown_test_chip(); -#endif - test_unaligned_read_write(NULL); -} +FLASH_TEST_CASE("SPI flash unaligned read/write", test_unaligned_read_write); +FLASH_TEST_CASE_3("SPI flash unaligned read/write", test_unaligned_read_write); -void test_single_read_write(void *chip) +void test_single_read_write(esp_flash_t* chip) { ESP_LOGI(TAG, "Testing chip %p...", chip); uint32_t offs = erase_test_region(chip, 2); @@ -284,21 +295,14 @@ void test_single_read_write(void *chip) } } -TEST_CASE("SPI flash single byte reads/writes", "[esp_flash]") -{ - test_single_read_write(NULL); -#ifndef SKIP_EXTENDED_CHIP_TEST - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_single_read_write(test_chip); - teardown_test_chip(); -#endif -} +FLASH_TEST_CASE("SPI flash single byte reads/writes", test_single_read_write); +FLASH_TEST_CASE_3("SPI flash single byte reads/writes", test_single_read_write); /* this test is notable because it generates a lot of unaligned reads/writes, and also reads/writes across both a sector boundary & many page boundaries. */ -void test_three_byte_read_write(void *chip) +void test_three_byte_read_write(esp_flash_t *chip) { ESP_LOGI(TAG, "Testing chip %p...", chip); uint32_t offs = erase_test_region(chip, 2); @@ -315,15 +319,8 @@ void test_three_byte_read_write(void *chip) } } -TEST_CASE("SPI flash three byte reads/writes", "[esp_flash]") -{ -#ifndef SKIP_EXTENDED_CHIP_TEST - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_three_byte_read_write(test_chip); - teardown_test_chip(); -#endif - test_three_byte_read_write(NULL); -} +FLASH_TEST_CASE("SPI flash three byte reads/writes", test_three_byte_read_write); +FLASH_TEST_CASE_3("SPI flash three byte reads/writes", test_three_byte_read_write); void test_erase_large_region(esp_flash_t *chip) { @@ -355,15 +352,8 @@ void test_erase_large_region(esp_flash_t *chip) TEST_ASSERT_EQUAL_HEX32(0xFFFFFFFF, readback); } -TEST_CASE("SPI flash erase large region", "[esp_flash]") -{ - test_erase_large_region(NULL); -#ifndef SKIP_EXTENDED_CHIP_TEST - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_erase_large_region(test_chip); - teardown_test_chip(); -#endif -} +FLASH_TEST_CASE("SPI flash erase large region", test_erase_large_region); +FLASH_TEST_CASE_3("SPI flash erase large region", test_erase_large_region); static void test_write_protection(esp_flash_t* chip) { @@ -385,15 +375,8 @@ static void test_write_protection(esp_flash_t* chip) } } -TEST_CASE("Test esp_flash can enable/disable write protetion", "[esp_flash]") -{ - test_write_protection(NULL); -#ifndef SKIP_EXTENDED_CHIP_TEST - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_write_protection(test_chip); - teardown_test_chip(); -#endif -} +FLASH_TEST_CASE("Test esp_flash can enable/disable write protetion", test_write_protection); +FLASH_TEST_CASE_3("Test esp_flash can enable/disable write protetion", test_write_protection); static const uint8_t large_const_buffer[16400] = { 203, // first byte @@ -410,8 +393,73 @@ static void test_write_large_buffer(esp_flash_t *chip, const uint8_t *source, si static void write_large_buffer(esp_flash_t *chip, const esp_partition_t *part, const uint8_t *source, size_t length); static void read_and_check(esp_flash_t *chip, const esp_partition_t *part, const uint8_t *source, size_t length); -TEST_CASE("SPI flash test reading with all speed/mode permutations", "[esp_flash]") +// Internal functions for testing, from esp_flash_api.c +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); + +static bool check_winbond_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); + if ((flash_id >> 16) == 0xEF) { + return true; + } else { + return false; + } +} + +static void test_toggle_qe(esp_flash_t* chip) +{ + bool qe; + if (chip == NULL) { + chip = esp_flash_default_chip; + } + esp_flash_io_mode_t io_mode_before = chip->read_mode; + esp_err_t ret = esp_flash_get_io_mode(chip, &qe); + TEST_ESP_OK(ret); + + bool is_winbond_chip = check_winbond_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) { + //allows clear qe failure for Winbond chips + ret = ESP_OK; + } + TEST_ESP_OK(ret); + + bool qe_read; + 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."); + chip->read_mode = io_mode_before; + return; + } + TEST_ASSERT_EQUAL(qe, qe_read); + } + //restore the io_mode after test + 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); + + +void test_permutations(flashtest_config_t* config) +{ + //replace config pointer with pointer to internal temporary config + flashtest_config_t temp_cfg; + memcpy(&temp_cfg, config, sizeof(flashtest_config_t)); + flashtest_config_t* cfg = &temp_cfg; + esp_flash_t* chip; + const int length = sizeof(large_const_buffer); uint8_t *source_buf = malloc(length); TEST_ASSERT_NOT_NULL(source_buf); @@ -423,60 +471,75 @@ TEST_CASE("SPI flash test reading with all speed/mode permutations", "[esp_flash const esp_partition_t *part = get_test_data_partition(); TEST_ASSERT(part->size > length + 2 + SPI_FLASH_SEC_SIZE); -#ifndef SKIP_EXTENDED_CHIP_TEST - //use the lowest speed to write and read to make sure success - setup_new_chip(TEST_SPI_READ_MODE, ESP_FLASH_SPEED_MIN); - write_large_buffer(test_chip, part, source_buf, length); - read_and_check(test_chip, part, source_buf, length); - teardown_test_chip(); + //write data to be read, and use the lowest speed to write and read to make sure success + cfg->io_mode = SPI_FLASH_READ_MODE_MIN; + cfg->speed = ESP_FLASH_SPEED_MIN; + setup_new_chip(cfg, &chip); + write_large_buffer(chip, part, source_buf, length); + read_and_check(chip, part, source_buf, length); + teardown_test_chip(chip, cfg->host_id); - esp_flash_read_mode_t io_mode = SPI_FLASH_READ_MODE_MIN; - while (io_mode != SPI_FLASH_READ_MODE_MAX) { + + if (config->host_id != -1) { esp_flash_speed_t speed = ESP_FLASH_SPEED_MIN; while (speed != ESP_FLASH_SPEED_MAX) { - ESP_LOGI(TAG, "test flash io mode: %d, speed: %d", io_mode, speed); - setup_new_chip(io_mode, speed); - read_and_check(test_chip, part, source_buf, length); - teardown_test_chip(); + //test io_mode in the inner loop to test QE set/clear function, since + //the io mode will switch frequently. + esp_flash_io_mode_t io_mode = SPI_FLASH_READ_MODE_MIN; + while (io_mode != SPI_FLASH_READ_MODE_MAX) { + ESP_LOGI(TAG, "test flash io mode: %d, speed: %d", io_mode, speed); + cfg->io_mode = io_mode; + cfg->speed = speed; + setup_new_chip(cfg, &chip); + read_and_check(chip, part, source_buf, length); + teardown_test_chip(chip, cfg->host_id); + io_mode++; + } speed++; } - io_mode++; + } else { + //test main flash + write_large_buffer(NULL, part, source_buf, length); + read_and_check(NULL, part, source_buf, length); } -#endif - - //test main flash BTW - write_large_buffer(NULL, part, source_buf, length); - read_and_check(NULL, part, source_buf, length); free(source_buf); } -TEST_CASE("Test esp_flash_write large const buffer", "[esp_flash]") +TEST_CASE("SPI flash test reading with all speed/mode permutations", "[esp_flash]") { - //buffer in flash - test_write_large_buffer(NULL, large_const_buffer, sizeof(large_const_buffer)); -#ifndef SKIP_EXTENDED_CHIP_TEST - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_write_large_buffer(test_chip, large_const_buffer, sizeof(large_const_buffer)); - teardown_test_chip(); -#endif + test_permutations(&config_list[0]); } -#ifndef SKIP_EXTENDED_CHIP_TEST -TEST_CASE("Test esp_flash_write large RAM buffer", "[esp_flash]") +#ifndef CONFIG_ESP32_SPIRAM_SUPPORT +TEST_CASE("SPI flash test reading with all speed/mode permutations, 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH]") +{ + for (int i = 0; i < ALL_TEST_NUM; i++) { + test_permutations(&config_list[i]); + } +} +#endif + +static void test_write_large_const_buffer(esp_flash_t* chip) +{ + test_write_large_buffer(chip, large_const_buffer, sizeof(large_const_buffer)); +} + +FLASH_TEST_CASE("Test esp_flash_write large const buffer", test_write_large_const_buffer); +FLASH_TEST_CASE_3("Test esp_flash_write large const buffer", test_write_large_const_buffer); + +static void test_write_large_ram_buffer(esp_flash_t* chip) { // buffer in RAM uint8_t *source_buf = malloc(sizeof(large_const_buffer)); TEST_ASSERT_NOT_NULL(source_buf); memcpy(source_buf, large_const_buffer, sizeof(large_const_buffer)); - - setup_new_chip(TEST_SPI_READ_MODE, TEST_SPI_SPEED); - test_write_large_buffer(test_chip, source_buf, sizeof(large_const_buffer)); - teardown_test_chip(); - + test_write_large_buffer(chip, source_buf, sizeof(large_const_buffer)); free(source_buf); } -#endif + +FLASH_TEST_CASE("Test esp_flash_write large RAM buffer", test_write_large_ram_buffer); +FLASH_TEST_CASE_3("Test esp_flash_write large RAM buffer", test_write_large_ram_buffer); static void write_large_buffer(esp_flash_t *chip, const esp_partition_t *part, const uint8_t *source, size_t length) { diff --git a/tools/ci/config/target-test.yml b/tools/ci/config/target-test.yml index c2845ac96..f0f3f0822 100644 --- a/tools/ci/config/target-test.yml +++ b/tools/ci/config/target-test.yml @@ -457,6 +457,14 @@ UT_033: - UT_T2_Ethernet - psram +UT_034: + extends: .unit_test_template + parallel: 4 + tags: + - ESP32_IDF + - UT_T1_ESP_FLASH + + nvs_compatible_test: extends: .test_template artifacts: From eb104aa16f80fb9d54b3e5bf3b79550e95d51294 Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Tue, 10 Sep 2019 00:56:46 +0800 Subject: [PATCH 4/4] esp_flash: fix the quad issue for some GD flash chips The GD flash with product ID 40H, is already used in Wrover-nosufix modules. --- components/spi_flash/CMakeLists.txt | 3 +- components/spi_flash/Kconfig | 19 ++- components/spi_flash/esp_flash_api.c | 23 ++-- components/spi_flash/include/esp_flash.h | 1 + .../spi_flash/include/spi_flash_chip_gd.h | 32 ++++++ components/spi_flash/linker.lf | 1 + components/spi_flash/spi_flash_chip_drivers.c | 4 + components/spi_flash/spi_flash_chip_gd.c | 108 ++++++++++++++++++ components/spi_flash/spi_flash_chip_generic.c | 6 +- 9 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 components/spi_flash/include/spi_flash_chip_gd.h create mode 100644 components/spi_flash/spi_flash_chip_gd.c diff --git a/components/spi_flash/CMakeLists.txt b/components/spi_flash/CMakeLists.txt index 497125e47..61900d09b 100644 --- a/components/spi_flash/CMakeLists.txt +++ b/components/spi_flash/CMakeLists.txt @@ -14,7 +14,7 @@ else() "partition.c" "spi_flash_rom_patch.c" ) - # New implementation + # New implementation after IDF v4.0 list(APPEND cache_srcs "esp_flash_api.c" "esp_flash_spi_init.c" @@ -25,6 +25,7 @@ else() "spi_flash_chip_drivers.c" "spi_flash_chip_generic.c" "spi_flash_chip_issi.c" + "spi_flash_chip_gd.c" "memspi_host_driver.c" ) list(APPEND srcs ${cache_srcs}) diff --git a/components/spi_flash/Kconfig b/components/spi_flash/Kconfig index 8a10651ff..046146e00 100644 --- a/components/spi_flash/Kconfig +++ b/components/spi_flash/Kconfig @@ -90,8 +90,23 @@ menu "SPI Flash driver" bool "ISSI" default y help - Enable this to support auto detection of ISSI chips if chip vendor not specified. - This adds support for variant chips, however will extend detecting time. + Enable this to support auto detection of ISSI chips if chip vendor not directly + given by ``chip_drv`` member of the chip struct. This adds support for variant + chips, however will extend detecting time. + + config SPI_FLASH_SUPPORT_GD_CHIP + bool "GigaDevice" + default y + help + Enable this to support auto detection of GD (GigaDevice) chips if chip vendor not + directly given by ``chip_drv`` member of the chip struct. If you are using Wrover + modules, please don't disable this, otherwise your flash may not work in 4-bit + mode. + + This adds support for variant chips, however will extend detecting time and image + size. Note that the default chip driver supports the GD chips with product ID + 60H. + endmenu #auto detect flash chips endmenu diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 7cb0e0d13..ff9c49e4d 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -62,6 +62,7 @@ static const char io_mode_str[][IO_STR_LEN] = { _Static_assert(sizeof(io_mode_str)/IO_STR_LEN == SPI_FLASH_READ_MODE_MAX, "the io_mode_str should be consistent with the esp_flash_io_mode_t defined in spi_flash_ll.h"); +esp_err_t esp_flash_read_chip_id(esp_flash_t* chip, uint32_t* flash_id); /* Static function to notify OS of a new SPI flash operation. @@ -115,6 +116,18 @@ esp_err_t IRAM_ATTR esp_flash_init(esp_flash_t *chip) return ESP_ERR_INVALID_ARG; } + //read chip id + uint32_t flash_id; + int retries = 10; + do { + err = esp_flash_read_chip_id(chip, &flash_id); + } while (err == ESP_ERR_FLASH_NOT_INITIALISED && retries-- > 0); + + if (err != ESP_OK) { + return err; + } + chip->chip_id = flash_id; + if (!esp_flash_chip_driver_initialized(chip)) { // Detect chip_drv err = detect_spi_flash_chip(chip); @@ -175,15 +188,7 @@ esp_err_t IRAM_ATTR esp_flash_read_chip_id(esp_flash_t* chip, uint32_t* flash_id static esp_err_t IRAM_ATTR detect_spi_flash_chip(esp_flash_t *chip) { esp_err_t err; - uint32_t flash_id; - int retries = 10; - do { - err = esp_flash_read_chip_id(chip, &flash_id); - } while (err == ESP_ERR_FLASH_NOT_INITIALISED && retries-- > 0); - - if (err != ESP_OK) { - return err; - } + uint32_t flash_id = chip->chip_id; // Detect the chip and set the chip_drv structure for it const spi_flash_chip_t **drivers = esp_flash_registered_chips; diff --git a/components/spi_flash/include/esp_flash.h b/components/spi_flash/include/esp_flash.h index b2b38806d..afce78c5e 100644 --- a/components/spi_flash/include/esp_flash.h +++ b/components/spi_flash/include/esp_flash.h @@ -65,6 +65,7 @@ struct esp_flash_t { esp_flash_io_mode_t read_mode; ///< Configured SPI flash read mode. Set before ``esp_flash_init`` is called. uint32_t size; ///< Size of SPI flash in bytes. If 0, size will be detected during initialisation. + uint32_t chip_id; ///< Detected chip id. }; diff --git a/components/spi_flash/include/spi_flash_chip_gd.h b/components/spi_flash/include/spi_flash_chip_gd.h new file mode 100644 index 000000000..0d52435a3 --- /dev/null +++ b/components/spi_flash/include/spi_flash_chip_gd.h @@ -0,0 +1,32 @@ +// Copyright 2015-2019 Espressif Systems (Shanghai) PTE LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include +#include "esp_flash.h" +#include "spi_flash_chip_driver.h" + + +/** + * GD (GigaDevice) SPI flash chip_drv, uses all the above functions for its operations. In + * default autodetection, this is used as a catchall if a more specific chip_drv + * is not found. + * + * Note that this is for GD chips with product ID 40H (GD25Q) and 60H (GD25LQ). The chip diver uses + * different commands to write the SR2 register according to the chip ID. For GD25Q40 - GD25Q16 + * chips, and GD25LQ chips, WRSR (01H) command is used; while WRSR2 (31H) is used for GD25Q32 - + * GD25Q127 chips. + */ +extern const spi_flash_chip_t esp_flash_chip_gd; diff --git a/components/spi_flash/linker.lf b/components/spi_flash/linker.lf index e5d886ca4..5e2c1af32 100644 --- a/components/spi_flash/linker.lf +++ b/components/spi_flash/linker.lf @@ -4,5 +4,6 @@ entries: spi_flash_rom_patch (noflash_text) spi_flash_chip_generic (noflash) spi_flash_chip_issi (noflash) + spi_flash_chip_gd(noflash) memspi_host_driver (noflash) diff --git a/components/spi_flash/spi_flash_chip_drivers.c b/components/spi_flash/spi_flash_chip_drivers.c index ae10bcec3..316ac9ae1 100644 --- a/components/spi_flash/spi_flash_chip_drivers.c +++ b/components/spi_flash/spi_flash_chip_drivers.c @@ -16,6 +16,7 @@ #include "spi_flash_chip_driver.h" #include "spi_flash_chip_generic.h" #include "spi_flash_chip_issi.h" +#include "spi_flash_chip_gd.h" #include "sdkconfig.h" /* @@ -30,6 +31,9 @@ static const spi_flash_chip_t *default_registered_chips[] = { #ifdef CONFIG_SPI_FLASH_SUPPORT_ISSI_CHIP &esp_flash_chip_issi, +#endif +#ifdef CONFIG_SPI_FLASH_SUPPORT_GD_CHIP + &esp_flash_chip_gd, #endif &esp_flash_chip_generic, NULL, diff --git a/components/spi_flash/spi_flash_chip_gd.c b/components/spi_flash/spi_flash_chip_gd.c new file mode 100644 index 000000000..bc877d2cb --- /dev/null +++ b/components/spi_flash/spi_flash_chip_gd.c @@ -0,0 +1,108 @@ +// Copyright 2015-2019 Espressif Systems (Shanghai) PTE LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include "spi_flash_chip_generic.h" +#include "spi_flash_defs.h" + +#define FLASH_ID_MASK 0xFF00 +#define FLASH_SIZE_MASK 0xFF +#define GD25Q_PRODUCT_ID 0x4000 +#define GD25LQ_PRODUCT_ID 0x6000 + +#define WRSR_16B_REQUIRED(chip_id) (((chip_id) & FLASH_ID_MASK) == GD25LQ_PRODUCT_ID || \ + ((chip_id) & FLASH_SIZE_MASK) <= 0x15) + +/* Driver for GD flash chip */ + +esp_err_t spi_flash_chip_gd_probe(esp_flash_t *chip, uint32_t flash_id) +{ + /* Check manufacturer and product IDs match our desired masks */ + const uint8_t MFG_ID = 0xC8; + if (flash_id >> 16 != MFG_ID) { + return ESP_ERR_NOT_FOUND; + } + + uint32_t product_id = flash_id & FLASH_ID_MASK; + if (product_id != GD25Q_PRODUCT_ID && product_id != GD25LQ_PRODUCT_ID) { + return ESP_ERR_NOT_FOUND; + } + + return ESP_OK; +} + +esp_err_t spi_flash_chip_gd_set_io_mode(esp_flash_t *chip) +{ + if (WRSR_16B_REQUIRED(chip->chip_id)) { + const uint32_t qe = 1<<9; + return spi_flash_common_set_io_mode(chip, + spi_flash_common_write_status_16b_wrsr, + spi_flash_common_read_status_16b_rdsr_rdsr2, + qe); + } else { + const uint32_t qe = 1<<1; + return spi_flash_common_set_io_mode(chip, + spi_flash_common_write_status_8b_wrsr2, + spi_flash_common_read_status_8b_rdsr2, + qe); + } +} + +esp_err_t spi_flash_chip_gd_get_io_mode(esp_flash_t *chip, esp_flash_io_mode_t* out_io_mode) +{ + /* GD uses bit 1 of SR2 as Quad Enable */ + const uint8_t BIT_QE = 1 << 1; + uint32_t sr; + esp_err_t ret = spi_flash_common_read_status_8b_rdsr2(chip, &sr); + if (ret == ESP_OK) { + *out_io_mode = ((sr & BIT_QE)? SPI_FLASH_QOUT: 0); + } + return ret; +} + + +static const char chip_name[] = "gd"; + +// The issi chip can use the functions for generic chips except from set read mode and probe, +// So we only replace these two functions. +const spi_flash_chip_t esp_flash_chip_gd = { + .name = chip_name, + .probe = spi_flash_chip_gd_probe, + .reset = spi_flash_chip_generic_reset, + .detect_size = spi_flash_chip_generic_detect_size, + .erase_chip = spi_flash_chip_generic_erase_chip, + .erase_sector = spi_flash_chip_generic_erase_sector, + .erase_block = spi_flash_chip_generic_erase_block, + .sector_size = 4 * 1024, + .block_erase_size = 64 * 1024, + + .get_chip_write_protect = spi_flash_chip_generic_get_write_protect, + .set_chip_write_protect = spi_flash_chip_generic_set_write_protect, + + // TODO support protected regions on ISSI flash + .num_protectable_regions = 0, + .protectable_regions = NULL, + .get_protected_regions = NULL, + .set_protected_regions = NULL, + + .read = spi_flash_chip_generic_read, + .write = spi_flash_chip_generic_write, + .program_page = spi_flash_chip_generic_page_program, + .page_size = 256, + .write_encrypted = spi_flash_chip_generic_write_encrypted, + + .wait_idle = spi_flash_chip_generic_wait_idle, + .set_io_mode = spi_flash_chip_gd_set_io_mode, + .get_io_mode = spi_flash_chip_gd_get_io_mode, +}; diff --git a/components/spi_flash/spi_flash_chip_generic.c b/components/spi_flash/spi_flash_chip_generic.c index 16d6b85e1..23172fb93 100644 --- a/components/spi_flash/spi_flash_chip_generic.c +++ b/components/spi_flash/spi_flash_chip_generic.c @@ -60,12 +60,8 @@ esp_err_t spi_flash_chip_generic_reset(esp_flash_t *chip) esp_err_t spi_flash_chip_generic_detect_size(esp_flash_t *chip, uint32_t *size) { - uint32_t id = 0; + uint32_t id = chip->chip_id; *size = 0; - esp_err_t err = chip->host->read_id(chip->host, &id); - if (err != ESP_OK) { - return err; - } /* Can't detect size unless the high byte of the product ID matches the same convention, which is usually 0x40 or * 0xC0 or similar. */