[U-Boot] [PATCH] MTD/SPI/FLASH: add support for Ramtron FRAMs using SPI

[Mike Frysinger]

On Saturday, August 28, 2010 19:45:57 Reinhard Meyer wrote:
> On 29.08.2010 01:17, Mike Frysinger wrote:
> > On Saturday, August 28, 2010 17:48:39 Mike Frysinger wrote:
> >> +			/* Make sure the ID was jedec extended */
> >> +			j = flashes[i].shift - 1;
> >> +			while (j>= 0&&  idcode[j] == 0x7f)
> >> +				continue;
> > 
> > thinko ... this loops forever.  the tested fix:
> > 	j = flashes[i].shift;
> > 	while (--j>= 0)
> > 	
> > 		if (idcode[j] != 0x7f)
> > 		
> > 			break;
> > 
> > this whole change increases code size a bit (20 - 60 bytes on Blackfin
> > depending on how many flashes are supported), but it makes management
> > easier and allows for arbitrarily long manufacturers ids.  so i think
> > it's worth it.
> 
> arbitrarily long ids unfortunately require an arbitrarily long id_buffer :)

well right, but now the problem has been reduced to simply changing the len of 
the idcode buffer

> In that case I think its easier to right after READ_ID count the 0x7f's
> and search the table with count and the id:

hmm, that's true.  no point in rescanning the idcode multiple times.

> (n is either a compile time constant or determined by examining the table
> for largest 'shift'+3)

i'd prefer to stick to compile constant for now so we dont have to force the 
hardware to keep recomputing a number that gcc isnt capable of optimizing into 
a constant for us.

> read-id (n bytes)
> shift=count 7f's (max n-3)
> id=id_buffer[shift]
> search table for shift and id, call function.
> if function returns NULL, continue search in table.
> 
> That will allow several shift=0, id=0xff entries in the table, which should
> be ordered such that least likely to false detect probes come first:
> stmicro first since that probe actually has a way to really figure out
> the device, the FM25H20-ramtron has no other way except for a ram size
> test which certainly is not a good idea to do here...

hmm, pushing the non-jedec probes into the const table is attractive indeed

> It is disputable whether the function gets id_buffer or id_buffer+shift
> as parameter. I'd prefer the latter.

since we already need to calculate the value of id_buffer+shift to check the 
one byte, i think passing that down is OK for now.  i would just make a note 
in the code about this behavior.

> I am willing to code and test and submit a patch for that method.

if you want to take the patch i posted and extend that by itself with the 
comments here, and then do the ramtron patch separately on top of that, that'd 
be great.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100828/14fdc655/attachment.pgp