[PATCH v6 0/2] sf: Check protection before writing/erasing flash
Changes in v6: - fixed embarrassingly inverted logic in unlock check (and properly tested it this time)
Changes in v5: - adjust unused is_locked callback to our is_unlocked needs - use this callback in sf command instead
Jan
Jan Kiszka (2): mtd: spi: Convert is_locked callback to is_unlocked sf: Query write-protection status before operating the flash
cmd/sf.c | 12 ++++++++++++ drivers/mtd/spi/spi-nor-core.c | 26 +++++++++++++------------- include/linux/mtd/spi-nor.h | 6 +++--- 3 files changed, 28 insertions(+), 16 deletions(-)
From: Jan Kiszka jan.kiszka@siemens.com
There was no user of this callback after 5b66fdb29dc3 anymore, and its semantic as now inconsistent between stm and sst26. What we need for the upcoming new usecase is a "completely unlocked" semantic. So consolidate over this.
Signed-off-by: Jan Kiszka jan.kiszka@siemens.com --- drivers/mtd/spi/spi-nor-core.c | 26 +++++++++++++------------- include/linux/mtd/spi-nor.h | 6 +++--- 2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index a70fbda4bbc..74aa7fc4b58 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -1308,13 +1308,13 @@ 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 + * Check if a region of the flash is (completely) unlocked. See stm_lock() for * more info. * - * Returns 1 if entire region is locked, 0 if any portion is unlocked, and + * Returns 1 if entire region is unlocked, 0 if any portion is locked, and * negative on errors. */ -static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len) +static int stm_is_unlocked(struct spi_nor *nor, loff_t ofs, uint64_t len) { int status;
@@ -1322,7 +1322,7 @@ static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len) if (status < 0) return status;
- return stm_is_locked_sr(nor, ofs, len, status); + return stm_is_unlocked_sr(nor, ofs, len, status); } #endif /* CONFIG_SPI_FLASH_STMICRO */
@@ -1555,16 +1555,16 @@ static int sst26_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) }
/* - * Returns EACCES (positive value) if region is locked, 0 if region is unlocked, - * and negative on errors. + * Returns EACCES (positive value) if region is (partially) locked, 0 if region + * is completely unlocked, and negative on errors. */ -static int sst26_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len) +static int sst26_is_unlocked(struct spi_nor *nor, loff_t ofs, uint64_t len) { /* - * 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. + * is_unlocked function is used for check before reading or erasing + * flash 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; @@ -3785,7 +3785,7 @@ int spi_nor_scan(struct spi_nor *nor) info->flags & SPI_NOR_HAS_LOCK) { nor->flash_lock = stm_lock; nor->flash_unlock = stm_unlock; - nor->flash_is_locked = stm_is_locked; + nor->flash_is_unlocked = stm_is_unlocked; } #endif
@@ -3797,7 +3797,7 @@ int spi_nor_scan(struct spi_nor *nor) if (info->flags & SPI_NOR_HAS_SST26LOCK) { nor->flash_lock = sst26_lock; nor->flash_unlock = sst26_unlock; - nor->flash_is_locked = sst26_is_locked; + nor->flash_is_unlocked = sst26_is_unlocked; } #endif
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 4ceeae623de..f857a94db71 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -506,8 +506,8 @@ struct spi_flash { * spi-nor will send the erase opcode via write_reg() * @flash_lock: [FLASH-SPECIFIC] lock a region of the SPI NOR * @flash_unlock: [FLASH-SPECIFIC] unlock a region of the SPI NOR - * @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is - * completely locked + * @flash_is_unlocked: [FLASH-SPECIFIC] check if a region of the SPI NOR is + * completely unlocked * @quad_enable: [FLASH-SPECIFIC] enables SPI NOR quad mode * @octal_dtr_enable: [FLASH-SPECIFIC] enables SPI NOR octal DTR mode. * @ready: [FLASH-SPECIFIC] check if the flash is ready @@ -556,7 +556,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_unlocked)(struct spi_nor *nor, loff_t ofs, uint64_t len); int (*quad_enable)(struct spi_nor *nor); int (*octal_dtr_enable)(struct spi_nor *nor); int (*ready)(struct spi_nor *nor);
From: Jan Kiszka jan.kiszka@siemens.com
Do not suggest successful operation if a flash area to be changed is actually locked, thus will not execute the request. Rather report an error and bail out. That's way more user-friendly than asking them to manually check for this case.
Derived from original patch by Chao Zeng.
Signed-off-by: Jan Kiszka jan.kiszka@siemens.com --- cmd/sf.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/cmd/sf.c b/cmd/sf.c index 8bdebd9fd8f..c97d0e28bb8 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -287,6 +287,12 @@ static int do_spi_flash_read_write(int argc, char *const argv[]) return 1; }
+ if (strncmp(argv[0], "read", 4) != 0 && flash->flash_is_unlocked && + !flash->flash_is_unlocked(flash, offset, len)) { + printf("ERROR: flash area is locked\n"); + return 1; + } + buf = map_physmem(addr, len, MAP_WRBACK); if (!buf && addr) { puts("Failed to map physical memory\n"); @@ -343,6 +349,12 @@ static int do_spi_flash_erase(int argc, char *const argv[]) return 1; }
+ if (flash->flash_is_unlocked && + !flash->flash_is_unlocked(flash, offset, len)) { + printf("ERROR: flash area is locked\n"); + return 1; + } + ret = spi_flash_erase(flash, offset, size); printf("SF: %zu bytes @ %#x Erased: ", (size_t)size, (u32)offset); if (ret)
On 02.03.22 15:01, Jan Kiszka wrote:
Changes in v6:
- fixed embarrassingly inverted logic in unlock check (and properly tested it this time)
Changes in v5:
- adjust unused is_locked callback to our is_unlocked needs
- use this callback in sf command instead
Jan
Jan Kiszka (2): mtd: spi: Convert is_locked callback to is_unlocked sf: Query write-protection status before operating the flash
cmd/sf.c | 12 ++++++++++++ drivers/mtd/spi/spi-nor-core.c | 26 +++++++++++++------------- include/linux/mtd/spi-nor.h | 6 +++--- 3 files changed, 28 insertions(+), 16 deletions(-)
Ping for these.
Jan
On Sat, May 28, 2022 at 02:07:17PM +0200, Jan Kiszka wrote:
On 02.03.22 15:01, Jan Kiszka wrote:
Changes in v6:
- fixed embarrassingly inverted logic in unlock check (and properly tested it this time)
Changes in v5:
- adjust unused is_locked callback to our is_unlocked needs
- use this callback in sf command instead
Jan
Jan Kiszka (2): mtd: spi: Convert is_locked callback to is_unlocked sf: Query write-protection status before operating the flash
cmd/sf.c | 12 ++++++++++++ drivers/mtd/spi/spi-nor-core.c | 26 +++++++++++++------------- include/linux/mtd/spi-nor.h | 6 +++--- 3 files changed, 28 insertions(+), 16 deletions(-)
Ping for these.
Jagan, do you have more commentary? Should I take these in to -next, next week when it opens? Thanks.
participants (2)
-
Jan Kiszka -
Tom Rini