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

[Ye Li]

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 at 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