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

[Uwe Hermann]

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.

 
> +/* start is an offset to the base address of the flash chip */

Please start sentences with capital letter and end them with full stop
for consistency.


> +/* start is an offset to the base address of the flash chip */

Ditto.


> @@ -146,26 +156,41 @@
>  	printf("total_size/page_size = %d\n", total_size / page_size);
>  	for (i = 0; i < (total_size / page_size) - 1; i++) {
>  		printf("%04d at address: 0x%08x\n", i, i * page_size);
> -		block_erase_m29f400bt(bios, bios + i * page_size);
> +		if (block_erase_m29f400bt(flash, i * page_size, page_size)) {
> +			fprintf(stderr, "ERASE FAILED!\n");
> +			return -1;
> +		}
>  		write_page_m29f400bt(bios, buf + i * page_size,
>  				     bios + i * page_size, page_size);
>  		printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
>  	}
>  
>  	printf("%04d at address: 0x%08x\n", 7, 0x70000);
> -	block_erase_m29f400bt(bios, bios + 0x70000);
> +	if (block_erase_m29f400bt(flash, 0x70000, 32 * 1024)) {
> +		fprintf(stderr, "ERASE FAILED!\n");
> +		return -1;
> +	}
>  	write_page_m29f400bt(bios, buf + 0x70000, bios + 0x70000, 32 * 1024);
>  
>  	printf("%04d at address: 0x%08x\n", 8, 0x78000);
> -	block_erase_m29f400bt(bios, bios + 0x78000);
> +	if (block_erase_m29f400bt(flash, 0x78000, 8 * 1024)) {
> +		fprintf(stderr, "ERASE FAILED!\n");
> +		return -1;
> +	}
>  	write_page_m29f400bt(bios, buf + 0x78000, bios + 0x78000, 8 * 1024);
>  
>  	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).


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org