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

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

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