January 2021 - flashrom-gerrit

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Richard Hughes (Code Review) 18 Jan '21

18 Jan '21

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/49643 to look at the new patch set (#2). Change subject: libflashrom: Return progress state to the library user ...................................................................... libflashrom: Return progress state to the library user Include test for the dummy spi25 device. Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Signed-off-by: Richard Hughes <richard(a)hughsie.com> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M en29lv640b.c M flash.h M it87spi.c M jedec.c M libflashrom.c M libflashrom.h M linux_mtd.c M lspcon_i2c_spi.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 20 files changed, 123 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/49643/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Gerrit-Change-Number: 49643 Gerrit-PatchSet: 2 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: programmer: remove unused noop_shutdown function
by Angel Pons (Code Review) 18 Jan '21

18 Jan '21

Attention is currently required from: Alan Green. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49631 ) Change subject: programmer: remove unused noop_shutdown function ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49631 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b67223aed8be54b60771aa1b2d498836ed28060 Gerrit-Change-Number: 49631 Gerrit-PatchSet: 1 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Comment-Date: Mon, 18 Jan 2021 14:48:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Richard Hughes (Code Review) 18 Jan '21

18 Jan '21

Attention is currently required from: Edward O'Callaghan. Richard Hughes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 ) Change subject: libflashrom: Return progress state to the library user ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Adding Edward as requested. -- To view, visit https://review.coreboot.org/c/flashrom/+/49643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Gerrit-Change-Number: 49643 Gerrit-PatchSet: 1 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 18 Jan 2021 11:22:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Richard Hughes (Code Review) 18 Jan '21

18 Jan '21

Richard Hughes has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/49643 ) Change subject: libflashrom: Return progress state to the library user ...................................................................... libflashrom: Return progress state to the library user Include test for the dummy spi25 device. Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Signed-off-by: Richard Hughes <richard(a)hughsie.com> --- M flash.h M libflashrom.c M libflashrom.h M spi.c M spi25.c M tests/spi25.c M tests/tests.c M tests/tests.h 8 files changed, 85 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/49643/1 diff --git a/flash.h b/flash.h index 883b6f4..66010ce 100644 --- a/flash.h +++ b/flash.h @@ -285,6 +285,9 @@ chip_restore_fn_cb_t func; uint8_t status; } chip_restore_fn[MAX_CHIP_RESTORE_FUNCTIONS]; + /* Progress reporting */ + flashrom_progress_callback *progress_callback; + void *progress_userdata; }; /* Timing used in probe routines. ZERO is -2 to differentiate between an unset @@ -415,6 +418,7 @@ #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 set_progress(struct flashctx *flash, enum flashrom_progress_stage stage, size_t current, size_t total); /* layout.c */ int register_include_arg(struct layout_include_args **args, char *name); diff --git a/libflashrom.c b/libflashrom.c index ae2d33d..10d8882 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -95,6 +95,29 @@ return 0; } +/** + * @brief Set the progress callback function. + * + * Set a callback function which will be invoked whenever libflashrom wants + * to indicate the progress has chaned. 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 userdata Userdata pointer to include with the progress callback. + */ +void flashrom_set_progress_callback(struct flashrom_flashctx *flashctx, flashrom_progress_callback *progress_callback, void *progress_userdata) +{ + flashctx->progress_callback = progress_callback; + flashctx->progress_userdata = progress_userdata; +} +/** @private */ +void set_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t current, size_t total) +{ + if (flashctx->progress_callback == NULL) + return; + flashctx->progress_callback(flashctx, stage, current, total, flashctx->progress_userdata); +} + /** @} */ /* end flashrom-general */ diff --git a/libflashrom.h b/libflashrom.h index d0d5826..4f2474c 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -35,6 +35,13 @@ FLASHROM_MSG_DEBUG2 = 4, FLASHROM_MSG_SPEW = 5, }; + +/** @ingroup flashrom-general */ +enum flashrom_progress_stage { + FLASHROM_PROGRESS_READ, + FLASHROM_PROGRESS_WRITE, +}; + /** @ingroup flashrom-general */ typedef int(flashrom_log_callback)(enum flashrom_log_level, const char *format, va_list); void flashrom_set_log_callback(flashrom_log_callback *); @@ -93,6 +100,10 @@ int flashrom_flash_erase(struct flashrom_flashctx *); void flashrom_flash_release(struct flashrom_flashctx *); +/** @ingroup flashrom-general */ +typedef void(flashrom_progress_callback)(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t current, size_t total, void *user_data); +void flashrom_set_progress_callback(struct flashrom_flashctx *const flashctx, flashrom_progress_callback *, void *); + /** @ingroup flashrom-flash */ enum flashrom_flag { FLASHROM_FLAG_FORCE, diff --git a/spi.c b/spi.c index aed2a92..49b23fc 100644 --- a/spi.c +++ b/spi.c @@ -101,6 +101,8 @@ { int ret; size_t to_read; + size_t progress_start = start; + size_t progress_total = 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 @@ -110,6 +112,7 @@ ret = flash->mst->spi.read(flash, buf, start, to_read); if (ret) return ret; + set_progress(flash, FLASHROM_PROGRESS_READ, start - progress_start, progress_total); } return 0; } diff --git a/spi25.c b/spi25.c index 213273f..58f3936 100644 --- a/spi25.c +++ b/spi25.c @@ -669,11 +669,14 @@ { int ret; size_t to_read; + size_t progress_start = start; + size_t progress_total = 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; + set_progress(flash, FLASHROM_PROGRESS_READ, start - progress_start, progress_total); } return 0; } @@ -693,6 +696,8 @@ * we're OK for now. */ unsigned int page_size = flash->chip->page_size; + size_t progress_start = start; + size_t progress_total = 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 @@ -717,6 +722,7 @@ if (rc) return rc; } + set_progress(flash, FLASHROM_PROGRESS_WRITE, start - progress_start, progress_total); } return 0; @@ -736,6 +742,7 @@ for (i = start; i < start + len; i++) { if (spi_nbyte_program(flash, i, buf + i - start, 1)) return 1; + set_progress(flash, FLASHROM_PROGRESS_WRITE, i - start, len - start); } return 0; } diff --git a/tests/spi25.c b/tests/spi25.c index 942fe6e..2d4b18d 100644 --- a/tests/spi25.c +++ b/tests/spi25.c @@ -49,6 +49,41 @@ .write = NULL, }; +static void spi_read_progress_cb(struct flashrom_flashctx *flashctx, + enum flashrom_progress_stage stage, + size_t current, + size_t total, + void *user_data) +{ + uint32_t *cnt = (uint32_t *) user_data; + assert_int_equal(0x300, total); + (*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 }; + flashrom_set_progress_callback(&flashctx, spi_read_progress_cb, (void *) &cnt); + 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 3124607..e749f77 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -52,6 +52,7 @@ 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 cb905fd..cf8f0ba 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -31,6 +31,7 @@ /* 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/+/49643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Gerrit-Change-Number: 49643 Gerrit-PatchSet: 1 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: util/getrevision.sh: Fallback when git tags is missing
by Patrick Georgi (Code Review) 18 Jan '21

18 Jan '21

Attention is currently required from: Sam McNally, Nico Huber. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49618 ) Change subject: util/getrevision.sh: Fallback when git tags is missing ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib9f85b2be8b6f5e1332ba98a8a71fcad12331818 Gerrit-Change-Number: 49618 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 18 Jan 2021 09:32:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: release I/Os on shutdown
by Alan Green (Code Review) 18 Jan '21

18 Jan '21

Attention is currently required from: Edward O'Callaghan. Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49632 ) Change subject: ft2232_spi.c: release I/Os on shutdown ...................................................................... Patch Set 2: (2 comments) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/49632/comment/2b671720_11610e25 PS1, Line 175: ret = -8; : goto ftdi_err; : } : : ftdi_err: > I feel like this goto makes things more complex then it needs to be for a single branch, the ret of […] Good call. I had copy-paste from elsewhere, which left a bit of a mess. https://review.coreboot.org/c/flashrom/+/49632/comment/008388e7_7389de67 PS1, Line 183: return f; > ret = f; would be consistent with the rest of the control flow. I think this makes more sense now. -- To view, visit https://review.coreboot.org/c/flashrom/+/49632 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fae55e532595752983f55fac2298f81699dbe5b Gerrit-Change-Number: 49632 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 18 Jan 2021 08:09:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: release I/Os on shutdown
by Alan Green (Code Review) 18 Jan '21

18 Jan '21

Attention is currently required from: Alan Green. Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/49632 to look at the new patch set (#2). Change subject: ft2232_spi.c: release I/Os on shutdown ...................................................................... ft2232_spi.c: release I/Os on shutdown Resets FTDI I/O pins to high-Z (input) when shutting down. This allows other devices to use the SPI bus without having to disconnect the programmer. This change will introduce a backward incompatibility in the case where a user is relying on the state of FTDI outputs post-programming (eg. to disallow another device from driving CS low). However, there are likely more cases where releasing the SPI bus is the correct thing to do. Signed-off-by: Alan Green <avg(a)google.com> Change-Id: I9fae55e532595752983f55fac2298f81699dbe5b --- M ft2232_spi.c 1 file changed, 26 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/49632/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49632 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fae55e532595752983f55fac2298f81699dbe5b Gerrit-Change-Number: 49632 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: programmer: remove unused noop_shutdown function
by Paul Menzel (Code Review) 18 Jan '21

18 Jan '21

Attention is currently required from: Alan Green. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49631 ) Change subject: programmer: remove unused noop_shutdown function ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49631/comment/ac81fcdb_2d7d0af8 PS1, Line 9: vestigal vestig*i*al -- To view, visit https://review.coreboot.org/c/flashrom/+/49631 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b67223aed8be54b60771aa1b2d498836ed28060 Gerrit-Change-Number: 49631 Gerrit-PatchSet: 1 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Comment-Date: Mon, 18 Jan 2021 08:01:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Generalized GPIOL pin control
by Alan Green (Code Review) 18 Jan '21

18 Jan '21

Alan Green has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/49637 ) Change subject: ft2232_spi.c: Generalized GPIOL pin control ...................................................................... ft2232_spi.c: Generalized GPIOL pin control Adds a new arg "gpiol" to allow the four FT2232 GPIOL pins to be set to any combination of high, low or high-impedance. The existing arg "csgpiol", is intended to function as an additional "cs" signal, allowing pins to be set high but not low. This patch preserves the csgpiol arg for backward compatibility. In the event that both arguments are specified, gpiol is used. Signed-off-by: Alan Green <avg(a)google.com> Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab --- M ft2232_spi.c 1 file changed, 38 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/49637/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index 6cf7a3c..60327ee 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -370,7 +370,7 @@ } free(arg); - /* Allows setting multiple GPIOL states, for example: csgpiol=012 */ + /* Allows setting multiple GPIOL pins to high, for example: csgpiol=012 */ arg = extract_programmer_param("csgpiol"); if (arg) { unsigned int ngpios = strlen(arg); @@ -391,6 +391,43 @@ } free(arg); + /* Allows setting GPIOL pins high, low or input (high-z) */ + arg = extract_programmer_param("gpiol"); + if (arg) { + int ok = 0; + if (strlen(arg) == 4) { + ok = 1; + for (int i = 0; i < 4; i++) { + unsigned int pin = i + 4; + switch (toupper(arg[i])) { + case 'H': + cs_bits |= 1 << pin; + pindir |= 1 << pin; + break; + case 'L': + pindir |= 1 << pin; + break; + case 'X': + break; + default: + ok = 0; + } + } + } + if (!ok) { + msg_perr("Error: Invalid GPIOLs specified: \"%s\".\n" + "Valid values are 4 character strings of H, L and X.\n" + " H - Set GPIOL high\n" + " L - Set GPIOL low\n" + " X - Leave GPIOL at high impedance\n" + "Example: gpiol=HXLX drives GPIOL 0 to high, GPIOL 2 to " + "high, and leaves GPIOLs\n1 and 3 undriven.\n", arg); + free(arg); + return -2; + } + } + free(arg); + msg_pdbg("Using device type %s %s ", get_ft2232_vendorname(ft2232_vid, ft2232_type), get_ft2232_devicename(ft2232_vid, ft2232_type)); -- To view, visit https://review.coreboot.org/c/flashrom/+/49637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Gerrit-Change-Number: 49637 Gerrit-PatchSet: 1 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ft2232_spi.c: release I/Os on shutdown
by Edward O'Callaghan (Code Review) 18 Jan '21

18 Jan '21

Attention is currently required from: Alan Green. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49632 ) Change subject: ft2232_spi.c: release I/Os on shutdown ...................................................................... Patch Set 1: Code-Review+1 (2 comments) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/49632/comment/05d83332_aa755d0e PS1, Line 175: ret = -8; : goto ftdi_err; : } : : ftdi_err: I feel like this goto makes things more complex then it needs to be for a single branch, the ret of '-8' can also be overwritten with 'f'. Some questions here are, what is '-8' and how about a log line then drop the goto and press on with closing the ftdicontext. https://review.coreboot.org/c/flashrom/+/49632/comment/96ec232f_597a86ed PS1, Line 183: return f; ret = f; would be consistent with the rest of the control flow. -- To view, visit https://review.coreboot.org/c/flashrom/+/49632 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fae55e532595752983f55fac2298f81699dbe5b Gerrit-Change-Number: 49632 Gerrit-PatchSet: 1 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Comment-Date: Mon, 18 Jan 2021 06:43:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2021