[PATCH v1] mtd: spi-nor: Specify wait ready time for every chip
Wang Haikun
Haikun.Wang at freescale.com
Mon May 25 03:44:57 PDT 2015
On 5/21/2015 5:44 AM, Brian Norris wrote:
> On Thu, May 14, 2015 at 04:45:52PM +0800, Haikun Wang wrote:
>> Chip erase cycle time is different among different chips.
>> Cycle time is also differnet among different operations for a special chip.
>> So we had better use corresponding timeout value for a specail chip and
>> a special operation when wait chip ready.
>> This patch add the method to specify the timeout value and use them
>> when wait chip ready.
>
> (Might want to spell-check your comments above.)
>
> I can't take this until you show me a real user for this. Right now,
> there is no example demonstrating why you need to make these delays
OK, we have a flash "n25q512a" on our LS2085ARDB board.
Its typical chip erase cycle time is 240s, max is 480s.
m25p80 spi0.0: found n25q512a, expected m25p80
m25p80 spi0.0: n25q512a (65536 Kbytes)
> configurable. And I'm not at all convinced that we need them. If you
> need a few extra seconds for the timeout, we might as well just increase
> it for all chips. We shouldn't be running up against the timeout in
> practice anyway, so this is just an error handling parameter.
I approve of what you said. But if we increase it to 480s, we need wait
480s in case of program a page fail, its max cycle time is less than 1s
usually.
>
>> Signed-off-by: Haikun Wang <haikun.wang at freescale.com>
>> ---
>> drivers/mtd/spi-nor/spi-nor.c | 60 ++++++++++++++++++++++++++++++++-----------
>> include/linux/mtd/spi-nor.h | 4 +++
>> 2 files changed, 49 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 14a5d23..63a7488 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -46,6 +46,14 @@ struct flash_info {
>> u16 page_size;
>> u16 addr_width;
>>
>> + /*
>> + * common_wait, wait ready time applying to most operations .
>> + * max_wait defines the max wait ready time.
>> + * Specified in seconds.
>> + */
>
> These comments are extremely non-specific. You need to be more precise
> than "most". Also, split the comments up to describe each field
> separately, instead of trying to tackle both fields in one comment
> block.
>
> Brian
>
>> + s16 common_wait;
>> + s16 max_wait;
>> +
>> u16 flags;
>> #define SECT_4K 0x01 /* SPINOR_OP_BE_4K works uniformly */
>> #define SPI_NOR_NO_ERASE 0x02 /* No erase command needed */
>> @@ -231,12 +239,15 @@ static int spi_nor_ready(struct spi_nor *nor)
>> * Service routine to read status register until ready, or timeout occurs.
>> * Returns non-zero if error.
>> */
>> -static int spi_nor_wait_till_ready(struct spi_nor *nor)
>> +static int spi_nor_wait_till_ready(struct spi_nor *nor, int wait_time)
>> {
>> unsigned long deadline;
>> int timeout = 0, ret;
>>
>> - deadline = jiffies + MAX_READY_WAIT_JIFFIES;
>> + if (wait_time > 0)
>> + deadline = jiffies + wait_time * HZ;
>> + else
>> + deadline = jiffies + MAX_READY_WAIT_JIFFIES;
>>
>> while (!timeout) {
>> if (time_after_eq(jiffies, deadline))
>> @@ -326,7 +337,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
>> goto erase_err;
>> }
>>
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->max_wait);
>> if (ret)
>> goto erase_err;
>>
>> @@ -348,7 +359,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
>> addr += mtd->erasesize;
>> len -= mtd->erasesize;
>>
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->common_wait);
>> if (ret)
>> goto erase_err;
>> }
>> @@ -467,8 +478,9 @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>> return ret;
>> }
>>
>> -/* Used when the "_ext_id" is two bytes at most */
>> -#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
>> +/* Used when the "_ext_id" is two bytes at most, WATTM: wait time */
>> +#define INFO_WATTM(_jedec_id, _ext_id, _sector_size, _n_sectors, \
>> + _common_wait, _max_wait, _flags) \
>> ((kernel_ulong_t)&(struct flash_info) { \
>> .id = { \
>> ((_jedec_id) >> 16) & 0xff, \
>> @@ -481,10 +493,13 @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>> .sector_size = (_sector_size), \
>> .n_sectors = (_n_sectors), \
>> .page_size = 256, \
>> + .common_wait = (_common_wait), \
>> + .max_wait = (_max_wait), \
>> .flags = (_flags), \
>> })
>>
>> -#define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
>> +#define INFO6_WATTM(_jedec_id, _ext_id, _sector_size, _n_sectors, \
>> + _common_wait, _max_wait, _flags) \
>> ((kernel_ulong_t)&(struct flash_info) { \
>> .id = { \
>> ((_jedec_id) >> 16) & 0xff, \
>> @@ -498,6 +513,8 @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>> .sector_size = (_sector_size), \
>> .n_sectors = (_n_sectors), \
>> .page_size = 256, \
>> + .common_wait = (_common_wait), \
>> + .max_wait = (_max_wait), \
>> .flags = (_flags), \
>> })
>>
>> @@ -510,6 +527,16 @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>> .flags = (_flags), \
>> })
>>
>> +#define DEFAULT_WAIT_TIME 40 /* in seconds */
>> +
>> +#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
>> + INFO_WATTM(_jedec_id, _ext_id, _sector_size, _n_sectors, \
>> + DEFAULT_WAIT_TIME, DEFAULT_WAIT_TIME, _flags)
>> +
>> +#define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
>> + INFO6_WATTM(_jedec_id, _ext_id, _sector_size, _n_sectors, \
>> + DEFAULT_WAIT_TIME, DEFAULT_WAIT_TIME, _flags)
>> +
>> /* NOTE: double check command sets and memory organization when you add
>> * more nor chips. This current list focusses on newer chips, which
>> * have been converging on command sets which including JEDEC ID.
>> @@ -756,7 +783,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>>
>> /* write one byte. */
>> nor->write(nor, to, 1, retlen, buf);
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->common_wait);
>> if (ret)
>> goto time_out;
>> }
>> @@ -768,7 +795,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>>
>> /* write two bytes. */
>> nor->write(nor, to, 2, retlen, buf + actual);
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->common_wait);
>> if (ret)
>> goto time_out;
>> to += 2;
>> @@ -777,7 +804,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>> nor->sst_write_second = false;
>>
>> write_disable(nor);
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->common_wait);
>> if (ret)
>> goto time_out;
>>
>> @@ -788,7 +815,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>> nor->program_opcode = SPINOR_OP_BP;
>> nor->write(nor, to, 1, retlen, buf + actual);
>>
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->common_wait);
>> if (ret)
>> goto time_out;
>> write_disable(nor);
>> @@ -834,7 +861,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>> if (page_size > nor->page_size)
>> page_size = nor->page_size;
>>
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->common_wait);
>> if (ret)
>> goto write_err;
>>
>> @@ -844,7 +871,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>> }
>> }
>>
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->common_wait);
>> write_err:
>> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_WRITE);
>> return ret;
>> @@ -860,7 +887,7 @@ static int macronix_quad_enable(struct spi_nor *nor)
>> nor->cmd_buf[0] = val | SR_QUAD_EN_MX;
>> nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 1, 0);
>>
>> - if (spi_nor_wait_till_ready(nor))
>> + if (spi_nor_wait_till_ready(nor, nor->common_wait))
>> return 1;
>>
>> ret = read_sr(nor);
>> @@ -931,7 +958,7 @@ static int micron_quad_enable(struct spi_nor *nor)
>> return ret;
>> }
>>
>> - ret = spi_nor_wait_till_ready(nor);
>> + ret = spi_nor_wait_till_ready(nor, nor->common_wait);
>> if (ret)
>> return ret;
>>
>> @@ -1184,6 +1211,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>
>> nor->read_dummy = spi_nor_read_dummy_cycles(nor);
>>
>> + nor->common_wait = info->common_wait;
>> + nor->max_wait = info->max_wait;
>> +
>> dev_info(dev, "%s (%lld Kbytes)\n", id->name,
>> (long long)mtd->size >> 10);
>>
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index e540952..a0f29c7 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -158,6 +158,8 @@ enum spi_nor_option_flags {
>> * @lock: [FLASH-SPECIFIC] lock a region of the SPI NOR
>> * @unlock: [FLASH-SPECIFIC] unlock a region of the SPI NOR
>> * @priv: the private data
>> + * @common_wait: wait ready time applying to most operations, in seconds
>> + * @max_wait: max wait ready time of all operations, in seconds
>> */
>> struct spi_nor {
>> struct mtd_info *mtd;
>> @@ -195,6 +197,8 @@ struct spi_nor {
>> int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>>
>> void *priv;
>> + s16 common_wait;
>> + s16 max_wait;
>> };
>>
>> /**
>
More information about the linux-mtd
mailing list