diff mbox series

[v2,2/6] mtd: spi-nor: add erase die (chip) capability

Message ID 20231101145853.524045-3-tudor.ambarus@linaro.org (mailing list archive)
State New, archived
Headers show
Series mtd: spi-nor: introduce die erase | expand

Commit Message

Tudor Ambarus Nov. 1, 2023, 2:58 p.m. UTC
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    | 104 +++++++++++++++++++++++-----------
 drivers/mtd/spi-nor/core.h    |   8 ++-
 drivers/mtd/spi-nor/debugfs.c |   2 +-
 3 files changed, 78 insertions(+), 36 deletions(-)

Comments

Tudor Ambarus Nov. 1, 2023, 4:04 p.m. UTC | #1
Hi, Fabio,

On 11/1/23 14:58, 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    | 104 +++++++++++++++++++++++-----------
>  drivers/mtd/spi-nor/core.h    |   8 ++-
>  drivers/mtd/spi-nor/debugfs.c |   2 +-
>  3 files changed, 78 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 25a64c65717d..ac2651e76285 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1065,19 +1065,25 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int spi_nor_erase_chip(struct spi_nor *nor)
> +static int spi_nor_erase_die(struct spi_nor *nor, loff_t addr, size_t die_size)
>  {
> +	bool multi_die = nor->mtd.size == die_size;

this is wrong, it should have been
	bool multi_die = nor->mtd.size != die_size;

Sorry :)
Fabio Estevam Nov. 1, 2023, 4:17 p.m. UTC | #2
Hi Tudor,

On 01/11/2023 13:04, Tudor Ambarus wrote:

> this is wrong, it should have been
> 	bool multi_die = nor->mtd.size != die_size;


I applied the following changes on top of v2:

--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1067,12 +1067,17 @@ static int spi_nor_read_sr2(struct spi_nor *nor, 
u8 *sr2)
   */
  static int spi_nor_erase_die(struct spi_nor *nor, loff_t addr, size_t 
die_size)
  {
-       bool multi_die = nor->mtd.size == die_size;
+       bool multi_die = nor->mtd.size != die_size;
         int ret;

-       dev_dbg(nor->dev, " %lldKiB\n", (long long)(die_size >> 10));
+       dev_info(nor->dev, " %lldKiB\n", (long long)(die_size >> 10));

         if (nor->spimem) {
+               dev_info(nor->dev, "***** nor->params->die_erase_opcode: 
0x%x\n", nor->params->die_erase_opcode);
+               dev_info(nor->dev, "***** nor->addr_nbytes: %d\n", 
nor->addr_nbytes);
+               dev_info(nor->dev, "***** addr 0x%llx\n", addr);
+               dev_info(nor->dev, "***** multi_die: %d\n", multi_die);
+
                 struct spi_mem_op op =
                         
SPI_NOR_DIE_ERASE_OP(nor->params->die_erase_opcode,
                                              nor->addr_nbytes, addr, 
multi_die);
, but erase is still not working:


~# time flash_erase /dev/mtd0 0 0
Erasing 131072 Kibyte @ 0 --  0 [   14.733446] spi-nor spi0.0:  65536KiB
% complete [   14.739932] spi-nor spi0.0: ***** 
nor->params->die_erase_opcode: 0xc4
[   14.747311] spi-nor spi0.0: ***** nor->addr_nbytes: 4
[   14.752402] spi-nor spi0.0: ***** addr 0x0
[   14.756524] spi-nor spi0.0: ***** multi_die: 1
[   14.761121] spi-nor spi0.0:  65536KiB
[   14.764807] spi-nor spi0.0: ***** nor->params->die_erase_opcode: 0xc4
[   14.771289] spi-nor spi0.0: ***** nor->addr_nbytes: 4
[   14.776369] spi-nor spi0.0: ***** addr 0x4000000
[   14.781004] spi-nor spi0.0: ***** multi_die: 1
Erasing 131072 Kibyte @ 0 -- 100 % complete

real	0m0.061s
user	0m0.006s
sys	0m0.053s

~# hexdump -C /dev/mtd0
00000000  d4 a1 8c 16 ad 4d b2 df  3d 2a af c2 ae 0a 8a c1  
|.....M..=*......|
00000010  5f 2d 7a 17 9f c3 a4 46  cd f9 80 b8 1e 33 43 25  
|_-z....F.....3C%|
....

Thanks
Tudor Ambarus Nov. 1, 2023, 5:27 p.m. UTC | #3
On 11/1/23 16:17, Fabio Estevam wrote:
> Hi Tudor,
> 

Hi!

> On 01/11/2023 13:04, Tudor Ambarus wrote:
> 
>> this is wrong, it should have been
>>     bool multi_die = nor->mtd.size != die_size;
> 
> 
> I applied the following changes on top of v2:
> 
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1067,12 +1067,17 @@ static int spi_nor_read_sr2(struct spi_nor *nor,
> u8 *sr2)
>   */
>  static int spi_nor_erase_die(struct spi_nor *nor, loff_t addr, size_t
> die_size)
>  {
> -       bool multi_die = nor->mtd.size == die_size;
> +       bool multi_die = nor->mtd.size != die_size;
>         int ret;
> 
> -       dev_dbg(nor->dev, " %lldKiB\n", (long long)(die_size >> 10));
> +       dev_info(nor->dev, " %lldKiB\n", (long long)(die_size >> 10));
> 
>         if (nor->spimem) {
> +               dev_info(nor->dev, "***** nor->params->die_erase_opcode:
> 0x%x\n", nor->params->die_erase_opcode);
> +               dev_info(nor->dev, "***** nor->addr_nbytes: %d\n",
> nor->addr_nbytes);
> +               dev_info(nor->dev, "***** addr 0x%llx\n", addr);
> +               dev_info(nor->dev, "***** multi_die: %d\n", multi_die);
> +
>                 struct spi_mem_op op =
>                         SPI_NOR_DIE_ERASE_OP(nor->params->die_erase_opcode,
>                                              nor->addr_nbytes, addr,
> multi_die);

cool!

> , but erase is still not working:
>>
> ~# time flash_erase /dev/mtd0 0 0
> Erasing 131072 Kibyte @ 0 --  0 [   14.733446] spi-nor spi0.0:  65536KiB
> % complete [   14.739932] spi-nor spi0.0: *****
> nor->params->die_erase_opcode: 0xc4
> [   14.747311] spi-nor spi0.0: ***** nor->addr_nbytes: 4
> [   14.752402] spi-nor spi0.0: ***** addr 0x0
> [   14.756524] spi-nor spi0.0: ***** multi_die: 1
> [   14.761121] spi-nor spi0.0:  65536KiB
> [   14.764807] spi-nor spi0.0: ***** nor->params->die_erase_opcode: 0xc4
> [   14.771289] spi-nor spi0.0: ***** nor->addr_nbytes: 4
> [   14.776369] spi-nor spi0.0: ***** addr 0x4000000
> [   14.781004] spi-nor spi0.0: ***** multi_die: 1
> Erasing 131072 Kibyte @ 0 -- 100 % complete
> 
> real    0m0.061s
> user    0m0.006s
> sys    0m0.053s
> 
> ~# hexdump -C /dev/mtd0
> 00000000  d4 a1 8c 16 ad 4d b2 df  3d 2a af c2 ae 0a 8a c1 
> |.....M..=*......|
> 00000010  5f 2d 7a 17 9f c3 a4 46  cd f9 80 b8 1e 33 43 25 
> |_-z....F.....3C%|
> ....

Thanks, Fabio. I can't tell what's happening and it's getting late here.
Die erase does not execute if either of the status register BP bits are
set to 1 (some sectors are locked), but that shouldn't be the case I
guess as you could erase the entire flash before. So maybe dump SR/FSR
before and after the chip erase op. Other idea is to dump the spi_mem_op
fields, after spi_nor_spimem_setup_op() is called, maybe I got something
wrong there, but on a short look it seems fine.

I'll re-read this tomorrow. Cheers,
ta
Fabio Estevam Nov. 2, 2023, 4:43 p.m. UTC | #4
Hi Tudor,

On 01/11/2023 14:27, Tudor Ambarus wrote:

> Thanks, Fabio. I can't tell what's happening and it's getting late 
> here.
> Die erase does not execute if either of the status register BP bits are
> set to 1 (some sectors are locked), but that shouldn't be the case I
> guess as you could erase the entire flash before. So maybe dump SR/FSR

Correct, I could erase the entire flash before.

> before and after the chip erase op. Other idea is to dump the 
> spi_mem_op

SR is the same before and after the chip erase operation.

I haven't dumped FSR as I didn't find an easy way to do it from the 
core.

> fields, after spi_nor_spimem_setup_op() is called, maybe I got 
> something
> wrong there, but on a short look it seems fine.
> 
> I'll re-read this tomorrow. Cheers,

Not sure if it helps to give some ideas, but there was an old submission
for die erase support:

http://www.infradead.org/pipermail/linux-mtd/2016-October/069960.html

http://www.infradead.org/pipermail/linux-mtd/2016-October/069961.html

http://www.infradead.org/pipermail/linux-mtd/2016-October/069959.html

Thanks for your help.
Tudor Ambarus Nov. 2, 2023, 5:36 p.m. UTC | #5
On 11/2/23 16:43, Fabio Estevam wrote:
> Hi Tudor,

Hi, Fabio,

> 
> On 01/11/2023 14:27, Tudor Ambarus wrote:
> 
>> Thanks, Fabio. I can't tell what's happening and it's getting late here.
>> Die erase does not execute if either of the status register BP bits are
>> set to 1 (some sectors are locked), but that shouldn't be the case I
>> guess as you could erase the entire flash before. So maybe dump SR/FSR
> 
> Correct, I could erase the entire flash before.
> 
>> before and after the chip erase op. Other idea is to dump the spi_mem_op
> 
> SR is the same before and after the chip erase operation.

what value did it have?

> 
> I haven't dumped FSR as I didn't find an easy way to do it from the core.
> 
>> fields, after spi_nor_spimem_setup_op() is called, maybe I got something
>> wrong there, but on a short look it seems fine.
>>
>> I'll re-read this tomorrow. Cheers,
> 
> Not sure if it helps to give some ideas, but there was an old submission
> for die erase support:
> > http://www.infradead.org/pipermail/linux-mtd/2016-October/069960.html
> 
> http://www.infradead.org/pipermail/linux-mtd/2016-October/069961.html
> 
> http://www.infradead.org/pipermail/linux-mtd/2016-October/069959.html
> 

yeah, nothing special here. It reminds me that I could have updated the
uniform and non-uniform erases to use the die erase when possible, but I
was too lazy to do it.
Fabio Estevam Nov. 2, 2023, 5:40 p.m. UTC | #6
Hi Tudor,

On 02/11/2023 14:36, Tudor Ambarus wrote:

>> SR is the same before and after the chip erase operation.
> 
> what value did it have?

[   35.726305] spi-nor spi0.0: ***** SR before erase: 0x2
[   35.731543] spi-nor spi0.0: ***** SR after erase: 0x2

If you have some debug patch to dump more information about this issue,
I will be glad to run it here.

Thanks for your help.
Tudor Ambarus Nov. 2, 2023, 5:47 p.m. UTC | #7
On 11/2/23 17:40, Fabio Estevam wrote:
> Hi Tudor,
> 
> On 02/11/2023 14:36, Tudor Ambarus wrote:
> 
>>> SR is the same before and after the chip erase operation.
>>
>> what value did it have?
> 
> [   35.726305] spi-nor spi0.0: ***** SR before erase: 0x2
> [   35.731543] spi-nor spi0.0: ***** SR after erase: 0x2
> 
> If you have some debug patch to dump more information about this issue,
> I will be glad to run it here.

I think I found what I got wrong. Give me few minutes.
Tudor Ambarus Nov. 2, 2023, 5:54 p.m. UTC | #8
Does this help?

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 05f3e93794bd..54e342a7e73c 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1870,7 +1870,7 @@ static int spi_nor_erase(struct mtd_info *mtd,
struct erase_info *instr)

        if (n_dice) {
                die_size = div_u64(mtd->size, n_dice);
-               if (len == die_size && (addr & (die_size - 1)))
+               if (!(len & (die_size - 1)) && !(addr & (die_size - 1)))
                        multi_die_erase = true;
        } else {
                die_size = mtd->size;
Tudor Ambarus Nov. 2, 2023, 5:59 p.m. UTC | #9
On 11/2/23 17:54, Tudor Ambarus wrote:
> Does this help?
> 

I guess not, because you used the full flash erase in the first place,
but still a bug. Ok, I'll add some prints.
Fabio Estevam Nov. 2, 2023, 6:01 p.m. UTC | #10
On 02/11/2023 14:59, Tudor Ambarus wrote:
> On 11/2/23 17:54, Tudor Ambarus wrote:
>> Does this help?
>> 
> 
> I guess not, because you used the full flash erase in the first place,
> but still a bug. Ok, I'll add some prints.

Yes, it did not help:

root@mcde3000a:~# time flash_erase /dev/mtd0 0 0
Erasing 131072 Kibyte @ 0 --  0 [   27.693305] ****** n_dice: 2
% complete [   27.699053] ****** die_size: 67108864
[   27.703650] ****** multi_die_erase: 1
[   27.707342] ****** addr: 0
[   27.710070] ****** len: 134217728
[   27.713406] **** Logic addr & (die_size - 1): 0
[   27.718017] **************** spi_nor_erase_dice 1
[   27.722793] spi-nor spi0.0:  **** Die size is 65536KiB
[   27.727964] spi-nor spi0.0: ***** nor->params->die_erase_opcode: 0xc4
[   27.734430] spi-nor spi0.0: ***** nor->addr_nbytes: 4
[   27.739498] spi-nor spi0.0: ***** addr 0x0
[   27.743614] spi-nor spi0.0: ***** multi_die: 1
[   27.748094] spi-nor spi0.0: ***** SR before erase: 0x2
[   27.753278] spi-nor spi0.0: ***** SR after erase: 0x2
[   27.758387] spi-nor spi0.0:  **** Die size is 65536KiB
[   27.763559] spi-nor spi0.0: ***** nor->params->die_erase_opcode: 0xc4
[   27.770017] spi-nor spi0.0: ***** nor->addr_nbytes: 4
[   27.775085] spi-nor spi0.0: ***** addr 0x4000000
[   27.779724] spi-nor spi0.0: ***** multi_die: 1
[   27.784249] spi-nor spi0.0: ***** SR before erase: 0x2
[   27.789496] spi-nor spi0.0: ***** SR after erase: 0x2
Erasing 131072 Kibyte @ 0 -- 100 % complete

real	0m0.110s
user	0m0.001s
sys	0m0.107s
root@mcde3000a:~# hexdump /dev/mtd0
0000000 a1d4 168c 4dad dfb2 2a3d c2af 0aae c18a
0000010 2d5f 177a c39f 46a4 f9cd b880 331e 2543
0000020 dd47 8981 1eaa aab4 9626 336c 4f74 2b22
....
Tudor Ambarus Nov. 2, 2023, 6:21 p.m. UTC | #11
On 11/2/23 18:01, Fabio Estevam wrote:
> On 02/11/2023 14:59, Tudor Ambarus wrote:
>> On 11/2/23 17:54, Tudor Ambarus wrote:
>>> Does this help?
>>>
>>
>> I guess not, because you used the full flash erase in the first place,
>> but still a bug. Ok, I'll add some prints.
> 
> Yes, it did not help:


Let's see what gets to the SPI controller. Which SPI controller do you use?

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index af8f3fc30256..5584bf9cbfd1 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1067,7 +1067,7 @@ static int spi_nor_read_sr2(struct spi_nor *nor,
u8 *sr2)
  */
 static int spi_nor_erase_die(struct spi_nor *nor, loff_t addr, size_t
die_size)
 {
-       bool multi_die = nor->mtd.size == die_size;
+       bool multi_die = nor->mtd.size != die_size;
        int ret;

        dev_dbg(nor->dev, " %lldKiB\n", (long long)(die_size >> 10));
@@ -1079,6 +1079,23 @@ static int spi_nor_erase_die(struct spi_nor *nor,
loff_t addr, size_t die_size)

                spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);

+               dev_info(nor->dev, "***** nor->reg_proto = 0x%08x \n",
nor->reg_proto);
+               dev_info(nor->dev, "*****\n");
+               dev_info(nor->dev, "***** op.cmd.nbytes = 0x%02x \n",
op.cmd.nbytes);
+               dev_info(nor->dev, "***** op.cmd.buswidth = 0x%02x \n",
op.cmd.buswidth);
+               dev_info(nor->dev, "***** op.cmd.opcode = 0x%02x \n",
(u8) op.cmd.opcode);
+               dev_info(nor->dev, "*****\n");
+               dev_info(nor->dev, "***** op.addr.nbytes = 0x%02x \n",
op.addr.nbytes);
+               dev_info(nor->dev, "***** op.addr.buswidth = 0x%02x \n",
op.addr.buswidth);
+               dev_info(nor->dev, "***** op.addr.buswidth = 0x%llx \n",
op.addr.val);
+               dev_info(nor->dev, "*****\n");
+               dev_info(nor->dev, "***** op.dummy.nbytes = 0x%02x \n",
op.dummy.nbytes);
+               dev_info(nor->dev, "***** op.dummy.buswidth = 0x%02x
\n", op.dummy.buswidth);
+               dev_info(nor->dev, "*****\n");
+               dev_info(nor->dev, "***** op.data.buswidth = 0x%02x \n",
op.data.buswidth);
+               dev_info(nor->dev, "***** op.data.nbytes = %d \n",
op.data.nbytes);
+
+
                ret = spi_mem_exec_op(nor->spimem, &op);
        } else {
                if (multi_die)
@@ -1870,7 +1887,7 @@ static int spi_nor_erase(struct mtd_info *mtd,
struct erase_info *instr)

        if (n_dice) {
                die_size = div_u64(mtd->size, n_dice);
-               if (len == die_size && (addr & (die_size - 1)))
+               if (!(len & (die_size - 1)) && !(addr & (die_size - 1)))
                        multi_die_erase = true;
        } else {
                die_size = mtd->size;
Fabio Estevam Nov. 2, 2023, 6:33 p.m. UTC | #12
On 02/11/2023 15:21, Tudor Ambarus wrote:

> Let's see what gets to the SPI controller. Which SPI controller do you 
> use?

I am using i.MX8MP, which has drivers/spi/spi-nxp-fspi.c.

Here is the result:

root@mcde3000a:~# flash_erase /dev/mtd0 0 0
Erasing 131072 Kibyte @ 0 --  0 [   26.040903] spi-nor spi0.0: ***** 
nor->reg_proto = 0x00010101
% complete [   26.049570] spi-nor spi0.0: *****
[   26.053849] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
[   26.059118] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
[   26.064539] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
[   26.069787] spi-nor spi0.0: *****
[   26.073118] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
[   26.078451] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
[   26.083949] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
[   26.089368] spi-nor spi0.0: *****
[   26.092699] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
[   26.098123] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
[   26.103713] spi-nor spi0.0: *****
[   26.107045] spi-nor spi0.0: ***** op.data.buswidth = 0x00
[   26.112549] spi-nor spi0.0: ***** op.data.nbytes = 0
[   26.117727] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
[   26.123589] spi-nor spi0.0: *****
[   26.127012] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
[   26.132274] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
[   26.137706] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
[   26.142956] spi-nor spi0.0: *****
[   26.146290] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
[   26.151625] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
[   26.157132] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
[   26.163065] spi-nor spi0.0: *****
[   26.166402] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
[   26.171815] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
[   26.177405] spi-nor spi0.0: *****
[   26.180737] spi-nor spi0.0: ***** op.data.buswidth = 0x00
[   26.186241] spi-nor spi0.0: ***** op.data.nbytes = 0
Erasing 131072 Kibyte @ 0 -- 100 % complete

root@mcde3000a:~# hexdump /dev/mtd0
0000000 a1d4 168c 4dad dfb2 2a3d c2af 0aae c18a
0000010 2d5f 177a c39f 46a4 f9cd b880 331e 2543
....

Thanks
Tudor Ambarus Nov. 2, 2023, 6:46 p.m. UTC | #13
On 11/2/23 18:33, Fabio Estevam wrote:
> On 02/11/2023 15:21, Tudor Ambarus wrote:
> 
>> Let's see what gets to the SPI controller. Which SPI controller do you
>> use?
> 
> I am using i.MX8MP, which has drivers/spi/spi-nxp-fspi.c.
> 
> Here is the result:
> 
> root@mcde3000a:~# flash_erase /dev/mtd0 0 0
> Erasing 131072 Kibyte @ 0 --  0 [   26.040903] spi-nor spi0.0: *****
> nor->reg_proto = 0x00010101
> % complete [   26.049570] spi-nor spi0.0: *****
> [   26.053849] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> [   26.059118] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> [   26.064539] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
> [   26.069787] spi-nor spi0.0: *****
> [   26.073118] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> [   26.078451] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> [   26.083949] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
> [   26.089368] spi-nor spi0.0: *****
> [   26.092699] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> [   26.098123] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> [   26.103713] spi-nor spi0.0: *****
> [   26.107045] spi-nor spi0.0: ***** op.data.buswidth = 0x00
> [   26.112549] spi-nor spi0.0: ***** op.data.nbytes = 0
> [   26.117727] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> [   26.123589] spi-nor spi0.0: *****
> [   26.127012] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> [   26.132274] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> [   26.137706] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
> [   26.142956] spi-nor spi0.0: *****
> [   26.146290] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> [   26.151625] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> [   26.157132] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
> [   26.163065] spi-nor spi0.0: *****
> [   26.166402] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> [   26.171815] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> [   26.177405] spi-nor spi0.0: *****
> [   26.180737] spi-nor spi0.0: ***** op.data.buswidth = 0x00
> [   26.186241] spi-nor spi0.0: ***** op.data.nbytes = 0
> Erasing 131072 Kibyte @ 0 -- 100 % complete
> 

It looks good ...

> root@mcde3000a:~# hexdump /dev/mtd0
> 0000000 a1d4 168c 4dad dfb2 2a3d c2af 0aae c18a
> 0000010 2d5f 177a c39f 46a4 f9cd b880 331e 2543

but the patient is dead :). Would be good if Takahiro can test on IFX
too. In the meantime I'll try to find a n25q00, I remember I had one in
the past.
Tudor Ambarus Nov. 2, 2023, 6:56 p.m. UTC | #14
On 11/2/23 18:46, Tudor Ambarus wrote:
> 
> 
> On 11/2/23 18:33, Fabio Estevam wrote:
>> On 02/11/2023 15:21, Tudor Ambarus wrote:
>>
>>> Let's see what gets to the SPI controller. Which SPI controller do you
>>> use?
>>
>> I am using i.MX8MP, which has drivers/spi/spi-nxp-fspi.c.
>>
>> Here is the result:
>>
>> root@mcde3000a:~# flash_erase /dev/mtd0 0 0
>> Erasing 131072 Kibyte @ 0 --  0 [   26.040903] spi-nor spi0.0: *****
>> nor->reg_proto = 0x00010101
>> % complete [   26.049570] spi-nor spi0.0: *****
>> [   26.053849] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>> [   26.059118] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>> [   26.064539] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>> [   26.069787] spi-nor spi0.0: *****
>> [   26.073118] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>> [   26.078451] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>> [   26.083949] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
>> [   26.089368] spi-nor spi0.0: *****
>> [   26.092699] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>> [   26.098123] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>> [   26.103713] spi-nor spi0.0: *****
>> [   26.107045] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>> [   26.112549] spi-nor spi0.0: ***** op.data.nbytes = 0
>> [   26.117727] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>> [   26.123589] spi-nor spi0.0: *****
>> [   26.127012] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>> [   26.132274] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>> [   26.137706] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>> [   26.142956] spi-nor spi0.0: *****
>> [   26.146290] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>> [   26.151625] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>> [   26.157132] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
>> [   26.163065] spi-nor spi0.0: *****
>> [   26.166402] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>> [   26.171815] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>> [   26.177405] spi-nor spi0.0: *****
>> [   26.180737] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>> [   26.186241] spi-nor spi0.0: ***** op.data.nbytes = 0
>> Erasing 131072 Kibyte @ 0 -- 100 % complete
>>
> 
> It looks good ...
> 
>> root@mcde3000a:~# hexdump /dev/mtd0
>> 0000000 a1d4 168c 4dad dfb2 2a3d c2af 0aae c18a
>> 0000010 2d5f 177a c39f 46a4 f9cd b880 331e 2543
> 
> but the patient is dead :). Would be good if Takahiro can test on IFX
> too. In the meantime I'll try to find a n25q00, I remember I had one in
> the past.


Let's try something else:
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 5584bf9cbfd1..a0af7a7eb727 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1074,7 +1074,7 @@ static int spi_nor_erase_die(struct spi_nor *nor,
loff_t addr, size_t die_size)

        if (nor->spimem) {
                struct spi_mem_op op =
-                       SPI_NOR_DIE_ERASE_OP(nor->params->die_erase_opcode,
+                       SPI_NOR_DIE_ERASE_OP(nor->erase_opcode,
                                             nor->addr_nbytes, addr,
multi_die);

                spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);

I expect that with this the first sector from each die will be erased.
Fabio Estevam Nov. 2, 2023, 9:42 p.m. UTC | #15
On 02/11/2023 15:56, Tudor Ambarus wrote:

> Let's try something else:
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 5584bf9cbfd1..a0af7a7eb727 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1074,7 +1074,7 @@ static int spi_nor_erase_die(struct spi_nor *nor,
> loff_t addr, size_t die_size)
> 
>         if (nor->spimem) {
>                 struct spi_mem_op op =
> -                       
> SPI_NOR_DIE_ERASE_OP(nor->params->die_erase_opcode,
> +                       SPI_NOR_DIE_ERASE_OP(nor->erase_opcode,
>                                              nor->addr_nbytes, addr,
> multi_die);
> 
>                 spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> 
> I expect that with this the first sector from each die will be erased.

You are right. This is exactly what happens with this change:

First die:

~# hexdump  /dev/mtd0
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0010000 9231 efb8 d664 d77b 231c 3522 d7e8 4f6e

Second die:

~# hexdump -s 0x4000000 /dev/mtd0
4000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
4010000 0418 0518 60a0 9214 2411 2004 c144 9e2b
Tudor Ambarus Nov. 3, 2023, 11:47 a.m. UTC | #16
On 11/2/23 21:42, Fabio Estevam wrote:
> On 02/11/2023 15:56, Tudor Ambarus wrote:
> 
>> Let's try something else:
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 5584bf9cbfd1..a0af7a7eb727 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -1074,7 +1074,7 @@ static int spi_nor_erase_die(struct spi_nor *nor,
>> loff_t addr, size_t die_size)
>>
>>         if (nor->spimem) {
>>                 struct spi_mem_op op =
>> -                      
>> SPI_NOR_DIE_ERASE_OP(nor->params->die_erase_opcode,
>> +                       SPI_NOR_DIE_ERASE_OP(nor->erase_opcode,
>>                                              nor->addr_nbytes, addr,
>> multi_die);
>>
>>                 spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>>
>> I expect that with this the first sector from each die will be erased.
> 
> You are right. This is exactly what happens with this change:
> 
> First die:
> 
> ~# hexdump  /dev/mtd0
> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff
> *
> 0010000 9231 efb8 d664 d77b 231c 3522 d7e8 4f6e
> 
> Second die:
> 
> ~# hexdump -s 0x4000000 /dev/mtd0
> 4000000 ffff ffff ffff ffff ffff ffff ffff ffff
> *
> 4010000 0418 0518 60a0 9214 2411 2004 c144 9e2b


I think I know what happens with your flash. Please try the debug patch
from https://github.com/ambarus/linux-0day.git
spi-nor/next-die-erase-v2-debug.

I assume your flash works in 3-byte mode. The die erase cmd needs an
extended register in 3-byte mode otherwise is ignored. Please try the
patch and let me know if it works.

Reading the datasheet I found out that
"The device supports 3-byte addressing (default), with A[23:0] input
during address cycle. After any READ command is executed, the device
will output data from the selected address. After the boundary is
reached, the device will start reading again from the beginning."

So I expect up to now we tested just the first die, all tests in 3-byte
mode. Thus when we wanted to do a cross-die test, reading from the last
2 MB of first die to the first 2MB of the second die, what we actually
did was to read the last and the first 2MB of the first die.

Cheers,
ta
Fabio Estevam Nov. 3, 2023, 12:30 p.m. UTC | #17
Hi Tudor,

On 03/11/2023 08:47, Tudor Ambarus wrote:

> I think I know what happens with your flash. Please try the debug patch
> from https://github.com/ambarus/linux-0day.git
> spi-nor/next-die-erase-v2-debug.
> 
> I assume your flash works in 3-byte mode. The die erase cmd needs an
> extended register in 3-byte mode otherwise is ignored. Please try the
> patch and let me know if it works.

Progress: with this one, I see that the whole flash was erased:

~# flash_erase /dev/mtd0 0 0
Erasing 131072 Kibyte @ 0 --  0 [   39.865773] spi-nor spi0.0: ***** 
nor->reg_proto = 0x00010101
% complete [   39.874441] spi-nor spi0.0: *****
[   39.878712] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
[   39.883983] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
[   39.889402] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
[   39.894648] spi-nor spi0.0: *****
[   39.897979] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
[   39.903309] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
[   39.908810] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
[   39.914226] spi-nor spi0.0: *****
[   39.917558] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
[   39.922981] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
[   39.928570] spi-nor spi0.0: *****
[   39.931905] spi-nor spi0.0: ***** op.data.buswidth = 0x00
[   39.937403] spi-nor spi0.0: ***** op.data.nbytes = 0

[  157.398677] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
[  157.404543] spi-nor spi0.0: *****
[  157.407878] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
[  157.413133] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
[  157.418549] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
[  157.423799] spi-nor spi0.0: *****
[  157.427130] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
[  157.432461] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
[  157.437963] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
[  157.443900] spi-nor spi0.0: *****
[  157.447229] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
[  157.452648] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
[  157.458239] spi-nor spi0.0: *****
[  157.461558] spi-nor spi0.0: ***** op.data.buswidth = 0x00
[  157.467066] spi-nor spi0.0: ***** op.data.nbytes = 0
Erasing 131072 Kibyte @ 0 -- 100 % complete

~# hexdump /dev/mtd0
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
8000000

The problem I see is that the erase operation was super slow.

Please see the timestamps to get an idea.

Is this slow-erase behavior expected? Please note that the
SPI controller on the i.MX8MP does not have DMA support at the moment.

Thanks!
Fabio Estevam Nov. 3, 2023, 12:53 p.m. UTC | #18
On 03/11/2023 09:30, Fabio Estevam wrote:

> The problem I see is that the erase operation was super slow.
> 
> Please see the timestamps to get an idea.
> 
> Is this slow-erase behavior expected? Please note that the
> SPI controller on the i.MX8MP does not have DMA support at the moment.

Measuring the time it takes to erase the whole flash:

root@mcde3000a:~# time flash_erase /dev/mtd0 0 0
Erasing 131072 Kibyte @ 0 --  0 [ 1502.857687] spi-nor spi0.0: ***** 
nor->reg_proto = 0x00010101
% complete [ 1502.866354] spi-nor spi0.0: *****
[ 1502.870631] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
[ 1502.875891] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
[ 1502.881311] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
[ 1502.886559] spi-nor spi0.0: *****
[ 1502.889888] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
[ 1502.895220] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
[ 1502.900724] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
[ 1502.906141] spi-nor spi0.0: *****
[ 1502.909474] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
[ 1502.914899] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
[ 1502.920489] spi-nor spi0.0: *****
[ 1502.923823] spi-nor spi0.0: ***** op.data.buswidth = 0x00
[ 1502.929323] spi-nor spi0.0: ***** op.data.nbytes = 0
[ 1620.329673] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
[ 1620.335538] spi-nor spi0.0: *****
[ 1620.338869] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
[ 1620.344127] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
[ 1620.349543] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
[ 1620.354791] spi-nor spi0.0: *****
[ 1620.358123] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
[ 1620.363452] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
[ 1620.368953] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
[ 1620.374890] spi-nor spi0.0: *****
[ 1620.378229] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
[ 1620.383646] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
[ 1620.389234] spi-nor spi0.0: *****
[ 1620.392566] spi-nor spi0.0: ***** op.data.buswidth = 0x00
[ 1620.398068] spi-nor spi0.0: ***** op.data.nbytes = 0
Erasing 131072 Kibyte @ 0 -- 100 % complete

real	3m56.795s
user	0m0.001s
sys	3m15.840s

Originally, I was also getting around 4 minutes to erase the whole 
flash,
so the erase-die performance is comparable to the erase one sector at 
time.
Tudor Ambarus Nov. 3, 2023, 1:26 p.m. UTC | #19
On 11/3/23 12:53, Fabio Estevam wrote:
> On 03/11/2023 09:30, Fabio Estevam wrote:
> 
>> The problem I see is that the erase operation was super slow.
>>
>> Please see the timestamps to get an idea.
>>
>> Is this slow-erase behavior expected? Please note that the

No.

>> SPI controller on the i.MX8MP does not have DMA support at the moment.

We don't use DMA for erases, it doesn't matter whether you use DMA or not.

> 
> Measuring the time it takes to erase the whole flash:
> 
> root@mcde3000a:~# time flash_erase /dev/mtd0 0 0
> Erasing 131072 Kibyte @ 0 --  0 [ 1502.857687] spi-nor spi0.0: *****
> nor->reg_proto = 0x00010101
> % complete [ 1502.866354] spi-nor spi0.0: *****
> [ 1502.870631] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> [ 1502.875891] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> [ 1502.881311] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
> [ 1502.886559] spi-nor spi0.0: *****
> [ 1502.889888] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> [ 1502.895220] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> [ 1502.900724] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
> [ 1502.906141] spi-nor spi0.0: *****
> [ 1502.909474] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> [ 1502.914899] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> [ 1502.920489] spi-nor spi0.0: *****
> [ 1502.923823] spi-nor spi0.0: ***** op.data.buswidth = 0x00
> [ 1502.929323] spi-nor spi0.0: ***** op.data.nbytes = 0
> [ 1620.329673] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> [ 1620.335538] spi-nor spi0.0: *****
> [ 1620.338869] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> [ 1620.344127] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> [ 1620.349543] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
> [ 1620.354791] spi-nor spi0.0: *****
> [ 1620.358123] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> [ 1620.363452] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> [ 1620.368953] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
> [ 1620.374890] spi-nor spi0.0: *****
> [ 1620.378229] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> [ 1620.383646] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> [ 1620.389234] spi-nor spi0.0: *****
> [ 1620.392566] spi-nor spi0.0: ***** op.data.buswidth = 0x00
> [ 1620.398068] spi-nor spi0.0: ***** op.data.nbytes = 0
> Erasing 131072 Kibyte @ 0 -- 100 % complete
> 
> real    3m56.795s
> user    0m0.001s
> sys    3m15.840s
> 
> Originally, I was also getting around 4 minutes to erase the whole flash,
> so the erase-die performance is comparable to the erase one sector at time.
> 

Which version of mtd-utils are you using? I guess the flash-erase
utility is written in a bad way. Please use the following while I check
what flash_erase is doing:

time mtd_debug erase /dev/mtd0 0 134217728

ta
Fabio Estevam Nov. 3, 2023, 1:37 p.m. UTC | #20
On 03/11/2023 10:26, Tudor Ambarus wrote:

> Which version of mtd-utils are you using? I guess the flash-erase

mtd-utils 2.1.5

> utility is written in a bad way. Please use the following while I check
> what flash_erase is doing:
> 
> time mtd_debug erase /dev/mtd0 0 134217728

"mtd_debug erase" gives the same time as well:

root@mcde3000a:~# time mtd_debug erase /dev/mtd0 0 134217728
[ 4322.114967] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
[ 4322.120861] spi-nor spi0.0: *****
[ 4322.124210] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
[ 4322.129478] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
[ 4322.134903] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
[ 4322.140154] spi-nor spi0.0: *****
[ 4322.143491] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
[ 4322.148831] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
[ 4322.154341] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
[ 4322.159761] spi-nor spi0.0: *****
[ 4322.163098] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
[ 4322.168524] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
[ 4322.174118] spi-nor spi0.0: *****
[ 4322.177439] spi-nor spi0.0: ***** op.data.buswidth = 0x00
[ 4322.182948] spi-nor spi0.0: ***** op.data.nbytes = 0
[ 4439.966060] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
[ 4439.971920] spi-nor spi0.0: *****
[ 4439.975252] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
[ 4439.980511] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
[ 4439.985928] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
[ 4439.991174] spi-nor spi0.0: *****
[ 4439.994504] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
[ 4439.999834] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
[ 4440.005335] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
[ 4440.011272] spi-nor spi0.0: *****
[ 4440.014604] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
[ 4440.020018] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
[ 4440.025606] spi-nor spi0.0: *****
[ 4440.028937] spi-nor spi0.0: ***** op.data.buswidth = 0x00
[ 4440.034438] spi-nor spi0.0: ***** op.data.nbytes = 0
Erased 134217728 bytes from address 0x00000000 in flash

real	3m57.384s
user	0m0.005s
sys	3m35.211s

Thanks
Tudor Ambarus Nov. 3, 2023, 1:48 p.m. UTC | #21
On 03.11.2023 15:37, Fabio Estevam wrote:
> On 03/11/2023 10:26, Tudor Ambarus wrote:
> 
>> Which version of mtd-utils are you using? I guess the flash-erase
> 
> mtd-utils 2.1.5
> 
>> utility is written in a bad way. Please use the following while I check
>> what flash_erase is doing:
>>
>> time mtd_debug erase /dev/mtd0 0 134217728
> 
> "mtd_debug erase" gives the same time as well:
> 
> root@mcde3000a:~# time mtd_debug erase /dev/mtd0 0 134217728
> [ 4322.114967] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> [ 4322.120861] spi-nor spi0.0: *****
> [ 4322.124210] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> [ 4322.129478] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> [ 4322.134903] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
> [ 4322.140154] spi-nor spi0.0: *****
> [ 4322.143491] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> [ 4322.148831] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> [ 4322.154341] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
> [ 4322.159761] spi-nor spi0.0: *****
> [ 4322.163098] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> [ 4322.168524] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> [ 4322.174118] spi-nor spi0.0: *****
> [ 4322.177439] spi-nor spi0.0: ***** op.data.buswidth = 0x00
> [ 4322.182948] spi-nor spi0.0: ***** op.data.nbytes = 0
> [ 4439.966060] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> [ 4439.971920] spi-nor spi0.0: *****
> [ 4439.975252] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> [ 4439.980511] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> [ 4439.985928] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
> [ 4439.991174] spi-nor spi0.0: *****
> [ 4439.994504] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> [ 4439.999834] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> [ 4440.005335] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
> [ 4440.011272] spi-nor spi0.0: *****
> [ 4440.014604] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> [ 4440.020018] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> [ 4440.025606] spi-nor spi0.0: *****
> [ 4440.028937] spi-nor spi0.0: ***** op.data.buswidth = 0x00
> [ 4440.034438] spi-nor spi0.0: ***** op.data.nbytes = 0
> Erased 134217728 bytes from address 0x00000000 in flash
> 
> real    3m57.384s
> user    0m0.005s
> sys    3m35.211s
> 

Yep, it's strange, we'll have to check what's happening. I found my
n25q00 flash, on my side all its 4 dice are erased in 5 sec.  SFDP
defines how long the erase die should take, see BFPT dword 11. You can
start with that.

# time mtd_debug erase /dev/mtd1 0 134217728
spi-nor spi1.0: ***** nor->reg_proto = 0x00010101
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.cmd.nbytes = 0x01
spi-nor spi1.0: ***** op.cmd.buswidth = 0x01
spi-nor spi1.0: ***** op.cmd.opcode = 0xc4
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.addr.nbytes = 0x04
spi-nor spi1.0: ***** op.addr.buswidth = 0x01
spi-nor spi1.0: ***** op.addr.buswidth = 0x0
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.dummy.nbytes = 0x00
spi-nor spi1.0: ***** op.dummy.buswidth = 0x00
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.data.buswidth = 0x00
spi-nor spi1.0: ***** op.data.nbytes = 0
spi-nor spi1.0: ***** nor->reg_proto = 0x00010101
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.cmd.nbytes = 0x01
spi-nor spi1.0: ***** op.cmd.buswidth = 0x01
spi-nor spi1.0: ***** op.cmd.opcode = 0xc4
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.addr.nbytes = 0x04
spi-nor spi1.0: ***** op.addr.buswidth = 0x01
spi-nor spi1.0: ***** op.addr.buswidth = 0x2000000
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.dummy.nbytes = 0x00
spi-nor spi1.0: ***** op.dummy.buswidth = 0x00
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.data.buswidth = 0x00
spi-nor spi1.0: ***** op.data.nbytes = 0
spi-nor spi1.0: ***** nor->reg_proto = 0x00010101
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.cmd.nbytes = 0x01
spi-nor spi1.0: ***** op.cmd.buswidth = 0x01
spi-nor spi1.0: ***** op.cmd.opcode = 0xc4
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.addr.nbytes = 0x04
spi-nor spi1.0: ***** op.addr.buswidth = 0x01
spi-nor spi1.0: ***** op.addr.buswidth = 0x4000000
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.dummy.nbytes = 0x00
spi-nor spi1.0: ***** op.dummy.buswidth = 0x00
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.data.buswidth = 0x00
spi-nor spi1.0: ***** op.data.nbytes = 0
spi-nor spi1.0: ***** nor->reg_proto = 0x00010101
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.cmd.nbytes = 0x01
spi-nor spi1.0: ***** op.cmd.buswidth = 0x01
spi-nor spi1.0: ***** op.cmd.opcode = 0xc4
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.addr.nbytes = 0x04
spi-nor spi1.0: ***** op.addr.buswidth = 0x01
spi-nor spi1.0: ***** op.addr.buswidth = 0x6000000
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.dummy.nbytes = 0x00
spi-nor spi1.0: ***** op.dummy.buswidth = 0x00
spi-nor spi1.0: *****
spi-nor spi1.0: ***** op.data.buswidth = 0x00
spi-nor spi1.0: ***** op.data.nbytes = 0
Erased 134217728 bytes from address 0x00000000 in flash

real	0m5.485s
user	0m0.000s
sys	0m5.461s
# hexdump /dev/mtd1
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
8000000
Fabio Estevam Nov. 3, 2023, 2:16 p.m. UTC | #22
On 03/11/2023 10:48, Tudor Ambarus wrote:

> Yep, it's strange, we'll have to check what's happening. I found my
> n25q00 flash, on my side all its 4 dice are erased in 5 sec.  SFDP
> defines how long the erase die should take, see BFPT dword 11. You can
> start with that.

Where does BFPT dword 11 reside inside SFDP?

~# hexdump -C 
/sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp
00000000  53 46 44 50 06 01 01 ff  00 06 01 10 30 00 00 ff  
|SFDP........0...|
00000010  84 00 01 02 80 00 00 ff  ff ff ff ff ff ff ff ff  
|................|
00000020  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  
|................|
00000030  e5 20 fb ff ff ff ff 3f  29 eb 27 6b 27 3b 27 bb  |. 
.....?).'k';'.|
00000040  ff ff ff ff ff ff 27 bb  ff ff 29 eb 0c 20 10 d8  
|......'...).. ..|
00000050  0f 52 00 00 24 4a 99 00  8b 8e 03 e1 ac 01 27 38  
|.R..$J........'8|
00000060  7a 75 7a 75 fb bd d5 5c  4a 0f 82 ff 81 bd 3d 36  
|zuzu...\J.....=6|
00000070  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  
|................|
00000080  ff e7 ff ff 21 dc ff ff                           |....!...|
00000088

Do we need a fixup to tell the erase die to be faster?

I haven't found the 'BFPT' term in the JEDEC spec.

> real	0m5.485s
> user	0m0.000s
> sys	0m5.461s

Before erasing, did the flash contain 128MB of random data on your test?

On my tests, when the flash contains 128MB of random data, the first 
erase
takes 4 minutes. Subsequent erases take only 2 seconds.

Thanks
Tudor Ambarus Nov. 3, 2023, 2:37 p.m. UTC | #23
On 03.11.2023 16:16, Fabio Estevam wrote:
> On 03/11/2023 10:48, Tudor Ambarus wrote:
> 
>> Yep, it's strange, we'll have to check what's happening. I found my
>> n25q00 flash, on my side all its 4 dice are erased in 5 sec.  SFDP
>> defines how long the erase die should take, see BFPT dword 11. You can
>> start with that.
> 
> Where does BFPT dword 11 reside inside SFDP?

BFPT stands for Basic Flash Parameter Table. BFPT is the first table of
SFDP. Please compare the standard with the dump. We haven't written yet
an utility to decode the SFDP data.
> 
> ~# hexdump -C
> /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp
> 00000000  53 46 44 50 06 01 01 ff  00 06 01 10 30 00 00 ff 
> |SFDP........0...|
> 00000010  84 00 01 02 80 00 00 ff  ff ff ff ff ff ff ff ff 
> |................|
> 00000020  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff 
> |................|
> 00000030  e5 20 fb ff ff ff ff 3f  29 eb 27 6b 27 3b 27 bb  |.
> .....?).'k';'.|
> 00000040  ff ff ff ff ff ff 27 bb  ff ff 29 eb 0c 20 10 d8 
> |......'...).. ..|
> 00000050  0f 52 00 00 24 4a 99 00  8b 8e 03 e1 ac 01 27 38 
> |.R..$J........'8|
> 00000060  7a 75 7a 75 fb bd d5 5c  4a 0f 82 ff 81 bd 3d 36 
> |zuzu...\J.....=6|
> 00000070  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff 
> |................|
> 00000080  ff e7 ff ff 21 dc ff ff                           |....!...|
> 00000088
> 
> Do we need a fixup to tell the erase die to be faster?

no. We can retrieve the erase time from BFPT and use it to narrow the
wait time.
> 
> I haven't found the 'BFPT' term in the JEDEC spec.
> 
>> real    0m5.485s
>> user    0m0.000s
>> sys    0m5.461s
> 
> Before erasing, did the flash contain 128MB of random data on your test?

not everywhere

> 
> On my tests, when the flash contains 128MB of random data, the first erase
> takes 4 minutes. Subsequent erases take only 2 seconds.
> 

I confirm I see the same behavior on n25q00 flash.
Erased 134217728 bytes from address 0x00000000 in flash

real	3m49.082s
user	0m0.000s
sys	3m48.485s

I'll continue to investigate this at a best effort rate, expect for some
delays, I'm handling SPI NOR mostly in my spare time. Happy to guide you
though. You may join #mtd irc channel for short questions, see where it
is in the MAINTAINERS file.
Fabio Estevam Nov. 3, 2023, 2:58 p.m. UTC | #24
On 03/11/2023 11:37, Tudor Ambarus wrote:

> I confirm I see the same behavior on n25q00 flash.
> Erased 134217728 bytes from address 0x00000000 in flash
> 
> real	3m49.082s
> user	0m0.000s
> sys	3m48.485s
> 
> I'll continue to investigate this at a best effort rate, expect for 
> some
> delays, I'm handling SPI NOR mostly in my spare time. Happy to guide 
> you
> though. You may join #mtd irc channel for short questions, see where it
> is in the MAINTAINERS file.

Thanks, Tudor.

Reading the datasheet at:
https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_01g_bbb_0.pdf

The last entry of Table 49 shows:

512Mb bulk erase time: Typ: 153 s Max: 460 s

512Mb corresponds to the size of one die.

mt25qu01g has two dies so the bulk erase time would be from 306 to 920 s 
(5 to 15 minutes!)

It seems that there is not much we can do about this long erase time.

Is my understanding correct?
Michael Walle Nov. 6, 2023, 9:34 a.m. UTC | #25
Am 2023-11-03 14:48, schrieb Tudor Ambarus:
> On 03.11.2023 15:37, Fabio Estevam wrote:
>> On 03/11/2023 10:26, Tudor Ambarus wrote:
>> 
>>> Which version of mtd-utils are you using? I guess the flash-erase
>> 
>> mtd-utils 2.1.5
>> 
>>> utility is written in a bad way. Please use the following while I 
>>> check
>>> what flash_erase is doing:
>>> 
>>> time mtd_debug erase /dev/mtd0 0 134217728
>> 
>> "mtd_debug erase" gives the same time as well:
>> 
>> root@mcde3000a:~# time mtd_debug erase /dev/mtd0 0 134217728
>> [ 4322.114967] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>> [ 4322.120861] spi-nor spi0.0: *****
>> [ 4322.124210] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>> [ 4322.129478] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>> [ 4322.134903] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>> [ 4322.140154] spi-nor spi0.0: *****
>> [ 4322.143491] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>> [ 4322.148831] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>> [ 4322.154341] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
>> [ 4322.159761] spi-nor spi0.0: *****
>> [ 4322.163098] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>> [ 4322.168524] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>> [ 4322.174118] spi-nor spi0.0: *****
>> [ 4322.177439] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>> [ 4322.182948] spi-nor spi0.0: ***** op.data.nbytes = 0
>> [ 4439.966060] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>> [ 4439.971920] spi-nor spi0.0: *****
>> [ 4439.975252] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>> [ 4439.980511] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>> [ 4439.985928] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>> [ 4439.991174] spi-nor spi0.0: *****
>> [ 4439.994504] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>> [ 4439.999834] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>> [ 4440.005335] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
>> [ 4440.011272] spi-nor spi0.0: *****
>> [ 4440.014604] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>> [ 4440.020018] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>> [ 4440.025606] spi-nor spi0.0: *****
>> [ 4440.028937] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>> [ 4440.034438] spi-nor spi0.0: ***** op.data.nbytes = 0
>> Erased 134217728 bytes from address 0x00000000 in flash
>> 
>> real    3m57.384s
>> user    0m0.005s
>> sys    3m35.211s
>> 
> 
> Yep, it's strange, we'll have to check what's happening. I found my
> n25q00 flash, on my side all its 4 dice are erased in 5 sec.  SFDP
> defines how long the erase die should take, see BFPT dword 11. You can
> start with that.

Had the flash some contents or was it all-ff? Maybe the Micron flash 
will
check if all bytes are one and will skip the erase.

Die/Chip erases will take much longer most of the time and are 
comparable
to individual sector erases (as Fabio also found out). You'll probably
just save the overhead of the indivudal commands.

I've looked at the N25Q00AA datasheet and the erase time there is 153s
(typ) for *one* die.

-michael
Tudor Ambarus Nov. 6, 2023, 2:23 p.m. UTC | #26
On 11/6/23 09:34, Michael Walle wrote:
> Am 2023-11-03 14:48, schrieb Tudor Ambarus:
>> On 03.11.2023 15:37, Fabio Estevam wrote:
>>> On 03/11/2023 10:26, Tudor Ambarus wrote:
>>>
>>>> Which version of mtd-utils are you using? I guess the flash-erase
>>>
>>> mtd-utils 2.1.5
>>>
>>>> utility is written in a bad way. Please use the following while I check
>>>> what flash_erase is doing:
>>>>
>>>> time mtd_debug erase /dev/mtd0 0 134217728
>>>
>>> "mtd_debug erase" gives the same time as well:
>>>
>>> root@mcde3000a:~# time mtd_debug erase /dev/mtd0 0 134217728
>>> [ 4322.114967] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>>> [ 4322.120861] spi-nor spi0.0: *****
>>> [ 4322.124210] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>>> [ 4322.129478] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>>> [ 4322.134903] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>>> [ 4322.140154] spi-nor spi0.0: *****
>>> [ 4322.143491] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>>> [ 4322.148831] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>>> [ 4322.154341] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
>>> [ 4322.159761] spi-nor spi0.0: *****
>>> [ 4322.163098] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>>> [ 4322.168524] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>>> [ 4322.174118] spi-nor spi0.0: *****
>>> [ 4322.177439] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>>> [ 4322.182948] spi-nor spi0.0: ***** op.data.nbytes = 0
>>> [ 4439.966060] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>>> [ 4439.971920] spi-nor spi0.0: *****
>>> [ 4439.975252] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>>> [ 4439.980511] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>>> [ 4439.985928] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>>> [ 4439.991174] spi-nor spi0.0: *****
>>> [ 4439.994504] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>>> [ 4439.999834] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>>> [ 4440.005335] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
>>> [ 4440.011272] spi-nor spi0.0: *****
>>> [ 4440.014604] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>>> [ 4440.020018] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>>> [ 4440.025606] spi-nor spi0.0: *****
>>> [ 4440.028937] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>>> [ 4440.034438] spi-nor spi0.0: ***** op.data.nbytes = 0
>>> Erased 134217728 bytes from address 0x00000000 in flash
>>>
>>> real    3m57.384s
>>> user    0m0.005s
>>> sys    3m35.211s
>>>
>>
>> Yep, it's strange, we'll have to check what's happening. I found my
>> n25q00 flash, on my side all its 4 dice are erased in 5 sec.  SFDP
>> defines how long the erase die should take, see BFPT dword 11. You can
>> start with that.
> 
> Had the flash some contents or was it all-ff? Maybe the Micron flash will
> check if all bytes are one and will skip the erase.

it had some contents, but not different than 0xff
> 
> Die/Chip erases will take much longer most of the time and are comparable
> to individual sector erases (as Fabio also found out). You'll probably
> just save the overhead of the indivudal commands.

There is a speed benefit in using die erase instead of individual sector
erases.
> 
> I've looked at the N25Q00AA datasheet and the erase time there is 153s
> (typ) for *one* die.
> 

you mean mt25q. Table 49 in
https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_01g_bbb_0.pdf


Each die has 64MB. A die is composed of either 16384 4KB sectors or 2048
sectors of 32KB.

4KB typical erase time is 0.05s, thus a die will be erased in 819.2s.
32KB typical erase time is 0.1s, thus a die will be erased in 204.8s.
die erase typical erase time is 153s.

4K max erase time is 0.4s, thus a die will be erased in 6553.6s
32KB max erase time is 1, thus a die will be erased in 2048s.
die erase max time is 460s.

so you might say that 32KB typical erase time might be comparable to a
die erase command when erasing an entire die with 32KB erases, but even
so, it should be preferable to use die erase cmd. Instead of sending a
write enable followed by a sector erase command for each sector, you
could instead use a single write enable followed by a single die erase
command.

ta
Tudor Ambarus Nov. 6, 2023, 2:24 p.m. UTC | #27
On 11/3/23 14:58, Fabio Estevam wrote:
> On 03/11/2023 11:37, Tudor Ambarus wrote:
> 
>> I confirm I see the same behavior on n25q00 flash.
>> Erased 134217728 bytes from address 0x00000000 in flash
>>
>> real    3m49.082s
>> user    0m0.000s
>> sys    3m48.485s
>>
>> I'll continue to investigate this at a best effort rate, expect for some
>> delays, I'm handling SPI NOR mostly in my spare time. Happy to guide you
>> though. You may join #mtd irc channel for short questions, see where it
>> is in the MAINTAINERS file.
> 
> Thanks, Tudor.
> 
> Reading the datasheet at:
> https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_01g_bbb_0.pdf
> 
> The last entry of Table 49 shows:
> 
> 512Mb bulk erase time: Typ: 153 s Max: 460 s
> 
> 512Mb corresponds to the size of one die.
> 
> mt25qu01g has two dies so the bulk erase time would be from 306 to 920 s
> (5 to 15 minutes!)
> 
> It seems that there is not much we can do about this long erase time.
> 
> Is my understanding correct?

Yes, I find your understanding correct.
Tudor Ambarus Nov. 6, 2023, 2:56 p.m. UTC | #28
On 11/6/23 14:23, Tudor Ambarus wrote:
> it had some contents, but not different than 0xff
*not all different*
Takahiro Kuwano Nov. 8, 2023, 8:06 a.m. UTC | #29
Hi,

On 11/3/2023 3:46 AM, Tudor Ambarus wrote:
> 
> 
> On 11/2/23 18:33, Fabio Estevam wrote:
>> On 02/11/2023 15:21, Tudor Ambarus wrote:
>>
>>> Let's see what gets to the SPI controller. Which SPI controller do you
>>> use?
>>
>> I am using i.MX8MP, which has drivers/spi/spi-nxp-fspi.c.
>>
>> Here is the result:
>>
>> root@mcde3000a:~# flash_erase /dev/mtd0 0 0
>> Erasing 131072 Kibyte @ 0 --  0 [   26.040903] spi-nor spi0.0: *****
>> nor->reg_proto = 0x00010101
>> % complete [   26.049570] spi-nor spi0.0: *****
>> [   26.053849] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>> [   26.059118] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>> [   26.064539] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>> [   26.069787] spi-nor spi0.0: *****
>> [   26.073118] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>> [   26.078451] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>> [   26.083949] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
>> [   26.089368] spi-nor spi0.0: *****
>> [   26.092699] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>> [   26.098123] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>> [   26.103713] spi-nor spi0.0: *****
>> [   26.107045] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>> [   26.112549] spi-nor spi0.0: ***** op.data.nbytes = 0
>> [   26.117727] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>> [   26.123589] spi-nor spi0.0: *****
>> [   26.127012] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>> [   26.132274] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>> [   26.137706] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>> [   26.142956] spi-nor spi0.0: *****
>> [   26.146290] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>> [   26.151625] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>> [   26.157132] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
>> [   26.163065] spi-nor spi0.0: *****
>> [   26.166402] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>> [   26.171815] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>> [   26.177405] spi-nor spi0.0: *****
>> [   26.180737] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>> [   26.186241] spi-nor spi0.0: ***** op.data.nbytes = 0
>> Erasing 131072 Kibyte @ 0 -- 100 % complete
>>
> 
> It looks good ...
> 
>> root@mcde3000a:~# hexdump /dev/mtd0
>> 0000000 a1d4 168c 4dad dfb2 2a3d c2af 0aae c18a
>> 0000010 2d5f 177a c39f 46a4 f9cd b880 331e 2543
> 
> but the patient is dead :). Would be good if Takahiro can test on IFX
> too. In the meantime I'll try to find a n25q00, I remember I had one in
> the past.
> 
With your spi-nor/next-die-erase-v2-debug in linux-0day, die erase for IFX part
(s25hs02gt) is working. Here is the log.

1st die:

zynq> time mtd_debug erase /dev/mtd0 0 0x8000000
spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
spi-nor spi0.0: ***** op.cmd.opcode = 0x61
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.addr.nbytes = 0x04
spi-nor spi0.0: ***** op.addr.buswidth = 0x01
spi-nor spi0.0: ***** op.addr.buswidth = 0x0
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.data.buswidth = 0x00
spi-nor spi0.0: ***** op.data.nbytes = 0
Erased 134217728 bytes from address 0x00000000 in flash
real    7m 47.92s
user    0m 0.00s
sys     7m 47.92s


2nd die:

zynq> time mtd_debug erase /dev/mtd0 0x8000000 0x8000000
spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
spi-nor spi0.0: ***** op.cmd.opcode = 0x61
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.addr.nbytes = 0x04
spi-nor spi0.0: ***** op.addr.buswidth = 0x01
spi-nor spi0.0: ***** op.addr.buswidth = 0x8000000
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.data.buswidth = 0x00
spi-nor spi0.0: ***** op.data.nbytes = 0
Erased 134217728 bytes from address 0x08000000 in flash
real    7m 44.97s
user    0m 0.00s
sys     7m 44.96s


Both dice at once:

zynq> time mtd_debug erase /dev/mtd0 0x0 0x10000000
spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
spi-nor spi0.0: ***** op.cmd.opcode = 0x61
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.addr.nbytes = 0x04
spi-nor spi0.0: ***** op.addr.buswidth = 0x01
spi-nor spi0.0: ***** op.addr.buswidth = 0x0
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.data.buswidth = 0x00
spi-nor spi0.0: ***** op.data.nbytes = 0
spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
spi-nor spi0.0: ***** op.cmd.opcode = 0x61
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.addr.nbytes = 0x04
spi-nor spi0.0: ***** op.addr.buswidth = 0x01
spi-nor spi0.0: ***** op.addr.buswidth = 0x8000000
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.data.buswidth = 0x00
spi-nor spi0.0: ***** op.data.nbytes = 0
Erased 268435456 bytes from address 0x00000000 in flash
real    15m 44.17s
user    0m 0.00s
sys     15m 44.15s


Infineon SEMPER flash family has 'blank check' feature that skips erase ops
if the sector is already erased. It can be enabled by setting CFR3[5]=1.
After enabling this, the subsequent erase times are reduced.

1st die:

zynq> time mtd_debug erase /dev/mtd0 0 0x8000000
spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
spi-nor spi0.0: ***** op.cmd.opcode = 0x61
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.addr.nbytes = 0x04
spi-nor spi0.0: ***** op.addr.buswidth = 0x01
spi-nor spi0.0: ***** op.addr.buswidth = 0x0
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.data.buswidth = 0x00
spi-nor spi0.0: ***** op.data.nbytes = 0
Erased 134217728 bytes from address 0x00000000 in flash
real    0m 6.84s
user    0m 0.00s
sys     0m 6.84s


2nd die:

zynq> time mtd_debug erase /dev/mtd0 0x8000000 0x8000000
spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
spi-nor spi0.0: ***** op.cmd.opcode = 0x61
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.addr.nbytes = 0x04
spi-nor spi0.0: ***** op.addr.buswidth = 0x01
spi-nor spi0.0: ***** op.addr.buswidth = 0x8000000
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.data.buswidth = 0x00
spi-nor spi0.0: ***** op.data.nbytes = 0
Erased 134217728 bytes from address 0x08000000 in flash
real    0m 6.70s
user    0m 0.00s
sys     0m 6.70s


Both dice at once:

zynq> time mtd_debug erase /dev/mtd0 0 0x10000000
spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
spi-nor spi0.0: ***** op.cmd.opcode = 0x61
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.addr.nbytes = 0x04
spi-nor spi0.0: ***** op.addr.buswidth = 0x01
spi-nor spi0.0: ***** op.addr.buswidth = 0x0
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.data.buswidth = 0x00
spi-nor spi0.0: ***** op.data.nbytes = 0
spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
spi-nor spi0.0: ***** op.cmd.opcode = 0x61
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.addr.nbytes = 0x04
spi-nor spi0.0: ***** op.addr.buswidth = 0x01
spi-nor spi0.0: ***** op.addr.buswidth = 0x8000000
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
spi-nor spi0.0: *****
spi-nor spi0.0: ***** op.data.buswidth = 0x00
spi-nor spi0.0: ***** op.data.nbytes = 0
Erased 268435456 bytes from address 0x00000000 in flash
real    0m 13.54s
user    0m 0.00s
sys     0m 13.54s

Thanks,
Takahiro
Tudor Ambarus Nov. 8, 2023, 8:54 a.m. UTC | #30
On 11/8/23 08:06, Takahiro Kuwano wrote:
> Hi,
> 
> On 11/3/2023 3:46 AM, Tudor Ambarus wrote:
>>
>>
>> On 11/2/23 18:33, Fabio Estevam wrote:
>>> On 02/11/2023 15:21, Tudor Ambarus wrote:
>>>
>>>> Let's see what gets to the SPI controller. Which SPI controller do you
>>>> use?
>>>
>>> I am using i.MX8MP, which has drivers/spi/spi-nxp-fspi.c.
>>>
>>> Here is the result:
>>>
>>> root@mcde3000a:~# flash_erase /dev/mtd0 0 0
>>> Erasing 131072 Kibyte @ 0 --  0 [   26.040903] spi-nor spi0.0: *****
>>> nor->reg_proto = 0x00010101
>>> % complete [   26.049570] spi-nor spi0.0: *****
>>> [   26.053849] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>>> [   26.059118] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>>> [   26.064539] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>>> [   26.069787] spi-nor spi0.0: *****
>>> [   26.073118] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>>> [   26.078451] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>>> [   26.083949] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
>>> [   26.089368] spi-nor spi0.0: *****
>>> [   26.092699] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>>> [   26.098123] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>>> [   26.103713] spi-nor spi0.0: *****
>>> [   26.107045] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>>> [   26.112549] spi-nor spi0.0: ***** op.data.nbytes = 0
>>> [   26.117727] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>>> [   26.123589] spi-nor spi0.0: *****
>>> [   26.127012] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>>> [   26.132274] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>>> [   26.137706] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>>> [   26.142956] spi-nor spi0.0: *****
>>> [   26.146290] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>>> [   26.151625] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>>> [   26.157132] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
>>> [   26.163065] spi-nor spi0.0: *****
>>> [   26.166402] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>>> [   26.171815] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>>> [   26.177405] spi-nor spi0.0: *****
>>> [   26.180737] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>>> [   26.186241] spi-nor spi0.0: ***** op.data.nbytes = 0
>>> Erasing 131072 Kibyte @ 0 -- 100 % complete
>>>
>>
>> It looks good ...
>>
>>> root@mcde3000a:~# hexdump /dev/mtd0
>>> 0000000 a1d4 168c 4dad dfb2 2a3d c2af 0aae c18a
>>> 0000010 2d5f 177a c39f 46a4 f9cd b880 331e 2543
>>
>> but the patient is dead :). Would be good if Takahiro can test on IFX
>> too. In the meantime I'll try to find a n25q00, I remember I had one in
>> the past.
>>
> With your spi-nor/next-die-erase-v2-debug in linux-0day, die erase for IFX part
> (s25hs02gt) is working. Here is the log.
> 
> 1st die:
> 
> zynq> time mtd_debug erase /dev/mtd0 0 0x8000000
> spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> spi-nor spi0.0: ***** op.cmd.opcode = 0x61
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> spi-nor spi0.0: ***** op.addr.buswidth = 0x0
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.data.buswidth = 0x00
> spi-nor spi0.0: ***** op.data.nbytes = 0
> Erased 134217728 bytes from address 0x00000000 in flash
> real    7m 47.92s
> user    0m 0.00s
> sys     7m 47.92s
> 
> 
> 2nd die:
> 
> zynq> time mtd_debug erase /dev/mtd0 0x8000000 0x8000000
> spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> spi-nor spi0.0: ***** op.cmd.opcode = 0x61
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> spi-nor spi0.0: ***** op.addr.buswidth = 0x8000000
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.data.buswidth = 0x00
> spi-nor spi0.0: ***** op.data.nbytes = 0
> Erased 134217728 bytes from address 0x08000000 in flash
> real    7m 44.97s
> user    0m 0.00s
> sys     7m 44.96s
> 
> 
> Both dice at once:
> 
> zynq> time mtd_debug erase /dev/mtd0 0x0 0x10000000
> spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> spi-nor spi0.0: ***** op.cmd.opcode = 0x61
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> spi-nor spi0.0: ***** op.addr.buswidth = 0x0
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.data.buswidth = 0x00
> spi-nor spi0.0: ***** op.data.nbytes = 0
> spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> spi-nor spi0.0: ***** op.cmd.opcode = 0x61
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> spi-nor spi0.0: ***** op.addr.buswidth = 0x8000000
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.data.buswidth = 0x00
> spi-nor spi0.0: ***** op.data.nbytes = 0
> Erased 268435456 bytes from address 0x00000000 in flash
> real    15m 44.17s
> user    0m 0.00s
> sys     15m 44.15s
> 

great

> 
> Infineon SEMPER flash family has 'blank check' feature that skips erase ops
> if the sector is already erased. It can be enabled by setting CFR3[5]=1.
> After enabling this, the subsequent erase times are reduced.
> 
> 1st die:
> 
> zynq> time mtd_debug erase /dev/mtd0 0 0x8000000
> spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> spi-nor spi0.0: ***** op.cmd.opcode = 0x61
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> spi-nor spi0.0: ***** op.addr.buswidth = 0x0
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.data.buswidth = 0x00
> spi-nor spi0.0: ***** op.data.nbytes = 0
> Erased 134217728 bytes from address 0x00000000 in flash
> real    0m 6.84s
> user    0m 0.00s
> sys     0m 6.84s
> 
> 
> 2nd die:
> 
> zynq> time mtd_debug erase /dev/mtd0 0x8000000 0x8000000
> spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> spi-nor spi0.0: ***** op.cmd.opcode = 0x61
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> spi-nor spi0.0: ***** op.addr.buswidth = 0x8000000
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.data.buswidth = 0x00
> spi-nor spi0.0: ***** op.data.nbytes = 0
> Erased 134217728 bytes from address 0x08000000 in flash
> real    0m 6.70s
> user    0m 0.00s
> sys     0m 6.70s
> 
> 
> Both dice at once:
> 
> zynq> time mtd_debug erase /dev/mtd0 0 0x10000000
> spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> spi-nor spi0.0: ***** op.cmd.opcode = 0x61
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> spi-nor spi0.0: ***** op.addr.buswidth = 0x0
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.data.buswidth = 0x00
> spi-nor spi0.0: ***** op.data.nbytes = 0
> spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
> spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
> spi-nor spi0.0: ***** op.cmd.opcode = 0x61
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.addr.nbytes = 0x04
> spi-nor spi0.0: ***** op.addr.buswidth = 0x01
> spi-nor spi0.0: ***** op.addr.buswidth = 0x8000000
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
> spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
> spi-nor spi0.0: *****
> spi-nor spi0.0: ***** op.data.buswidth = 0x00
> spi-nor spi0.0: ***** op.data.nbytes = 0
> Erased 268435456 bytes from address 0x00000000 in flash
> real    0m 13.54s
> user    0m 0.00s
> sys     0m 13.54s
> 

wonderful, thanks Takahiro! I'll send a v3.

Cheers,
ta
Michael Walle Nov. 9, 2023, 9:09 a.m. UTC | #31
Am 2023-11-06 15:23, schrieb Tudor Ambarus:
> On 11/6/23 09:34, Michael Walle wrote:
>> Am 2023-11-03 14:48, schrieb Tudor Ambarus:
>>> On 03.11.2023 15:37, Fabio Estevam wrote:
>>>> On 03/11/2023 10:26, Tudor Ambarus wrote:
>>>> 
>>>>> Which version of mtd-utils are you using? I guess the flash-erase
>>>> 
>>>> mtd-utils 2.1.5
>>>> 
>>>>> utility is written in a bad way. Please use the following while I 
>>>>> check
>>>>> what flash_erase is doing:
>>>>> 
>>>>> time mtd_debug erase /dev/mtd0 0 134217728
>>>> 
>>>> "mtd_debug erase" gives the same time as well:
>>>> 
>>>> root@mcde3000a:~# time mtd_debug erase /dev/mtd0 0 134217728
>>>> [ 4322.114967] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>>>> [ 4322.120861] spi-nor spi0.0: *****
>>>> [ 4322.124210] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>>>> [ 4322.129478] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>>>> [ 4322.134903] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>>>> [ 4322.140154] spi-nor spi0.0: *****
>>>> [ 4322.143491] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>>>> [ 4322.148831] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>>>> [ 4322.154341] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
>>>> [ 4322.159761] spi-nor spi0.0: *****
>>>> [ 4322.163098] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>>>> [ 4322.168524] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>>>> [ 4322.174118] spi-nor spi0.0: *****
>>>> [ 4322.177439] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>>>> [ 4322.182948] spi-nor spi0.0: ***** op.data.nbytes = 0
>>>> [ 4439.966060] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>>>> [ 4439.971920] spi-nor spi0.0: *****
>>>> [ 4439.975252] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>>>> [ 4439.980511] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>>>> [ 4439.985928] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>>>> [ 4439.991174] spi-nor spi0.0: *****
>>>> [ 4439.994504] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>>>> [ 4439.999834] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>>>> [ 4440.005335] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
>>>> [ 4440.011272] spi-nor spi0.0: *****
>>>> [ 4440.014604] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>>>> [ 4440.020018] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>>>> [ 4440.025606] spi-nor spi0.0: *****
>>>> [ 4440.028937] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>>>> [ 4440.034438] spi-nor spi0.0: ***** op.data.nbytes = 0
>>>> Erased 134217728 bytes from address 0x00000000 in flash
>>>> 
>>>> real    3m57.384s
>>>> user    0m0.005s
>>>> sys    3m35.211s
>>>> 
>>> 
>>> Yep, it's strange, we'll have to check what's happening. I found my
>>> n25q00 flash, on my side all its 4 dice are erased in 5 sec.  SFDP
>>> defines how long the erase die should take, see BFPT dword 11. You 
>>> can
>>> start with that.
>> 
>> Had the flash some contents or was it all-ff? Maybe the Micron flash 
>> will
>> check if all bytes are one and will skip the erase.
> 
> it had some contents, but not different than 0xff
>> 
>> Die/Chip erases will take much longer most of the time and are 
>> comparable
>> to individual sector erases (as Fabio also found out). You'll probably
>> just save the overhead of the indivudal commands.
> 
> There is a speed benefit in using die erase instead of individual 
> sector
> erases.
>> 
>> I've looked at the N25Q00AA datasheet and the erase time there is 153s
>> (typ) for *one* die.
>> 
> 
> you mean mt25q. Table 49 in
> https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_01g_bbb_0.pdf
> 
> 
> Each die has 64MB. A die is composed of either 16384 4KB sectors or 
> 2048
> sectors of 32KB.
> 
> 4KB typical erase time is 0.05s, thus a die will be erased in 819.2s.
> 32KB typical erase time is 0.1s, thus a die will be erased in 204.8s.
> die erase typical erase time is 153s.
> 
> 4K max erase time is 0.4s, thus a die will be erased in 6553.6s
> 32KB max erase time is 1, thus a die will be erased in 2048s.
> die erase max time is 460s.
> 
> so you might say that 32KB typical erase time might be comparable to a
> die erase command when erasing an entire die with 32KB erases, but even
> so, it should be preferable to use die erase cmd. Instead of sending a
> write enable followed by a sector erase command for each sector, you
> could instead use a single write enable followed by a single die erase
> command.

I was just implying that your 5s chip erase time sounded unlikely.

I'm also still undecided on the use of a chip/die erase command. How
often is a flash erased entirely? I don't think very often, maybe during
board manufacturing. And is that worth the (maintenance) efforts?

Anyway, I won't stand in the way if you insist.

Nobody commented on that so far: The jedec spec doesn't say anything
about the chip/die erase command, right? There is a flag which
indicates wether the chip has one (in 8d8d8d mode) and there is a field
for its erase time, but not *what* the actual command is. Such a pity,
esp. because the die erase now differs among vendors.. whereas the chip
erase command seems to be common among vendors.

Btw. if we want to speed the erase up we should make use of the erase
regions. AFAIK, at the moment we do (slow) 4k erases most of the time
because distros have this kernel option enabled.

-michael
Tudor Ambarus Nov. 15, 2023, 7:06 a.m. UTC | #32
On 09.11.2023 11:09, Michael Walle wrote:
> Am 2023-11-06 15:23, schrieb Tudor Ambarus:
>> On 11/6/23 09:34, Michael Walle wrote:
>>> Am 2023-11-03 14:48, schrieb Tudor Ambarus:
>>>> On 03.11.2023 15:37, Fabio Estevam wrote:
>>>>> On 03/11/2023 10:26, Tudor Ambarus wrote:
>>>>>
>>>>>> Which version of mtd-utils are you using? I guess the flash-erase
>>>>>
>>>>> mtd-utils 2.1.5
>>>>>
>>>>>> utility is written in a bad way. Please use the following while I
>>>>>> check
>>>>>> what flash_erase is doing:
>>>>>>
>>>>>> time mtd_debug erase /dev/mtd0 0 134217728
>>>>>
>>>>> "mtd_debug erase" gives the same time as well:
>>>>>
>>>>> root@mcde3000a:~# time mtd_debug erase /dev/mtd0 0 134217728
>>>>> [ 4322.114967] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>>>>> [ 4322.120861] spi-nor spi0.0: *****
>>>>> [ 4322.124210] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>>>>> [ 4322.129478] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>>>>> [ 4322.134903] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>>>>> [ 4322.140154] spi-nor spi0.0: *****
>>>>> [ 4322.143491] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>>>>> [ 4322.148831] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>>>>> [ 4322.154341] spi-nor spi0.0: ***** op.addr.buswidth = 0x0
>>>>> [ 4322.159761] spi-nor spi0.0: *****
>>>>> [ 4322.163098] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>>>>> [ 4322.168524] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>>>>> [ 4322.174118] spi-nor spi0.0: *****
>>>>> [ 4322.177439] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>>>>> [ 4322.182948] spi-nor spi0.0: ***** op.data.nbytes = 0
>>>>> [ 4439.966060] spi-nor spi0.0: ***** nor->reg_proto = 0x00010101
>>>>> [ 4439.971920] spi-nor spi0.0: *****
>>>>> [ 4439.975252] spi-nor spi0.0: ***** op.cmd.nbytes = 0x01
>>>>> [ 4439.980511] spi-nor spi0.0: ***** op.cmd.buswidth = 0x01
>>>>> [ 4439.985928] spi-nor spi0.0: ***** op.cmd.opcode = 0xc4
>>>>> [ 4439.991174] spi-nor spi0.0: *****
>>>>> [ 4439.994504] spi-nor spi0.0: ***** op.addr.nbytes = 0x04
>>>>> [ 4439.999834] spi-nor spi0.0: ***** op.addr.buswidth = 0x01
>>>>> [ 4440.005335] spi-nor spi0.0: ***** op.addr.buswidth = 0x4000000
>>>>> [ 4440.011272] spi-nor spi0.0: *****
>>>>> [ 4440.014604] spi-nor spi0.0: ***** op.dummy.nbytes = 0x00
>>>>> [ 4440.020018] spi-nor spi0.0: ***** op.dummy.buswidth = 0x00
>>>>> [ 4440.025606] spi-nor spi0.0: *****
>>>>> [ 4440.028937] spi-nor spi0.0: ***** op.data.buswidth = 0x00
>>>>> [ 4440.034438] spi-nor spi0.0: ***** op.data.nbytes = 0
>>>>> Erased 134217728 bytes from address 0x00000000 in flash
>>>>>
>>>>> real    3m57.384s
>>>>> user    0m0.005s
>>>>> sys    3m35.211s
>>>>>
>>>>
>>>> Yep, it's strange, we'll have to check what's happening. I found my
>>>> n25q00 flash, on my side all its 4 dice are erased in 5 sec.  SFDP
>>>> defines how long the erase die should take, see BFPT dword 11. You can
>>>> start with that.
>>>
>>> Had the flash some contents or was it all-ff? Maybe the Micron flash
>>> will
>>> check if all bytes are one and will skip the erase.
>>
>> it had some contents, but not different than 0xff
>>>
>>> Die/Chip erases will take much longer most of the time and are
>>> comparable
>>> to individual sector erases (as Fabio also found out). You'll probably
>>> just save the overhead of the indivudal commands.
>>
>> There is a speed benefit in using die erase instead of individual sector
>> erases.
>>>
>>> I've looked at the N25Q00AA datasheet and the erase time there is 153s
>>> (typ) for *one* die.
>>>
>>
>> you mean mt25q. Table 49 in
>> https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_01g_bbb_0.pdf
>>
>>
>> Each die has 64MB. A die is composed of either 16384 4KB sectors or 2048
>> sectors of 32KB.
>>
>> 4KB typical erase time is 0.05s, thus a die will be erased in 819.2s.
>> 32KB typical erase time is 0.1s, thus a die will be erased in 204.8s.
>> die erase typical erase time is 153s.
>>
>> 4K max erase time is 0.4s, thus a die will be erased in 6553.6s
>> 32KB max erase time is 1, thus a die will be erased in 2048s.
>> die erase max time is 460s.
>>
>> so you might say that 32KB typical erase time might be comparable to a
>> die erase command when erasing an entire die with 32KB erases, but even
>> so, it should be preferable to use die erase cmd. Instead of sending a
>> write enable followed by a sector erase command for each sector, you
>> could instead use a single write enable followed by a single die erase
>> command.
> 
> I was just implying that your 5s chip erase time sounded unlikely.
> 
> I'm also still undecided on the use of a chip/die erase command. How
> often is a flash erased entirely? I don't think very often, maybe during
> board manufacturing. And is that worth the (maintenance) efforts?
> 
> Anyway, I won't stand in the way if you insist.

I don't insist, but I feel it is worth it, yes. We have some speed
improvement and we should benefit of it.

> 
> Nobody commented on that so far: The jedec spec doesn't say anything
> about the chip/die erase command, right? There is a flag which
> indicates wether the chip has one (in 8d8d8d mode) and there is a field
> for its erase time, but not *what* the actual command is. Such a pity,

jesd216 mentions die erase, but does not provide an opcode for it. Check
BFPT dword 11, bits 30:24, "Chip Erase, Typical time", it says:

"Typical time to erase one chip (die). User must poll device busy to
determine if the operation has completed. For a device consisting of
multiple dies, that are individually accessed, the time is for each die
to which a chip erase command is applied."

So when a flash consists of a single die, this is the erase time for the
full chip (die) erase, and when it consists of multiple dies, it's the
die erase time. Chip and die are the same thing.

> esp. because the die erase now differs among vendors.. whereas the chip
> erase command seems to be common among vendors.

Indeed we saw that the die erase opcode differs among vendors, nothing
that we can do about it now. But we can handle it in software, no big deal.

> 
> Btw. if we want to speed the erase up we should make use of the erase
> regions. AFAIK, at the moment we do (slow) 4k erases most of the time
> because distros have this kernel option enabled.
> 

yes, I know, there were some attempts and the main concern was how it
will cope with what filesystems are requiring. Anyway, different topic
that needs some thought.

Cheers,
ta
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 25a64c65717d..ac2651e76285 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1065,19 +1065,25 @@  static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int spi_nor_erase_chip(struct spi_nor *nor)
+static int spi_nor_erase_die(struct spi_nor *nor, loff_t addr, size_t die_size)
 {
+	bool multi_die = nor->mtd.size == die_size;
 	int ret;
 
-	dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
+	dev_dbg(nor->dev, " %lldKiB\n", (long long)(die_size >> 10));
 
 	if (nor->spimem) {
-		struct spi_mem_op op = SPI_NOR_CHIP_ERASE_OP;
+		struct spi_mem_op op =
+			SPI_NOR_DIE_ERASE_OP(nor->params->die_erase_opcode,
+					     nor->addr_nbytes, addr, multi_die);
 
 		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
 
 		ret = spi_mem_exec_op(nor->spimem, &op);
 	} else {
+		if (multi_die)
+			return -EOPNOTSUPP;
+
 		ret = spi_nor_controller_ops_write_reg(nor,
 						       SPINOR_OP_CHIP_ERASE,
 						       NULL, 0);
@@ -1792,6 +1798,51 @@  static int spi_nor_erase_multi_sectors(struct spi_nor *nor, u64 addr, u32 len)
 	return ret;
 }
 
+static int spi_nor_erase_dice(struct spi_nor *nor, loff_t addr,
+			      size_t len, size_t die_size)
+{
+	unsigned long timeout;
+	int ret;
+
+	/*
+	 * Scale the timeout linearly with the size of the flash, with
+	 * a minimum calibrated to an old 2MB flash. We could try to
+	 * pull these from CFI/SFDP, but these values should be good
+	 * enough for now.
+	 */
+	timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES,
+		      CHIP_ERASE_2MB_READY_WAIT_JIFFIES *
+		      (unsigned long)(nor->mtd.size / SZ_2M));
+
+	do {
+		ret = spi_nor_lock_device(nor);
+		if (ret)
+			return ret;
+
+		ret = spi_nor_write_enable(nor);
+		if (ret) {
+			spi_nor_unlock_device(nor);
+			return ret;
+		}
+
+		ret = spi_nor_erase_die(nor, addr, die_size);
+
+		spi_nor_unlock_device(nor);
+		if (ret)
+			return ret;
+
+		ret = spi_nor_wait_till_ready_with_timeout(nor, timeout);
+		if (ret)
+			return ret;
+
+		addr += die_size;
+		len -= die_size;
+
+	} while (len);
+
+	return 0;
+}
+
 /*
  * Erase an address range on the nor chip.  The address range may extend
  * one or more erase sectors. Return an error if there is a problem erasing.
@@ -1799,7 +1850,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 multi_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,39 +1868,22 @@  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)))
+			multi_die_erase = true;
+	} else {
+		die_size = mtd->size;
+	}
+
 	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)) {
-		unsigned long timeout;
-
-		ret = spi_nor_lock_device(nor);
-		if (ret)
-			goto erase_err;
-
-		ret = spi_nor_write_enable(nor);
-		if (ret) {
-			spi_nor_unlock_device(nor);
-			goto erase_err;
-		}
-
-		ret = spi_nor_erase_chip(nor);
-		spi_nor_unlock_device(nor);
-		if (ret)
-			goto erase_err;
-
-		/*
-		 * Scale the timeout linearly with the size of the flash, with
-		 * a minimum calibrated to an old 2MB flash. We could try to
-		 * pull these from CFI/SFDP, but these values should be good
-		 * enough for now.
-		 */
-		timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES,
-			      CHIP_ERASE_2MB_READY_WAIT_JIFFIES *
-			      (unsigned long)(mtd->size / SZ_2M));
-		ret = spi_nor_wait_till_ready_with_timeout(nor, timeout);
+	/* chip (die) erase? */
+	if ((len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) ||
+	    multi_die_erase) {
+		ret = spi_nor_erase_dice(nor, addr, len, die_size);
 		if (ret)
 			goto erase_err;
 
@@ -2902,6 +2939,9 @@  static int spi_nor_late_init_params(struct spi_nor *nor)
 			return ret;
 	}
 
+	if (!nor->params->die_erase_opcode)
+		nor->params->die_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..b43ea2d49e74 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -85,9 +85,9 @@ 
 		   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),		\
-		   SPI_MEM_OP_NO_ADDR,					\
+#define SPI_NOR_DIE_ERASE_OP(opcode, addr_nbytes, addr, dice)		\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 0),				\
+		   SPI_MEM_OP_ADDR(dice ? addr_nbytes : 0, addr, 0),	\
 		   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.
+ * @die_erase_opcode:	die erase opcode. Defaults to SPINOR_OP_CHIP_ERASE.
  * @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				die_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..2dbda6b6938a 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->die_erase_opcode, buf);
 	}
 
 	seq_puts(s, "\nsector map\n");