| Message ID | 1726051530-56280-1-git-send-email-ye.li@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Dario Binacchi |
| Headers | show |
| Series | mtd: spi-nor: Fix chip erase timeout issue | expand |
On 9/11/2024 7:05 PM, Marek Vasut wrote: > On 9/11/24 12:45 PM, 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 to set new timeout for chip erase. > > Is there a matching Linux kernel commit you could point to, from which > this was ported over ? Yes. 09b6a377687b mtd: spi-nor: scale up timeout for full-chip erase > >> Fixes: b91a0822d752 ("mtd: spi-nor: Add CHIP_ERASE optimization") >> >> Signed-off-by: Ye Li <ye.li@nxp.com> >> --- >> 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 aea611fef523..338e5856337e 100644 >> --- a/drivers/mtd/spi/spi-nor-core.c >> +++ b/drivers/mtd/spi/spi-nor-core.c >> @@ -44,6 +44,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 { >> @@ -991,6 +997,7 @@ static int spi_nor_erase(struct mtd_info *mtd, >> struct erase_info *instr) >> bool addr_known = false; >> u32 addr, len, rem; >> int ret, err; >> + unsigned long timeout = 0; > > Keep the variables in reverse xmas-tree please. Will update it in v2. > >> dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr, >> (long long)instr->len); >> @@ -1026,6 +1033,15 @@ 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. >> + */ >> + timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES, >> + CHIP_ERASE_2MB_READY_WAIT_JIFFIES * >> + (unsigned long)(mtd->size / SZ_2M)); > Is the type cast needed ? Yes. otherwise get below warning In file included from include/linux/bitops.h:22, from include/log.h:15, from drivers/mtd/spi/spi-nor-core.c:13: drivers/mtd/spi/spi-nor-core.c: In function ‘spi_nor_erase’: include/linux/kernel.h:190:24: warning: comparison of distinct pointer types lacks a cast 190 | (void) (&_max1 == &_max2); \ | ^~ drivers/mtd/spi/spi-nor-core.c:1042:35: note: in expansion of macro ‘max’ 1042 | timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES, | Best regards, Ye Li > > Thanks
On 9/12/24 10:48 AM, Ye Li wrote: [...] >>> + timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES, >>> + CHIP_ERASE_2MB_READY_WAIT_JIFFIES * >>> + (unsigned long)(mtd->size / SZ_2M)); >> Is the type cast needed ? > > Yes. otherwise get below warning > > > In file included from include/linux/bitops.h:22, > from include/log.h:15, > from drivers/mtd/spi/spi-nor-core.c:13: > drivers/mtd/spi/spi-nor-core.c: In function ‘spi_nor_erase’: > include/linux/kernel.h:190:24: warning: comparison of distinct pointer > types lacks a cast > 190 | (void) (&_max1 == &_max2); \ > | ^~ > drivers/mtd/spi/spi-nor-core.c:1042:35: note: in expansion of macro ‘max’ > 1042 | timeout = > max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES, > | mtd->size is uint64 , so you need to use do_div() : u64 sz = mtd->size; do_div(sz, SZ_2M); timeout = CHIP_ERASE_2MB_READY_WAIT_JIFFIES * max(1ULL, sz);
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index aea611fef523..338e5856337e 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -44,6 +44,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 { @@ -991,6 +997,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) bool addr_known = false; u32 addr, len, rem; int ret, err; + unsigned long timeout = 0; dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr, (long long)instr->len); @@ -1026,6 +1033,15 @@ 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. + */ + timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES, + CHIP_ERASE_2MB_READY_WAIT_JIFFIES * + (unsigned long)(mtd->size / SZ_2M)); } else { ret = spi_nor_erase_sector(nor, addr); } @@ -1035,7 +1051,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); + 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; }
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 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> --- drivers/mtd/spi/spi-nor-core.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)