[coreboot] [PATCH] flashrom: Check for failed SPI command execution

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> On 17.11.2008 14:28, Stefan Reinauer wrote:

> I tried factoring it out, but since we don't want to die() completely,
> we have to return with an error code from this function. That pretty
> much rules out factoring out the checks into a separate function. I
> could move all checks one level down in the call graph and clutter up
> only the end of each function.
> What do you think?
>   

I guess your current version is fine then
>
>   
>>> +int spi_chip_erase_60_c7(struct flashchip *flash)
>>> +{
>>> +	int result;
>>> +	result = spi_chip_erase_60(flash);
>>> +	if (result) {
>>> +		printf_debug("spi_chip_erase_60 failed, trying c7\n");
>>> +		result = spi_chip_erase_c7(flash);
>>> +	}
>>> +	return result;
>>> +}
>>>   
>>>     
>>>       
>> I don't particularly like this. Maybe we should have spi_chip_erase()
>> try all different erase functions in a row, and check whether the erase
>> was actually successfull (all bytes 0xff)?
>>   
>>     
>
> That's a bit difficult. There are some chips which have either the 0x60
> or the 0xc7 opcode used for something else, so we need to keep this chip
> specific.
>   
Hm.. So do we need to keep an array of supported read, write, erase, id
opcodes for each spi chip to handle this? That way we could check the
chip's capabilities and compare to the OPMENUs capabilities.

That one function does one job, but I'm a little concerned that we end
up writing functions for all possible variations of possible commands
and have a hard time tracking it afterwards.



Stefan


-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866