Hi,
On 10/03/2019 10:58, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
+ James and Yong from Cypress.
Hi, Jonas, James, Yong,
On 01/30/2019 12:07 AM, Jonas Bonn wrote:
The status register bit SRWD (status register write disable) is
described in many words in the datasheets but effectively boils down to:
i) if set, respect WP# when trying to change protection bits;
ii) if unset, ignore WP# when trying to change protection bits
In short, the bit determines whether the WP# signal is honored or not.
It's difficult to imagine the use-case where the WP# is connected and
asserted but the user doesn't want to respect its setting. As such,
this patch sets the SRWD bit unconditionally so that the WP# is _always_
respected; hardware that doesn't care about WP# normally won't even have
it connected.
Always setting SRWD to 1, dismisses the need of a SRWD bit in the first place.
Do you know why Cypress made this bit configurable and didn't enable it by default?
I suspect it has some historical relevance; as things currently stand, I
don't see how it's useful in its current form.
We initially thought that the WP# signal turned on/off block protection
but that's not the case; the block protection bits BP0-BP2 turn on/off
block protection and the WP# signal only protects the BP[0-2] bits from
being modified. Turning off the WP# functionality via the SRWD bits
means that the BP[0-2] bits are always modifiable and therefore the
flash device is never really protected from writes by a malicious actor.
The key question here is: if the WP# signal is connected, why would you
ever want its state to be ignored by the device? De-asserting the
signal has the same effect as setting the SRWD bit. And the default
state is un-asserted if the signal is not connected, so that case is
covered, too. I see no reason not to just always set SRWD and thereby
make the WP# signal do what one expects of it, always.
/Jonas
Cheers,
ta
Tested on a Cypress s25fl512s. With this patch, the WP# is always
respected, irregardless of whether any flash protection bits are set.
Signed-off-by: Jonas Bonn <jonas@xxxxxxxxxxx>
CC: Marek Vasut <marek.vasut@xxxxxxxxx>
CC: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
CC: Brian Norris <computersforpeace@xxxxxxxxx>
CC: Boris Brezillon <bbrezillon@xxxxxxxxxx>
CC: Richard Weinberger <richard@xxxxxx>
CC: linux-mtd@xxxxxxxxxxxxxxxxxxx
---
drivers/mtd/spi-nor/spi-nor.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 13a5055e5f3f..4c8ce2b90838 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1231,9 +1231,6 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
status_new = (status_old & ~mask & ~SR_TB) | val;
- /* Disallow further writes if WP pin is asserted */
- status_new |= SR_SRWD;
-
if (!use_top)
status_new |= SR_TB;
@@ -1313,10 +1310,6 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
status_new = (status_old & ~mask & ~SR_TB) | val;
- /* Don't protect status register if we're fully unlocked */
- if (lock_len == 0)
- status_new &= ~SR_SRWD;
-
if (!use_top)
status_new |= SR_TB;
@@ -3898,6 +3891,7 @@ static int spi_nor_setup(struct spi_nor *nor,
static int spi_nor_init(struct spi_nor *nor)
{
int err;
+ int sr;
/*
* Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
@@ -3933,7 +3927,14 @@ static int spi_nor_init(struct spi_nor *nor)
set_4byte(nor, true);
}
- return 0;
+ /* Always respect the WP# (write-protect) input */
+ sr = read_sr(nor);
+ if (sr < 0) {
+ dev_err(nor->dev, "error while reading status register\n");
+ return -EINVAL;
+ }
+ sr |= SR_SRWD;
+ return write_sr_and_check(nor, sr, SR_SRWD);
}
/* mtd resume handler */
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
