[LinuxBIOS] [PATCH] flashrom: Support EON EN29F002AT, JEDEC continuation IDs

[Carl-Daniel Hailfinger]

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