November 2022 - flashrom-gerrit

[S] Change in flashrom[master]: stlinkv3_spi: fix uninitialized pointer
by Alexander Goncharov (Code Review) 07 Nov '22

07 Nov '22

Alexander Goncharov has restored this change. ( https://review.coreboot.org/c/flashrom/+/67700 ) Change subject: stlinkv3_spi: fix uninitialized pointer ...................................................................... Restored -- To view, visit https://review.coreboot.org/c/flashrom/+/67700 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibaf25f67186724d9045ade849026782c3eac4952 Gerrit-Change-Number: 67700 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: restore

1 0

[M] Change in flashrom[master]: util: add bash completion script
by Alexander Goncharov (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68247 ) Change subject: util: add bash completion script ...................................................................... Patch Set 8: (2 comments) Patchset: PS7: > I just looked, there are unresolved comments. […] Yeah, I was waiting for a reply from Felix. I marked it as resolved for now. File meson.build: https://review.coreboot.org/c/flashrom/+/68247/comment/2ef2462e_73d89daa PS4, Line 611: share/bash-completion/completions > I went through sources at the link that Thomas sent (btw many thanks). […] Done. If I didn't do what you want, feel free to reopen the comment. -- To view, visit https://review.coreboot.org/c/flashrom/+/68247 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie68bc91c3cea4de2ffdbeffd07e48edd8d5590e1 Gerrit-Change-Number: 68247 Gerrit-PatchSet: 8 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 07 Nov 2022 09:34:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Add prefix to io_mock functions not to clash with macros
by Felix Singer (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68433 ) Change subject: tests: Add prefix to io_mock functions not to clash with macros ...................................................................... Patch Set 7: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68433/comment/e55bd827_a85ec18e PS5, Line 10: because the latter are allowed : to be macros. Adding a prefix to flashrom mock functions avoids : them being accidentally expanded. Standard I/O functions are : expanded and flashrom mocks stays as they are > > Why is it a problem that standard IO functions are macros? <... […] Now I understand. Thanks for your detailed explanation :) -- To view, visit https://review.coreboot.org/c/flashrom/+/68433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Gerrit-Change-Number: 68433 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 07 Nov 2022 07:26:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[1.3.x]: Revert "libflashrom: Return progress state to the library user"
by Felix Singer (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Peter Marheine. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69283 ) Change subject: Revert "libflashrom: Return progress state to the library user" ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69283 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: Id3d7ffcaf266a60a44eb453fd09b7c63c05349c2 Gerrit-Change-Number: 69283 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 07 Nov 2022 07:20:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[1.3.x]: Revert "libflashrom: Return progress state to the library user"
by Anastasia Klimchuk (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Peter Marheine. Hello build bot (Jenkins), Edward O'Callaghan, Nikolai Artemiev, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69283 to look at the new patch set (#2). Change subject: Revert "libflashrom: Return progress state to the library user" ...................................................................... Revert "libflashrom: Return progress state to the library user" This reverts commit 40892b0c08fbc8029921e91511dd3f91fc956f90. The feature of returning progress for libflashrom users was introduced in original commit, however later a bug was found and reported as https://ticket.coreboot.org/issues/390. Reverting in a release branch to unblock release candidate, since it is unknown how much time needed to fix the bug. Meanwhile the feature remains in a master branch and will be fixed under ticket 390. TEST=scenarios below run successfully 1) flashrom -h does not show --progress 2) flashrom -p dummy:emulate=W25Q128FV -r /tmp/dump.bin 3) flashrom -p dummy:emulate=W25Q128FV -v /tmp/dump.bin 4) flashrom -p dummy:emulate=W25Q128FV -E 5) head -c 16777216 </dev/urandom >/tmp/image.bin flashrom -p dummy:image=/tmp/image.bin,emulate=W25Q128FV \ -w /tmp/dump.bin Change-Id: Id3d7ffcaf266a60a44eb453fd09b7c63c05349c2 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M en29lv640b.c M flashrom.8.tmpl M include/flash.h M include/libflashrom.h M it87spi.c M jedec.c M libflashrom.c M libflashrom.map M linux_mtd.c M parade_lspcon.c M realtek_mst_i2c_spi.c M spi.c M spi25.c M sst28sf040.c M tests/spi25.c M tests/tests.c M tests/tests.h 22 files changed, 34 insertions(+), 184 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/69283/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69283 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: Id3d7ffcaf266a60a44eb453fd09b7c63c05349c2 Gerrit-Change-Number: 69283 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[1.3.x]: Revert "libflashrom: Return progress state to the library user"
by Anastasia Klimchuk (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Edward O'Callaghan. Hello Edward O'Callaghan, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/69283 to review the following change. Change subject: Revert "libflashrom: Return progress state to the library user" ...................................................................... Revert "libflashrom: Return progress state to the library user" This reverts commit 40892b0c08fbc8029921e91511dd3f91fc956f90. The feature of returning progress for libflashrom users was introduced in original commit, however later a bug was found and reported as https://ticket.coreboot.org/issues/390. Reverting in a release branch to unblock release candidate, since it is unknown how much time needed to fix the bug. Meanwhile the feature remains in a master branch and will be fixed under ticket 390. TEST=scenarios below run successfully 1) flashrom -h does not show --progress 2) flashrom -p dummy:emulate=W25Q128FV -r /tmp/dump.bin 3) flashrom -p dummy:emulate=W25Q128FV -v /tmp/dump.bin 4) flashrom -p dummy:emulate=W25Q128FV -E 5) head -c 16777216 </dev/urandom >/tmp/image.bin flashrom -p dummy:image=/tmp/image.bin,emulate=W25Q128FV \ -w /tmp/dump.bin Change-Id: Id3d7ffcaf266a60a44eb453fd09b7c63c05349c2 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M en29lv640b.c M flashrom.8.tmpl M include/flash.h M include/libflashrom.h M it87spi.c M jedec.c M libflashrom.c M libflashrom.map M linux_mtd.c M realtek_mst_i2c_spi.c M spi.c M spi25.c M sst28sf040.c M tests/spi25.c M tests/tests.c M tests/tests.h 21 files changed, 34 insertions(+), 182 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/69283/1 diff --git a/82802ab.c b/82802ab.c index 90c9cf0..f2f2707 100644 --- a/82802ab.c +++ b/82802ab.c @@ -135,7 +135,6 @@ chip_writeb(flash, 0x40, dst); chip_writeb(flash, *src++, dst++); wait_82802ab(flash); - update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, len); } /* FIXME: Ignore errors for now. */ diff --git a/at45db.c b/at45db.c index 992f3a4..074d924 100644 --- a/at45db.c +++ b/at45db.c @@ -550,7 +550,6 @@ msg_cerr("Writing page %u failed!\n", i); return 1; } - update_progress(flash, FLASHROM_PROGRESS_WRITE, i + page_size, len); } return 0; } diff --git a/cli_classic.c b/cli_classic.c index b66094c..a77422e 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -79,7 +79,6 @@ #if CONFIG_PRINT_WIKI == 1 " -z | --list-supported-wiki print supported devices in wiki syntax\n" #endif - " --progress show progress percentage on the standard output\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, " @@ -580,7 +579,6 @@ bool read_it = false, extract_it = false, write_it = false, erase_it = false, verify_it = false; bool dont_verify_it = false, dont_verify_all = false; bool list_supported = false; - bool show_progress = false; struct flashrom_layout *layout = NULL; static const struct programmer_entry *prog = NULL; enum { @@ -596,7 +594,6 @@ OPTION_WP_ENABLE, OPTION_WP_DISABLE, OPTION_WP_LIST, - OPTION_PROGRESS, }; int ret = 0; @@ -633,7 +630,6 @@ {"help", 0, NULL, 'h'}, {"version", 0, NULL, 'R'}, {"output", 1, NULL, 'o'}, - {"progress", 0, NULL, OPTION_PROGRESS}, {NULL, 0, NULL, 0}, }; @@ -874,9 +870,6 @@ cli_classic_abort_usage("No log filename specified.\n"); } break; - case OPTION_PROGRESS: - show_progress = true; - break; default: cli_classic_abort_usage(NULL); break; @@ -1048,13 +1041,6 @@ fill_flash = &flashes[0]; - unsigned int progress_user_data[FLASHROM_PROGRESS_NR]; - struct flashrom_progress progress_state = { - .user_data = progress_user_data - }; - if (show_progress) - flashrom_set_progress_callback(fill_flash, &flashrom_progress_cb, &progress_state); - print_chip_support_status(fill_flash->chip); unsigned int limitexceeded = count_max_decode_exceedings(fill_flash); diff --git a/cli_output.c b/cli_output.c index e5b829a..629db67 100644 --- a/cli_output.c +++ b/cli_output.c @@ -64,31 +64,6 @@ verbose_screen = oldverbose_screen; } -static const char *flashrom_progress_stage_to_string(enum flashrom_progress_stage stage) -{ - if (stage == FLASHROM_PROGRESS_READ) - return "READ"; - if (stage == FLASHROM_PROGRESS_WRITE) - return "WRITE"; - if (stage == FLASHROM_PROGRESS_ERASE) - return "ERASE"; - return "UNKNOWN"; -} - -void flashrom_progress_cb(struct flashrom_flashctx *flashctx) -{ - struct flashrom_progress *progress_state = flashctx->progress_state; - unsigned int pc = 0; - unsigned int *percentages = progress_state->user_data; - if (progress_state->current > 0 && progress_state->total > 0) - pc = ((unsigned long long) progress_state->current * 10000llu) / - ((unsigned long long) progress_state->total * 100llu); - if (percentages[progress_state->stage] != pc) { - percentages[progress_state->stage] = pc; - msg_ginfo("[%s] %u%% complete... ", flashrom_progress_stage_to_string(progress_state->stage), pc); - } -} - /* Please note that level is the verbosity, not the importance of the message. */ int flashrom_print_cb(enum flashrom_log_level level, const char *fmt, va_list ap) { diff --git a/dediprog.c b/dediprog.c index b4279d8..2117451 100644 --- a/dediprog.c +++ b/dediprog.c @@ -658,7 +658,6 @@ msg_perr("SPI bulk write failed, expected %i, got %s!\n", 512, libusb_error_name(ret)); return 1; } - update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, count); } return 0; diff --git a/en29lv640b.c b/en29lv640b.c index 8a8d641..07d8624 100644 --- a/en29lv640b.c +++ b/en29lv640b.c @@ -48,7 +48,6 @@ #endif dst += 2; src += 2; - update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 2, len); } /* FIXME: Ignore errors for now. */ diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index e672869..b977908 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -53,8 +53,7 @@ [\fB\-\-wp\-status\fR] [\fB\-\-wp\-list\fR] [\fB\-\-wp\-enable\fR|\fB\-\-wp\-disable\fR] [\fB\-\-wp\-range\fR <start>,<length>|\fB\-\-wp\-region\fR <region>] [\fB\-n\fR] [\fB\-N\fR] [\fB\-f\fR])] - [\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] [\fB\-\-progress\fR] - + [\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] .SH DESCRIPTION .B flashrom is a utility for detecting, reading, writing, verifying and erasing flash @@ -450,9 +449,6 @@ way to gather logs from flashrom because they will be verbose even if the on-screen messages are not verbose and don't require output redirection. .TP -.B "\-\-progress" -Show progress percentage of operations on the standard output. -.TP .B "\-R, \-\-version" Show version information and exit. .SH PROGRAMMER-SPECIFIC INFORMATION diff --git a/include/flash.h b/include/flash.h index 2ba235d..7738ee3 100644 --- a/include/flash.h +++ b/include/flash.h @@ -420,9 +420,6 @@ chip_restore_fn_cb_t func; uint8_t status; } chip_restore_fn[MAX_CHIP_RESTORE_FUNCTIONS]; - /* Progress reporting */ - flashrom_progress_callback *progress_callback; - struct flashrom_progress *progress_state; }; /* Timing used in probe routines. ZERO is -2 to differentiate between an unset @@ -518,7 +515,6 @@ 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); /* Let gcc and clang check for correct printf-style format strings. */ int print(enum flashrom_log_level level, const char *fmt, ...) #ifdef __MINGW32__ @@ -547,7 +543,6 @@ #define msg_gspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* general debug spew */ #define msg_pspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* programmer debug spew */ #define msg_cspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* chip debug spew */ -void update_progress(struct flashctx *flash, enum flashrom_progress_stage stage, size_t current, size_t total); /* spi.c */ struct spi_command { diff --git a/include/libflashrom.h b/include/libflashrom.h index 9bbdcc5..3c99b30 100644 --- a/include/libflashrom.h +++ b/include/libflashrom.h @@ -69,34 +69,6 @@ */ void flashrom_set_log_callback(flashrom_log_callback *log_callback); -enum flashrom_progress_stage { - FLASHROM_PROGRESS_READ, - FLASHROM_PROGRESS_WRITE, - FLASHROM_PROGRESS_ERASE, - FLASHROM_PROGRESS_NR, -}; -struct flashrom_progress { - enum flashrom_progress_stage stage; - size_t current; - size_t total; - void *user_data; -}; -struct flashrom_flashctx; -typedef void(flashrom_progress_callback)(struct flashrom_flashctx *flashctx); -/** - * @brief Set the progress callback function. - * - * Set a callback function which will be invoked whenever libflashrom wants - * to indicate the progress has changed. This allows frontends to do whatever - * they see fit with such values, e.g. update a progress bar in a GUI tool. - * - * @param progress_callback Pointer to the new progress callback function. - * @param progress_state Pointer to progress state to include with the progress - * callback. - */ -void flashrom_set_progress_callback(struct flashrom_flashctx *const flashctx, - flashrom_progress_callback *progress_callback, struct flashrom_progress *progress_state); - /** @} */ /* end flashrom-general */ /** @@ -203,6 +175,7 @@ * @{ */ +struct flashrom_flashctx; /** * @brief Probe for a flash chip. * diff --git a/it87spi.c b/it87spi.c index a022041..e1edd20 100644 --- a/it87spi.c +++ b/it87spi.c @@ -292,7 +292,6 @@ int ret = it8716f_spi_page_program(flash, buf, start); if (ret) return ret; - update_progress(flash, FLASHROM_PROGRESS_WRITE, chip->page_size - len, chip->page_size); start += chip->page_size; len -= chip->page_size; buf += chip->page_size; diff --git a/jedec.c b/jedec.c index c4ee0c7..1b35c8d 100644 --- a/jedec.c +++ b/jedec.c @@ -421,7 +421,6 @@ if (write_byte_program_jedec_common(flash, src, dst, mask)) failed = 1; dst++, src++; - update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, len); } if (failed) msg_cerr(" writing sector at 0x%" PRIxPTR " failed!\n", olddst); @@ -488,7 +487,6 @@ * we're OK for now. */ unsigned int page_size = flash->chip->page_size; - unsigned int nwrites = (start + len - 1) / page_size; /* Warning: This loop has a very unusual condition and body. * The loop needs to go through each page with at least one affected @@ -499,7 +497,7 @@ * (start + len - 1) / page_size. Since we want to include that last * page as well, the loop condition uses <=. */ - for (i = start / page_size; i <= nwrites; i++) { + for (i = start / page_size; i <= (start + len - 1) / page_size; i++) { /* Byte position of the first byte in the range in this page. */ /* starthere is an offset to the base address of the chip. */ starthere = max(start, i * page_size); @@ -508,7 +506,6 @@ if (write_page_write_jedec_common(flash, buf + starthere - start, starthere, lenhere)) return 1; - update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, nwrites + 1); } return 0; diff --git a/libflashrom.c b/libflashrom.c index cbc6243..ce9d351 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -65,25 +65,6 @@ return 0; } -void flashrom_set_progress_callback(struct flashrom_flashctx *flashctx, flashrom_progress_callback *progress_callback, struct flashrom_progress *progress_state) -{ - flashctx->progress_callback = progress_callback; - flashctx->progress_state = progress_state; -} -/** @private */ -void update_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t current, size_t total) -{ - if (flashctx->progress_callback == NULL) - return; - if (current > total) - current = total; - - flashctx->progress_state->stage = stage; - flashctx->progress_state->current = current; - flashctx->progress_state->total = total; - flashctx->progress_callback(flashctx); -} - const char *flashrom_version_info(void) { return flashrom_version; diff --git a/libflashrom.map b/libflashrom.map index e3c7a76..7be9e2e 100644 --- a/libflashrom.map +++ b/libflashrom.map @@ -26,7 +26,6 @@ flashrom_programmer_init; flashrom_programmer_shutdown; flashrom_set_log_callback; - flashrom_set_progress_callback; flashrom_shutdown; flashrom_supported_boards; flashrom_supported_chipsets; diff --git a/linux_mtd.c b/linux_mtd.c index b8aee06..5a1360e 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -215,7 +215,6 @@ } i += step; - update_progress(flash, FLASHROM_PROGRESS_READ, i, len); } return 0; @@ -258,7 +257,6 @@ } i += step; - update_progress(flash, FLASHROM_PROGRESS_WRITE, i, len); } return 0; @@ -295,7 +293,6 @@ __func__, ret, strerror(errno)); return 1; } - update_progress(flash, FLASHROM_PROGRESS_ERASE, u + data->erasesize, len); } return 0; diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 2a5d5ed..fad3ba9 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -397,7 +397,6 @@ ret |= realtek_mst_i2c_execute_write(fd); if (ret) break; - update_progress(flash, FLASHROM_PROGRESS_WRITE, i + RTK_PAGE_SIZE, len); } return ret; diff --git a/spi.c b/spi.c index 38d8d01..13378e2 100644 --- a/spi.c +++ b/spi.c @@ -101,8 +101,6 @@ { int ret; size_t to_read; - size_t start_address = start; - size_t end_address = len - start; for (; len; len -= to_read, buf += to_read, start += to_read) { /* Do not cross 16MiB boundaries in a single transfer. This helps with @@ -112,7 +110,6 @@ ret = flash->mst->spi.read(flash, buf, start, to_read); if (ret) return ret; - update_progress(flash, FLASHROM_PROGRESS_READ, start - start_address + to_read, end_address); } return 0; } diff --git a/spi25.c b/spi25.c index 061a326..c8d13aa 100644 --- a/spi25.c +++ b/spi25.c @@ -693,14 +693,11 @@ { int ret; size_t to_read; - size_t start_address = start; - size_t end_address = len - start; for (; len; len -= to_read, buf += to_read, start += to_read) { to_read = min(chunksize, len); ret = spi_nbyte_read(flash, start, buf, to_read); if (ret) return ret; - update_progress(flash, FLASHROM_PROGRESS_READ, start - start_address + to_read, end_address); } return 0; } @@ -720,8 +717,6 @@ * we're OK for now. */ unsigned int page_size = flash->chip->page_size; - size_t start_address = start; - size_t end_address = len - start; /* Warning: This loop has a very unusual condition and body. * The loop needs to go through each page with at least one affected @@ -746,7 +741,6 @@ if (rc) return rc; } - update_progress(flash, FLASHROM_PROGRESS_WRITE, start - start_address + lenhere, end_address); } return 0; @@ -766,7 +760,6 @@ for (i = start; i < start + len; i++) { if (spi_nbyte_program(flash, i, buf + i - start, 1)) return 1; - update_progress(flash, FLASHROM_PROGRESS_WRITE, i - start, len - start); } return 0; } diff --git a/sst28sf040.c b/sst28sf040.c index 9080684..eb9aa58 100644 --- a/sst28sf040.c +++ b/sst28sf040.c @@ -92,7 +92,6 @@ /* wait for Toggle bit ready */ toggle_ready_jedec(flash, bios); - update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, len); } return 0; diff --git a/tests/spi25.c b/tests/spi25.c index 8729593..8768d09 100644 --- a/tests/spi25.c +++ b/tests/spi25.c @@ -67,65 +67,6 @@ return 0; } -static void spi_read_progress_cb(struct flashrom_flashctx *flashctx) -{ - struct flashrom_progress *progress_state = flashctx->progress_state; - uint32_t *cnt = (uint32_t *) progress_state->user_data; - assert_int_equal(0x300, progress_state->total); - switch (*cnt) { - case 0: - assert_int_equal(0x100, progress_state->current); - break; - case 1: - assert_int_equal(0x200, progress_state->current); - break; - case 2: - assert_int_equal(0x300, progress_state->current); - break; - case 3: - assert_int_equal(0x300, progress_state->current); - break; - case 4: - assert_int_equal(0x300, progress_state->current); - break; - default: - fail(); - } - (*cnt)++; -} - -void spi_read_chunked_test_success(void **state) -{ - (void) state; /* unused */ - uint8_t buf[0x400] = { 0x0 }; - uint32_t cnt = 0; - const unsigned int max_data_read = 0x100; - const unsigned int offset = 0x100; - struct registered_master mst = { - .spi.read = default_spi_read, - .spi.max_data_read = max_data_read - }; - - /* setup initial test state */ - struct flashctx flashctx = { - .chip = &mock_chip, - .mst = &mst - }; - struct flashrom_progress progress_state = { - .user_data = (void *) &cnt, - }; - flashrom_set_progress_callback(&flashctx, spi_read_progress_cb, &progress_state); - for (int i = 0; i < 4; i++) { - expect_memory(__wrap_spi_send_command, flash, - &flashctx, sizeof(flashctx)); - will_return(__wrap_spi_send_command, JEDEC_WRDI); - will_return(__wrap_spi_send_command, JEDEC_READ); - will_return(__wrap_spi_send_command, max_data_read); - } - assert_int_equal(0, spi_chip_read(&flashctx, buf, offset, sizeof(buf))); - assert_int_equal(5, cnt); -} - void spi_write_enable_test_success(void **state) { (void) state; /* unused */ diff --git a/tests/tests.c b/tests/tests.c index 53bad65..30ff5b5 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -405,7 +405,6 @@ const struct CMUnitTest spi25_tests[] = { cmocka_unit_test(spi_write_enable_test_success), cmocka_unit_test(spi_write_disable_test_success), - cmocka_unit_test(spi_read_chunked_test_success), cmocka_unit_test(probe_spi_rdid_test_success), cmocka_unit_test(probe_spi_rdid4_test_success), cmocka_unit_test(probe_spi_rems_test_success), diff --git a/tests/tests.h b/tests/tests.h index 7d84101..3ad4f4f 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -31,7 +31,6 @@ /* spi25.c */ void spi_write_enable_test_success(void **state); void spi_write_disable_test_success(void **state); -void spi_read_chunked_test_success(void **state); void probe_spi_rdid_test_success(void **state); void probe_spi_rdid4_test_success(void **state); void probe_spi_rems_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/69283 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: Id3d7ffcaf266a60a44eb453fd09b7c63c05349c2 Gerrit-Change-Number: 69283 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: flashrom_tester: Only print color when stdout isatty
by Evan Benn (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Edward O'Callaghan. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69270 ) Change subject: flashrom_tester: Only print color when stdout isatty ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: This is about the smallest change I could manage without totally refactoring the thing. -- To view, visit https://review.coreboot.org/c/flashrom/+/69270 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia3cc527fb98e53eda6773622340cf10764df2cba Gerrit-Change-Number: 69270 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 07 Nov 2022 05:51:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Only print color when stdout isatty
by Evan Benn (Code Review) 07 Nov '22

07 Nov '22

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69270 ) Change subject: flashrom_tester: Only print color when stdout isatty ...................................................................... flashrom_tester: Only print color when stdout isatty Add the atty crate as a dependency. Print log and report in color only when isatty is true. BUG=b:246250254 BRANCH=None TEST=ssh dut flashrom_tester # no color TEST=ssh -t dut flashrom_tester # color Change-Id: Ia3cc527fb98e53eda6773622340cf10764df2cba --- M util/flashrom_tester/Cargo.toml M util/flashrom_tester/src/logger.rs M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/types.rs 4 files changed, 74 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/69270/1 diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml index b57f04e..a7b6881 100644 --- a/util/flashrom_tester/Cargo.toml +++ b/util/flashrom_tester/Cargo.toml @@ -14,6 +14,7 @@ required-features = ["cli"] [dependencies] +atty = "0.2" built = { version = "0.5", features = ["chrono"] } chrono = { version = "0.4", optional = true } clap = { version = "2.33", default-features = false, optional = true } diff --git a/util/flashrom_tester/src/logger.rs b/util/flashrom_tester/src/logger.rs index 3c9b74b..fb26bde 100644 --- a/util/flashrom_tester/src/logger.rs +++ b/util/flashrom_tester/src/logger.rs @@ -41,6 +41,7 @@ struct Logger<W: Write + Send> { level: log::LevelFilter, target: LogTarget<W>, + color: types::Color, } enum LogTarget<W> @@ -57,16 +58,14 @@ } fn log(&self, record: &log::Record) { - fn log_internal<W: Write>(mut w: W, record: &log::Record) -> std::io::Result<()> { + fn log_internal<W: Write>( + mut w: W, + record: &log::Record, + color: &types::Color, + ) -> std::io::Result<()> { let now = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Micros, true); - write!(w, "{}{} ", types::MAGENTA, now)?; - write!( - w, - "{}[ {} ]{} ", - types::YELLOW, - record.level(), - types::RESET - )?; + write!(w, "{}{} ", color.magenta, now)?; + write!(w, "{}[ {} ]{} ", color.yellow, record.level(), color.reset)?; writeln!(w, "{}", record.args()) } @@ -75,11 +74,11 @@ LogTarget::Terminal => { let stdout = std::io::stdout(); let mut lock = stdout.lock(); - log_internal(&mut lock, record) + log_internal(&mut lock, record, &self.color) } LogTarget::Write(ref mutex) => { let mut lock = mutex.lock().unwrap(); - log_internal(&mut *lock, record) + log_internal(&mut *lock, record, &self.color) } }; } @@ -97,6 +96,11 @@ let mut logger = Logger { level: log::LevelFilter::Info, target: LogTarget::Terminal, + color: if atty::is(atty::Stream::Stdout) { + types::COLOR + } else { + types::NOCOLOR + }, }; if debug { @@ -106,6 +110,7 @@ logger.target = LogTarget::Write(Mutex::new( std::fs::File::create(path).expect("Unable to open log file for writing"), )); + logger.color = types::NOCOLOR; } log::set_max_level(logger.level); @@ -115,6 +120,7 @@ #[cfg(test)] mod tests { use super::{LogTarget, Logger}; + use flashrom_tester::types; use log::{Level, LevelFilter, Log, Record}; use std::sync::Mutex; @@ -125,6 +131,7 @@ let logger = Logger { level: LevelFilter::Info, target: LogTarget::Write(lock), + color: types::COLOR, }; for record in records { diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index f2c3846..8017997 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -543,6 +543,11 @@ ) { match format { OutputFormat::Pretty => { + let color = if atty::is(atty::Stream::Stdout) { + types::COLOR + } else { + types::NOCOLOR + }; println!(); println!(" ============================="); println!(" ===== AVL qual RESULTS ===="); @@ -561,8 +566,8 @@ if *result != TestConclusion::Pass { println!( " {} {}", - style!(format!(" <+> {} test:", name), types::BOLD), - style_dbg!(result, types::RED) + style!(format!(" <+> {} test:", name), color.bold, color), + style_dbg!(result, color.red, color) ); match error { None => {} @@ -571,8 +576,8 @@ } else { println!( " {} {}", - style!(format!(" <+> {} test:", name), types::BOLD), - style_dbg!(result, types::GREEN) + style!(format!(" <+> {} test:", name), color.bold, color), + style_dbg!(result, color.green, color) ); } } diff --git a/util/flashrom_tester/src/types.rs b/util/flashrom_tester/src/types.rs index b22ded2..9cefb27 100644 --- a/util/flashrom_tester/src/types.rs +++ b/util/flashrom_tester/src/types.rs @@ -33,21 +33,40 @@ // Software Foundation. // -pub const BOLD: &str = "\x1b[1m"; +pub struct Color { + pub bold: &'static str, + pub reset: &'static str, + pub magenta: &'static str, + pub yellow: &'static str, + pub green: &'static str, + pub red: &'static str, +} -pub const RESET: &str = "\x1b[0m"; -pub const MAGENTA: &str = "\x1b[35m"; -pub const YELLOW: &str = "\x1b[33m"; -pub const GREEN: &str = "\x1b[92m"; -pub const RED: &str = "\x1b[31m"; +pub const COLOR: Color = Color { + bold: "\x1b[1m", + reset: "\x1b[0m", + magenta: "\x1b[35m", + yellow: "\x1b[33m", + green: "\x1b[92m", + red: "\x1b[31m", +}; + +pub const NOCOLOR: Color = Color { + bold: "", + reset: "", + magenta: "", + yellow: "", + green: "", + red: "", +}; macro_rules! style_dbg { - ($s: expr, $c: expr) => { - format!("{}{:?}{}", $c, $s, types::RESET) + ($s: expr, $c: expr, $col: expr) => { + format!("{}{:?}{}", $c, $s, $col.reset) }; } macro_rules! style { - ($s: expr, $c: expr) => { - format!("{}{}{}", $c, $s, types::RESET) + ($s: expr, $c: expr, $col: expr) => { + format!("{}{}{}", $c, $s, $col.reset) }; } -- To view, visit https://review.coreboot.org/c/flashrom/+/69270 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia3cc527fb98e53eda6773622340cf10764df2cba Gerrit-Change-Number: 69270 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom_tester: Change timestamp to UTC microsecond
by Evan Benn (Code Review) 07 Nov '22

07 Nov '22

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69269 ) Change subject: flashrom_tester: Change timestamp to UTC microsecond ...................................................................... flashrom_tester: Change timestamp to UTC microsecond Match the timestamp with dmesg and other logs for ease of comparing. BUG=b:246250254 BRANCH=None TEST=flashrom_tester --libflashrom host Change-Id: I05182f52c0e9392a4fa2b388fdc30633e5d6e5ef --- M util/flashrom_tester/src/logger.rs 1 file changed, 17 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/69269/1 diff --git a/util/flashrom_tester/src/logger.rs b/util/flashrom_tester/src/logger.rs index 12e5495..3c9b74b 100644 --- a/util/flashrom_tester/src/logger.rs +++ b/util/flashrom_tester/src/logger.rs @@ -58,8 +58,8 @@ fn log(&self, record: &log::Record) { fn log_internal<W: Write>(mut w: W, record: &log::Record) -> std::io::Result<()> { - let now = chrono::Local::now(); - write!(w, "{}{} ", types::MAGENTA, now.format("%Y-%m-%dT%H:%M:%S"))?; + let now = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Micros, true); + write!(w, "{}{} ", types::MAGENTA, now)?; write!( w, "{}[ {} ]{} ", -- To view, visit https://review.coreboot.org/c/flashrom/+/69269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I05182f52c0e9392a4fa2b388fdc30633e5d6e5ef Gerrit-Change-Number: 69269 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: tests: Add llvm-cov option and run target for code coverage
by Evan Benn (Code Review) 07 Nov '22

07 Nov '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Hello build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69268 to look at the new patch set (#2). Change subject: tests: Add llvm-cov option and run target for code coverage ...................................................................... tests: Add llvm-cov option and run target for code coverage Code coverage can be requested with -Dllvm_cov and run with ninja llvm-cov. BUG=b:187647884 BRANCH=None TEST=meson test; ninja llvm-cov Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M Documentation/building.md M meson.build M meson_options.txt A scripts/llvm-cov 4 files changed, 40 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/69268/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69268 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Gerrit-Change-Number: 69268 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022