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

[Carl-Daniel Hailfinger]

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
>
> With the two bugfixes discussed on IRC this is:
>
> Acked-by: Uwe Hermann
>
> 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