December 2024 - flashrom-gerrit

[S] Change in flashrom[main]: cli_classic: Add option to use first detected chip if multiple found
by Matt DeVillier (Code Review) 07 Dec '24

07 Dec '24

Attention is currently required from: Anastasia Klimchuk, David Hendricks. Matt DeVillier has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/85160?usp=email ) Change subject: cli_classic: Add option to use first detected chip if multiple found ...................................................................... Patch Set 2: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/85160/comment/021bfeb3_32c4dc71?us… : PS2, Line 133: " --use-first-chip in cases where multiple chips are detected, use the first one found\n" > For your use case, are you interested at all in WP feature? If yes, I am not sure it would work with the strategy of choosing first matching model always. in 99% of cases, WP is already disabled when doing the initial flash via ChromeOS, but I do use `--wp-status` and `--wp-disable` to be sure. > If most of chips in this context are new enough to support SFDP, would you consider trying SFDP auto-detection first? Specifically, that's adding -c "SFDP-capable chip" to command line, so that instead of going through the full list flashrom only tries to talk to chip as a generic "SFDP-capable chip"? I tested this on a few older devices (Haswell, Broadwell, Sany/IvyBridge) and none of them support SFDP, so report chip not found when using the `-c "SFDP-capable chip` param -- To view, visit https://review.coreboot.org/c/flashrom/+/85160?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I0427b1ef80e4eca16623f0fc9119d79f7dd62551 Gerrit-Change-Number: 85160 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 08 Dec 2024 02:42:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: cli_classic: Add option to use first detected chip if multiple found
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Attention is currently required from: David Hendricks, Matt DeVillier. Anastasia Klimchuk has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/85160?usp=email ) Change subject: cli_classic: Add option to use first detected chip if multiple found ...................................................................... Patch Set 2: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/85160/comment/9e9a871e_3c7edd88?us… : PS2, Line 133: " --use-first-chip in cases where multiple chips are detected, use the first one found\n" > The context is very useful, thank you. […] Hello Matt, just checking, what do you think about all the above? Purely as an example, an output for `flashrom -p dummy:emulate=MX25L6436 -c "SFDP-capable chip" -VV` has these lines (at the time of writing): ``` Probing for Unknown SFDP-capable chip, 0 kB: SFDP revision = 1.0 SFDP number of parameter headers is 2 (NPH = 1). SFDP parameter table header 0/1: ID 0x00, version 1.0 Length 36 B, Parameter Table Pointer 0x00001c Parsing JEDEC flash parameter table... 3-Byte only addressing. Status register is non-volatile and the standard does not allow vendors to tell us whether EWSR/WREN is needed for status register writes - assuming EWSR. Write chunk size is at least 64 B. Flash chip size is 8192 kB. Block eraser 0: 2048 x 4096 B with opcode 0x20 Tried to add a duplicate block eraser: 2048 x 4096 B with opcode 0x20. Block eraser 1: 256 x 32768 B with opcode 0x52 Block eraser 2: 128 x 65536 B with opcode 0xd8 done. SFDP parameter table header 1/1: ID 0xc2, version 1.0 Length 16 B, Parameter Table Pointer 0x000048 Support for SFDP Page with ID 0xc2 not implemented, skipping it. ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/85160?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I0427b1ef80e4eca16623f0fc9119d79f7dd62551 Gerrit-Change-Number: 85160 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Comment-Date: Fri, 06 Dec 2024 08:37:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips.c: Add reg_bits for W25Q256JW
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Attention is currently required from: Hsuan Ting Chen, Kapil Porwal. Anastasia Klimchuk has posted comments on this change by Kapil Porwal. ( https://review.coreboot.org/c/flashrom/+/85323?usp=email ) Change subject: flashchips.c: Add reg_bits for W25Q256JW ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Thanks everyone, for contribution and reviews! […] Kapil, the patch CB:83307 is submitted, so you can go ahead. You need to move your code to `winbond.c`, upload new patchset (make sure it builds), and then resolve this comment. Thanks. -- To view, visit https://review.coreboot.org/c/flashrom/+/85323?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I050754b28a90911a50f891869297524ce9a6720e Gerrit-Change-Number: 85323 Gerrit-PatchSet: 2 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Fri, 06 Dec 2024 06:33:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: Extract cli_output declarations to a separate header.
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/85134?usp=email ) Change subject: Extract cli_output declarations to a separate header. ...................................................................... Extract cli_output declarations to a separate header. This is a simple refactor that aims to simplify maintenance and to clarify file dependency inside the project. Currently, many declarations reside in flash.h making it difficult to really understand file dependency. Change-Id: I4209d5ed205ca14c39e83aa923e103b7282a7059 Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/85134 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M cli_output.c A include/cli_output.h M include/flash.h 4 files changed, 35 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index f622720..1a0565b 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -26,6 +26,7 @@ #include <stdlib.h> #include <stdint.h> #include <cli_getopt.h> +#include <cli_output.h> #include "flash.h" #include "flashchips.h" #include "fmap.h" diff --git a/cli_output.c b/cli_output.c index 20295b8..a8b8d7c 100644 --- a/cli_output.c +++ b/cli_output.c @@ -15,11 +15,12 @@ * GNU General Public License for more details. */ +#include <cli_output.h> + #include <stdio.h> #include <stdarg.h> #include <string.h> #include <errno.h> -#include "flash.h" enum flashrom_log_level verbose_screen = FLASHROM_MSG_INFO; enum flashrom_log_level verbose_logfile = FLASHROM_MSG_DEBUG2; diff --git a/include/cli_output.h b/include/cli_output.h new file mode 100644 index 0000000..3a3db88 --- /dev/null +++ b/include/cli_output.h @@ -0,0 +1,31 @@ +/* + * This file is part of the flashrom project. + * + * Copyright 2024 Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __CLI_OUTPUT_H__ +#define __CLI_OUTPUT_H__ + +#include <stdarg.h> +#include <flash.h> + +extern enum flashrom_log_level verbose_screen; +extern enum flashrom_log_level verbose_logfile; +int open_logfile(const char * const filename); +int close_logfile(void); +void start_logging(void); +int flashrom_print_cb(enum flashrom_log_level level, const char *fmt, va_list ap); +void flashrom_progress_cb(struct flashrom_flashctx *flashctx); + +#endif /* __CLI_OUTPUT_H__ */ diff --git a/include/flash.h b/include/flash.h index a1f1551..629ef3a 100644 --- a/include/flash.h +++ b/include/flash.h @@ -726,14 +726,7 @@ /* cli_common.c */ void print_chip_support_status(const struct flashchip *chip); -/* cli_output.c */ -extern enum flashrom_log_level verbose_screen; -extern enum flashrom_log_level verbose_logfile; -int open_logfile(const char * const filename); -int close_logfile(void); -void start_logging(void); -int flashrom_print_cb(enum flashrom_log_level level, const char *fmt, va_list ap); -void flashrom_progress_cb(struct flashrom_flashctx *flashctx); +/* libflashrom.c */ /* Let gcc and clang check for correct printf-style format strings. */ int print(enum flashrom_log_level level, const char *fmt, ...) #ifdef __MINGW32__ -- To view, visit https://review.coreboot.org/c/flashrom/+/85134?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4209d5ed205ca14c39e83aa923e103b7282a7059 Gerrit-Change-Number: 85134 Gerrit-PatchSet: 4 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in flashrom[main]: Extract cli_output declarations to a separate header.
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Attention is currently required from: Antonio Vázquez Blanco. Anastasia Klimchuk has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/85134?usp=email ) Change subject: Extract cli_output declarations to a separate header. ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS2: > I leave this unresolved comment as a note this patch will be submitted after v1.5.0 tag is done. […] Done, and I rebased the patch because there was another patch modifying cli_classic -- To view, visit https://review.coreboot.org/c/flashrom/+/85134?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4209d5ed205ca14c39e83aa923e103b7282a7059 Gerrit-Change-Number: 85134 Gerrit-PatchSet: 3 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Comment-Date: Fri, 06 Dec 2024 06:27:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: doc: Double-clarify that mailing list is public
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/85277?usp=email ) Change subject: doc: Double-clarify that mailing list is public ...................................................................... doc: Double-clarify that mailing list is public Change-Id: Id8e0daf75e25e6153a80fb9444547bdf91d1d343 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/85277 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M doc/contact.rst 1 file changed, 5 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/doc/contact.rst b/doc/contact.rst index 38888fb..3ddea6e 100644 --- a/doc/contact.rst +++ b/doc/contact.rst @@ -16,6 +16,11 @@ Please note that the list is moderated for non-subscribers and we recommend to subscribe first. +Our mailing list is public and archived, see below for the links to the archives. +There are also a bunch of independent public archives services that are not operated by the flashrom team. +All those archives are also public, everyone on the internet can read them, +please be careful what kind of information you are sending to the mailing list. + Subscription """""""""""" https://mail.coreboot.org/postorius/lists/flashrom.flashrom.org/ -- To view, visit https://review.coreboot.org/c/flashrom/+/85277?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Id8e0daf75e25e6153a80fb9444547bdf91d1d343 Gerrit-Change-Number: 85277 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in flashrom[main]: doc: Double-clarify that mailing list is public
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85277?usp=email ) Change subject: doc: Double-clarify that mailing list is public ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Leaving unresolved comment as a note to submit this after v1.5.0 tag is done. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/85277?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Id8e0daf75e25e6153a80fb9444547bdf91d1d343 Gerrit-Change-Number: 85277 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 06 Dec 2024 06:16:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: cli_classic.c: Make -r/-w/-v argument optional when using -i
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/85159?usp=email ) Change subject: cli_classic.c: Make -r/-w/-v argument optional when using -i ...................................................................... cli_classic.c: Make -r/-w/-v argument optional when using -i Make the filename parameter directly following -r/-w/-v optional, since the -i parameter allows the image to be written to be sourced from multiple files, regions to be read from flash and written to separate image files, and regions to be verified using an image file only containing that region. Since the filename parameter following -w/-v was ignored when a filename was specified following `-i <region>:<filename>`, this patch essentially removes the requirement to provide an unused parameter. Based on https://review.coreboot.org/c/flashrom/+/52362. TEST=run the following commands on a supported board: flashrom -p internal -r /tmp/coreboot.rom flashrom -p internal -r --ifd -i bios:/tmp/coreboot.rom flashrom -p internal -r /tmp/coreboot.rom --ifd -i bios:/tmp/bios.bin flashrom -p internal -w /tmp/coreboot.rom flashrom -p internal -w --ifd -i bios:/tmp/coreboot.rom flashrom -p internal -w /tmp/coreboot.rom --ifd -i bios:/tmp/bios.bin flashrom -p internal -v /tmp/coreboot.rom flashrom -p internal -v --ifd -i bios:/tmp/coreboot.rom flashrom -p internal -v /tmp/coreboot.rom --ifd -i bios:/tmp/bios.bin Change-Id: I6eba095d478f1a7bdbc3854627a656f93dd9e452 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/85159 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M doc/classic_cli_manpage.rst M include/layout.h M layout.c 4 files changed, 176 insertions(+), 38 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/cli_classic.c b/cli_classic.c index 8f37019..f622720 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -108,17 +108,17 @@ printf("Usage: %s [-h|-R|-L|" "\n\t-p <programmername>[:<parameters>] [-c <chipname>]\n" "\t\t(--flash-name|--flash-size|\n" - "\t\t [-E|-x|(-r|-w|-v) <file>]\n" + "\t\t [-E|-x|(-r|-w|-v) [<file>]]\n" "\t\t [(-l <layoutfile>|--ifd| --fmap|--fmap-file <file>) [-i <region>[:<file>]]...]\n" "\t\t [-n] [-N] [-f])]\n" "\t[-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> or the content provided\n" + " -r | --read [<file>] read flash and save to <file>\n" + " -w | --write [<file>|-] write <file> or the content provided\n" " on the standard input to flash\n" - " -v | --verify (<file>|-) verify flash against <file>\n" + " -v | --verify [<file>|-] verify flash against <file>\n" " or the content provided on the standard input\n" " -E | --erase erase flash memory\n" " -V | --verbose more verbose output\n" @@ -616,6 +616,23 @@ return 0; } +static char *get_optional_filename(char *argv[]) +{ + char *filename = NULL; + + /* filename was supplied in optarg (i.e. -rfilename) */ + if (optarg != NULL) + filename = strdup(optarg); + /* filename is on optind if it is not another flag (i.e. -r filename) + * - is treated as stdin, so we still strdup in this case + */ + else if (optarg == NULL && argv[optind] != NULL && + (argv[optind][0] != '-' || argv[optind][1] == '\0')) + filename = strdup(argv[optind++]); + + return filename; +} + static int do_read(struct flashctx *const flash, const char *const filename) { int ret; @@ -663,8 +680,10 @@ } /* Read '-w' argument first... */ - if (read_buf_from_file(newcontents, flash_size, filename)) - goto _free_ret; + if (filename) { + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + } /* * ... then update newcontents with contents from files provided to '-i' * args if needed. @@ -697,8 +716,10 @@ } /* Read '-v' argument first... */ - if (read_buf_from_file(newcontents, flash_size, filename)) - goto _free_ret; + if (filename) { + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + } /* * ... then update newcontents with contents from files provided to '-i' * args if needed. @@ -773,12 +794,12 @@ switch (opt) { case 'r': cli_classic_validate_singleop(&operation_specified); - options->filename = strdup(optarg); + options->filename = get_optional_filename(argv); options->read_it = true; break; case 'w': cli_classic_validate_singleop(&operation_specified); - options->filename = strdup(optarg); + options->filename = get_optional_filename(argv); options->write_it = true; break; case 'v': @@ -787,7 +808,7 @@ if (options->dont_verify_it) { cli_classic_abort_usage("--verify and --noverify are mutually exclusive. Aborting.\n"); } - options->filename = strdup(optarg); + options->filename = get_optional_filename(argv); options->verify_it = true; break; case 'n': @@ -1033,12 +1054,12 @@ int ret = 0; struct cli_options options = { 0 }; - static const char optstring[] = "r:Rw:v:nNVEfc:l:i:p:Lzho:x"; + static const char optstring[] = "r::Rw::v::nNVEfc:l:i:p:Lzho:x"; static const struct option long_options[] = { - {"read", 1, NULL, 'r'}, - {"write", 1, NULL, 'w'}, + {"read", 2, NULL, 'r'}, + {"write", 2, NULL, 'w'}, {"erase", 0, NULL, 'E'}, - {"verify", 1, NULL, 'v'}, + {"verify", 2, NULL, 'v'}, {"noverify", 0, NULL, 'n'}, {"noverify-all", 0, NULL, 'N'}, {"extract", 0, NULL, 'x'}, @@ -1098,7 +1119,7 @@ parse_options(argc, argv, optstring, long_options, &options); - if ((options.read_it | options.write_it | options.verify_it) && check_filename(options.filename, "image")) + if (options.filename && check_filename(options.filename, "image")) cli_classic_abort_usage(NULL); if (options.layoutfile && check_filename(options.layoutfile, "layout")) cli_classic_abort_usage(NULL); @@ -1316,6 +1337,57 @@ goto out_shutdown; } + /* + * Common rules for -r/-w/-v syntax parsing: + * + * - If no filename is specified at all, quit. + * + * - If a file is specified for -r/-w/-v and no files are specified with + * -i args (or -i is not used), then that file will be used for reading/ + * writing/verifying the entire ROM. + * + * - If no filename is specified for -r/-w/-v, but files are specified + * for -i, then the number of file arguments for -i options must be + * equal to the total number of -i options. + * + * Rules for reading: + * + * - If files are specified for -i args but not -r, do partial reads for + * each -i arg, creating a new file for each region. Each -i option + * must specify a filename. + * + * - If filenames are specified for -r and -i args, then: + * - Do partial read for each -i arg, creating a new file for + * each region where a filename is provided (-i region:filename). + * - Create a ROM-sized file with partially filled content. For each + * -i arg, fill the corresponding offset with content from ROM. + * + * Rules for writing and verifying: + * + * - If files are specified for both -w/-v and -i args, -i files take + * priority (files specified for -w/-v are unused). + * + * - If files are specified for -i args but not -w, do partial writes + * for each -i arg. Likewise for -v and -i args. All -i args must + * supply a filename. Any omission is considered ambiguous. + * + * - Regions with a filename associated must not overlap. This is also + * considered ambiguous. Note: This is checked later since it requires + * processing the layout/fmap first. + */ + if ((options.read_it | options.write_it | options.verify_it) && !options.filename) { + if (!options.include_args) { + msg_gerr("Error: No image file specified.\n"); + ret = 1; + goto out_shutdown; + } + + if (check_include_args_filename(options.include_args)) { + ret = 1; + goto out_shutdown; + } + } + if (options.flash_name) { if (fill_flash->chip->vendor && fill_flash->chip->name) { printf("vendor=\"%s\" name=\"%s\"\n", diff --git a/doc/classic_cli_manpage.rst b/doc/classic_cli_manpage.rst index 47df6c7..4f2e68e 100644 --- a/doc/classic_cli_manpage.rst +++ b/doc/classic_cli_manpage.rst @@ -14,7 +14,7 @@ | **flashrom** [-h|-R|-L| | -p <programmername>[:<parameters>] [-c <chipname>] | (--flash-name|--flash-size| -| [-E|-x|-r <file>|-w <file>|-v <file>] +| [-E|-x|-r [<file>]|-w [<file>]|-v [<file>]] | [(-l <file>|--ifd|--fmap|--fmap-file <file>) | [-i <include>[:<file>]]] | [--wp-status] [--wp-list] [--wp-enable|--wp-disable] @@ -50,10 +50,13 @@ All operations involving any chip access (probe/read/write/...) require the ``-p/--programmer`` option to be used (please see below). -**-r, --read <file>** +**-r, --read [<file>]** Read flash ROM contents and save them into the given **<file>**. If the file already exists, it will be overwritten. + The **<file>** parameter is required here unless reading is restricted to one or more flash regions via the ``-i/--include`` parameter + and the file is specified there. See the ``--include`` section below for examples. + **-w, --write (<file>|-)** Write **<file>** into flash ROM. If **-** is provided instead, contents will be read from stdin. @@ -64,6 +67,9 @@ This copy is updated along with the write operation. In case of erase errors it is even re-read completely. After writing has finished and if verification is enabled, the whole flash chip is read out and compared with the input image. + The **<file>** parameter is required here unless writing is restricted to one or more flash regions via the ``-i/--include`` parameter + and the file is specified there. See the ``--include`` section below for examples. + **-n, --noverify** Skip the automatic verification of flash ROM contents after writing. Using this option is **not** recommended, @@ -91,6 +97,9 @@ Verify the flash ROM contents against the given **<file>**. If **-** is provided instead, contents will be written to the stdout. + The **<file>** parameter is required here unless verification is restricted to one or more flash regions via the ``-i/--include`` parameter + and the file is specified there. See the ``--include`` section below for examples. + **-E, --erase** Erase the flash ROM chip. @@ -187,23 +196,66 @@ **-i, --include <region>[:<file>]** - Read or write only **<region>** to or from ROM. - The **-i** option may be used multiple times if the user wishes to read or write multiple regions using a single command. + Read, write, or verify only **<region>** to or from ROM. + The **-i** option may be used multiple times if the user wishes to read, write, or verify multiple regions using a single command. The user may optionally specify a corresponding **<file>** for any region they wish to read or write. A read operation will read the corresponding regions from ROM and write individual files for each one. A write option will read file(s) and write to the corresponding region(s) in ROM. - For write operations, files specified using ``-i`` take precedence over content from the argument to ``-w``. + For all read/write/verify operations, the **<file>** parameter following those operations becomes optional and will be ignored + if present whenever the <file> is specified following the region. + + Common rules for -r/-w/-v syntax parsing: + + - If no filename is specified at all, quit. + + - If a file is specified for -r/-w/-v and no files are specified with + -i args (or -i is not used), then that file will be used for reading/ + writing/verifying the entire ROM. + + - If no filename is specified for -r/-w/-v, but files are specified + for -i, then the number of file arguments for -i options must be + equal to the total number of -i options. + + Rules for reading: + + - If files are specified for -i args but not -r, do partial reads for + each -i arg, creating a new file for each region. Each -i option + must specify a filename. + + - If filenames are specified for -r and -i args, then: + + - Do partial read for each -i arg, creating a new file for + each region where a filename is provided (-i region:filename). + - Create a ROM-sized file with partially filled content. For each + -i arg, fill the corresponding offset with content from ROM. + + Rules for writing and verifying: + + - If files are specified for both -w/-v and -i args, -i files take + priority (files specified for -w/-v are unused). + + - If files are specified for -i args but not -w, do partial writes + for each -i arg. Likewise for -v and -i args. All -i args must + supply a filename. Any omission is considered ambiguous. + + - Regions with a filename associated must not overlap. This is also + considered ambiguous. Note: This is checked later since it requires + processing the layout/fmap first. Examples: To read regions named **foo** and **bar** in layout file **<layout>** into region-sized files **foo.bin** and **bar.bin**, run:: - flashrom -p prog -l <layout> -i foo:foo.bin -i bar:bar.bin -r rom.bin + flashrom -p prog -r -l <layout> -i foo:foo.bin -i bar:bar.bin To write files **foo.bin** and **bar.bin** into regions named **foo** and **bar** in layout file **<layout>** to the ROM, run:: - flashrom -p prog -l <layout> -i foo:foo.bin -i bar:bar.bin -w rom.bin + flashrom -p prog -w -l <layout> -i foo:foo.bin -i bar:bar.bin + + To verify regions named **foo** and **bar** using layout file **<layout>** and files **foo.bin** and **bar.bin**, run:: + + flashrom -p prog -v -l <layout> -i foo:foo.bin -i bar:bar.bin **--wp-status** @@ -321,18 +373,18 @@ **--progress** - Show progress percentage of operations on the standard output. + Show progress percentage of operations on the standard output. **--sacrifice-ratio <ratio>** - Fraction (as a percentage, 0-50) of an erase block that may be erased even if unmodified. - Larger values may complete programming faster, but may also hurt chip longevity by erasing cells unnecessarily. + Fraction (as a percentage, 0-50) of an erase block that may be erased even if unmodified. + Larger values may complete programming faster, but may also hurt chip longevity by erasing cells unnecessarily. - Default is 0, S+1 size block only selected if all the S size blocks inside it need to be erased in full. - 50 means that if more than a half of the area needs to be erased, - a S+1 size block can be selected to cover all the area with one erase. - The tradeoff is the speed of programming operation VS the longevity of the chip. Default is longevity. + Default is 0, S+1 size block only selected if all the S size blocks inside it need to be erased in full. + 50 means that if more than a half of the area needs to be erased, + a S+1 size block can be selected to cover all the area with one erase. + The tradeoff is the speed of programming operation VS the longevity of the chip. Default is longevity. - DANGEROUS! It wears your chip faster! + DANGEROUS! It wears your chip faster! **-R, --version** @@ -707,16 +759,16 @@ write-protected (on real hardware the pin is usually negated, but not here). **Frequency** - Frequency can be specified in ``Hz`` (default), ``KHz``, or ``MHz`` (not case sensitive). - If ``freq`` parameter is passed in from command line, commands will delay for certain time before returning, - so that to emulate the requested frequency. + Frequency can be specified in ``Hz`` (default), ``KHz``, or ``MHz`` (not case sensitive). + If ``freq`` parameter is passed in from command line, commands will delay for certain time before returning, + so that to emulate the requested frequency. - Valid range is [1Hz, 8000Mhz] and there is no delay by default. + Valid range is [1Hz, 8000Mhz] and there is no delay by default. - The delay of an SPI command is proportional to the number of bits send over SPI bus in both directions - and is calculated based on the assumption that we transfer at 1 bit/Hz:: + The delay of an SPI command is proportional to the number of bits send over SPI bus in both directions + and is calculated based on the assumption that we transfer at 1 bit/Hz:: - flashrom -p dummy:emulate=W25Q128FV,freq=64mhz + flashrom -p dummy:emulate=W25Q128FV,freq=64mhz nic3com, nicrealtek, nicnatsemi, nicintel, nicintel_eeprom, nicintel_spi, gfxnvidia, ogp_spi, drkaiser, satasii, satamv, atahpt, atavia, atapromise, it8212 programmers diff --git a/include/layout.h b/include/layout.h index d03a15b..ce3dd4b 100644 --- a/include/layout.h +++ b/include/layout.h @@ -69,6 +69,7 @@ int register_include_arg(struct layout_include_args **, const char *arg); int process_include_args(struct flashrom_layout *, const struct layout_include_args *); +int check_include_args_filename(const struct layout_include_args *); void cleanup_include_args(struct layout_include_args **); const struct romentry *layout_next_included_region(const struct flashrom_layout *, chipoff_t); diff --git a/layout.c b/layout.c index e46e61a..a1aa964 100644 --- a/layout.c +++ b/layout.c @@ -288,6 +288,19 @@ return 0; } +int check_include_args_filename(const struct layout_include_args *include_args) +{ + const struct layout_include_args *arg; + for (arg = include_args; arg; arg = arg->next) { + if (!arg->file || (arg->file[0] == '\0')) { + fprintf(stderr, "Error: No region file specified for -i/--include option.\n"); + return 1; + } + } + + return 0; +} + /* returns boolean 1 if any regions overlap, 0 otherwise */ int included_regions_overlap(const struct flashrom_layout *const l) { -- To view, visit https://review.coreboot.org/c/flashrom/+/85159?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I6eba095d478f1a7bdbc3854627a656f93dd9e452 Gerrit-Change-Number: 85159 Gerrit-PatchSet: 9 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in flashrom[main]: cli_classic.c: Make -r/-w/-v argument optional when using -i
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Attention is currently required from: Matt DeVillier. Anastasia Klimchuk has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/85159?usp=email ) Change subject: cli_classic.c: Make -r/-w/-v argument optional when using -i ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: > I leave this unresolved comment as a note this patch will be submitted after v1.5.0 tag is done. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/85159?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I6eba095d478f1a7bdbc3854627a656f93dd9e452 Gerrit-Change-Number: 85159 Gerrit-PatchSet: 8 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Fri, 06 Dec 2024 06:15:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XL] Change in flashrom[main]: flashchips: Splitting flashchips into separate files by vendor
by Anastasia Klimchuk (Code Review) 06 Dec '24

06 Dec '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/83307?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashchips: Splitting flashchips into separate files by vendor ...................................................................... flashchips: Splitting flashchips into separate files by vendor To make the flashchips "database" easier to manage, split it by vendor into several smaller files. This commit transfers the bulk of the data to separate files and includes them from `flashchips.c`. Although this is ugly (.c includes are usually frowned upon), it is a necessary evil to make this commit reproducible. Tested in two ways: 1) Output of `flashrom -L` has no diffs with/without the patch compared with diff and cmp tools 2) flashrom binary has no diffs with/without the patch compared with diff and cmp tools Note for binary comparison documentation and manpages need to be disabled (documentation is actually modified in the patch), also version in meson.build set to "none" (otherwise git version counts every commit). Change-Id: I3a9ebb0575e2700c5871d16875495d9c8943b30b Co-developed-by: Angel Pons <th3fanbus(a)gmail.com> Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/83307 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M MAINTAINERS M doc/contrib_howtos/how_to_add_new_chip.rst M flashchips.c A flashchips/amd.c A flashchips/amic.c A flashchips/atmel.c A flashchips/boya_bohong.c A flashchips/bright.c A flashchips/catalyst.c A flashchips/ene.c A flashchips/eon.c A flashchips/esi.c A flashchips/esmt.c A flashchips/fudan.c A flashchips/fujitsu.c A flashchips/gigadevice.c A flashchips/hyundai.c A flashchips/intel.c A flashchips/issi.c A flashchips/macronix.c A flashchips/micron.c A flashchips/micron_numonyx_st.c A flashchips/mosel_vitelic.c A flashchips/nantronics.c A flashchips/pmc.c A flashchips/puya.c A flashchips/sanyo.c A flashchips/sharp.c A flashchips/spansion.c A flashchips/sst.c A flashchips/st.c A flashchips/syncmos_mosel_vitelic.c A flashchips/ti.c A flashchips/winbond.c A flashchips/xmc.c A flashchips/xtx.c A flashchips/zetta.c 37 files changed, 24,143 insertions(+), 23,458 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved -- To view, visit https://review.coreboot.org/c/flashrom/+/83307?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I3a9ebb0575e2700c5871d16875495d9c8943b30b Gerrit-Change-Number: 83307 Gerrit-PatchSet: 5 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2024