Re: [PATCH] CFI: prevent the false software timeouts on writes

Sergei Shtylylov
2005-11-07T13:19:44Z

Hail you!

> yeah it works with XIP! :)
> I guess you can ask for commit now if no one objects.

So, I beseech you sirs to have a mercy on my patch, and commit it with
least possible delay.

God speed.

> Vitaly Wool wrote:
>
>> Hi Sergey,
>>
>> I'm going to verify if this works with XIP and get back to you.
>>
>> Best regards,
>> Vitaly
>>
>> Sergei Shtylylov wrote:
>>
>>> Hello.
>>>
>>> We've noticed that sometimes "MTD do_write_buffer(): software
>>> timeout"
>>> message was printed out when writing to a Fujitsu NOR flash.
>>> It turned out that this was because of a race in the timeout
>>> handling
>>> do_write_buffer(). A small timeout of (HZ / 1000) + 1 is used there, and
>>> sometimes if the timer interrupt handling takes more than one or even
>>> two
>>> jiffies (which is 1-2 ms with HZ == 1000) and that interrupt happens
>>> just
>>> after chip_ready() call, the driver bails out from a ready polling loop
>>> despite the chip has actually become ready while all those interrupts
>>> were
>>> handled. To deal with this issue, extra check for chip ready is
>>> neccessary on
>>> timeout expiration (and the checks should better be reordered).
>>> As do_write_oneword() uses the same approach, it needs to also
>>> be changed.
>>>
>>> Signed-off-by: Konstantin Baidarov <kbaidarov@ru.mvista.com>
>>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>>>
>>>
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> Index: drivers/mtd/chips/cfi_cmdset_0002.c
>>> ===================================================================
>>> RCS file: /home/cvs/mtd/drivers/mtd/chips/cfi_cmdset_0002.c,v
>>> retrieving revision 1.120
>>> diff -a -u -p -r1.120 cfi_cmdset_0002.c
>>> --- drivers/mtd/chips/cfi_cmdset_0002.c 20 Jul 2005 21:01:13
>>> -0000 1.120
>>> +++ drivers/mtd/chips/cfi_cmdset_0002.c 25 Oct 2005 15:38:44 -0000
>>> @@ -1014,16 +1014,16 @@ static int __xipram do_write_oneword(str
>>> continue;
>>> }
>>>
>>> - if (chip_ready(map, adr))
>>> - break;
>>> -
>>> - if (time_after(jiffies, timeo)) {
>>> + if (time_after(jiffies, timeo) && !chip_ready(map, adr)){
>>> xip_enable(map, chip, adr);
>>> printk(KERN_WARNING "MTD %s(): software timeout\n",
>>> __func__);
>>> xip_disable(map, chip, adr);
>>> - break;
>>> + break;
>>> }
>>>
>>> + if (chip_ready(map, adr))
>>> + break;
>>> +
>>> /* Latency issues. Drop the lock, wait a while and retry */
>>> UDELAY(map, chip, adr, 1);
>>> }
>>> @@ -1275,13 +1275,13 @@ static int __xipram do_write_buffer(stru
>>> continue;
>>> }
>>>
>>> + if (time_after(jiffies, timeo) && !chip_ready(map, adr))
>>> + break;
>>> +
>>> if (chip_ready(map, adr)) {
>>> xip_enable(map, chip, adr);
>>> goto op_done;
>>> }
>>> - - if( time_after(jiffies, timeo))
>>> - break;
>>>
>>> /* Latency issues. Drop the lock, wait a while and retry */
>>> UDELAY(map, chip, adr, 1);