mtd/drivers/mtd/chips cfi_cmdset_0002.c,1.67,1.68

Thayne Harbaugh tharbaugh at lnxi.com
Mon Apr 21 19:26:28 EDT 2003 

Update of /home/cvs/mtd/drivers/mtd/chips
In directory phoenix.infradead.org:/tmp/cvs-serv17616

Modified Files:
	cfi_cmdset_0002.c 
Log Message:
Remove dq5 check from erase/write status polling.
dq5 is now only used to decode failures - using dq5 to
check status _greatly_ complicates the logic and only
optimizes the error case (which should be infrequent at
best).
Added benefit - chips that do not use dq5 now work - they
might have strange error reports when they timeout.
Future fixes should select cmdset_0002 capabilities for
chips that support dq7, dq6, dq5, dq2, erase suspend/resume,
etc.

Index: cfi_cmdset_0002.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/chips/cfi_cmdset_0002.c,v
retrieving revision 1.67
retrieving revision 1.68
diff -u -r1.67 -r1.68
--- cfi_cmdset_0002.c	15 Apr 2003 22:32:13 -0000	1.67
+++ cfi_cmdset_0002.c	21 Apr 2003 23:26:25 -0000	1.68
@@ -472,6 +472,7 @@
     static unsigned long uWriteTimeout = ( HZ / 1000 ) + 1;
 	DECLARE_WAITQUEUE(wait, current);
 	int ret = 0;
+	int ta = 0;
 
  retry:
 	cfi_spin_lock(chip->mutex);
@@ -554,30 +555,23 @@
 	       __func__, oldstatus, status );
 
 	/*
-	 * This assumes that dq5 == dq6 >> 1
-	 * It's done this way so that we only look for asserted dq5
-	 * on interleaved chips that are still toggling - we can
-	 * ignore chips that have already completed and might have
-	 * dq5 asserted.  It is important to use oldstatus because
-	 * a completed state might look like it has toggled from
-	 * the previous read of status bits and it might trigger
-	 * an erroneous error.
+	 * This only checks if dq6 is still toggling and that our
+	 * timer hasn't expired.  We purposefully ignore the chips
+	 * internal timer that will assert dq5 and leave dq6 toggling.
+	 * This is done for a variety of reasons:
+	 * 1) Not all chips support dq5.
+	 * 2) Dealing with asynchronous status bit and data updates
+	 *    and reading a device two more times creates _messy_
+	 *    logic when trying to deal with interleaved devices -
+	 *    some may be changing while others are still busy.
+	 * 3) Checking dq5 only helps to optimize an error case that
+	 *    should at worst be infrequent and at best non-existent.
 	 *
-	 * FIXME - this is rather complicated - trying to deal with
-	 * asynchronous transitions and cases where the toggle bit
-	 * appears to toggle when it has really just settled to true
-	 * data and having to read the locations an additional two
-	 * times to verify bits.  It might be that we just need
-	 * to watch dq6 and our timeout and ignore dq5.  If the
-	 * timeout is exceeded and we still have dq6 toggling we then
-	 * look for asserted dq5 for each toggling dq6.  That would
-	 * be safer, easier to code and get correct, and marginally
-	 * less efficient.  Who cares if the error case, which should
-	 * be very infrequent, is optimized.
+	 * If our timeout occurs _then_ we will check dq5 to see
+	 * if the device also had an internal timeout.
 	 */
 	while( ( ( status ^ oldstatus ) & dq6 )
-	       && ! ( oldstatus & ( ( status ^ oldstatus ) & dq6 ) >> 1 )
-	       && ! time_after(jiffies, timeo) ) {
+	       && ! ( ta = time_after(jiffies, timeo) ) ) {
 
 		if (need_resched()) {
 			cfi_spin_unlock(chip->mutex);
@@ -592,35 +586,19 @@
 		       __func__, oldstatus, status );
 	}
 
-	if ( ( ( status ^ oldstatus ) & dq6 )
-	     && ! ( oldstatus & ( ( status ^ oldstatus ) & dq6 ) >> 1 ) ) {
-		/* dq6 still toggling and dq5 still low - must be timeout */
-		printk( KERN_WARNING
-			"MTD %s(): Timed out while waiting for write to complete.\n",
-			__func__ );
-		goto write_failed;
-	}
-
 	/*
-	 * Either dq5 went high or dq6 finish toggling.  For
-	 * the correct completion of certain types of writes
-	 * dq5 might _appear_ to go high while dq6 might
-	 * _appear_ to still be toggling - it depends on the
-	 * last toggle-state dq6 was in and what the final
-	 * state will be with true data.  We alread know that
-	 * something changed state with the last set of reads.
+	 * Something kicked us out of the read-back loop.  We'll
+	 * check success befor checking failure.
+	 * Even though dq6 might be true data, it is unkown if
+	 * all of the other bits have changed to true data due to
+	 * the asynchronous nature of the internal state machine.
 	 * We will read two more times and use this to either
 	 * verify that the write completed successfully or
 	 * that something really went wrong.  BOTH reads
 	 * must match what was written - this certifies that
-	 * dq6 is not still toggling  and that the status
+	 * bits aren't still changing  and that the status
 	 * bits erroneously match the datum that was written.
 	 */
-
-	/*
-	 * Be optimistic and check success first - then decode any
-	 * failures.
-	 */
 	prev_oldstatus = oldstatus;
 	prev_status = status;
 	oldstatus = cfi_read(map, adr);
@@ -633,24 +611,31 @@
 		goto write_done;
 	}
 
-	/* something has failed */
-	if ( ( ( status ^ oldstatus ) & dq6 )
-	     && ( status & ( ( status ^ oldstatus ) & dq6 >> 1 ) ) ) {
-		printk(KERN_WARNING
-		       "MTD %s(): Internal flash device timeout occured.\n",
-		       __func__ );
-	} else {
-		/*
-		 * If we get to here then it means that something failed
-		 * and that the usual error state of toggling dq6
-		 * while dq5 goes high did not happen.  Something
-		 * is seriously wacky!  Dump some debug info.
-		 */
-		printk(KERN_WARNING
-		       "MTD %s(): Wacky!  Unable to decode failure status\n",
-		       __func__ );
+	if ( ta ) {
+		int dq5mask = ( ( status ^ oldstatus ) & dq6 ) >> 1;
+		if ( status & dq5mask ) {
+			/* dq5 asserted - decode interleave chips */
+			printk( KERN_WARNING
+				"MTD %s(): FLASH internal timeout: 0x%.8x\n",
+				__func__,
+				status & dq5mask );
+		} else {
+			printk( KERN_WARNING
+				"MTD %s(): Software timed out during write.\n",
+				__func__ );
+		}
+		goto write_failed;
 	}
 
+	/*
+	 * If we get to here then it means that something
+	 * is wrong and it's not a timeout.  Something
+	 * is seriously wacky!  Dump some debug info.
+	 */
+	printk(KERN_WARNING
+	       "MTD %s(): Wacky!  Unable to decode failure status\n",
+	       __func__ );
+
 	printk(KERN_WARNING
 	       "MTD %s(): 0x%.8lx(0x%.8x): 0x%.8x 0x%.8x 0x%.8x 0x%.8x\n",
 	       __func__, adr, datum,
@@ -825,6 +810,7 @@
 	struct cfi_private *cfi = map->fldrv_priv;
 	DECLARE_WAITQUEUE(wait, current);
 	int ret = 0;
+	int ta = 0;
 	__u32 ones = 0;
 
  retry:
@@ -882,8 +868,7 @@
 	       __func__, oldstatus, status );
 
 	while( ( ( status ^ oldstatus ) & dq6 )
-	       && ! ( oldstatus & ( ( status ^ oldstatus ) & dq6 >> 1 ) )
-	       && ! time_after(jiffies, timeo) ) {
+	       && ! ( ta = time_after(jiffies, timeo) ) ) {
 		int wait_reps;
 
 		/* an initial short sleep */
@@ -913,8 +898,7 @@
 		/* Busy wait for 1/10 of a milisecond */
 		for(wait_reps = 0;
 		    (wait_reps < 100)
-			    && ( ( status ^ oldstatus ) & dq6 )
-			    && ! ( oldstatus & ( ( status ^ oldstatus ) & dq6 >> 1 ) );
+			    && ( ( status ^ oldstatus ) & dq6 );
 		    wait_reps++) {
 			
 			/* Latency issues. Drop the lock, wait a while and retry */
@@ -934,62 +918,41 @@
 		       __func__, oldstatus, status );
 	}
 
-	if ( ( ( status ^ oldstatus ) & dq6 )
-	     && ! ( oldstatus & ( ( status ^ oldstatus ) & dq6 >> 1 ) ) ) {
-		/* dq6 still toggling and dq5 still low - must be timeout */
-		printk( KERN_WARNING
-			"MTD %s(): Timed out while waiting for erase to complete.\n",
-			__func__ );
-		goto erase_failed;
-	}
-
-	/*
-	 * Be optimistic and check success first - then decode any
-	 * failures.
-	 */
 	prev_oldstatus = oldstatus;
 	prev_status = status;
 	oldstatus = cfi_read(map, adr);
 	status = cfi_read(map, adr);
 	DEBUG( MTD_DEBUG_LEVEL3, "MTD %s(): Check 0x%.8x 0x%.8x\n",
 	       __func__, oldstatus, status );
-		
-	switch( CFIDEV_BUSWIDTH ) {
-	case 1:
-		ones = (__u8)~0;
-		break;
-	case 2:
-		ones = (__u16)~0;
-		break;
-	case 4:
-		ones = (__u32)~0;
-		break;
-	}
+
+	ones = CMD( (__u8)~0 );
 	
 	if ( oldstatus == ones && status == ones ) {
 		/* success - do nothing */
 		goto erase_done;
 	}
 
-	/* something has failed */
-	if ( ( ( status ^ oldstatus ) & dq6 )
-	     && ( oldstatus & ( ( status ^ oldstatus ) & dq6 >> 1 ) ) ) {
-		printk(KERN_WARNING
-		       "MTD %s(): Internal flash device timeout occured.\n",
-		       __func__ );
-	} else {
-		/*
-		 * This state means that the erase did not succeed
-		 * and that the usual error state of toggling dq6
-		 * while dq5 goes high did not happen.  Something
-		 * is seriously wacky!
-		 */
-		printk(KERN_WARNING
-		       "MTD %s(): Wacky!  Unable to decode failure status\n",
-		       __func__ );
+	if ( ta ) {
+		int dq5mask = ( ( status ^ oldstatus ) & dq6 ) >> 1;
+		if ( status & dq5mask ) {
+			/* dq5 asserted - decode interleave chips */
+			printk( KERN_WARNING
+				"MTD %s(): FLASH internal timeout: 0x%.8x\n",
+				__func__,
+				status & dq5mask );
+		} else {
+			printk( KERN_WARNING
+				"MTD %s(): Software timed out during write.\n",
+				__func__ );
+		}
+		goto erase_failed;
 	}
 
 	printk(KERN_WARNING
+	       "MTD %s(): Wacky!  Unable to decode failure status\n",
+	       __func__ );
+
+	printk(KERN_WARNING
 	       "MTD %s(): 0x%.8lx(0x%.8x): 0x%.8x 0x%.8x 0x%.8x 0x%.8x\n",
 	       __func__, adr, ones,
 	       prev_oldstatus, prev_status,
@@ -1017,6 +980,7 @@
 	struct cfi_private *cfi = map->fldrv_priv;
 	DECLARE_WAITQUEUE(wait, current);
 	int ret = 0;
+	int ta = 0;
 	__u32 ones = 0;
 
  retry:
@@ -1074,8 +1038,7 @@
 	       __func__, oldstatus, status );
 
 	while( ( ( status ^ oldstatus ) & dq6 )
-	       && ! ( oldstatus & ( ( status ^ oldstatus ) & dq6 >> 1 ) )
-	       && ! time_after(jiffies, timeo) ) {
+	       && ! ( ta = time_after(jiffies, timeo) ) ) {
 		int wait_reps;
 
 		/* an initial short sleep */
@@ -1105,8 +1068,7 @@
 		/* Busy wait for 1/10 of a milisecond */
 		for(wait_reps = 0;
 		    (wait_reps < 100)
-			    && ( ( status ^ oldstatus ) & dq6 )
-			    && ! ( oldstatus & ( ( status ^ oldstatus ) & dq6 >> 1 ) );
+			    && ( ( status ^ oldstatus ) & dq6 );
 		    wait_reps++) {
 			
 			/* Latency issues. Drop the lock, wait a while and retry */
@@ -1126,19 +1088,6 @@
 		       __func__, oldstatus, status );
 	}
 
-	if ( ( ( status ^ oldstatus ) & dq6 )
-	     && ! ( oldstatus & ( ( status ^ oldstatus ) & dq6 >> 1 ) ) ) {
-		/* dq6 still toggling and dq5 still low - must be timeout */
-		printk( KERN_WARNING
-			"MTD %s(): Timed out while waiting for erase to complete.\n",
-			__func__ );
-		goto erase_failed;
-	}
-
-	/*
-	 * Be optimistic and check success first - then decode any
-	 * failures.
-	 */
 	prev_oldstatus = oldstatus;
 	prev_status = status;
 	oldstatus = cfi_read(map, adr);
@@ -1146,40 +1095,32 @@
 	DEBUG( MTD_DEBUG_LEVEL3, "MTD %s(): Check 0x%.8x 0x%.8x\n",
 	       __func__, oldstatus, status );
 
-	switch( CFIDEV_BUSWIDTH ) {
-	case 1:
-		ones = (__u8)~0;
-		break;
-	case 2:
-		ones = (__u16)~0;
-		break;
-	case 4:
-		ones = (__u32)~0;
-		break;
-	}
-	
+	ones = CMD( (__u8)~0 );
+
 	if ( oldstatus == ones && status == ones ) {
 		/* success - do nothing */
 		goto erase_done;
 	}
 
-	/* something has failed */
-	if ( ( ( status ^ oldstatus ) & dq6 )
-	     && ( oldstatus & ( ( status ^ oldstatus ) & dq6 >> 1 ) ) ) {
-		printk(KERN_WARNING
-		       "MTD %s(): Internal flash device timeout occured.\n",
-		       __func__ );
-	} else {
-		/*
-		 * This state means that the erase did not succeed
-		 * and that the usual error state of toggling dq6
-		 * while dq5 goes high did not happen.  Something
-		 * is seriously wacky!
-		 */
-		printk(KERN_WARNING
-		       "MTD %s(): Wacky!  Unable to decode failure status\n",
-		       __func__ );
+	if ( ta ) {
+		int dq5mask = ( ( status ^ oldstatus ) & dq6 ) >> 1;
+		if ( status & dq5mask ) {
+			/* dq5 asserted - decode interleave chips */
+			printk( KERN_WARNING
+				"MTD %s(): FLASH internal timeout: 0x%.8x\n",
+				__func__,
+				status & dq5mask );
+		} else {
+			printk( KERN_WARNING
+				"MTD %s(): Software timed out during write.\n",
+				__func__ );
+		}
+		goto erase_failed;
 	}
+
+	printk(KERN_WARNING
+	       "MTD %s(): Wacky!  Unable to decode failure status\n",
+	       __func__ );
 
 	printk(KERN_WARNING
 	       "MTD %s(): 0x%.8lx(0x%.8x): 0x%.8x 0x%.8x 0x%.8x 0x%.8x\n",

More information about the linux-mtd-cvs mailing list