Messages in this thread | ![/](/images/icornerl.gif) | | From | <> | Subject | Re: [PATCH 1/2] mtd: spi-nor: Introduce erase_proto | Date | Mon, 18 Jul 2022 16:50:18 +0000 |
| |
On 12/16/21 22:05, Pratyush Yadav wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 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. >
Doesn't [1] solve Alexander's problem? Alexander, would you please test Patrice's patch and provide a Tested-by tag if everything is ok?
Thanks, ta
[1] https://patchwork.ozlabs.org/project/linux-mtd/patch/20220629133013.3382393-1-patrice.chotard@foss.st.com/
| ![\](/images/icornerr.gif) |