| Message ID | 20200916124418.833-12-p.yadav@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Vignesh R |
| Headers | show |
| Series | mtd: spi-nor: add xSPI Octal DTR support | expand |
On 16/09/20 06:14PM, Pratyush Yadav wrote: > Perform a Soft Reset on shutdown on flashes that support it so that the > flash can be reset to its initial state and any configurations made by > spi-nor (given that they're only done in volatile registers) will be > reset. This will hand back the flash in pristine state for any further > operations on it. > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > --- > drivers/mtd/spi-nor/core.c | 41 +++++++++++++++++++++++++++++++++++++ > include/linux/mtd/spi-nor.h | 2 ++ > 2 files changed, 43 insertions(+) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 6ee93544d72f..853dfa02f0de 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -40,6 +40,9 @@ > > #define SPI_NOR_MAX_ADDR_WIDTH 4 > > +#define SPI_NOR_SRST_SLEEP_MIN 200 > +#define SPI_NOR_SRST_SLEEP_MAX 400 > + > /** > * spi_nor_get_cmd_ext() - Get the command opcode extension based on the > * extension type. > @@ -3174,6 +3177,41 @@ static int spi_nor_init(struct spi_nor *nor) > return 0; > } > > +static void spi_nor_soft_reset(struct spi_nor *nor) > +{ > + struct spi_mem_op op; > + int ret; > + > + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8), The buswidth used here should be 1 instead of 8. It makes no difference in practice because the call to spi_nor_spimem_setup_op() immediately after will over-write it to the correct value anyway, but let's follow the style followed throughout the rest of the codebase. Will fix in the next version. > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_NO_ADDR, > + SPI_MEM_OP_NO_DATA); > + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); > + ret = spi_mem_exec_op(nor->spimem, &op); > + if (ret) { > + dev_warn(nor->dev, "Software reset failed: %d\n", ret); > + return; > + } > + > + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8), Same here. > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_NO_ADDR, > + SPI_MEM_OP_NO_DATA); > + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); > + ret = spi_mem_exec_op(nor->spimem, &op); > + if (ret) { > + dev_warn(nor->dev, "Software reset failed: %d\n", ret); > + return; > + } > + > + /* > + * Software Reset is not instant, and the delay varies from flash to > + * flash. Looking at a few flashes, most range somewhere below 100 > + * microseconds. So, sleep for a range of 200-400 us. > + */ > + usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX); > +} > + > /* mtd resume handler */ > static void spi_nor_resume(struct mtd_info *mtd) > {
On 9/29/20 4:08 PM, Pratyush Yadav wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 16/09/20 06:14PM, Pratyush Yadav wrote: >> Perform a Soft Reset on shutdown on flashes that support it so that the >> flash can be reset to its initial state and any configurations made by >> spi-nor (given that they're only done in volatile registers) will be >> reset. This will hand back the flash in pristine state for any further >> operations on it. >> >> Signed-off-by: Pratyush Yadav <p.yadav@ti.com> >> --- >> drivers/mtd/spi-nor/core.c | 41 +++++++++++++++++++++++++++++++++++++ >> include/linux/mtd/spi-nor.h | 2 ++ >> 2 files changed, 43 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c >> index 6ee93544d72f..853dfa02f0de 100644 >> --- a/drivers/mtd/spi-nor/core.c >> +++ b/drivers/mtd/spi-nor/core.c >> @@ -40,6 +40,9 @@ >> >> #define SPI_NOR_MAX_ADDR_WIDTH 4 >> >> +#define SPI_NOR_SRST_SLEEP_MIN 200 >> +#define SPI_NOR_SRST_SLEEP_MAX 400 >> + >> /** >> * spi_nor_get_cmd_ext() - Get the command opcode extension based on the >> * extension type. >> @@ -3174,6 +3177,41 @@ static int spi_nor_init(struct spi_nor *nor) >> return 0; >> } >> >> +static void spi_nor_soft_reset(struct spi_nor *nor) >> +{ >> + struct spi_mem_op op; >> + int ret; >> + >> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8), can we avoid the cast? > > The buswidth used here should be 1 instead of 8. It makes no difference > in practice because the call to spi_nor_spimem_setup_op() immediately > after will over-write it to the correct value anyway, but let's follow > the style followed throughout the rest of the codebase. Will fix in the > next version. or you can just set the buswidth to 0 so that the reader rises his eyebrow and search for where it is updated. If you like it better you'll have to change throughout the entire code base, maybe in 4/15 where setup_op is introduced. > >> + SPI_MEM_OP_NO_DUMMY, >> + SPI_MEM_OP_NO_ADDR, >> + SPI_MEM_OP_NO_DATA); >> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); Still not a big fan of this, but we can update the op init later on. How about some new lines around spi_nor_spimem_setup_op()? First time I read the code I haven't seen it. >> + ret = spi_mem_exec_op(nor->spimem, &op); >> + if (ret) { >> + dev_warn(nor->dev, "Software reset failed: %d\n", ret); >> + return; >> + } >> + >> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8), > > Same here. > >> + SPI_MEM_OP_NO_DUMMY, >> + SPI_MEM_OP_NO_ADDR, >> + SPI_MEM_OP_NO_DATA); >> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); >> + ret = spi_mem_exec_op(nor->spimem, &op); >> + if (ret) { >> + dev_warn(nor->dev, "Software reset failed: %d\n", ret); >> + return; >> + } >> + >> + /* >> + * Software Reset is not instant, and the delay varies from flash to >> + * flash. Looking at a few flashes, most range somewhere below 100 >> + * microseconds. So, sleep for a range of 200-400 us. >> + */ >> + usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX); >> +} >> + >> /* mtd resume handler */ >> static void spi_nor_resume(struct mtd_info *mtd) >> { > > -- > Regards, > Pratyush Yadav > Texas Instruments India >
On 30/09/20 07:32AM, Tudor.Ambarus@microchip.com wrote: > On 9/29/20 4:08 PM, Pratyush Yadav wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On 16/09/20 06:14PM, Pratyush Yadav wrote: > >> Perform a Soft Reset on shutdown on flashes that support it so that the > >> flash can be reset to its initial state and any configurations made by > >> spi-nor (given that they're only done in volatile registers) will be > >> reset. This will hand back the flash in pristine state for any further > >> operations on it. > >> > >> Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > >> --- > >> drivers/mtd/spi-nor/core.c | 41 +++++++++++++++++++++++++++++++++++++ > >> include/linux/mtd/spi-nor.h | 2 ++ > >> 2 files changed, 43 insertions(+) > >> > >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > >> index 6ee93544d72f..853dfa02f0de 100644 > >> --- a/drivers/mtd/spi-nor/core.c > >> +++ b/drivers/mtd/spi-nor/core.c > >> @@ -40,6 +40,9 @@ > >> > >> #define SPI_NOR_MAX_ADDR_WIDTH 4 > >> > >> +#define SPI_NOR_SRST_SLEEP_MIN 200 > >> +#define SPI_NOR_SRST_SLEEP_MAX 400 > >> + > >> /** > >> * spi_nor_get_cmd_ext() - Get the command opcode extension based on the > >> * extension type. > >> @@ -3174,6 +3177,41 @@ static int spi_nor_init(struct spi_nor *nor) > >> return 0; > >> } > >> > >> +static void spi_nor_soft_reset(struct spi_nor *nor) > >> +{ > >> + struct spi_mem_op op; > >> + int ret; > >> + > >> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8), > > can we avoid the cast? No. The compiler complains about "expected expression before '{' token". We can avoid it if the assignment is with the declaration but then we can't re-use the variable op for the second command. I like the cast better than using separate variables for the two (or more in other cases) commands we execute. > > > > The buswidth used here should be 1 instead of 8. It makes no difference > > in practice because the call to spi_nor_spimem_setup_op() immediately > > after will over-write it to the correct value anyway, but let's follow > > the style followed throughout the rest of the codebase. Will fix in the > > next version. > > or you can just set the buswidth to 0 so that the reader rises his eyebrow > and search for where it is updated. If you like it better you'll have to change > throughout the entire code base, maybe in 4/15 where setup_op is introduced. Ok. Will change it. > > > >> + SPI_MEM_OP_NO_DUMMY, > >> + SPI_MEM_OP_NO_ADDR, > >> + SPI_MEM_OP_NO_DATA); > >> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); > > Still not a big fan of this, but we can update the op init later on. How > about some new lines around spi_nor_spimem_setup_op()? First time I read > the code I haven't seen it. Ok. > >> + ret = spi_mem_exec_op(nor->spimem, &op); > >> + if (ret) { > >> + dev_warn(nor->dev, "Software reset failed: %d\n", ret); > >> + return; > >> + } > >> + > >> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8), > > > > Same here. > > > >> + SPI_MEM_OP_NO_DUMMY, > >> + SPI_MEM_OP_NO_ADDR, > >> + SPI_MEM_OP_NO_DATA); > >> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); > >> + ret = spi_mem_exec_op(nor->spimem, &op); > >> + if (ret) { > >> + dev_warn(nor->dev, "Software reset failed: %d\n", ret); > >> + return; > >> + } > >> + > >> + /* > >> + * Software Reset is not instant, and the delay varies from flash to > >> + * flash. Looking at a few flashes, most range somewhere below 100 > >> + * microseconds. So, sleep for a range of 200-400 us. > >> + */ > >> + usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX); > >> +} > >> + > >> /* mtd resume handler */ > >> static void spi_nor_resume(struct mtd_info *mtd) > >> {
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 6ee93544d72f..853dfa02f0de 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -40,6 +40,9 @@ #define SPI_NOR_MAX_ADDR_WIDTH 4 +#define SPI_NOR_SRST_SLEEP_MIN 200 +#define SPI_NOR_SRST_SLEEP_MAX 400 + /** * spi_nor_get_cmd_ext() - Get the command opcode extension based on the * extension type. @@ -3174,6 +3177,41 @@ static int spi_nor_init(struct spi_nor *nor) return 0; } +static void spi_nor_soft_reset(struct spi_nor *nor) +{ + struct spi_mem_op op; + int ret; + + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_NO_ADDR, + SPI_MEM_OP_NO_DATA); + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); + ret = spi_mem_exec_op(nor->spimem, &op); + if (ret) { + dev_warn(nor->dev, "Software reset failed: %d\n", ret); + return; + } + + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_NO_ADDR, + SPI_MEM_OP_NO_DATA); + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); + ret = spi_mem_exec_op(nor->spimem, &op); + if (ret) { + dev_warn(nor->dev, "Software reset failed: %d\n", ret); + return; + } + + /* + * Software Reset is not instant, and the delay varies from flash to + * flash. Looking at a few flashes, most range somewhere below 100 + * microseconds. So, sleep for a range of 200-400 us. + */ + usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX); +} + /* mtd resume handler */ static void spi_nor_resume(struct mtd_info *mtd) { @@ -3194,6 +3232,9 @@ void spi_nor_restore(struct spi_nor *nor) nor->flags & SNOR_F_BROKEN_RESET) nor->params->set_4byte_addr_mode(nor, false); + if (nor->flags & SNOR_F_SOFT_RESET) + spi_nor_soft_reset(nor); + spi_nor_quad_enable(nor, false); } EXPORT_SYMBOL_GPL(spi_nor_restore); diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index cd549042c53d..299685d15dc2 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -51,6 +51,8 @@ #define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */ #define SPINOR_OP_RDEAR 0xc8 /* Read Extended Address Register */ #define SPINOR_OP_WREAR 0xc5 /* Write Extended Address Register */ +#define SPINOR_OP_SRSTEN 0x66 /* Software Reset Enable */ +#define SPINOR_OP_SRST 0x99 /* Software Reset */ /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */ #define SPINOR_OP_READ_4B 0x13 /* Read data bytes (low frequency) */
Perform a Soft Reset on shutdown on flashes that support it so that the flash can be reset to its initial state and any configurations made by spi-nor (given that they're only done in volatile registers) will be reset. This will hand back the flash in pristine state for any further operations on it. Signed-off-by: Pratyush Yadav <p.yadav@ti.com> --- drivers/mtd/spi-nor/core.c | 41 +++++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 2 ++ 2 files changed, 43 insertions(+)