spi25_statusreg: Allow to write (non-)volatile bits specifically
There's a subtle difference between prepending a write-status-register
command with a write enable (WREN) or an enable write status register
(EWSR): The former targets non-volatile bits, while the latter targets
volatile bits, i.e. register bits that do not survive a reset.
Sometimes bits are implemented as both volatile and non-volatile. Then,
the non-volatile state is loaded into the volatile registers after chip
reset, and writes with a WREN target both. So far, we simply used WREN
when possible. This can, however, lead to unnecessary wear of the non-
volatile bits. Flash datasheets do not mention any maximum write cycles
for them. However, it is unclear if this is an academic issue, i.e. the
manufacturers account for the wear and implement redundancy, or if they
simply don't expect that many configuration changes.
For a start, allow to specify explicitly which kind of register bits we
want to write. We keep the current behavior. However, the logic to dis-
able block protections automatically should be revised to prefer vola-
tile writes.
Change-Id: I807a2c48f4eaa85d5a10b37362e71818359a4c93
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.sourcearcade.org/c/flashprog/+/190
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
diff --git a/include/chipdrivers.h b/include/chipdrivers.h
index 94db44b..b38ac55 100644
--- a/include/chipdrivers.h
+++ b/include/chipdrivers.h
@@ -66,8 +66,13 @@
/* spi25_statusreg.c */
+enum wrsr_target {
+ WRSR_VOLATILE_BITS = 1,
+ WRSR_NON_VOLATILE_BITS = 2,
+ WRSR_EITHER = 3,
+};
int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t *value);
-int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value);
+int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value, enum wrsr_target);
void spi_prettyprint_status_register_bit(uint8_t status, int bit);
int spi_prettyprint_status_register_plain(struct flashctx *flash);
int spi_prettyprint_status_register_default_welwip(struct flashctx *flash);
diff --git a/spi25_statusreg.c b/spi25_statusreg.c
index 24bef6c..8d5e525 100644
--- a/spi25_statusreg.c
+++ b/spi25_statusreg.c
@@ -56,7 +56,8 @@
return 0;
}
-int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value)
+int spi_write_register(const struct flashctx *flash, enum flash_reg reg,
+ uint8_t value, enum wrsr_target target)
{
int feature_bits = flash->chip->feature_bits;
@@ -127,14 +128,18 @@
}
uint8_t enable_cmd;
- if (feature_bits & FEATURE_WRSR_WREN) {
+ if (((feature_bits & FEATURE_WRSR_EITHER) == 0) && (target & WRSR_VOLATILE_BITS)) {
+ /* TODO: check database and remove this case! */
+ msg_cwarn("Missing status register write definition, assuming EWSR is needed\n");
+ enable_cmd = JEDEC_EWSR;
+ } else if ((feature_bits & FEATURE_WRSR_WREN) && (target & WRSR_NON_VOLATILE_BITS)) {
enable_cmd = JEDEC_WREN;
- } else if (feature_bits & FEATURE_WRSR_EWSR) {
+ } else if ((feature_bits & FEATURE_WRSR_EWSR) && (target & WRSR_VOLATILE_BITS)) {
enable_cmd = JEDEC_EWSR;
} else {
- msg_cdbg("Missing status register write definition, assuming "
- "EWSR is needed\n");
- enable_cmd = JEDEC_EWSR;
+ msg_cerr("Chip doesn't support %svolatile status register writes.\n",
+ target & WRSR_NON_VOLATILE_BITS ? "non-" : "");
+ return 1;
}
struct spi_command cmds[] = {
@@ -240,7 +245,7 @@
static int spi_restore_status(struct flashctx *flash, uint8_t status)
{
msg_cdbg("restoring chip status (0x%02x)\n", status);
- return spi_write_register(flash, STATUS1, status);
+ return spi_write_register(flash, STATUS1, status, WRSR_EITHER);
}
/* A generic block protection disable.
@@ -288,7 +293,7 @@
return 1;
}
/* All bits except the register lock bit (often called SPRL, SRWD, WPEN) are readonly. */
- result = spi_write_register(flash, STATUS1, status & ~lock_mask);
+ result = spi_write_register(flash, STATUS1, status & ~lock_mask, WRSR_EITHER);
if (result) {
msg_cerr("Could not write status register 1.\n");
return result;
@@ -305,7 +310,8 @@
msg_cdbg("done.\n");
}
/* Global unprotect. Make sure to mask the register lock bit as well. */
- result = spi_write_register(flash, STATUS1, status & ~(bp_mask | lock_mask) & unprotect_mask);
+ result = spi_write_register(flash, STATUS1,
+ status & ~(bp_mask | lock_mask) & unprotect_mask, WRSR_EITHER);
if (result) {
msg_cerr("Could not write status register 1.\n");
return result;
diff --git a/writeprotect.c b/writeprotect.c
index 5424518..302451b 100644
--- a/writeprotect.c
+++ b/writeprotect.c
@@ -169,7 +169,7 @@
value = (value & ~write_masks[reg]) | expected;
- if (spi_write_register(flash, reg, value))
+ if (spi_write_register(flash, reg, value, WRSR_EITHER))
return FLASHPROG_WP_ERR_WRITE_FAILED;
}