Handle erase failure in partial write. Clean up erase function checking. Update a few comments and messages to improve readability.
The erase failure handling is a genuine bugfix which is needed on locked down chipsets and in the unlikely case that write fails halfway through or an incorrect chip definition is used. Signed-off-by: Carl-Daniel Hailfinger <[email protected]> Index: flashrom-cleanup_erase_and_write/flashrom.c =================================================================== --- flashrom-cleanup_erase_and_write/flashrom.c (Revision 1236) +++ flashrom-cleanup_erase_and_write/flashrom.c (Arbeitskopie) @@ -1429,57 +1429,84 @@ return 0; } +static int check_block_eraser(struct flashchip *flash, int k, int log) +{ + struct block_eraser eraser = flash->block_erasers[k]; + + if (log) + msg_cdbg("Looking at blockwise erase function %i... ", k); + if (!eraser.block_erase && !eraser.eraseblocks[0].count) { + if (log) + msg_cdbg("not defined. "); + return 1; + } + if (!eraser.block_erase && eraser.eraseblocks[0].count) { + if (log) + msg_cdbg("eraseblock layout is known, but no " + "matching block erase function found. "); + return 1; + } + if (eraser.block_erase && !eraser.eraseblocks[0].count) { + if (log) + msg_cdbg("block erase function found, but " + "eraseblock layout is unknown. "); + return 1; + } + return 0; +} + int erase_and_write_flash(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents) { - int k, ret = 0, found = 0; + int k, ret = 0; uint8_t *curcontents; unsigned long size = flash->total_size * 1024; + int usable_erasefunctions = 0; + for (k = 0; k < NUM_ERASEFUNCTIONS; k++) + if (!check_block_eraser(flash, k, 0)) + usable_erasefunctions++; + msg_cinfo("Erasing and writing flash chip... "); + if (!usable_erasefunctions) { + msg_cerr("ERROR: flashrom has no erase function for this flash " + "chip.\n"); + return 1; + } + curcontents = (uint8_t *) malloc(size); /* Copy oldcontents to curcontents to avoid clobbering oldcontents. */ memcpy(curcontents, oldcontents, size); - msg_cinfo("Erasing and writing flash chip... "); for (k = 0; k < NUM_ERASEFUNCTIONS; k++) { - struct block_eraser eraser = flash->block_erasers[k]; - - msg_cdbg("Looking at blockwise erase function %i... ", k); - if (!eraser.block_erase && !eraser.eraseblocks[0].count) { - msg_cdbg("not defined. " - "Looking for another erase function.\n"); + if (check_block_eraser(flash, k, 1) && usable_erasefunctions) { + msg_cdbg("Looking for another erase function.\n"); continue; } - if (!eraser.block_erase && eraser.eraseblocks[0].count) { - msg_cdbg("eraseblock layout is known, but no " - "matching block erase function found. " - "Looking for another erase function.\n"); - continue; - } - if (eraser.block_erase && !eraser.eraseblocks[0].count) { - msg_cdbg("block erase function found, but " - "eraseblock layout is unknown. " - "Looking for another erase function.\n"); - continue; - } - found = 1; + usable_erasefunctions--; msg_cdbg("trying... "); ret = walk_eraseregions(flash, k, &erase_and_write_block_helper, curcontents, newcontents); msg_cdbg("\n"); /* If everything is OK, don't try another erase function. */ if (!ret) break; - /* FIXME: Reread the whole chip here so we know the current - * chip contents? curcontents might be up to date, but this - * code is only reached if something failed, and then we don't - * know exactly what failed, and how. + /* Write/erase failed, so try to find out what the current chip + * contents are. If no usable erase functions remain, we could + * abort the loop instead of continuing, the effect is the same. + * The only difference is whether the reason for other unusable + * functions is printed or not. If in doubt, verbosity wins. */ + if (!usable_erasefunctions) + continue; + if (flash->read(flash, curcontents, 0, size)) { + /* Now we are truly screwed. Read failed as well. */ + msg_cerr("Can't read anymore!\n"); + /* We have no idea about the flash chip contents, so + * retrying with another erase function is pointless. + */ + break; + } } /* Free the scratchpad. */ free(curcontents); - if (!found) { - msg_cerr("ERROR: flashrom has no erase function for this flash chip.\n"); - return 1; - } if (ret) { msg_cerr("FAILED!\n"); @@ -1827,18 +1854,15 @@ * preserved, but in that case we might perform unneeded erase which * takes time as well. */ - msg_cdbg("Reading old flash chip contents...\n"); + msg_cdbg("Reading current flash chip contents...\n"); if (flash->read(flash, oldcontents, 0, size)) { ret = 1; goto out; } - // This should be moved into each flash part's code to do it - // cleanly. This does the job. + /* Build a new image from the given layout. */ handle_romentries(flash, oldcontents, newcontents); - // //////////////////////////////////////////////////////////// - if (write_it) { if (erase_and_write_flash(flash, oldcontents, newcontents)) { msg_cerr("Uh oh. Erase/write failed. Checking if " -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
