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

[Thayne Harbaugh]

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

> > From: David Woodhouse <dwmw2 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: This is a digitally signed message part
Url : http://lists.infradead.org/pipermail/linux-mtd/attachments/20030508/fad5393c/attachment.bin