Hello Tudor, > -----Original Message----- > From: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> > Sent: Friday, June 30, 2023 2:18 PM > To: Michael Walle <michael@xxxxxxxx>; Mahapatra, Amit Kumar > <amit.kumar-mahapatra@xxxxxxx> > Cc: pratyush@xxxxxxxxxx; miquel.raynal@xxxxxxxxxxx; richard@xxxxxx; > vigneshr@xxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; > conor+dt@xxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>; linux- > mtd@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; amitrkcian2002@xxxxxxxxx > 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@xxxxxxx> > >> --- > >> 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