[PATCH 2/3] Add support for 4 bytes rdid

[Rudolf Marek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

Mine AMIC flash chip needs 4 bytes RDID. Following patch adds support for that.

Signed-off-by: Rudolf Marek

Thanks,
Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIaUZb3J9wPJqZRNURAl74AKCIbhYCudIyKL/hJ3aHYEzb1O3O0ACgrEg7
Wr8KvR7+GfSGb3l875Ibk3I=
=9ePr
-----END PGP SIGNATURE-----

[Peter Stuge]

On Mon, Jun 30, 2008 at 10:47:23PM +0200, Rudolf Marek wrote:
> @@ -97,7 +97,8 @@
>    	if (!oddparity(readarr[1]))
>    		printf_debug("RDID byte 1 parity violation.\n");
>    	manuf_id = (readarr[0] << 8) | readarr[1];
> model_id = readarr[2];
> model_id = (bytes == 3) ? readarr[2] :
> 				((readarr[2] << 8) | readarr[3]);

Please improve this a bit, I suggest:

model_id=readarr[2];
if(bytes>3) { /* or 4==bytes */
  model_id<<=8;
  model_id|=readarr[3];
}

With a fix (not neccessarily mine) for the above, this is:

Acked-by: Peter Stuge

[Rudolf Marek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Okay I fixed that.

Committed revision 3399.

Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD4DBQFIaVQl3J9wPJqZRNURAhiRAKCTQzScPL6KHT34P4Hn+rVgs+0bFACXUwSN
xO8nOWu8vg752/+8B6yz+A==
=Cp23
-----END PGP SIGNATURE-----

[Carl-Daniel Hailfinger]

On 30.06.2008 23:19, Peter Stuge wrote:
> On Mon, Jun 30, 2008 at 10:47:23PM +0200, Rudolf Marek wrote:
> > @@ -97,7 +97,8 @@
> >    	if (!oddparity(readarr[1]))
> >    		printf_debug("RDID byte 1 parity violation.\n");
> >    	manuf_id = (readarr[0] << 8) | readarr[1];
> > model_id = readarr[2];
> > model_id = (bytes == 3) ? readarr[2] :
> > 				((readarr[2] << 8) | readarr[3]);
>
> Please improve this a bit, I suggest:
>
> model_id=readarr[2];
> if(bytes>3) { /* or 4==bytes */
>   model_id<<=8;
>   model_id|=readarr[3];
> }
>
> With a fix (not neccessarily mine) for the above, this is:

Technically, both variants will fail for some chips. There are at least
the following RDID responses:
1 byte manuf, 2 bytes model
2 bytes manuf, 1 byte model
2 bytes manuf, 2 bytes model
and all of the above with additional data appended.

The existing code handles the first two cases, but not the third. The
new code handles the first and third case, but not the second.
> Acked-by: Peter Stuge

I know of some SPI chips which return 16 bytes for RDID. How about
reading 16 bytes, cropping the result as needed?

Regards,
Carl-Daniel

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> On 30.06.2008 23:19, Peter Stuge wrote:
> > On Mon, Jun 30, 2008 at 10:47:23PM +0200, Rudolf Marek wrote:
> > > @@ -97,7 +97,8 @@
> > >    	if (!oddparity(readarr[1]))
> > >    		printf_debug("RDID byte 1 parity violation.\n");
> > >    	manuf_id = (readarr[0] << 8) | readarr[1];
> > > model_id = readarr[2];
> > > model_id = (bytes == 3) ? readarr[2] :
> > > 				((readarr[2] << 8) | readarr[3]);
> >
> > Please improve this a bit, I suggest:
> >
> > model_id=readarr[2];
> > if(bytes>3) { /* or 4==bytes */
> >   model_id<<=8;
> >   model_id|=readarr[3];
> > }
> >
> > With a fix (not neccessarily mine) for the above, this is:
>
> Technically, both variants will fail for some chips. There are at least
> the following RDID responses:
> 1 byte manuf, 2 bytes model
> 2 bytes manuf, 1 byte model
> 2 bytes manuf, 2 bytes model
> and all of the above with additional data appended.
>
> The existing code handles the first two cases, but not the third. The
> new code handles the first and third case, but not the second.

What chips are there that send the second case?
> I know of some SPI chips which return 16 bytes for RDID. How about
> reading 16 bytes, cropping the result as needed?

Yes the numonyx do that, but what you read is a customer specific serial 
number, not part of the identification process.

Reading that might allow for some nice use cases though. (Does it?)

Stefan

[Carl-Daniel Hailfinger]

On 01.07.2008 01:49, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
> > On 30.06.2008 23:19, Peter Stuge wrote:
> > > On Mon, Jun 30, 2008 at 10:47:23PM +0200, Rudolf Marek wrote:
> > > > @@ -97,7 +97,8 @@
> > > >          if (!oddparity(readarr[1]))
> > > >              printf_debug("RDID byte 1 parity violation.\n");
> > > >          manuf_id = (readarr[0] << 8) | readarr[1];
> > > >    model_id = readarr[2];
> > > >    model_id = (bytes == 3) ? readarr[2] :
> > > >                    ((readarr[2] << 8) | readarr[3]);
> > >
> > > Please improve this a bit, I suggest:
> > >
> > > model_id=readarr[2];
> > > if(bytes>3) { /* or 4==bytes */
> > >   model_id<<=8;
> > >   model_id|=readarr[3];
> > > }
> > >
> > > With a fix (not neccessarily mine) for the above, this is:
> >
> > Technically, both variants will fail for some chips. There are at least
> > the following RDID responses:
> > 1 byte manuf, 2 bytes model
> > 2 bytes manuf, 1 byte model
> > 2 bytes manuf, 2 bytes model
> > and all of the above with additional data appended.
> >
> > The existing code handles the first two cases, but not the third. The
> > new code handles the first and third case, but not the second.
>
> What chips are there that send the second case?

#define PMC_25LV512             0x7B
#define PMC_25LV010             0x7C
#define PMC_25LV020             0x7D
#define PMC_25LV040             0x7E
#define PMC_25LV080B            0x13
#define PMC_25LV016B            0x14
> > I know of some SPI chips which return 16 bytes for RDID. How about
> > reading 16 bytes, cropping the result as needed?
>
> Yes the numonyx do that, but what you read is a customer specific
> serial number, not part of the identification process.

Well, I have seen other chips which return a flash descriptor with
eraseblock info etc. No idea which vendor that was, though.
> Reading that might allow for some nice use cases though. (Does it?)

The flash descriptor would certainly be useful. Serial number would be
interesting, but only for inventory.

Regards,
Carl-Daniel