* [PATCH v2] mtd: spi-nor: Do not proceed with spi_nor_sr_unlock if WP
@ 2020-09-17 7:31 Matija Glavinic Pecotic
2020-09-17 9:59 ` Alexander Sverdlin
2020-09-30 9:20 ` Vignesh Raghavendra
0 siblings, 2 replies; 3+ messages in thread
From: Matija Glavinic Pecotic @ 2020-09-17 7:31 UTC (permalink / raw)
To: linux-mtd, tudor.ambarus; +Cc: Sverdlin, Alexander (Nokia - DE/Ulm)
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;
+
/* 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;
--
2.10.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] mtd: spi-nor: Do not proceed with spi_nor_sr_unlock if WP
2020-09-17 7:31 [PATCH v2] mtd: spi-nor: Do not proceed with spi_nor_sr_unlock if WP Matija Glavinic Pecotic
@ 2020-09-17 9:59 ` Alexander Sverdlin
2020-09-30 9:20 ` Vignesh Raghavendra
1 sibling, 0 replies; 3+ messages in thread
From: Alexander Sverdlin @ 2020-09-17 9:59 UTC (permalink / raw)
To: Matija Glavinic Pecotic, linux-mtd, tudor.ambarus
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;
>
--
Best regards,
Alexander Sverdlin.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] mtd: spi-nor: Do not proceed with spi_nor_sr_unlock if WP
2020-09-17 7:31 [PATCH v2] mtd: spi-nor: Do not proceed with spi_nor_sr_unlock if WP Matija Glavinic Pecotic
2020-09-17 9:59 ` Alexander Sverdlin
@ 2020-09-30 9:20 ` Vignesh Raghavendra
1 sibling, 0 replies; 3+ messages in thread
From: Vignesh Raghavendra @ 2020-09-30 9:20 UTC (permalink / raw)
To: Matija Glavinic Pecotic, linux-mtd, tudor.ambarus
Cc: Sverdlin, Alexander (Nokia - DE/Ulm)
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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-30 9:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 7:31 [PATCH v2] mtd: spi-nor: Do not proceed with spi_nor_sr_unlock if WP Matija Glavinic Pecotic
2020-09-17 9:59 ` Alexander Sverdlin
2020-09-30 9:20 ` Vignesh Raghavendra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).