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

[Vitaly Wool <vwool@ru.mvista.com>]

Greetings,

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

Best regards,
   Vitaly

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);
>>
>>
>>
>>
>>  
>>
>> ------------------------------------------------------------------------
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>  
>>
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
>