On 30.12.2007 05:40, Corey Osgood wrote:
>> + /* Check if it is a continuation ID, this should be a while
>> loop. */
>> + if (id1 == 0x7F) {
>> + largeid1 <<= 8;
>> + id1 = *(volatile uint8_t *)(bios + 0x100);
>> + largeid1 |= id1;
>> + }
>> + if (id2 == 0x7F) {
>> + largeid2 <<= 8;
>> + id2 = *(volatile uint8_t *)(bios + 0x101);
>> + largeid2 |= id2;
>> + }
>>
>
> are you sure this is right? The IDs are, at most, just 0x7f7f7fXX? Just
>
Actually, the code above does not go further than checking for IDs of
the type 0x7fXX, but does this for vendor and product ID. The current
published JEDEC spec has a list where the largest vendor ID is 7 bytes
long, but all leading bytes are 0x7f. The list will grow in the future,
and using a 64bit variable will not be enough anymore.
Suggestion for a new encoding:
Use a two-byte data type for the ID, the lower byte contains the only
non-0x7f byte, the upper byte contains the number of 0x7f bytes used as
prefix (which is the "page" number the vendor ID appears in.
> doesn't seem quite right, but it might be (yes, I realize it means 4x
> more IDs). We also break the ability to use IMT flash chips (which
> someone may, eventually...). The other things is, if it should be a
> while loop, why isn't it?
>
Oh, I had not seen the IMT entry in flash.h. It is obviously wrong. With
the current code, at least EON and IMT will collide, and neither have a
real vendor ID of 0x7f. I'll dig out the real IMT vendor ID.
> You also forgot your email in the copyright header in flashchips.c.
>
That is intentional.
Thanks for your review!
Regards,
Carl-Daniel
--
linuxbios mailing list
linuxbios@linuxbios.org
http://www.linuxbios.org/mailman/listinfo/linuxbios
