Re: [PATCH] 2.4.27 - MTD cfi_cmdset_0002.c - Duplicate cleanup in error path

[Rainer Weikusat <rainer.weikusat@sncag.com>]

"bradgoodman.com" <bkgoodman@bradgoodman.com> writes:
> Patch to 2.4.x: Corrects an obvious error where all of the cleanups are done
> twice in the event of a chip programming error. This can result in
> kernel BUG() getting called on subsequent programming attempts.
>
>
> --- linux-2.4.22.prepatch/drivers/mtd/chips/cfi_cmdset_0002.c	Fri Jun 13 10:51:34 2003
> +++ linux-2.4.22/drivers/mtd/chips/cfi_cmdset_0002.new	Thu Jul 15 14:44:30 2004
> @@ -549,11 +549,6 @@
>  			}
>  		} else {
>  			printk(KERN_WARNING "Waiting for write to complete timed out in do_write_oneword.");        
> -			
> -			chip->state = FL_READY;
> -			wake_up(&chip->wq);
> -			cfi_spin_unlock(chip->mutex);
> -			DISABLE_VPP(map);
>  			ret = -EIO;
>  		}
>  	}

I suggest the following instead:

--------------------------
--- cfi_cmdset_0002.c.orig	2004-07-24 15:05:31.000000000 +0800
+++ cfi_cmdset_0002.c	2004-07-24 15:06:06.000000000 +0800
@@ -461,7 +461,6 @@
 	unsigned int dq6, dq5;	
 	struct cfi_private *cfi = map->fldrv_priv;
 	DECLARE_WAITQUEUE(wait, current);
-	int ret = 0;
 
  retry:
 	cfi_spin_lock(chip->mutex);
@@ -554,7 +553,7 @@
 			wake_up(&chip->wq);
 			cfi_spin_unlock(chip->mutex);
 			DISABLE_VPP(map);
-			ret = -EIO;
+			return -EIO;
 		}
 	}
 
@@ -563,7 +562,7 @@
 	wake_up(&chip->wq);
 	cfi_spin_unlock(chip->mutex);
 
-	return ret;
+	return 0;
 }
 
 static int cfi_amdstd_write (struct mtd_info *mtd, loff_t to , size_t len, size_t *retlen, const u_char *buf)
----------------------------

That way, it is consistent with the other low-level chip access
functions. But the algorithm is per se buggy, anyway, because except
if DQ5 was raised before, the chip is not 'ready' (for reading array
data), but still in programming mode and will remain there until the
'embedded programming algorithm' stops, because (according to the
docs) a reset command will not be accepted until DQ5 has been raised
and the opportunityto check for that is gone after the syscall
returned to the caller.