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

[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