Hi,
Could we get the review moving on this patch? I first sent it over 4 months ago and got no feedback. I'd really like to have this feature merged, as I'm using it very often!
Thanks
Le lundi 29 février 2016 à 19:44 +0100, Paul Kocialkowski a écrit :
When developing software that has to be flashed to a flash chip to be executed, it often takes a long time to read the current flash contents (for flashrom to know what pages to erase and reprogram) each time, when writing the new image. However, when the flash was just reprogrammed, its current state is known to be the previous image that was flashed (assuming it was verified).
Thus, it makes sense to provide that image as a file for the flash contents instead of wasting valuable time read the whole flash each time.
Signed-off-by: Paul Kocialkowski contact@paulk.fr
cli_classic.c | 46 ++++++++++++++++++++++++++++------------------ flash.h | 2 +- flashrom.c | 14 +++++++++++--- 3 files changed, 40 insertions(+), 22 deletions(-)
diff --git a/cli_classic.c b/cli_classic.c index a2c2014..dc0164d 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -44,24 +44,25 @@ static void cli_classic_usage(const char *name) "[-E|(-r|-w|-v) <file>] [-l <layoutfile> [-i <imagename>]...] [-n] [-f]]\n" "[-V[V[V]]] [-o <logfile>]\n\n", name);
- printf(" -h | --help print this help text\n"
- " -R | --version print version
(release)\n"
- " -r | --read <file> read flash and save to
<file>\n"
- " -w | --write <file> write <file> to flash\n"
- " -v | --verify <file> verify flash against
<file>\n"
- " -E | --erase erase flash memory\n"
- " -V | --verbose more verbose output\n"
- " -c | --chip <chipname> probe only for specified
flash chip\n"
- " -f | --force force specific operations
(see man page)\n"
- " -n | --noverify don't auto-verify\n"
- " -l | --layout <layoutfile> read ROM layout from
<layoutfile>\n"
- " -i | --image <name> only flash image <name>
from flash layout\n"
- " -o | --output <logfile> log output to
<logfile>\n"
- " -L | --list-supported print supported
devices\n"
- printf(" -h | --help print this help text\n"
- " -R | --version print version
(release)\n"
- " -r | --read <file> read flash and save to
<file>\n"
- " -w | --write <file> write <file> to
flash\n"
- " -v | --verify <file> verify flash against
<file>\n"
- " -E | --erase erase flash memory\n"
- " -V | --verbose more verbose output\n"
- " -c | --chip <chipname> probe only for
specified flash chip\n"
- " -f | --force force specific
operations (see man page)\n"
- " -n | --noverify don't auto-verify\n"
- " -l | --layout <layoutfile> read ROM layout from
<layoutfile>\n"
- " -i | --image <name> only flash image <name>
from flash layout\n"
- " -o | --output <logfile> log output to
<logfile>\n"
- " -C | --flash-contents <contentsfile> assume flash contents
to be <contentsfile>\n"
- " -L | --list-supported print supported
devices\n" #if CONFIG_PRINT_WIKI == 1
- " -z | --list-supported-wiki print supported devices
in wiki syntax\n"
- " -z | --list-supported-wiki print supported devices
in wiki syntax\n" #endif
- " -p | --programmer <name>[:<param>] specify the programmer
device. One of\n");
- " -p | --programmer <name>[:<param>] specify the programmer
device. One of\n"); list_programmers_linebreak(4, 80, 0); printf(".\n\nYou can specify one of -h, -R, -L, " #if CONFIG_PRINT_WIKI == 1 @@ -106,7 +107,7 @@ int main(int argc, char *argv[]) enum programmer prog = PROGRAMMER_INVALID; int ret = 0;
- static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:Lzho:";
- static const char optstring[] = "r:Rw:v:nVEfc:l:i:C:p:Lzho:";
static const struct option long_options[] = { {"read", 1, NULL, 'r'}, {"write", 1, NULL, 'w'}, @@ -118,6 +119,7 @@ int main(int argc, char *argv[]) {"force", 0, NULL, 'f'}, {"layout", 1, NULL, 'l'}, {"image", 1, NULL, 'i'},
{"flash-contents", 1, NULL, 'C'},{"list-supported", 0, NULL, 'L'}, {"list-supported-wiki", 0, NULL, 'z'}, {"programmer", 1, NULL, 'p'}, @@ -128,6 +130,7 @@ int main(int argc, char *argv[]) }; char *filename = NULL;
- char *contentsfile = NULL;
char *layoutfile = NULL; #ifndef STANDALONE char *logfile = NULL; @@ -221,6 +224,9 @@ int main(int argc, char *argv[]) cli_classic_abort_usage(); } break;
case 'C':contentsfile = strdup(optarg);break;case 'L': if (++operation_specified > 1) { fprintf(stderr, "More than one operation " @@ -332,6 +338,9 @@ int main(int argc, char *argv[]) if (layoutfile && check_filename(layoutfile, "layout")) { cli_classic_abort_usage(); }
- if (contentsfile && check_filename(contentsfile, "contents")) {
cli_classic_abort_usage();- }
#ifndef STANDALONE if (logfile && check_filename(logfile, "log")) @@ -542,7 +551,7 @@ int main(int argc, char *argv[]) * Give the chip time to settle. */ programmer_delay(100000);
- ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it,
verify_it);
- ret |= doit(fill_flash, force, filename, contentsfile, read_it,
write_it, erase_it, verify_it); unmap_flash(fill_flash); out_shutdown: @@ -554,6 +563,7 @@ out: layout_cleanup(); free(filename); free(layoutfile);
- free(contentsfile);
free(pparam); /* clean up global variables */ free((char *)chip_to_probe); /* Silence! Freeing is not modifying contents. */ diff --git a/flash.h b/flash.h index dc5c140..2cf7ca6 100644 --- a/flash.h +++ b/flash.h @@ -284,7 +284,7 @@ void print_buildinfo(void); void print_banner(void); void list_programmers_linebreak(int startcol, int cols, int paren); int selfcheck(void); -int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it); +int doit(struct flashctx *flash, int force, const char *filename, const char *contentsfile, int read_it, int write_it, int erase_it, int verify_it); int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename); int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename); diff --git a/flashrom.c b/flashrom.c index d5c3238..d11f89d 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1978,8 +1978,9 @@ int chip_safety_check(const struct flashctx *flash, int force, int read_it, int * but right now it allows us to split off the CLI code. * Besides that, the function itself is a textbook example of abysmal code flow. */ -int doit(struct flashctx *flash, int force, const char *filename, int read_it,
- int write_it, int erase_it, int verify_it)
+int doit(struct flashctx *flash, int force, const char *filename,
- const char *contentsfile, int read_it, int write_it, int erase_it,
- int verify_it)
{ uint8_t *oldcontents; uint8_t *newcontents; @@ -2068,7 +2069,14 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it, * preserved, but in that case we might perform unneeded erase which * takes time as well. */
- if (read_all_first) {
- if (contentsfile) {
msg_cinfo("Reading old flash chip contents from file... ");if (read_buf_from_file(oldcontents, size, contentsfile)) {ret = 1;msg_cinfo("FAILED.\n");goto out;}- } else if (read_all_first) {
msg_cinfo("Reading old flash chip contents... "); if (flash->chip->read(flash, oldcontents, 0, size)) { ret = 1;