Re: [PATCH v2] mtd: spi-nor: Fix chip erase timeout issue

On 9/13/24 8:29 AM, Ye Li wrote:

Chip erase support was added to spi_nor_erase, but the timeout for polling SR ready is not updated and still for sector erase. So the timeout value is not enough for chip erase on some NOR flash. Follow kernel implementation 09b6a377687b ("mtd: spi-nor: scale up timeout for full-chip erase") to set new timeout for chip erase.

Fixes: b91a0822d752 ("mtd: spi-nor: Add CHIP_ERASE optimization")

Signed-off-by: Ye Li ye.li@nxp.com

Changes in v2:

• use do_div for u64
• Keep the variables in reverse xmas-tree
• Add kernel commit info

drivers/mtd/spi/spi-nor-core.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index ce01d42cfe0f..83335b235605 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -45,6 +45,12 @@

#define DEFAULT_READY_WAIT_JIFFIES (40UL * HZ)

+/*

• • For full-chip erase, calibrated to a 2MB flash (M25P16); should be scaled up
• • for larger flash
• */

+#define CHIP_ERASE_2MB_READY_WAIT_JIFFIES (40UL * HZ)

• #define ROUND_UP_TO(x, y) (((x) + (y) - 1) / (y) * (y))

  struct sfdp_parameter_header {

@@ -990,7 +996,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr) static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) { struct spi_nor *nor = mtd_to_spi_nor(mtd);

• unsigned long timeout = 0;

Set this to = DEFAULT_READY_WAIT_JIFFIES; instead of = 0;

bool addr_known = false;

• u64 sz = mtd->size; u32 addr, len, rem; int ret, err;

@@ -1028,6 +1036,14 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) if (len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) { ret = spi_nor_erase_chip(nor);

• 	/*
  

• 	 * 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.
  

• 	 */
  

• 	do_div(sz, SZ_2M);
  

• 	timeout = CHIP_ERASE_2MB_READY_WAIT_JIFFIES * max(1ULL, sz);
  

  } else { ret = spi_nor_erase_sector(nor, addr); }

@@ -1037,7 +1053,10 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) addr += ret; len -= ret;

• ret = spi_nor_wait_till_ready(nor);
  

Finally, do a simple:

ret = spi_nor_wait_till_ready_with_timeout(nor, timeout);

here, no need for if/else statement anymore.

• if (timeout)
  

• 	ret = spi_nor_wait_till_ready_with_timeout(nor, timeout);
  

• else
  

• 	ret = spi_nor_wait_till_ready(nor);
  

  if (ret) goto erase_err; }

That should simplify the code I hope.