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;
_______________________________________________
flashrom mailing list
flashrom@flashrom.org
https://www.flashrom.org/mailman/listinfo/flashrom