[coreboot] [PATCH] flashrom: Check if erase worked, check return codes

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Jun 15 19:24:19 CEST 2009


On 15.06.2009 18:12, Uwe Hermann wrote:
> On Mon, Jun 15, 2009 at 12:26:46PM +0200, Carl-Daniel Hailfinger wrote:
>   
>> I wasn't really awake while changing that code. Sorry. Same for the
>> other bugs you found.
>>
>> Compared to your version, I changed the following:
>> - Check inside all erase_*_jedec routines if erase worked, not outside.
>> - Rename rv to ret. Most functions in flashrom call the return variable ret.
>> - erase_sector_jedec and erase_block_jedec have changed prototypes to
>> enable erase checking.
>> - verify_range uses goto out_free to make sure we don't forget to free().
>> - Convert all remaining erase functions and actually check return codes
>> almost everywhere.
>>
>> Urja, would you remind reviewing the whole patch again? At 1087 lines of
>> manual conversions, it is too big for me to be 100% confident that I got
>> everything right.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>     
>
> With the two bugfixes discussed on IRC this is:
>
> Acked-by: Uwe Hermann <uwe at hermann-uwe.de>
>
> I successfully tested LPC on an CK804 box and SPI on some sb600 box.
>   

Thanks, committed with your suggestions in r595.
I added some doxygen comments to verify_range() as well.


>>  	printf("%04d at address: 0x%08x\n", 9, 0x7a000);
>> -	block_erase_m29f400bt(bios, bios + 0x7a000);
>> +	if (block_erase_m29f400bt(flash, 0x7a000, 8 * 1024)) {
>> +		fprintf(stderr, "ERASE FAILED!\n");
>> +		return -1;
>> +	}
>>  	write_page_m29f400bt(bios, buf + 0x7a000, bios + 0x7a000, 8 * 1024);
>>  
>>  	printf("%04d at address: 0x%08x\n", 10, 0x7c000);
>> -	block_erase_m29f400bt(bios, bios + 0x7c000);
>> +	if (block_erase_m29f400bt(flash, 0x7c000, 16 * 1024)) {
>> +		fprintf(stderr, "ERASE FAILED!\n");
>> +		return -1;
>> +	}
>>     
>
> This code need some refactoring IMHO, we can easily make an array
> of block sizes and loop over that (that's for another patch though).
>   

Actually, my eraseblock patch solves this nicely.

Regards,
Carl-Daniel

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





More information about the coreboot mailing list