Messages in this thread | | | Date | Fri, 17 Dec 2021 01:35:37 +0530 | From | Pratyush Yadav <> | Subject | Re: [PATCH 1/2] mtd: spi-nor: Introduce erase_proto |
| |
Hi Alexander,
On 09/12/21 11:08AM, Alexander A Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@nokia.com> > > I've been looking into non-working erase on mt25qu256a and pinpointed it to > be write_proto 1-4-4 selected from SFDP while the chip only supports 1-1-0 > erase. > > For now just introduce the separate protocol without functional change and > leave the real fix for the following patch. > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> > --- > drivers/mtd/spi-nor/core.c | 9 ++++++--- > include/linux/mtd/spi-nor.h | 4 +++- > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 2e21d5a..dcd02ea 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -177,7 +177,7 @@ static int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode, > > static int spi_nor_controller_ops_erase(struct spi_nor *nor, loff_t offs) > { > - if (spi_nor_protocol_is_dtr(nor->write_proto)) > + if (spi_nor_protocol_is_dtr(nor->erase_proto)) > return -EOPNOTSUPP; > > return nor->controller_ops->erase(nor, offs); > @@ -1186,7 +1186,7 @@ static int spi_nor_erase_chip(struct spi_nor *nor) > SPI_MEM_OP_NO_DUMMY, > SPI_MEM_OP_NO_DATA); > > - spi_nor_spimem_setup_op(nor, &op, nor->write_proto); > + spi_nor_spimem_setup_op(nor, &op, nor->erase_proto); > > ret = spi_mem_exec_op(nor->spimem, &op); > } else { > @@ -1331,7 +1331,7 @@ int spi_nor_erase_sector(struct spi_nor *nor, u32 addr) > SPI_MEM_OP_NO_DUMMY, > SPI_MEM_OP_NO_DATA); > > - spi_nor_spimem_setup_op(nor, &op, nor->write_proto); > + spi_nor_spimem_setup_op(nor, &op, nor->erase_proto); > > return spi_mem_exec_op(nor->spimem, &op); > } else if (nor->controller_ops->erase) { > @@ -2727,6 +2727,9 @@ static void spi_nor_late_init_params(struct spi_nor *nor) > */ > if (nor->flags & SNOR_F_HAS_LOCK && !nor->params->locking_ops) > spi_nor_init_default_locking_ops(nor); > + > + if (!nor->erase_proto) > + nor->erase_proto = nor->write_proto;
I get that you are trying to not break any existing flashes with this, but I don't quite like it. We should keep the same initialization flow with erase_proto as with write_proto, read_proto, etc. That is, initialize it to SNOR_PROTO_1_1_1 in spi_nor_scan() and then let the initialization procedure change it as needed.
The problem with this is of course that it could break some flashes by selecting the wrong erase. I would expect _most_ flashes to use erase_proto as 1-1-1 but I of course haven't went and looked at every single flash to point out the exceptions.
I would like to hear from others if they think it is okay to do this.
> } > > /**
-- Regards, Pratyush Yadav Texas Instruments Inc.
| |