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 <[email protected]>
With the two bugfixes discussed on IRC this is: Acked-by: Uwe Hermann <[email protected]> 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 -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
