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.2...@gmx.net>
>>
>
> With the two bugfixes discussed on IRC this is:
>
> Acked-by: Uwe Hermann <u...@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/
--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
