From 1cc860216e0fa43a7ebadb5b8d86eafbf1de7566 Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Fri, 2 Aug 2019 13:04:48 +0800 Subject: [PATCH 1/2] esp_flash: fix the set/get write protection functions Add support for get write protection support, fixed the duplicated set_write_protection link. All the write_protection check in the top layer are removed. The lower levels (chip) should ensure to disable write protection before the operation start. --- components/spi_flash/esp_flash_api.c | 30 +++------------ components/spi_flash/include/esp_flash.h | 2 - .../spi_flash/include/spi_flash_chip_driver.h | 7 +--- .../include/spi_flash_chip_generic.h | 14 ++++++- components/spi_flash/spi_flash_chip_generic.c | 38 +++++++++++-------- components/spi_flash/spi_flash_chip_issi.c | 5 +-- components/spi_flash/test/test_esp_flash.c | 33 +++++++++++++++- 7 files changed, 77 insertions(+), 52 deletions(-) diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 1ce52e47b..8f68be541 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -265,23 +265,13 @@ esp_err_t IRAM_ATTR esp_flash_erase_chip(esp_flash_t *chip) { VERIFY_OP(erase_chip); CHECK_WRITE_ADDRESS(chip, 0, chip->size); - bool write_protect = false; esp_err_t err = spiflash_start(chip); if (err != ESP_OK) { return err; } - err = esp_flash_get_chip_write_protect(chip, &write_protect); - - if (err == ESP_OK && write_protect) { - err = ESP_ERR_FLASH_PROTECTED; - } - - if (err == ESP_OK) { - err = chip->chip_drv->erase_chip(chip); - } - + err = chip->chip_drv->erase_chip(chip); return spiflash_end(chip, err); } @@ -292,7 +282,6 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui CHECK_WRITE_ADDRESS(chip, start, len); uint32_t block_erase_size = chip->chip_drv->erase_block == NULL ? 0 : chip->chip_drv->block_erase_size; uint32_t sector_size = chip->chip_drv->sector_size; - bool write_protect = false; if (sector_size == 0 || (block_erase_size % sector_size) != 0) { return ESP_ERR_FLASH_NOT_INITIALISED; @@ -310,16 +299,9 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui return err; } - // Check for write protection on whole chip - if (chip->chip_drv->get_chip_write_protect != NULL) { - err = chip->chip_drv->get_chip_write_protect(chip, &write_protect); - if (err == ESP_OK && write_protect) { - err = ESP_ERR_FLASH_PROTECTED; - } - } - // Check for write protected regions overlapping the erase region - if (err == ESP_OK && chip->chip_drv->get_protected_regions != NULL && chip->chip_drv->num_protectable_regions > 0) { + if (chip->chip_drv->get_protected_regions != NULL && + chip->chip_drv->num_protectable_regions > 0) { uint64_t protected = 0; err = chip->chip_drv->get_protected_regions(chip, &protected); if (err == ESP_OK && protected != 0) { @@ -360,10 +342,10 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui return err; } -esp_err_t IRAM_ATTR esp_flash_get_chip_write_protect(esp_flash_t *chip, bool *write_protected) +esp_err_t IRAM_ATTR esp_flash_get_chip_write_protect(esp_flash_t *chip, bool *out_write_protected) { VERIFY_OP(get_chip_write_protect); - if (write_protected == NULL) { + if (out_write_protected == NULL) { return ESP_ERR_INVALID_ARG; } @@ -372,7 +354,7 @@ esp_err_t IRAM_ATTR esp_flash_get_chip_write_protect(esp_flash_t *chip, bool *wr return err; } - err = chip->chip_drv->get_chip_write_protect(chip, write_protected); + err = chip->chip_drv->get_chip_write_protect(chip, out_write_protected); return spiflash_end(chip, err); } diff --git a/components/spi_flash/include/esp_flash.h b/components/spi_flash/include/esp_flash.h index 86737f791..95d815b7b 100644 --- a/components/spi_flash/include/esp_flash.h +++ b/components/spi_flash/include/esp_flash.h @@ -160,8 +160,6 @@ esp_err_t esp_flash_get_chip_write_protect(esp_flash_t *chip, bool *write_protec * @note Correct behaviour of this function depends on the SPI flash chip model and chip_drv in use (via the 'chip->drv' * field). * - * If write protection is enabled, destructive operations will fail with ESP_ERR_FLASH_PROTECTED. - * * Some SPI flash chips may require a power cycle before write protect status can be cleared. Otherwise, * write protection can be removed via a follow-up call to this function. * diff --git a/components/spi_flash/include/spi_flash_chip_driver.h b/components/spi_flash/include/spi_flash_chip_driver.h index fd5c1e4c9..9dd8ffa97 100644 --- a/components/spi_flash/include/spi_flash_chip_driver.h +++ b/components/spi_flash/include/spi_flash_chip_driver.h @@ -90,10 +90,10 @@ struct spi_flash_chip_t { uint32_t block_erase_size; /* Optimal (fastest) block size for multi-sector erases on this chip */ /* Read the write protect status of the entire chip. */ - esp_err_t (*get_chip_write_protect)(esp_flash_t *chip, bool *write_protected); + esp_err_t (*get_chip_write_protect)(esp_flash_t *chip, bool *out_write_protected); /* Set the write protect status of the entire chip. */ - esp_err_t (*set_chip_write_protect)(esp_flash_t *chip, bool write_protect_chip); + esp_err_t (*set_chip_write_protect)(esp_flash_t *chip, bool chip_write_protect); /* Number of individually write protectable regions on this chip. Range 0-63. */ uint8_t num_protectable_regions; @@ -135,9 +135,6 @@ struct spi_flash_chip_t { /* Perform an encrypted write to the chip, using internal flash encryption hardware. */ esp_err_t (*write_encrypted)(esp_flash_t *chip, const void *buffer, uint32_t address, uint32_t length); - /* Set the write enable flag. This function is called internally by other functions in this structure, before a destructive - operation takes place. */ - esp_err_t (*set_write_protect)(esp_flash_t *chip, bool write_protect); /* Wait for the SPI flash chip to be idle (any write operation to be complete.) This function is both called from the higher-level API functions, and from other functions in this structure. diff --git a/components/spi_flash/include/spi_flash_chip_generic.h b/components/spi_flash/include/spi_flash_chip_generic.h index 676e17311..885bd72e7 100644 --- a/components/spi_flash/include/spi_flash_chip_generic.h +++ b/components/spi_flash/include/spi_flash_chip_generic.h @@ -173,7 +173,19 @@ spi_flash_chip_generic_write_encrypted(esp_flash_t *chip, const void *buffer, ui * - ESP_OK if success * - or other error passed from the ``wait_idle``, ``read_status`` or ``set_write_protect`` function of host driver */ -esp_err_t spi_flash_chip_generic_write_enable(esp_flash_t *chip, bool write_protect); +esp_err_t spi_flash_chip_generic_set_write_protect(esp_flash_t *chip, bool write_protect); + +/** + * @brief Check whether WEL (write enable latch) bit is set in the Status Register read from RDSR (05h). + * + * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. + * @param out_write_protect Output of whether the write protect is set. + * + * @return + * - ESP_OK if success + * - or other error passed from the ``read_status`` function of host driver + */ +esp_err_t spi_flash_chip_generic_get_write_protect(esp_flash_t *chip, bool *out_write_protect); /** * @brief Read flash status via the RDSR command (05h) and wait for bit 0 (write diff --git a/components/spi_flash/spi_flash_chip_generic.c b/components/spi_flash/spi_flash_chip_generic.c index 0e80e8eb1..8bbb9d137 100644 --- a/components/spi_flash/spi_flash_chip_generic.c +++ b/components/spi_flash/spi_flash_chip_generic.c @@ -82,7 +82,7 @@ esp_err_t spi_flash_chip_generic_erase_chip(esp_flash_t *chip) { esp_err_t err; - err = chip->chip_drv->set_write_protect(chip, false); + err = chip->chip_drv->set_chip_write_protect(chip, false); if (err == ESP_OK) { err = chip->chip_drv->wait_idle(chip, DEFAULT_IDLE_TIMEOUT); } @@ -102,7 +102,7 @@ esp_err_t spi_flash_chip_generic_erase_chip(esp_flash_t *chip) esp_err_t spi_flash_chip_generic_erase_sector(esp_flash_t *chip, uint32_t start_address) { - esp_err_t err = chip->chip_drv->set_write_protect(chip, false); + esp_err_t err = chip->chip_drv->set_chip_write_protect(chip, false); if (err == ESP_OK) { err = chip->chip_drv->wait_idle(chip, DEFAULT_IDLE_TIMEOUT); } @@ -122,7 +122,7 @@ esp_err_t spi_flash_chip_generic_erase_sector(esp_flash_t *chip, uint32_t start_ esp_err_t spi_flash_chip_generic_erase_block(esp_flash_t *chip, uint32_t start_address) { - esp_err_t err = chip->chip_drv->set_write_protect(chip, false); + esp_err_t err = chip->chip_drv->set_chip_write_protect(chip, false); if (err == ESP_OK) { err = chip->chip_drv->wait_idle(chip, DEFAULT_IDLE_TIMEOUT); } @@ -185,7 +185,7 @@ esp_err_t spi_flash_chip_generic_write(esp_flash_t *chip, const void *buffer, ui page_len = page_size - (address % page_size); } - err = chip->chip_drv->set_write_protect(chip, false); + err = chip->chip_drv->set_chip_write_protect(chip, false); if (err == ESP_OK) { err = chip->chip_drv->program_page(chip, buffer, address, page_len); @@ -205,7 +205,7 @@ esp_err_t spi_flash_chip_generic_write_encrypted(esp_flash_t *chip, const void * return ESP_ERR_FLASH_UNSUPPORTED_HOST; // TODO } -esp_err_t spi_flash_chip_generic_write_enable(esp_flash_t *chip, bool write_protect) +esp_err_t spi_flash_chip_generic_set_write_protect(esp_flash_t *chip, bool write_protect) { esp_err_t err = ESP_OK; @@ -215,17 +215,26 @@ esp_err_t spi_flash_chip_generic_write_enable(esp_flash_t *chip, bool write_prot chip->host->set_write_protect(chip->host, write_protect); } + bool wp_read; + err = chip->chip_drv->get_chip_write_protect(chip, &wp_read); + if (err == ESP_OK && wp_read != write_protect) { + // WREN flag has not been set! + err = ESP_ERR_NOT_FOUND; + } + return err; +} + +esp_err_t spi_flash_chip_generic_get_write_protect(esp_flash_t *chip, bool *out_write_protect) +{ + esp_err_t err = ESP_OK; uint8_t status; + assert(out_write_protect!=NULL); err = chip->host->read_status(chip->host, &status); if (err != ESP_OK) { return err; } - if ((status & SR_WREN) == 0) { - // WREN flag has not been set! - err = ESP_ERR_NOT_FOUND; - } - + *out_write_protect = ((status & SR_WREN) == 0); return err; } @@ -329,7 +338,7 @@ esp_err_t spi_flash_common_set_read_mode(esp_flash_t *chip, uint8_t qe_rdsr_comm 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_write_protect(chip, false); + chip->chip_drv->set_chip_write_protect(chip, false); sr |= qe_sr_bit; spi_flash_trans_t t = { @@ -354,7 +363,7 @@ esp_err_t spi_flash_common_set_read_mode(esp_flash_t *chip, uint8_t qe_rdsr_comm return ESP_ERR_FLASH_NO_RESPONSE; } - chip->chip_drv->set_write_protect(chip, true); + chip->chip_drv->set_chip_write_protect(chip, true); } } return ESP_OK; @@ -383,8 +392,8 @@ const spi_flash_chip_t esp_flash_chip_generic = { .block_erase_size = 64 * 1024, // TODO: figure out if generic chip-wide protection bits exist across some manufacturers - .get_chip_write_protect = NULL, - .set_chip_write_protect = NULL, + .get_chip_write_protect = spi_flash_chip_generic_get_write_protect, + .set_chip_write_protect = spi_flash_chip_generic_set_write_protect, // Chip write protection regions do not appear to be standardised // at all, this is implemented in chip-specific drivers only. @@ -399,7 +408,6 @@ const spi_flash_chip_t esp_flash_chip_generic = { .page_size = 256, .write_encrypted = spi_flash_chip_generic_write_encrypted, - .set_write_protect = spi_flash_chip_generic_write_enable, .wait_idle = spi_flash_chip_generic_wait_idle, .set_read_mode = spi_flash_chip_generic_set_read_mode, }; diff --git a/components/spi_flash/spi_flash_chip_issi.c b/components/spi_flash/spi_flash_chip_issi.c index d32c3a192..d0b025637 100644 --- a/components/spi_flash/spi_flash_chip_issi.c +++ b/components/spi_flash/spi_flash_chip_issi.c @@ -58,8 +58,8 @@ const spi_flash_chip_t esp_flash_chip_issi = { .block_erase_size = 64 * 1024, // TODO: support get/set chip write protect for ISSI flash - .get_chip_write_protect = NULL, - .set_chip_write_protect = NULL, + .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, @@ -73,7 +73,6 @@ const spi_flash_chip_t esp_flash_chip_issi = { .page_size = 256, .write_encrypted = spi_flash_chip_generic_write_encrypted, - .set_write_protect = spi_flash_chip_generic_write_enable, .wait_idle = spi_flash_chip_generic_wait_idle, .set_read_mode = spi_flash_chip_issi_set_read_mode, }; diff --git a/components/spi_flash/test/test_esp_flash.c b/components/spi_flash/test/test_esp_flash.c index bf1384545..6d229404d 100644 --- a/components/spi_flash/test/test_esp_flash.c +++ b/components/spi_flash/test/test_esp_flash.c @@ -366,6 +366,36 @@ TEST_CASE("SPI flash erase large region", "[esp_flash]") #endif } +static void test_write_protection(esp_flash_t* chip) +{ + bool wp = true; + esp_err_t ret = ESP_OK; + ret = esp_flash_get_chip_write_protect(chip, &wp); + TEST_ESP_OK(ret); + + for (int i = 0; i < 4; i ++) { + bool wp_write = !wp; + ret = esp_flash_set_chip_write_protect(chip, wp_write); + TEST_ESP_OK(ret); + + bool wp_read; + ret = esp_flash_get_chip_write_protect(chip, &wp_read); + TEST_ESP_OK(ret); + TEST_ASSERT(wp_read == wp_write); + wp = wp_read; + } +} + +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 +} + static const uint8_t large_const_buffer[16400] = { 203, // first byte 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, @@ -491,5 +521,4 @@ static void test_write_large_buffer(esp_flash_t *chip, const uint8_t *source, si write_large_buffer(chip, part, source, length); read_and_check(chip, part, source, length); -} - +} \ No newline at end of file From a626b26cf97c00546e546e4a69cdfece1d1edc08 Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Sat, 3 Aug 2019 09:59:10 +0800 Subject: [PATCH 2/2] esp_flash: improve the comments a bit --- components/soc/include/hal/spi_flash_hal.h | 15 +++++--- .../spi_flash/include/memspi_host_driver.h | 2 + .../include/spi_flash_chip_generic.h | 37 +++++++++++-------- components/spi_flash/spi_flash_chip_issi.c | 1 - 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/components/soc/include/hal/spi_flash_hal.h b/components/soc/include/hal/spi_flash_hal.h index 3a18e4ca5..bf89c4148 100644 --- a/components/soc/include/hal/spi_flash_hal.h +++ b/components/soc/include/hal/spi_flash_hal.h @@ -88,14 +88,15 @@ esp_err_t spi_flash_hal_device_config(spi_flash_host_driver_t *driver); esp_err_t spi_flash_hal_common_command(spi_flash_host_driver_t *driver, spi_flash_trans_t *trans); /** - * Erase whole flash chip. + * Erase whole flash chip by using the erase chip (C7h) command. * * @param driver The driver context. */ void spi_flash_hal_erase_chip(spi_flash_host_driver_t *driver); /** - * Erase a specific sector by its start address. + * Erase a specific sector by its start address through the sector erase (20h) + * command. * * @param driver The driver context. * @param start_address Start address of the sector to erase. @@ -103,7 +104,8 @@ void spi_flash_hal_erase_chip(spi_flash_host_driver_t *driver); void spi_flash_hal_erase_sector(spi_flash_host_driver_t *driver, uint32_t start_address); /** - * Erase a specific block by its start address. + * Erase a specific 64KB block by its start address through the 64KB block + * erase (D8h) command. * * @param driver The driver context. * @param start_address Start address of the block to erase. @@ -111,7 +113,7 @@ void spi_flash_hal_erase_sector(spi_flash_host_driver_t *driver, uint32_t start_ void spi_flash_hal_erase_block(spi_flash_host_driver_t *driver, uint32_t start_address); /** - * Program a page of the flash. + * Program a page of the flash using the page program (02h) command. * * @param driver The driver context. * @param address Address of the page to program @@ -121,7 +123,8 @@ void spi_flash_hal_erase_block(spi_flash_host_driver_t *driver, uint32_t start_a void spi_flash_hal_program_page(spi_flash_host_driver_t *driver, const void *buffer, uint32_t address, uint32_t length); /** - * Read from the flash. The read command should be set by ``spi_flash_hal_configure_host_read_mode`` before. + * Read from the flash. Call ``spi_flash_hal_configure_host_read_mode`` to + * configure the read command before calling this function. * * @param driver The driver context. * @param buffer Buffer to store the read data @@ -133,7 +136,7 @@ void spi_flash_hal_program_page(spi_flash_host_driver_t *driver, const void *buf esp_err_t spi_flash_hal_read(spi_flash_host_driver_t *driver, void *buffer, uint32_t address, uint32_t read_len); /** - * Enable or disable the write protection of the flash chip. + * @brief Send the write enable (06h) or write disable (04h) command to the flash chip. * * @param driver The driver context. * @param wp true to enable the write protection, otherwise false. diff --git a/components/spi_flash/include/memspi_host_driver.h b/components/spi_flash/include/memspi_host_driver.h index 2347398aa..434361d97 100644 --- a/components/spi_flash/include/memspi_host_driver.h +++ b/components/spi_flash/include/memspi_host_driver.h @@ -62,6 +62,8 @@ esp_err_t memspi_host_init_pointers(spi_flash_host_driver_t *host, memspi_host_d ******************************************************************************/ /** + * @brief Read the Status Register read from RDSR (05h). + * * High speed implementation of RDID through memspi interface relying on the * ``common_command``. * diff --git a/components/spi_flash/include/spi_flash_chip_generic.h b/components/spi_flash/include/spi_flash_chip_generic.h index 885bd72e7..672ca6550 100644 --- a/components/spi_flash/include/spi_flash_chip_generic.h +++ b/components/spi_flash/include/spi_flash_chip_generic.h @@ -66,7 +66,7 @@ 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); /** - * @brief Erase chip by using the generic erase chip (C7h) command. + * @brief Erase chip by using the generic erase chip command. * * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. * @@ -77,7 +77,7 @@ esp_err_t spi_flash_chip_generic_detect_size(esp_flash_t *chip, uint32_t *size); esp_err_t spi_flash_chip_generic_erase_chip(esp_flash_t *chip); /** - * @brief Erase sector by using the generic sector erase (20h) command. + * @brief Erase sector by using the generic sector erase command. * * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. * @param start_address Start address of the sector to erase @@ -89,7 +89,7 @@ esp_err_t spi_flash_chip_generic_erase_chip(esp_flash_t *chip); esp_err_t spi_flash_chip_generic_erase_sector(esp_flash_t *chip, uint32_t start_address); /** - * @brief Erase block by using the generic 64KB block erase (D8h) command + * @brief Erase block by the generic 64KB block erase command * * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. * @param start_address Start address of the block to erase @@ -114,7 +114,7 @@ esp_err_t spi_flash_chip_generic_erase_block(esp_flash_t *chip, uint32_t start_a esp_err_t spi_flash_chip_generic_read(esp_flash_t *chip, void *buffer, uint32_t address, uint32_t length); /** - * @brief Perform a page program using the page program (02h) command. + * @brief Perform a page program using the page program command. * * @note Length of each call should not excced the limitation in * ``chip->host->max_write_bytes``. This function is called in @@ -163,7 +163,7 @@ esp_err_t spi_flash_chip_generic_write_encrypted(esp_flash_t *chip, const void *buffer, uint32_t address, uint32_t length); /** - * @brief Send the write enable (06h) command and verify the expected bit (1) in + * @brief Send the write enable or write disable command and verify the expected bit (1) in * the status register is set. * * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. @@ -171,12 +171,13 @@ spi_flash_chip_generic_write_encrypted(esp_flash_t *chip, const void *buffer, ui * * @return * - ESP_OK if success - * - or other error passed from the ``wait_idle``, ``read_status`` or ``set_write_protect`` function of host driver + * - or other error passed from the ``wait_idle``, ``read_status`` or + * ``set_write_protect`` function of host driver */ esp_err_t spi_flash_chip_generic_set_write_protect(esp_flash_t *chip, bool write_protect); /** - * @brief Check whether WEL (write enable latch) bit is set in the Status Register read from RDSR (05h). + * @brief Check whether WEL (write enable latch) bit is set in the Status Register read from RDSR. * * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. * @param out_write_protect Output of whether the write protect is set. @@ -188,8 +189,8 @@ esp_err_t spi_flash_chip_generic_set_write_protect(esp_flash_t *chip, bool write esp_err_t spi_flash_chip_generic_get_write_protect(esp_flash_t *chip, bool *out_write_protect); /** - * @brief Read flash status via the RDSR command (05h) and wait for bit 0 (write - * in progress bit) to be cleared. + * @brief Read flash status via the RDSR command and wait for bit 0 (write in + * progress bit) to be cleared. * * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. * @param timeout_ms Time to wait before timeout, in ms. @@ -244,13 +245,15 @@ 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); /** - * @brief Utility function for set_read_mode chip_drv function + * @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. * - * Most setting of read mode follows a common pattern, except for how to enable Quad I/O modes (QIO/QOUT). - * These use different commands to read/write the status register, and a different bit is set/cleared. + * Most chip QE enable follows a common pattern, though commands to read/write + * the status register may be different, as well as the position of QE bit. * - * This is a generic utility function to implement set_read_mode() for this pattern. Also configures host - * registers via spi_flash_common_configure_host_read_mode(). + * 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(). * * @param qe_rdsr_command SPI flash command to read status register * @param qe_wrsr_command SPI flash command to write status register @@ -262,7 +265,11 @@ esp_err_t spi_flash_generic_wait_host_idle(esp_flash_t *chip, uint32_t *timeout_ 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); /** - * @brief Configure the host to use the specified read mode set in the ``chip->read_mode``. + * @brief Configure the host registers to use the specified read mode set in + * the ``chip->read_mode``. + * + * Usually called in chip_drv read() functions before actual reading + * transactions. Also prepare the command to be sent in read functions. * * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. * diff --git a/components/spi_flash/spi_flash_chip_issi.c b/components/spi_flash/spi_flash_chip_issi.c index d0b025637..71684ec19 100644 --- a/components/spi_flash/spi_flash_chip_issi.c +++ b/components/spi_flash/spi_flash_chip_issi.c @@ -57,7 +57,6 @@ const spi_flash_chip_t esp_flash_chip_issi = { .sector_size = 4 * 1024, .block_erase_size = 64 * 1024, - // TODO: support get/set chip write protect for ISSI flash .get_chip_write_protect = spi_flash_chip_generic_get_write_protect, .set_chip_write_protect = spi_flash_chip_generic_set_write_protect,