| Message ID | 20231101094325.95851-3-tudor.ambarus@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mtd: spi-nor: introduce die erase | expand |
On 01.11.2023 11:43, Tudor Ambarus wrote: > JESD216 defines a chip as a die, one and the other are the same thing. > JESD216 clarifies that the chip erase time defined in BFPT dword(11) > applies separately to each die for multi-die devices in which the dice > are individually accessed. Based on this, update the > spi_nor_erase_chip() method to support multi-die devices. > > For now, benefit of the die erase when addr and len are aligned with die > size. This could be improved however for the uniform and non-uniform > erases cases to use the die erase when possible. For example if one > requests that an erase of a 2 die device starting from the last 64KB of > the first die to the end of the flash size, we could use just 2 > commands, a 64KB erase and a die erase. This improvement is left as an > exercise for the reader, as I don't have multi die flashes at hand. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> > --- > drivers/mtd/spi-nor/core.c | 22 +++++++++++++++++++--- > drivers/mtd/spi-nor/core.h | 6 ++++-- > drivers/mtd/spi-nor/debugfs.c | 2 +- > 3 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 25a64c65717d..360fce7ffe82 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -1067,17 +1067,21 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2) > */ > static int spi_nor_erase_chip(struct spi_nor *nor) > { > + u8 opcode = nor->params->chip_erase_opcode; > int ret; > > dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10)); > > if (nor->spimem) { > - struct spi_mem_op op = SPI_NOR_CHIP_ERASE_OP; > + struct spi_mem_op op = SPI_NOR_CHIP_ERASE_OP(opcode); > > spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); > > ret = spi_mem_exec_op(nor->spimem, &op); > } else { > + if (opcode != SPINOR_OP_CHIP_ERASE) > + return -EOPNOTSUPP; > + > ret = spi_nor_controller_ops_write_reg(nor, > SPINOR_OP_CHIP_ERASE, > NULL, 0); > @@ -1799,7 +1803,10 @@ static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len) > static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) > { > struct spi_nor *nor = mtd_to_spi_nor(mtd); > + u8 n_dice = nor->params->n_dice; > + bool die_erase = false; > u32 addr, len, rem; > + size_t die_size; > int ret; > > dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr, > @@ -1814,12 +1821,18 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) > addr = instr->addr; > len = instr->len; > > + if (n_dice) { > + die_size = div_u64(mtd->size, n_dice); > + if (len == die_size && (addr & (die_size - 1))) > + die_erase = true; > + } > + > ret = spi_nor_prep_and_lock_pe(nor, instr->addr, instr->len); > if (ret) > return ret; > > - /* whole-chip erase? */ > - if (len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) { > + /* chip (die) erase? */ > + if ((len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) || die_erase) { > unsigned long timeout; > > ret = spi_nor_lock_device(nor); > @@ -2902,6 +2915,9 @@ static int spi_nor_late_init_params(struct spi_nor *nor) > return ret; > } > > + if (nor->params->chip_erase_opcode) oops, here I wanted to check for null, when null, set the default. Takahiro and Fabio, would you please update when/if testing? if (!nor->params->chip_erase_opcode) Thanks!
On 11/1/23 09:43, Tudor Ambarus wrote: > JESD216 defines a chip as a die, one and the other are the same thing. > JESD216 clarifies that the chip erase time defined in BFPT dword(11) > applies separately to each die for multi-die devices in which the dice > are individually accessed. Based on this, update the > spi_nor_erase_chip() method to support multi-die devices. > > For now, benefit of the die erase when addr and len are aligned with die > size. This could be improved however for the uniform and non-uniform > erases cases to use the die erase when possible. For example if one > requests that an erase of a 2 die device starting from the last 64KB of > the first die to the end of the flash size, we could use just 2 > commands, a 64KB erase and a die erase. This improvement is left as an > exercise for the reader, as I don't have multi die flashes at hand. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> > --- > drivers/mtd/spi-nor/core.c | 22 +++++++++++++++++++--- > drivers/mtd/spi-nor/core.h | 6 ++++-- > drivers/mtd/spi-nor/debugfs.c | 2 +- > 3 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 25a64c65717d..360fce7ffe82 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -1067,17 +1067,21 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2) > */ > static int spi_nor_erase_chip(struct spi_nor *nor) > { > + u8 opcode = nor->params->chip_erase_opcode; > int ret; > > dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10)); > > if (nor->spimem) { > - struct spi_mem_op op = SPI_NOR_CHIP_ERASE_OP; > + struct spi_mem_op op = SPI_NOR_CHIP_ERASE_OP(opcode); > > spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); > > ret = spi_mem_exec_op(nor->spimem, &op); > } else { > + if (opcode != SPINOR_OP_CHIP_ERASE) > + return -EOPNOTSUPP; > + > ret = spi_nor_controller_ops_write_reg(nor, > SPINOR_OP_CHIP_ERASE, > NULL, 0); > @@ -1799,7 +1803,10 @@ static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len) > static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) > { > struct spi_nor *nor = mtd_to_spi_nor(mtd); > + u8 n_dice = nor->params->n_dice; > + bool die_erase = false; > u32 addr, len, rem; > + size_t die_size; > int ret; > > dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr, > @@ -1814,12 +1821,18 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) > addr = instr->addr; > len = instr->len; > > + if (n_dice) { > + die_size = div_u64(mtd->size, n_dice); > + if (len == die_size && (addr & (die_size - 1))) > + die_erase = true; > + } > + > ret = spi_nor_prep_and_lock_pe(nor, instr->addr, instr->len); > if (ret) > return ret; > > - /* whole-chip erase? */ > - if (len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) { > + /* chip (die) erase? */ > + if ((len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) || die_erase) { and here I should have iterated over all dices, whereas now I erase just one die :).
Hi Tudor, Thanks for working on this series, appreciated it. On 01/11/2023 09:05, Tudor Ambarus wrote: > and here I should have iterated over all dices, whereas now I erase > just > one die :). Would you like to send a v2 or let me know the change I should make here? I will be glad to test it. Thanks
Hi Tudor, On 01/11/2023 06:48, Tudor Ambarus wrote: > oops, here I wanted to check for null, when null, set the default. > Takahiro and Fabio, would you please update when/if testing? > > if (!nor->params->chip_erase_opcode) I applied this change on top of your series. flash_erase command completes very fast: ~# flash_erase /dev/mtd0 0 0 Erasing 131072 Kibyte @ 0 -- 100 % complete After that, I run 'hexdump -C /dev/mtd0', but I see that the original content is still there. Thanks
On 11/1/23 12:55, Fabio Estevam wrote: > Hi Tudor, > > On 01/11/2023 06:48, Tudor Ambarus wrote: > >> oops, here I wanted to check for null, when null, set the default. >> Takahiro and Fabio, would you please update when/if testing? >> >> if (!nor->params->chip_erase_opcode) > > I applied this change on top of your series. > > flash_erase command completes very fast: > > ~# flash_erase /dev/mtd0 0 0 > Erasing 131072 Kibyte @ 0 -- 100 % complete > > After that, I run 'hexdump -C /dev/mtd0', but I see that the original > content is still there. Yes, I had a wrong configuration for the die erase command. Will send a v2. Thanks!
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 25a64c65717d..360fce7ffe82 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -1067,17 +1067,21 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2) */ static int spi_nor_erase_chip(struct spi_nor *nor) { + u8 opcode = nor->params->chip_erase_opcode; int ret; dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10)); if (nor->spimem) { - struct spi_mem_op op = SPI_NOR_CHIP_ERASE_OP; + struct spi_mem_op op = SPI_NOR_CHIP_ERASE_OP(opcode); spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); ret = spi_mem_exec_op(nor->spimem, &op); } else { + if (opcode != SPINOR_OP_CHIP_ERASE) + return -EOPNOTSUPP; + ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0); @@ -1799,7 +1803,10 @@ static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len) static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) { struct spi_nor *nor = mtd_to_spi_nor(mtd); + u8 n_dice = nor->params->n_dice; + bool die_erase = false; u32 addr, len, rem; + size_t die_size; int ret; dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr, @@ -1814,12 +1821,18 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) addr = instr->addr; len = instr->len; + if (n_dice) { + die_size = div_u64(mtd->size, n_dice); + if (len == die_size && (addr & (die_size - 1))) + die_erase = true; + } + ret = spi_nor_prep_and_lock_pe(nor, instr->addr, instr->len); if (ret) return ret; - /* whole-chip erase? */ - if (len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) { + /* chip (die) erase? */ + if ((len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) || die_erase) { unsigned long timeout; ret = spi_nor_lock_device(nor); @@ -2902,6 +2915,9 @@ static int spi_nor_late_init_params(struct spi_nor *nor) return ret; } + if (nor->params->chip_erase_opcode) + nor->params->chip_erase_opcode = SPINOR_OP_CHIP_ERASE; + /* Default method kept for backward compatibility. */ if (!params->set_4byte_addr_mode) params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_brwr; diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index a456042379ee..f681a139772f 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -85,8 +85,8 @@ SPI_MEM_OP_NO_DUMMY, \ SPI_MEM_OP_NO_DATA) -#define SPI_NOR_CHIP_ERASE_OP \ - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CHIP_ERASE, 0), \ +#define SPI_NOR_CHIP_ERASE_OP(opcode) \ + SPI_MEM_OP(SPI_MEM_OP_CMD((opcode), 0), \ SPI_MEM_OP_NO_ADDR, \ SPI_MEM_OP_NO_DUMMY, \ SPI_MEM_OP_NO_DATA) @@ -362,6 +362,7 @@ struct spi_nor_otp { * command in octal DTR mode. * @n_banks: number of banks. * @n_dice: number of dice in the flash memory. + * @chip_erase_opcode: chip (die) erase opcode. Shouldn't be set for single die devices. * @vreg_offset: volatile register offset for each die. * @hwcaps: describes the read and page program hardware * capabilities. @@ -399,6 +400,7 @@ struct spi_nor_flash_parameter { u8 rdsr_addr_nbytes; u8 n_banks; u8 n_dice; + u8 chip_erase_opcode; u32 *vreg_offset; struct spi_nor_hwcaps hwcaps; diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c index 6e163cb5b478..f242a18e46fc 100644 --- a/drivers/mtd/spi-nor/debugfs.c +++ b/drivers/mtd/spi-nor/debugfs.c @@ -138,7 +138,7 @@ static int spi_nor_params_show(struct seq_file *s, void *data) if (!(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) { string_get_size(params->size, 1, STRING_UNITS_2, buf, sizeof(buf)); - seq_printf(s, " %02x (%s)\n", SPINOR_OP_CHIP_ERASE, buf); + seq_printf(s, " %02x (%s)\n", nor->params->chip_erase_opcode, buf); } seq_puts(s, "\nsector map\n");
JESD216 defines a chip as a die, one and the other are the same thing. JESD216 clarifies that the chip erase time defined in BFPT dword(11) applies separately to each die for multi-die devices in which the dice are individually accessed. Based on this, update the spi_nor_erase_chip() method to support multi-die devices. For now, benefit of the die erase when addr and len are aligned with die size. This could be improved however for the uniform and non-uniform erases cases to use the die erase when possible. For example if one requests that an erase of a 2 die device starting from the last 64KB of the first die to the end of the flash size, we could use just 2 commands, a 64KB erase and a die erase. This improvement is left as an exercise for the reader, as I don't have multi die flashes at hand. Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- drivers/mtd/spi-nor/core.c | 22 +++++++++++++++++++--- drivers/mtd/spi-nor/core.h | 6 ++++-- drivers/mtd/spi-nor/debugfs.c | 2 +- 3 files changed, 24 insertions(+), 6 deletions(-)