[PATCH v1 0/7] fix and improve Micron/SST26* SPI NOR protection handling

these patches improve the existing SPI NOR flash protection features by enabling hardware write protection where applicable, by fixing bugs, and by providing more information on the status of protection. this shall make using the protection feature more user-friendly and help meeting user expectations.
Bernhard Kirchen (7): command sf: help text format sf protect: warn about failed (un)lock operation SST26* locking: need to enable write write WPEN bit for SST26* devices when locking sf protect (un)lock for SST26*: test BPR values fix sst26_process_bpr check provide "sf protect check" command
cmd/sf.c | 74 ++++++++----- drivers/mtd/spi/spi-nor-core.c | 186 ++++++++++++++++++++++++++++----- include/linux/mtd/spi-nor.h | 18 +++- include/spi_flash.h | 8 ++ 4 files changed, 232 insertions(+), 54 deletions(-)

properly indent the help text and use single quotes consistently to mark variable parameters.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com ---
cmd/sf.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index c0d6a8f8a0..a991ae0d03 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -564,7 +564,7 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc,
/* The remaining commands require a selected device */ if (!flash) { - puts("No SPI flash selected. Please run `sf probe'\n"); + puts("No SPI flash selected. Please run 'sf probe'\n"); return 1; }
@@ -590,31 +590,27 @@ usage: return CMD_RET_USAGE; }
-#ifdef CONFIG_CMD_SF_TEST -#define SF_TEST_HELP "\nsf test offset len " \ - "- run a very basic destructive test" -#else -#define SF_TEST_HELP -#endif - U_BOOT_CMD( - sf, 5, 1, do_spi_flash, + sf, 5, 1, do_spi_flash, "SPI flash sub-system", - "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n" - " and chip select\n" - "sf read addr offset|partition len - read `len' bytes starting at\n" - " `offset' or from start of mtd\n" - " `partition'to memory at `addr'\n" - "sf write addr offset|partition len - write `len' bytes from memory\n" - " at `addr' to flash at `offset'\n" - " or to start of mtd `partition'\n" - "sf erase offset|partition [+]len - erase `len' bytes from `offset'\n" - " or from start of mtd `partition'\n" - " `+len' round up `len' to block size\n" - "sf update addr offset|partition len - erase and write `len' bytes from memory\n" - " at `addr' to flash at `offset'\n" - " or to start of mtd `partition'\n" - "sf protect lock/unlock sector len - protect/unprotect 'len' bytes starting\n" - " at address 'sector'\n" - SF_TEST_HELP + "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n" + " and chip select\n" + "sf read addr offset|partition len - read 'len' bytes starting at\n" + " 'offset' or from start of mtd\n" + " 'partition' to memory at 'addr'\n" + "sf write addr offset|partition len - write 'len' bytes from memory\n" + " at 'addr' to flash at 'offset'\n" + " or to start of mtd 'partition'\n" + "sf erase offset|partition [+]len - erase 'len' bytes from 'offset'\n" + " or from start of mtd 'partition'.\n" + " '+len' round up 'len' to block size\n" + "sf update addr offset|partition len - erase and write 'len' bytes from memory\n" + " at 'addr' to flash at 'offset'\n" + " or to start of mtd 'partition'\n" + "sf protect lock|unlock sector len - protect/unprotect 'len' bytes starting\n" + " at address 'sector'\n" +#ifdef CONFIG_CMD_SF_TEST + "sf test offset len - run a very basic destructive test\n" + " ranging 'len' bytes from 'offset'\n" +#endif );

On Thu, 28 Jan 2021 at 09:19, Bernhard Kirchen schlimmchen@gmail.com wrote:
properly indent the help text and use single quotes consistently to mark variable parameters.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com
cmd/sf.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

it is not guaranteed that there is a human readable message when the lock or unlock operation failed. make sure there is a message emitted by the "sf protect" implementation if the subcommand failed.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com ---
cmd/sf.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/cmd/sf.c b/cmd/sf.c index a991ae0d03..ecd2918cbc 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -378,7 +378,12 @@ static int do_spi_protect(int argc, char *const argv[])
ret = spi_flash_protect(flash, start, len, prot);
- return ret == 0 ? 0 : 1; + if (ret != 0) { + printf("ERROR: %slocking operation failed (%d)\n", (prot ? "" : "un"), ret); + return 1; + } + + return 0; }
#ifdef CONFIG_CMD_SF_TEST

On Thu, 28 Jan 2021 at 09:19, Bernhard Kirchen schlimmchen@gmail.com wrote:
it is not guaranteed that there is a human readable message when the lock or unlock operation failed. make sure there is a message emitted by the "sf protect" implementation if the subcommand failed.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com
cmd/sf.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Thu, Jan 28, 2021 at 9:49 PM Bernhard Kirchen schlimmchen@gmail.com wrote:
it is not guaranteed that there is a human readable message when the lock or unlock operation failed. make sure there is a message emitted by the "sf protect" implementation if the subcommand failed.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com
The E-mail address of the patch author and Signed-off-by seems to be different. please fix it in v2. Seems to be for all patches in the series.

prior to using the WBPR (write block protection register) command to write new block protection register values, the WREN command must be sent. otherwise the new values are not applied.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com ---
drivers/mtd/spi/spi-nor-core.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index e16b0e1462..050aeac3fa 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1091,12 +1091,20 @@ static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, enum lo if (ctl == SST26_CTL_CHECK) return 0;
+ ret = write_enable(nor); + if (ret < 0) + return ret; + ret = nor->write_reg(nor, SPINOR_OP_WRITE_BPR, bpr_buff, bpr_size); if (ret < 0) { dev_err(nor->dev, "fail to write block-protection register\n"); return ret; }
+ // ignore return value. even if write disable failed, the actual task + // (write block protecton register) was completed successfully. + write_disable(nor); + return 0; }

"sf protect lock" did only protect against accidental writes by software. it did not lock down the config or block-protection registers if the WP# pin was deasserted. hardware write protection was never enabled for these devices.
this change implements setting the WPEN bit and clearing the IOC bit in SST26* devices' config register if any block is protected by "sf protect lock" command, thus enabling hardware write protection. this behavior is similar to the "sf protect lock" implementation for Micron (and compatible) chips.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com ---
drivers/mtd/spi/spi-nor-core.c | 64 ++++++++++++++++++++++++++++++++-- include/linux/mtd/spi-nor.h | 1 + 2 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 050aeac3fa..10c01cf20e 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -185,7 +185,9 @@ static int read_fsr(struct spi_nor *nor) * location. Return the configuration register value. * Returns negative if error occurred. */ -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) +#if defined(CONFIG_SPI_FLASH_SPANSION) || \ + defined(CONFIG_SPI_FLASH_WINBOND) || \ + defined(CONFIG_SPI_FLASH_SST) static int read_cr(struct spi_nor *nor) { int ret; @@ -972,6 +974,8 @@ read_err: #define SST26_MAX_BPR_REG_LEN (18 + 1) #define SST26_BOUND_REG_SIZE ((32 + SST26_BPR_8K_NUM * 8) * SZ_1K)
+static int write_cr(struct spi_nor *nor, u8 value); + enum lock_ctl { SST26_CTL_LOCK, SST26_CTL_UNLOCK, @@ -994,6 +998,35 @@ static bool sst26_process_bpr(u32 bpr_size, u8 *cmd, u32 bit, enum lock_ctl ctl) return false; }
+static int sst26_hardware_write_protection(struct spi_nor *nor, bool lock) +{ + int ret; + u8 cr_val; + + cr_val = read_cr(nor); + if (cr_val < 0) { + dev_err(nor->dev, "fail to read config register value\n"); + return cr_val; + } + + cr_val &= ~CR_WPEN; + + if (lock) { + // disallow further writes if WP# pin is not asserted. + cr_val |= CR_WPEN; + // force quad SPI disabled as otherwise WP# pin works as data line. + cr_val &= ~CR_QUAD_EN_SPAN; + } + + ret = write_cr(nor, cr_val); + if (ret < 0) { + dev_err(nor->dev, "fail to configure WP# pin (hardware write protection)\n"); + return ret; + } + + return 0; +} + /* * Lock, unlock or check lock status of the flash region of the flash (depending * on the lock_ctl value) @@ -1101,6 +1134,14 @@ static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, enum lo return ret; }
+ // enable hardware write protection iff any protection bit is set + for (i = 0; i < bpr_size; ++i) + if (bpr_buff[i] != 0) + break; + ret = sst26_hardware_write_protection(nor, i < bpr_size); + if (ret < 0) + return ret; + // ignore return value. even if write disable failed, the actual task // (write block protecton register) was completed successfully. write_disable(nor); @@ -1339,7 +1380,9 @@ static int macronix_quad_enable(struct spi_nor *nor) } #endif
-#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) +#if defined(CONFIG_SPI_FLASH_SPANSION) || \ + defined(CONFIG_SPI_FLASH_WINBOND) || \ + defined(CONFIG_SPI_FLASH_SST) /* * Write status Register and configuration register with 2 bytes * The first byte will be written to the status register, while the @@ -1369,6 +1412,23 @@ static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr) return 0; }
+static int write_cr(struct spi_nor *nor, u8 value) +{ + int ret; + u8 sr_cr[2]; + + ret = read_sr(nor); + if (ret < 0) + return ret; + + sr_cr[0] = ret; + sr_cr[1] = value; + + return write_sr_cr(nor, sr_cr); +} +#endif + +#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) /** * spansion_read_cr_quad_enable() - set QE bit in Configuration Register. * @nor: pointer to a 'struct spi_nor' diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 4a8e19ee4f..ee5a59cf0a 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -151,6 +151,7 @@
/* Configuration Register bits. */ #define CR_QUAD_EN_SPAN BIT(1) /* Spansion Quad I/O */ +#define CR_WPEN BIT(7) /* SST26* Write-Protection Pin (WP#) Enable */
/* Status Register 2 bits. */ #define SR2_QUAD_EN_BIT7 BIT(7)

after writing new block protection bits, read back the block protection bit register and test whether the desired settings were accepted. fail with error message otherwise.
this adjusts the behavior of "sf protect (un)lock" for SST26* chips to be similar to the (un)locking behavior of Micron-compatible chips, where the statur register is read back and checked after writing it with the new block protection bit setting.
comparing the desired and current values fails if hardware write protection is enabled (SRWD (Micron) or WPEN (SST26*) bit set), and the WP# pin is not asserted, as the respective registers are write-protected in that case.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com ---
drivers/mtd/spi/spi-nor-core.c | 55 +++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 10c01cf20e..b3873aaf6e 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -998,6 +998,47 @@ static bool sst26_process_bpr(u32 bpr_size, u8 *cmd, u32 bit, enum lock_ctl ctl) return false; }
+static int sst26_read_bpr(struct spi_nor *nor, u8 *buff, u32 *len) +{ + struct mtd_info *mtd = &nor->mtd; + u32 bpr_size; + int ret; + + bpr_size = 2 + (mtd->size / SZ_64K / 8); + + if (*len < bpr_size) { + dev_err(nor->dev, "block-protection register buffer too small\n"); + return -EINVAL; + } + + ret = nor->read_reg(nor, SPINOR_OP_READ_BPR, buff, bpr_size); + if (ret < 0) { + dev_err(nor->dev, "failed to read block-protection register\n"); + return ret; + } + + *len = bpr_size; + return 0; +} + +static int sst26_check_bpr(struct spi_nor *nor, u8 *expected_buf, u32 expected_len) +{ + u8 actual_buf[SST26_MAX_BPR_REG_LEN] = {}; + u32 actual_len = SST26_MAX_BPR_REG_LEN; + int ret; + + ret = sst26_read_bpr(nor, actual_buf, &actual_len); + if (ret < 0) + return ret; + + if (expected_len != actual_len || memcmp(expected_buf, actual_buf, actual_len)) { + dev_err(nor->dev, "device did not accept new block protection bits\n"); + return -EACCES; + } + + return 0; +} + static int sst26_hardware_write_protection(struct spi_nor *nor, bool lock) { int ret; @@ -1034,7 +1075,7 @@ static int sst26_hardware_write_protection(struct spi_nor *nor, bool lock) static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, enum lock_ctl ctl) { struct mtd_info *mtd = &nor->mtd; - u32 i, bpr_ptr, rptr_64k, lptr_64k, bpr_size; + u32 i, bpr_ptr, rptr_64k, lptr_64k, bpr_size = SST26_MAX_BPR_REG_LEN; bool lower_64k = false, upper_64k = false; u8 bpr_buff[SST26_MAX_BPR_REG_LEN] = {}; int ret; @@ -1057,13 +1098,9 @@ static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, enum lo mtd->size != SZ_8M) return -EINVAL;
- bpr_size = 2 + (mtd->size / SZ_64K / 8); - - ret = nor->read_reg(nor, SPINOR_OP_READ_BPR, bpr_buff, bpr_size); - if (ret < 0) { - dev_err(nor->dev, "fail to read block-protection register\n"); + ret = sst26_read_bpr(nor, bpr_buff, &bpr_size); + if (ret < 0) return ret; - }
rptr_64k = min_t(u32, ofs + len, mtd->size - SST26_BOUND_REG_SIZE); lptr_64k = max_t(u32, ofs, SST26_BOUND_REG_SIZE); @@ -1134,6 +1171,10 @@ static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, enum lo return ret; }
+ ret = sst26_check_bpr(nor, bpr_buff, bpr_size); + if (ret < 0) + return ret; + // enable hardware write protection iff any protection bit is set for (i = 0; i < bpr_size; ++i) if (bpr_buff[i] != 0)

the sst26_process_bpr function is supposed to check whether a particular protection bit is set (write-lock active). the caller always returns with EACCES when sst26_process_bpr returns true. the previous implementation (double negation) causes the caller to abort checking protection bits when the first bit expected to be set is set.
in order to check whether a particular region is (completely) write-protected, all bits enabling write-protection for all relevant blocks (blocks within the region) must be set. checking for bits can be aborted if a required bit is NOT set, i.e., if sst26_process_bpr returns true. the test whether the region is locked then fails, indicated by (positive) return value EACCES. checking for bits must continue while the required bits are found to be set. only if all expected bits were found to be set, return value 0 indicates that no error occurred and all relevant bits are set.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com ---
drivers/mtd/spi/spi-nor-core.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index b3873aaf6e..0b48e068be 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -982,6 +982,9 @@ enum lock_ctl { SST26_CTL_CHECK };
+/* + * returns false for "no error" and true for "expected protection bit is NOT set" + */ static bool sst26_process_bpr(u32 bpr_size, u8 *cmd, u32 bit, enum lock_ctl ctl) { switch (ctl) { @@ -992,7 +995,7 @@ static bool sst26_process_bpr(u32 bpr_size, u8 *cmd, u32 bit, enum lock_ctl ctl) cmd[bpr_size - (bit / 8) - 1] &= ~BIT(bit % 8); break; case SST26_CTL_CHECK: - return !!(cmd[bpr_size - (bit / 8) - 1] & BIT(bit % 8)); + return !(cmd[bpr_size - (bit / 8) - 1] & BIT(bit % 8)); }
return false; @@ -1071,6 +1074,11 @@ static int sst26_hardware_write_protection(struct spi_nor *nor, bool lock) /* * Lock, unlock or check lock status of the flash region of the flash (depending * on the lock_ctl value) + * + * if lock_ctl == SST26_CTL_CHECK, returns EACCES (positive value) if region is + * NOT locked, 0 if region is locked, and negative on errors. + * + * otherwise returns 0 on success, negative value on error. */ static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, enum lock_ctl ctl) { @@ -1200,10 +1208,6 @@ static int sst26_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) return sst26_lock_ctl(nor, ofs, len, SST26_CTL_LOCK); }
-/* - * Returns EACCES (positive value) if region is locked, 0 if region is unlocked, - * and negative on errors. - */ static int sst26_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len) { /*

this change exposes checking the protection mechanism of SPI NOR flash chips to the CLI. it expands what was already there to communicate not only the protection state of a particular region, but also whether or not the hardware write protection mechanism of the chip is enabled.
the new command works for Micron (and compatible) SPI NOR flash chips as well as the SST26* series of SPI NOR flash chips.
the changes were tested on proprietary boards using a M25P16 and a SST26VF016B.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com ---
cmd/sf.c | 23 +++++++++++++++-- drivers/mtd/spi/spi-nor-core.c | 45 +++++++++++++++++++++++++--------- include/linux/mtd/spi-nor.h | 17 ++++++++++++- include/spi_flash.h | 8 ++++++ 4 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index ecd2918cbc..664442813d 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -355,6 +355,7 @@ static int do_spi_protect(int argc, char *const argv[]) int ret = 0; loff_t start, len; bool prot = false; + struct lock_info info;
if (argc != 4) return -1; @@ -369,6 +370,24 @@ static int do_spi_protect(int argc, char *const argv[]) return 1; }
+ if (strcmp(argv[1], "check") == 0) { + info.ofs = start; + info.len = len; + ret = spi_flash_is_protected(flash, &info); + if (ret != 0) { + printf("ERROR: operation failed (%d)\n", ret); + return 1; + } + + printf("region [0x%06x, 0x%06x] is %sfully write-protected\n", + info.ofs, (info.ofs + info.len - 1), + (info.is_locked ? "" : "NOT ")); + printf("hardware write protection is %sactive\n", + (info.hw_protection ? "" : "NOT ")); + + return 0; + } + if (strcmp(argv[1], "lock") == 0) prot = true; else if (strcmp(argv[1], "unlock") == 0) @@ -612,8 +631,8 @@ U_BOOT_CMD( "sf update addr offset|partition len - erase and write 'len' bytes from memory\n" " at 'addr' to flash at 'offset'\n" " or to start of mtd 'partition'\n" - "sf protect lock|unlock sector len - protect/unprotect 'len' bytes starting\n" - " at address 'sector'\n" + "sf protect lock|unlock|check offs len - (un)protect or check protection of 'len'\n" + " bytes starting at address 'offs'\n" #ifdef CONFIG_CMD_SF_TEST "sf test offset len - run a very basic destructive test\n" " ranging 'len' bytes from 'offset'\n" diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 0b48e068be..f77f22684d 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -869,18 +869,25 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) * Check if a region of the flash is (completely) locked. See stm_lock() for * more info. * - * Returns 1 if entire region is locked, 0 if any portion is unlocked, and - * negative on errors. + * Updates struct lock_info in-place to reflect protection state. */ -static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len) +static int stm_is_locked(struct spi_nor *nor, struct lock_info *info) { - int status; + int status, ret;
status = read_sr(nor); if (status < 0) return status;
- return stm_is_locked_sr(nor, ofs, len, status); + info->hw_protection = ((status & SR_SRWD) > 0); + + ret = stm_is_locked_sr(nor, info->ofs, info->len, status); + if (ret < 0) + return ret; + + info->is_locked = (ret > 0); + + return 0; } #endif /* CONFIG_SPI_FLASH_STMICRO */
@@ -1208,18 +1215,32 @@ static int sst26_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) return sst26_lock_ctl(nor, ofs, len, SST26_CTL_LOCK); }
-static int sst26_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len) +static int sst26_is_locked(struct spi_nor *nor, struct lock_info *info) { + int ret; + /* * is_locked function is used for check before reading or erasing flash - * region, so offset and length might be not 64k allighned, so adjust - * them to be 64k allighned as sst26_lock_ctl works only with 64k - * allighned regions. + * region, so offset and length might be not 64k aligned, so adjust + * them to be 64k aligned as sst26_lock_ctl works only with 64k + * aligned regions. */ - ofs -= ofs & (SZ_64K - 1); - len = len & (SZ_64K - 1) ? (len & ~(SZ_64K - 1)) + SZ_64K : len; + info->ofs -= info->ofs & (SZ_64K - 1); + info->len = info->len & (SZ_64K - 1) ? (info->len & ~(SZ_64K - 1)) + SZ_64K : info->len;
- return sst26_lock_ctl(nor, ofs, len, SST26_CTL_CHECK); + ret = sst26_lock_ctl(nor, info->ofs, info->len, SST26_CTL_CHECK); + if (ret < 0) + return ret; + + info->is_locked = (ret == 0); + + ret = read_cr(nor); + if (ret < 0) + return ret; + + info->hw_protection = ((ret & (CR_WPEN | CR_QUAD_EN_SPAN)) == CR_WPEN); + + return 0; }
static int sst_write_byteprogram(struct spi_nor *nor, loff_t to, size_t len, diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index ee5a59cf0a..bfd503b1b2 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -256,6 +256,21 @@ enum spi_nor_option_flags { */ struct flash_info;
+/** + * struct lock_info - structure describing flash protection + * @ofs: start address of region + * @len: length of region + * @is_locked: true if the region is write-protected (locked) + * @hw_protection: true if the write-protection is backed by hardware, e.g., + * the lock can only be removed by setting WP# pin high + */ +struct lock_info { + u32 ofs; + u32 len; + bool is_locked; + bool hw_protection; +}; + /* * TODO: Remove, once all users of spi_flash interface are moved to MTD * @@ -344,7 +359,7 @@ struct spi_nor {
int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len); int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len); - int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len); + int (*flash_is_locked)(struct spi_nor *nor, struct lock_info *lock); int (*quad_enable)(struct spi_nor *nor);
void *priv; diff --git a/include/spi_flash.h b/include/spi_flash.h index 85cae32cc7..09a4fdad1c 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -179,4 +179,12 @@ static inline int spi_flash_protect(struct spi_flash *flash, u32 ofs, u32 len, return flash->flash_unlock(flash, ofs, len); }
+static inline int spi_flash_is_protected(struct spi_flash *flash, struct lock_info *info) +{ + if (!flash->flash_is_locked) + return -EOPNOTSUPP; + + return flash->flash_is_locked(flash, info); +} + #endif /* _SPI_FLASH_H_ */

Hi,
On Thu, 28 Jan 2021 at 09:19, Bernhard Kirchen schlimmchen@gmail.com wrote:
this change exposes checking the protection mechanism of SPI NOR flash chips to the CLI. it expands what was already there to communicate not only the protection state of a particular region, but also whether or not the hardware write protection mechanism of the chip is enabled.
the new command works for Micron (and compatible) SPI NOR flash chips as well as the SST26* series of SPI NOR flash chips.
the changes were tested on proprietary boards using a M25P16 and a SST26VF016B.
Signed-off-by: Bernhard Kirchen bernhard.kirchen@mbconnectline.com
cmd/sf.c | 23 +++++++++++++++-- drivers/mtd/spi/spi-nor-core.c | 45 +++++++++++++++++++++++++--------- include/linux/mtd/spi-nor.h | 17 ++++++++++++- include/spi_flash.h | 8 ++++++ 4 files changed, 78 insertions(+), 15 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Can we get a sandbox test for this in test/dm/sf.c ?
Regards, Simon
participants (3)
-
Bernhard Kirchen
-
Jagan Teki
-
Simon Glass