[coreboot] [PATCH] flashrom: Group probe function together with associated IDs

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Tue Dec 9 10:51:37 CET 2008

On 09.12.2008 09:14, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
>> The flash probe function and the IDs for that specific probe function
>> belong together. Group them together on a single line.
>> We already have chips with multiple probe functions and multiple IDs.
>> This patch is the first step in consolidating the present way of
>> creating a new chip definition for each ID of a given chip into multiple
>> IDs per chip definition.
>> Attached for Gmail users.
> Thunderbird users also love this.
>> -	{"AMD", "Am29F002(N)BB", AMD_ID, AM_29F002BB,
>> +	{"AMD", "Am29F002(N)BB",
>> +	 AMD_ID, AM_29F002BB, probe_jedec,
> Please leave the chip name and ID on the same line. Once we fixed
> flashrom to use the non-hidden IDs here, that'll be a very convenient
> way for associating IDs and chips.
>>  	 256, 256,
>> -	 probe_jedec, erase_chip_jedec, write_en29f002a
>> +	 erase_chip_jedec, write_en29f002a 
>>  	},
> I think function pointers should not be intercepted by the sized above.
> This patch obfuscates the code.

How do you propose to handle chips which have multiple different IDs and
multiple different probe functions? I hope you're not seriously
suggesting something like this:

{"Vendor", "Chip", 0x01, 0x23, 0x45, 0x56, 0x78, 0x9a,
256, 256,
probe_spi_rdid, probe_spi_rems, probe_spi_res, erase_spi_foo, write_spi,

If we picked that layout, it would be really difficult to find out which
ID belongs to which probe function.

And having 3 entries for almost all SPI flash chips is definitely not
the way to go. Multiply that with the number of erase functions (up to 5
for some SPI chips) and you have 15 entries per real chip. Sure, I can
generate such entries, but people are going to hate that.

If the problem is not obvious, I'll gladly send a patch to illustrate my



More information about the coreboot mailing list