[coreboot] r3393 - trunk/util/flashrom

[Carl-Daniel Hailfinger]

On 28.06.2008 19:37, Stefan Reinauer wrote:
> I was having a cleanup of the whole SPI/LPC bus handling in mind:
>
> we have _1_ global variable (per instance of "flashes"?) instead of one
> per supported chipset.
>
> This variable is an ENUM
>
> enum flash_bus {
>    FLASH_BUS_LPC,
>    FLASH_BUS_ICH7_SPI,
>    FLASH_BUS_ICH9_SPI,
>    FLASH_BUS_VIA_SPI,
>    FLASH_BUS_IT87_SPI
> }
>   

Looks nice. Can we add a bus for those old (29*) parallel flash chips?
I assume differentiating between LPC and FWH will be difficult because
FWH is just a LPC variant and some chipsets do not allow us to detect
FWH/LPC


> I think this matches the physical actualities much better than the
> design-less hack that is used at the moment. Also the code could be much
> nicer in many places, as we could use case statements instead of endless
> if () else if () else if() statements
>   

What happens if there is more than one flash bus? Do we call the probe
functions compatible with the bus once per bus? If we do that, we can
stop after the first detected chip.

for (i=0; i<buscount; i++) {
	for (j=0; j<chipcount; j++) {
		if (!compatible(bus[i], flashchips[j])
			continue;
		found = flashchips[j].probe(bus[i], flashchips[j]);
		if (!found)
			continue;
		//add (chip,bus) pair to list of found chips
		break;
	}
}


Benefits of that approach:
* my "generic unknown flash chip" entries will work perfectly as expected
* it allows us to integrate drivers for external flashers
* probing for chips strapped to a non-default location (ID!=0) is trivial.

Regards
Carl-Daniel

-- 
http://www.hailfinger.org/