| Message ID | 25c209e2-f48e-f4aa-4043-1f259e26a7f1@nokia.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Vignesh R |
| Headers | show |
| Series | [v2] mtd: spi-nor: Do not proceed with spi_nor_sr_unlock if WP | expand |
On 17/09/2020 09:31, Matija Glavinic Pecotic wrote: > In case Status register 1:SR_SRWD was set (either by software or write > protect pin), sr_unlock fails on spi_nor_write_sr_and_check in 8-bit > mode due to fact that written value will not match read value. > > Problem was observed with n25q128a11 device having protection enabled > by W# pin. Failures lead to device probe failure. > > Problem was uncovered by 82de6a6fb67e16 ("mtd: spi-nor: Fix the writing > of the Status Register on micron flashes"). Commit selected 8-bit mode > for micron devices. In 16-bit mode, CR content is verified, while in > 8-bit SR is checked for sanity. > > Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com> Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> > --- > v2: Fix whitespaces added by mail client > > drivers/mtd/spi-nor/core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 65eff4c..4c3acc0 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -1777,6 +1777,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) > > status_old = nor->bouncebuf[0]; > > + /* If SR is write protected, we cannot unlock */ > + if (status_old & SR_SRWD) > + return 0; > + > /* If nothing in our range is locked, we don't need to do anything */ > if (spi_nor_is_unlocked_sr(nor, ofs, len, status_old)) > return 0; >
On 9/17/20 1:01 PM, Matija Glavinic Pecotic wrote: > In case Status register 1:SR_SRWD was set (either by software or write > protect pin), sr_unlock fails on spi_nor_write_sr_and_check in 8-bit > mode due to fact that written value will not match read value. > > Problem was observed with n25q128a11 device having protection enabled > by W# pin. Failures lead to device probe failure. > > Problem was uncovered by 82de6a6fb67e16 ("mtd: spi-nor: Fix the writing > of the Status Register on micron flashes"). Commit selected 8-bit mode > for micron devices. In 16-bit mode, CR content is verified, while in > 8-bit SR is checked for sanity. > > Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com> > --- > v2: Fix whitespaces added by mail client > > drivers/mtd/spi-nor/core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 65eff4c..4c3acc0 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -1777,6 +1777,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) > > status_old = nor->bouncebuf[0]; > > + /* If SR is write protected, we cannot unlock */ > + if (status_old & SR_SRWD) > + return 0; > + With this, MEMUNLOCK ioctl would succeed with no errors but would fail to modify lock bits when SR_SRWD is set. This would give user a wrong impression that region has been unlocked as there is no error code that's being returned (thus breaks ABI). Also, how would one unlock the flash once SR_SRWD is set? For example: (1) hardware WP# is deasserted (2) program flash (3) Set block protection bits and SR_SRWD (4) hardware WP# is asserted (5) flash protection range can no longer be changed, until WP# is deasserted Now to make flash writeable again: (1) hardware WP# is deasserted (2) Unlock the protection flash bits and SR_SRWD But (2) would always fail due to above check, right? Alternatively, spi_nor_init() can be modified to be non fatal when spi_nor_unlock_all() returns non zero value (but retain the dev_dbg()). Also add a comment inline. > /* If nothing in our range is locked, we don't need to do anything */ > if (spi_nor_is_unlocked_sr(nor, ofs, len, status_old)) > return 0; > Regards Vignesh
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 65eff4c..4c3acc0 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -1777,6 +1777,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len) status_old = nor->bouncebuf[0]; + /* If SR is write protected, we cannot unlock */ + if (status_old & SR_SRWD) + return 0; + /* If nothing in our range is locked, we don't need to do anything */ if (spi_nor_is_unlocked_sr(nor, ofs, len, status_old)) return 0;
In case Status register 1:SR_SRWD was set (either by software or write protect pin), sr_unlock fails on spi_nor_write_sr_and_check in 8-bit mode due to fact that written value will not match read value. Problem was observed with n25q128a11 device having protection enabled by W# pin. Failures lead to device probe failure. Problem was uncovered by 82de6a6fb67e16 ("mtd: spi-nor: Fix the writing of the Status Register on micron flashes"). Commit selected 8-bit mode for micron devices. In 16-bit mode, CR content is verified, while in 8-bit SR is checked for sanity. Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com> --- v2: Fix whitespaces added by mail client drivers/mtd/spi-nor/core.c | 4 ++++ 1 file changed, 4 insertions(+)