| Message ID | 20230625100251.31589-3-amit.kumar-mahapatra@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: spi-nor: Avoid setting SRWD bit in SR | expand |
Hello Tudor, > -----Original Message----- > From: Tudor Ambarus <tudor.ambarus@linaro.org> > Sent: Friday, June 30, 2023 2:18 PM > To: Michael Walle <michael@walle.cc>; Mahapatra, Amit Kumar > <amit.kumar-mahapatra@amd.com> > Cc: pratyush@kernel.org; miquel.raynal@bootlin.com; richard@nod.at; > vigneshr@ti.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > conor+dt@kernel.org; git (AMD-Xilinx) <git@amd.com>; linux- > mtd@lists.infradead.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; amitrkcian2002@gmail.com > Subject: Re: [PATCH v3 2/2] mtd: spi-nor: Avoid setting SRWD bit in SR if WP# > signal not connected > > > > On 6/27/23 07:14, Michael Walle wrote: > > Am 2023-06-25 12:02, schrieb Amit Kumar Mahapatra: > >> Setting the status register write disable (SRWD) bit in the status > >> register (SR) with WP# signal of the flash left floating or wrongly > >> tied to GND (that includes internal pull-downs), will configure the > >> SR permanently as read-only. If WP# signal is left floating or > >> wrongly tied to GND, avoid setting SRWD bit while writing the SR during > flash protection. > >> > >> Signed-off-by: Amit Kumar Mahapatra <amit.kumar- > mahapatra@amd.com> > >> --- > >> drivers/mtd/spi-nor/core.c | 3 +++ > >> drivers/mtd/spi-nor/core.h | 1 + > >> drivers/mtd/spi-nor/swp.c | 9 +++++++-- > >> 3 files changed, 11 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > >> index 0bb0ad14a2fc..520f5ab86d2b 100644 > >> --- a/drivers/mtd/spi-nor/core.c > >> +++ b/drivers/mtd/spi-nor/core.c > >> @@ -2864,6 +2864,9 @@ static void spi_nor_init_flags(struct spi_nor > >> *nor) > >> if (flags & NO_CHIP_ERASE) > >> nor->flags |= SNOR_F_NO_OP_CHIP_ERASE; > >> > >> + if (of_property_read_bool(np, "no-wp")) > >> + nor->flags |= SNOR_F_NO_WP; > >> + > > > > Please put it below the of_property_read_bool() which is already > > there, just to keep things sorted. > > > >> if (flags & SPI_NOR_RWW && nor->info->n_banks > 1 && > >> !nor->controller_ops) > >> nor->flags |= SNOR_F_RWW; > >> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h > >> index 4fb5ff09c63a..55b5e7abce6e 100644 > >> --- a/drivers/mtd/spi-nor/core.h > >> +++ b/drivers/mtd/spi-nor/core.h > >> @@ -132,6 +132,7 @@ enum spi_nor_option_flags { > >> SNOR_F_SWP_IS_VOLATILE = BIT(13), > >> SNOR_F_RWW = BIT(14), > >> SNOR_F_ECC = BIT(15), > >> + SNOR_F_NO_WP = BIT(16), > > > > See the comment right above this enum :/ > > > >> }; > >> > >> struct spi_nor_read_command { > >> diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c > >> index 0ba716e84377..cfaba41d74d6 100644 > >> --- a/drivers/mtd/spi-nor/swp.c > >> +++ b/drivers/mtd/spi-nor/swp.c > >> @@ -214,8 +214,13 @@ static int spi_nor_sr_lock(struct spi_nor *nor, > >> loff_t ofs, uint64_t len) > >> > >> status_new = (status_old & ~mask & ~tb_mask) | val; > >> > >> - /* Disallow further writes if WP pin is asserted */ > >> - status_new |= SR_SRWD; > >> + /* > >> + * Disallow further writes if WP# pin is neither left floating > >> +nor > >> + * wrongly tied to GND(that includes internal pull-downs). > > > > nit: space missing > > > > Otherwise looks good. > > > > Thanks, Michael. > > Amit, would be good if you can address Michael's comments and resubmit. If > not, I'll amend the patch by myself when applying. I will address Michael's comments and send the next series. Regards, Amit > > Cheers, > ta
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 0bb0ad14a2fc..520f5ab86d2b 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -2864,6 +2864,9 @@ static void spi_nor_init_flags(struct spi_nor *nor) if (flags & NO_CHIP_ERASE) nor->flags |= SNOR_F_NO_OP_CHIP_ERASE; + if (of_property_read_bool(np, "no-wp")) + nor->flags |= SNOR_F_NO_WP; + if (flags & SPI_NOR_RWW && nor->info->n_banks > 1 && !nor->controller_ops) nor->flags |= SNOR_F_RWW; diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index 4fb5ff09c63a..55b5e7abce6e 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -132,6 +132,7 @@ enum spi_nor_option_flags { SNOR_F_SWP_IS_VOLATILE = BIT(13), SNOR_F_RWW = BIT(14), SNOR_F_ECC = BIT(15), + SNOR_F_NO_WP = BIT(16), }; struct spi_nor_read_command { diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c index 0ba716e84377..cfaba41d74d6 100644 --- a/drivers/mtd/spi-nor/swp.c +++ b/drivers/mtd/spi-nor/swp.c @@ -214,8 +214,13 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, uint64_t len) status_new = (status_old & ~mask & ~tb_mask) | val; - /* Disallow further writes if WP pin is asserted */ - status_new |= SR_SRWD; + /* + * Disallow further writes if WP# pin is neither left floating nor + * wrongly tied to GND(that includes internal pull-downs). + * WP# pin hard strapped to GND can be a valid use case. + */ + if (!(nor->flags & SNOR_F_NO_WP)) + status_new |= SR_SRWD; if (!use_top) status_new |= tb_mask;
Setting the status register write disable (SRWD) bit in the status register (SR) with WP# signal of the flash left floating or wrongly tied to GND (that includes internal pull-downs), will configure the SR permanently as read-only. If WP# signal is left floating or wrongly tied to GND, avoid setting SRWD bit while writing the SR during flash protection. Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com> --- drivers/mtd/spi-nor/core.c | 3 +++ drivers/mtd/spi-nor/core.h | 1 + drivers/mtd/spi-nor/swp.c | 9 +++++++-- 3 files changed, 11 insertions(+), 2 deletions(-)