[Fwd: helper routines for flash chip probing, search sequence]

[Fwd: helper routines for flash chip probing, search sequence]

[David Woodhouse]

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

I meant the MTD list really, not linux-kernel. Sorry, I should have made
that explicit.

-- 
dwmw2

[-- Attachment #2: Forwarded message - helper routines for flash chip probing, search sequence --]
[-- Type: message/rfc822, Size: 7460 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 2328 bytes --]

I have sent this mail to David Woodhouse, the maintainer 
of MTD (memory technology devices) part. 
He answers 

> At first glance, you seem to be right -- you'll not get false matches
> for 4x interleave when in fact the interleave is 2x, so we should check
> for 4x first. 
>
> Can you send your patch (and explanation) to the mailing list too, so
> that others can comment on it? It's been a while since I've looked hard
> at anything but the CFI probes. Thanks.

And now my original mail:

I have a question, concerning the flash chip helper
routines
      linux/drivers/mtd/chips/gen_probe.c

In my opinion, the searching sequence, to find the
flash geometry, (cfi->device_type and cfi->interleave)
must be reversed, to prevent wrong decisions.
If the buswidth is given, the search should begin 
with the highest meaningfull interleave and the lowest
device type.
With the current searching order I get the following
effect.

I have 4 8bit flashes working on a 32 bit data bus.
The Manufacture ID is 0x89, the Device ID 0xAA.
The flash type is SHARP LH28F016SCT-L95
The correct values, which should be detected are:

- cfi->device_type =  1   CFI_DEVICETYPE_X8
- cfi->interleave  =  4   CFIDEV_INTERLEAVE_4

but the probing detected 

  cfi->interleave  = CFIDEV_INTERLEAVE_2 
  cfi->device_type = CFI_DEVICETYPE_X16.

as valid, so that the test with the correct values, following later,
will not be done.
The identifier codes of this SHARP flashes can be read at offset
0 and 1 after writing 0x90 at any arbitrary address in the flash space.
The unlock sequence 
  0xaa at cfi->addr_unlock1 = 0x555 and 
  0x55 at cfi->addr_unlock2 = 0x2aa
seems not to be neccessary.
The flashes 0 and 2 are unlocked by 0x00900090, flashes 1 and 4 not,
so that 0 and 2 answers with th correct ID and 1 and 3 with 0x00.

If the search order is reversed, the test with the correct values is made
before the test with wrong parameters.

I have attached a patch, which should prevent such wrong decisions, if
the flashes are not correctly designed.

Does anyone know reasons, that the search order should not be reversed?


-- 

--------------------------------------------

Datentechnik GmbH
Axel Ludszuweit
D-30855 Langenhagen

Tel.: +49 511 / 978197-630
Fax : +49 511 / 978197-670
http://www.keymile.com
mailto:axel.ludszuweit@keymile.com

[-- Attachment #2.1.2: patch_gen_probe.c --]
[-- Type: text/plain, Size: 2887 bytes --]

--- linux-2.4.20/drivers/mtd/chips/gen_probe.c.orig	Fri Oct  5 00:14:59 2001
+++ linux-2.4.20/drivers/mtd/chips/gen_probe.c	Thu May  8 15:34:52 2003
@@ -159,13 +159,6 @@
 
 #ifdef CFIDEV_BUSWIDTH_2		
 	case CFIDEV_BUSWIDTH_2:
-#ifdef CFIDEV_INTERLEAVE_1
-		cfi->interleave = CFIDEV_INTERLEAVE_1;
-
-		cfi->device_type = CFI_DEVICETYPE_X16;
-		if (cp->probe_chip(map, 0, NULL, cfi))
-			return 1;
-#endif /* CFIDEV_INTERLEAVE_1 */
 #ifdef CFIDEV_INTERLEAVE_2
 		cfi->interleave = CFIDEV_INTERLEAVE_2;
 
@@ -177,50 +170,56 @@
 		if (cp->probe_chip(map, 0, NULL, cfi))
 			return 1;
 #endif /* CFIDEV_INTERLEAVE_2 */
+#ifdef CFIDEV_INTERLEAVE_1
+		cfi->interleave = CFIDEV_INTERLEAVE_1;
+
+		cfi->device_type = CFI_DEVICETYPE_X16;
+		if (cp->probe_chip(map, 0, NULL, cfi))
+			return 1;
+#endif /* CFIDEV_INTERLEAVE_1 */
 		break;			
-#endif /* CFIDEV_BUSWIDTH_2 */
 
+#endif /* CFIDEV_BUSWIDTH_2 */
 #ifdef CFIDEV_BUSWIDTH_4
 	case CFIDEV_BUSWIDTH_4:
-#if defined(CFIDEV_INTERLEAVE_1) && defined(SOMEONE_ACTUALLY_MAKES_THESE)
-                cfi->interleave = CFIDEV_INTERLEAVE_1;
+#ifdef CFIDEV_INTERLEAVE_4
+		cfi->interleave = CFIDEV_INTERLEAVE_4;
 
-                cfi->device_type = CFI_DEVICETYPE_X32;
+		cfi->device_type = CFI_DEVICETYPE_X8;
 		if (cp->probe_chip(map, 0, NULL, cfi))
 			return 1;
-#endif /* CFIDEV_INTERLEAVE_1 */
-#ifdef CFIDEV_INTERLEAVE_2
-		cfi->interleave = CFIDEV_INTERLEAVE_2;
 
-#ifdef SOMEONE_ACTUALLY_MAKES_THESE
-		cfi->device_type = CFI_DEVICETYPE_X32;
+		cfi->device_type = CFI_DEVICETYPE_X16;
 		if (cp->probe_chip(map, 0, NULL, cfi))
 			return 1;
-#endif
-		cfi->device_type = CFI_DEVICETYPE_X16;
+
+#ifdef SOMEONE_ACTUALLY_MAKES_THESE
+		cfi->device_type = CFI_DEVICETYPE_X32;
 		if (cp->probe_chip(map, 0, NULL, cfi))
 			return 1;
+#endif /* SOMEONE_ACTUALLY_MAKES_THESE */
 
-		cfi->device_type = CFI_DEVICETYPE_X8;
+#endif /* CFIDEV_INTERLEAVE_4 */
+#ifdef CFIDEV_INTERLEAVE_2
+		cfi->interleave = CFIDEV_INTERLEAVE_2;
+
+		cfi->device_type = CFI_DEVICETYPE_X16;
 		if (cp->probe_chip(map, 0, NULL, cfi))
 			return 1;
-#endif /* CFIDEV_INTERLEAVE_2 */
-#ifdef CFIDEV_INTERLEAVE_4
-		cfi->interleave = CFIDEV_INTERLEAVE_4;
 
 #ifdef SOMEONE_ACTUALLY_MAKES_THESE
 		cfi->device_type = CFI_DEVICETYPE_X32;
 		if (cp->probe_chip(map, 0, NULL, cfi))
 			return 1;
-#endif
-		cfi->device_type = CFI_DEVICETYPE_X16;
-		if (cp->probe_chip(map, 0, NULL, cfi))
-			return 1;
+#endif /* SOMEONE_ACTUALLY_MAKES_THESE */
+#endif /* CFIDEV_INTERLEAVE_2 */
+#if defined(CFIDEV_INTERLEAVE_1) && defined(SOMEONE_ACTUALLY_MAKES_THESE)
+                cfi->interleave = CFIDEV_INTERLEAVE_1;
 
-		cfi->device_type = CFI_DEVICETYPE_X8;
+                cfi->device_type = CFI_DEVICETYPE_X32;
 		if (cp->probe_chip(map, 0, NULL, cfi))
 			return 1;
-#endif /* CFIDEV_INTERLEAVE_4 */
+#endif /* CFIDEV_INTERLEAVE_1 */
 		break;
 #endif /* CFIDEV_BUSWIDTH_4 */
 

Re: [Fwd: helper routines for flash chip probing, search sequence]

[Thayne Harbaugh]

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

> From: Axel Ludszuweit <alu@keymile.com>

> > From: David Woodhouse <dwmw2@infradead.org>
> > At first glance, you seem to be right -- you'll not get false matches
> > for 4x interleave when in fact the interleave is 2x, so we should check
> > for 4x first. 

I also agree - although I haven't put much thought into it.

> I have a question, concerning the flash chip helper
> routines
>       linux/drivers/mtd/chips/gen_probe.c
> 
> In my opinion, the searching sequence, to find the
> flash geometry, (cfi->device_type and cfi->interleave)
> must be reversed, to prevent wrong decisions.
> If the buswidth is given, the search should begin 
> with the highest meaningfull interleave and the lowest
> device type.

Sounds correct - haven't tried the patch.

> With the current searching order I get the following
> effect.
> 
> I have 4 8bit flashes working on a 32 bit data bus.
> The Manufacture ID is 0x89, the Device ID 0xAA.
> The flash type is SHARP LH28F016SCT-L95

Uhhh, something isn't correct here:

from jedec_probe.c:
#define MANUFACTURER_INTEL	0x0089
#define I28F016S3	0x00aa

Does Sharp mimic Intel?

I just checked the data sheets for both chips and both claim the same
manufacturer and part identifiers.  Did one license the others design? 
This is just _pure_evil_!  I don't care if they are supposed to be
compatible (they look similar at first glance), they need to have
different ID's.

> The correct values, which should be detected are:
> 
> - cfi->device_type =  1   CFI_DEVICETYPE_X8
> - cfi->interleave  =  4   CFIDEV_INTERLEAVE_4
> 
> but the probing detected 
> 
>   cfi->interleave  = CFIDEV_INTERLEAVE_2 
>   cfi->device_type = CFI_DEVICETYPE_X16.

This should be fixed in the current version (1.26) of jedec_probe.c - it
doesn't allow the matching of output data widths that aren't supported
by the chip. See the following lines in jedec_match():


	uaddr = finfo_uaddr(finfo, cfi->device_type);
	if ( MTD_UADDR_NOT_SUPPORTED ) {
		goto match_done;
	}

If the part data width doesn't match what is probed then it is
unsupported and the match fails.

This, however, doesn't mean that changing the probe order in
gen_probe.c: genprobe_new_chip() - I think that is needed _as_well_.

> as valid, so that the test with the correct values, following later,
> will not be done.
> The identifier codes of this SHARP flashes can be read at offset
> 0 and 1 after writing 0x90 at any arbitrary address in the flash space.
> The unlock sequence 
>   0xaa at cfi->addr_unlock1 = 0x555 and 
>   0x55 at cfi->addr_unlock2 = 0x2aa
> seems not to be neccessary.

Fixing this is slowly creeping into the code.  Right now we have the
unlock addresses enumerated.  It's just one more evolutionary step to
use that list when searching for chips and finding matches.  I had a few
concerns about how I was going to make the change and therefor I didn't
- now I don't remember what my concerns were . . ..  I have several
changes queued up that I want to make so that probes are more correct
and chips can be better configured.  Maybe one day I'll get a round
tuit.

> The flashes 0 and 2 are unlocked by 0x00900090, flashes 1 and 4 not,
> so that 0 and 2 answers with th correct ID and 1 and 3 with 0x00.
> 
> If the search order is reversed, the test with the correct values is made
> before the test with wrong parameters.

Sounds sane - once again, I haven't looked too closely.

> I have attached a patch, which should prevent such wrong decisions, if
> the flashes are not correctly designed.

Thanks for the work.

> Does anyone know reasons, that the search order should not be reversed?

I'll do some further investigation soon - hopefully today.

-- 
Thayne Harbaugh
Linux Networx

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 232 bytes --]