[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,
>> TEST_UNTESTED,
>> - 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,
TEST_UNTESTED,
probe_spi_rdid, probe_spi_rems, probe_spi_res, erase_spi_foo, write_spi,
read_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
point.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list