Bug in chip alias detection! (jedec_probe, possibly cfi_probe)

Bug in chip alias detection! (jedec_probe, possibly cfi_probe)

[Kremer, Alex]

[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]

Hi again,

I was testing the new support for Boot Block via jedec_probe.
I have 2 4MB boot block parts in 16 bit mode (no interleaving)
mapped over dc21285 (selected using A22)

The jedec_probe alias detection code misdetected an alias for a 3rd chip!

I think I have found the problem. When scanning over previously found
chips *THEY* (chips[i].start) should be checked if they also 
switched to autoselect mode when we switched the one we are checking now
(base)

        for (i=0; i<cfi->numchips; i++) {
                /* This chip should be in read mode if it's one
                   we've already touched. */
HERE --->>      if (jedec_read_mfr(map, base, osf) == cfi->mfr &&
HERE --->>          jedec_read_id(map, base, osf) == cfi->id) {
                        /* Eep. This chip also looks like it's in autoselect
mode.
                           Is it an alias for the new one? */

It should be 'chips[i].start' instead of 'base' in those lines. The current
one is
always in autoselect mode at this point.

I'm attaching a patch that fixed it for me (against cvs from yesterday Oct.
15).
The patch also includes a fix for a non critical bug -- the first
 chip is not reported and is not put back in read-array mode!

I think there is a somewhat similar bug in cfi_probe code, but I couldn't
verify this.

		if (qry_present(map,chips[i].start,cfi)) {
			/* Eep. This chip also had the QRY marker. 
			 * Is it an alias for the new one? */
			cfi_send_gen_cmd(0xF0, 0, chips[i].start, map, cfi,
cfi->device_type, NULL);

			/* If the QRY marker goes away, it's an alias */
HERE --->>>		if (!qry_present(map, chips[i].start, cfi)) {
				printk(KERN_DEBUG "%s: Found an alias at
0x%x for the chip at 0x%lx\n",
				       map->name, base, chips[i].start);
				return 0;
			}

when chips[i] is switched to read-array, the current one should be checked
(base),
not the chips[i], which was put in read-array mode in the previous line.

Kreso


[-- Attachment #2: mtd-jedec-alias.patch --]
[-- Type: application/octet-stream, Size: 1366 bytes --]

--- mtd/drivers/mtd/chips/jedec_probe.c	Sat Oct  6 17:56:57 2001
+++ mtd-fix/drivers/mtd/chips/jedec_probe.c	Tue Oct 16 14:06:20 2001
@@ -584,7 +584,17 @@
 		for (i=0; i<sizeof(jedec_table)/sizeof(jedec_table[0]); i++) {
 			if (mfr == jedec_table[i].mfr_id &&
 			    id == jedec_table[i].dev_id)
-				return cfi_jedec_setup(cfi, i);
+			      {
+				    /* Put it back into Read Mode */
+				    cfi_send_gen_cmd(0xF0, 0, base, map, cfi, cfi->device_type, NULL);
+				    cfi_send_gen_cmd(0xff, 0, base, map, cfi, cfi->device_type, NULL);
+
+				    printk(KERN_INFO "%s: Found %d x%d devices at 0x%x in %d-bit mode\n",
+					   map->name, cfi->interleave, cfi->device_type*8, base, 
+					   map->buswidth*8);
+				    
+				    return cfi_jedec_setup(cfi, i);
+			      }
 		}
 		if (!retried++) {
 			/* Deal with whichever strange chips these were */
@@ -599,8 +609,8 @@
 	for (i=0; i<cfi->numchips; i++) {
 		/* This chip should be in read mode if it's one
 		   we've already touched. */
-		if (jedec_read_mfr(map, base, osf) == cfi->mfr &&
-		    jedec_read_id(map, base, osf) == cfi->id) {
+		if (jedec_read_mfr(map, chips[i].start, osf) == cfi->mfr &&
+		    jedec_read_id(map, chips[i].start, osf) == cfi->id) {
 			/* Eep. This chip also looks like it's in autoselect mode.
 			   Is it an alias for the new one? */
 			

Re: Bug in chip alias detection! (jedec_probe, possibly cfi_probe)

[David Woodhouse]

alex.kremer@intel.com said:
>  I think I have found the problem. When scanning over previously found
> chips *THEY* (chips[i].start) should be checked if they also  switched
> to autoselect mode when we switched the one we are checking now (base)

Thanks. Please could you test this slightly edited fix:

Index: drivers/mtd/chips/jedec_probe.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/chips/jedec_probe.c,v
retrieving revision 1.5
diff -u -r1.5 jedec_probe.c
--- drivers/mtd/chips/jedec_probe.c	2001/10/06 15:56:57	1.5
+++ drivers/mtd/chips/jedec_probe.c	2001/10/16 14:44:31
@@ -583,8 +583,11 @@
 		
 		for (i=0; i<sizeof(jedec_table)/sizeof(jedec_table[0]); i++) {
 			if (mfr == jedec_table[i].mfr_id &&
-			    id == jedec_table[i].dev_id)
-				return cfi_jedec_setup(cfi, i);
+			    id == jedec_table[i].dev_id) {
+				if (!cfi_jedec_setup(cfi, i))
+					return 0;
+				goto ok_out;
+			}
 		}
 		if (!retried++) {
 			/* Deal with whichever strange chips these were */
@@ -599,8 +602,8 @@
 	for (i=0; i<cfi->numchips; i++) {
 		/* This chip should be in read mode if it's one
 		   we've already touched. */
-		if (jedec_read_mfr(map, base, osf) == cfi->mfr &&
-		    jedec_read_id(map, base, osf) == cfi->id) {
+		if (jedec_read_mfr(map, chips[i].start, osf) == cfi->mfr &&
+		    jedec_read_id(map, chips[i[.start, osf) == cfi->id) {
 			/* Eep. This chip also looks like it's in autoselect mode.
 			   Is it an alias for the new one? */
 			
@@ -640,6 +643,7 @@
 	chips[cfi->numchips].state = FL_READY;
 	cfi->numchips++;
 		
+ok_out:
 	/* Put it back into Read Mode */
 	cfi_send_gen_cmd(0xF0, 0, base, map, cfi, cfi->device_type, NULL);
 	cfi_send_gen_cmd(0xff, 0, base, map, cfi, cfi->device_type, NULL);


--
dwmw2