Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

esp_flash: fix the set/get write protection functions

Browse source 
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.
This commit is contained in:
Michael (XIAO Xufeng) 2019-08-02 13:04:48 +08:00
parent 16ee476a77
commit 1cc860216e
7 changed files with 77 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
components/spi_flash/esp_flash_api.c
View file

@ -265,23 +265,13 @@ esp_err_t IRAM_ATTR esp_flash_erase_chip(esp_flash_t *chip)
{ {
VERIFY_OP(erase_chip); VERIFY_OP(erase_chip);
CHECK_WRITE_ADDRESS(chip, 0, chip->size); CHECK_WRITE_ADDRESS(chip, 0, chip->size);
bool write_protect = false;
esp_err_t err = spiflash_start(chip); esp_err_t err = spiflash_start(chip);
if (err != ESP_OK) { if (err != ESP_OK) {
return err; 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); 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); 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 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; uint32_t sector_size = chip->chip_drv->sector_size;
bool write_protect = false;
if (sector_size == 0 || (block_erase_size % sector_size) != 0) { if (sector_size == 0 || (block_erase_size % sector_size) != 0) {
return ESP_ERR_FLASH_NOT_INITIALISED; 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; 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 // 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; uint64_t protected = 0;
err = chip->chip_drv->get_protected_regions(chip, &protected); err = chip->chip_drv->get_protected_regions(chip, &protected);
if (err == ESP_OK && protected != 0) { 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; 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); VERIFY_OP(get_chip_write_protect);
if (write_protected == NULL) { if (out_write_protected == NULL) {
return ESP_ERR_INVALID_ARG; 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; 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); return spiflash_end(chip, err);
} }

2
components/spi_flash/include/esp_flash.h
View file

@ -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' * @note Correct behaviour of this function depends on the SPI flash chip model and chip_drv in use (via the 'chip->drv'
* field). * 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, * 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. * write protection can be removed via a follow-up call to this function.
* *

7
components/spi_flash/include/spi_flash_chip_driver.h
View file

@ -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 */ 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. */ /* 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. */ /* 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. */ /* Number of individually write protectable regions on this chip. Range 0-63. */
uint8_t num_protectable_regions; 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. */ /* 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); 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. /* 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.

14
components/spi_flash/include/spi_flash_chip_generic.h
View file

@ -173,7 +173,19 @@ spi_flash_chip_generic_write_encrypted(esp_flash_t *chip, const void *buffer, ui
* - ESP_OK if success * - 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_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 * @brief Read flash status via the RDSR command (05h) and wait for bit 0 (write

38
components/spi_flash/spi_flash_chip_generic.c
View file

@ -82,7 +82,7 @@ esp_err_t spi_flash_chip_generic_erase_chip(esp_flash_t *chip)
{ {
esp_err_t err; 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) { if (err == ESP_OK) {
err = chip->chip_drv->wait_idle(chip, DEFAULT_IDLE_TIMEOUT); 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 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) { if (err == ESP_OK) {
err = chip->chip_drv->wait_idle(chip, DEFAULT_IDLE_TIMEOUT); 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 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) { if (err == ESP_OK) {
err = chip->chip_drv->wait_idle(chip, DEFAULT_IDLE_TIMEOUT); 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); 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) { if (err == ESP_OK) {
err = chip->chip_drv->program_page(chip, buffer, address, page_len); 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 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; 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); 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; uint8_t status;
assert(out_write_protect!=NULL);
err = chip->host->read_status(chip->host, &status); err = chip->host->read_status(chip->host, &status);
if (err != ESP_OK) { if (err != ESP_OK) {
return err; return err;
} }
if ((status & SR_WREN) == 0) { *out_write_protect = ((status & SR_WREN) == 0);
// WREN flag has not been set!
err = ESP_ERR_NOT_FOUND;
}
return err; 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); ESP_EARLY_LOGV(TAG, "set_read_mode: status before 0x%x", sr);
if ((sr & qe_sr_bit) == 0) { if ((sr & qe_sr_bit) == 0) {
//some chips needs the write protect to be disabled before writing to Status Register //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; sr |= qe_sr_bit;
spi_flash_trans_t t = { 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; 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; return ESP_OK;
@ -383,8 +392,8 @@ const spi_flash_chip_t esp_flash_chip_generic = {
.block_erase_size = 64 * 1024, .block_erase_size = 64 * 1024,
// TODO: figure out if generic chip-wide protection bits exist across some manufacturers // TODO: figure out if generic chip-wide protection bits exist across some manufacturers
.get_chip_write_protect = NULL, .get_chip_write_protect = spi_flash_chip_generic_get_write_protect,
.set_chip_write_protect = NULL, .set_chip_write_protect = spi_flash_chip_generic_set_write_protect,
// Chip write protection regions do not appear to be standardised // Chip write protection regions do not appear to be standardised
// at all, this is implemented in chip-specific drivers only. // 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, .page_size = 256,
.write_encrypted = spi_flash_chip_generic_write_encrypted, .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, .wait_idle = spi_flash_chip_generic_wait_idle,
.set_read_mode = spi_flash_chip_generic_set_read_mode, .set_read_mode = spi_flash_chip_generic_set_read_mode,
}; };

5
components/spi_flash/spi_flash_chip_issi.c
View file

@ -58,8 +58,8 @@ const spi_flash_chip_t esp_flash_chip_issi = {
.block_erase_size = 64 * 1024, .block_erase_size = 64 * 1024,
// TODO: support get/set chip write protect for ISSI flash // TODO: support get/set chip write protect for ISSI flash
.get_chip_write_protect = NULL, .get_chip_write_protect = spi_flash_chip_generic_get_write_protect,
.set_chip_write_protect = NULL, .set_chip_write_protect = spi_flash_chip_generic_set_write_protect,
// TODO support protected regions on ISSI flash // TODO support protected regions on ISSI flash
.num_protectable_regions = 0, .num_protectable_regions = 0,
@ -73,7 +73,6 @@ const spi_flash_chip_t esp_flash_chip_issi = {
.page_size = 256, .page_size = 256,
.write_encrypted = spi_flash_chip_generic_write_encrypted, .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, .wait_idle = spi_flash_chip_generic_wait_idle,
.set_read_mode = spi_flash_chip_issi_set_read_mode, .set_read_mode = spi_flash_chip_issi_set_read_mode,
}; };

31
components/spi_flash/test/test_esp_flash.c
View file

@ -366,6 +366,36 @@ TEST_CASE("SPI flash erase large region", "[esp_flash]")
#endif #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] = { static const uint8_t large_const_buffer[16400] = {
203, // first byte 203, // first byte
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
@ -492,4 +522,3 @@ static void test_write_large_buffer(esp_flash_t *chip, const uint8_t *source, si
write_large_buffer(chip, part, source, length); write_large_buffer(chip, part, source, length);
read_and_check(chip, part, source, length); read_and_check(chip, part, source, length);
} }