September 2022 - flashrom-gerrit

[S] Change in flashrom[1.1.x]: flashchips: Fix N25Q512 bulk erase
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Jacob Creedon. Hello build bot (Jenkins), Jacob Creedon, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67847 to review the following change. Change subject: flashchips: Fix N25Q512 bulk erase ...................................................................... flashchips: Fix N25Q512 bulk erase The N25Q is a stacked device, so it requires 0xC4 to perform a die erase. Signed-off-by: Jacob Creedon <jcreedon(a)google.com> Change-Id: Ib408fbe5633abd8b657e3907142b997e88b33f84 Reviewed-on: https://review.coreboot.org/c/flashrom/+/34489 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M flashchips.c 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/67847/1 diff --git a/flashchips.c b/flashchips.c index d85ea74..4dc50f7 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10555,8 +10555,8 @@ .eraseblocks = { {64 * 1024, 1024} }, .block_erase = spi_block_erase_d8, }, { - .eraseblocks = { {65536 * 1024, 1} }, - .block_erase = spi_block_erase_c7, + .eraseblocks = { {32768 * 1024, 2} }, + .block_erase = spi_block_erase_c4, } }, .printlock = spi_prettyprint_status_register_n25q, /* TODO: config, lock, flag regs */ -- To view, visit https://review.coreboot.org/c/flashrom/+/67847 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: Ib408fbe5633abd8b657e3907142b997e88b33f84 Gerrit-Change-Number: 67847 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Jacob Creedon <jcreedon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jacob Creedon <jcreedon(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.0.x]: pcidev: Always fetch ident info
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Thomas Heijligen, Daniel Verkamp. Hello build bot (Jenkins), Daniel Verkamp, Thomas Heijligen, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67845 to review the following change. Change subject: pcidev: Always fetch ident info ...................................................................... pcidev: Always fetch ident info As discovered earlier[1], the `vendor_id` and `device_id` fields are not always automatically set. However, we use these fields throughout flash- rom. To not lose track when we actually fetched them, let's always call pci_fill_info(PCI_FILL_IDENT) before returning a `pci_dev` handle. [1] Commit ca2e3bce0 (pcidev.c: populate IDs with pci_fill_info()) Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Ticket: https://ticket.coreboot.org/issues/367 Reviewed-on: https://review.coreboot.org/c/flashrom/+/64573 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Daniel Verkamp <dverkamp(a)chromium.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> --- M board_enable.c M internal.c 2 files changed, 39 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/67845/1 diff --git a/board_enable.c b/board_enable.c index 3b6755e..97bb15a 100644 --- a/board_enable.c +++ b/board_enable.c @@ -1514,6 +1514,7 @@ /* First, look for a known LPC bridge */ for (dev = pacc->devices; dev; dev = dev->next) { + pci_fill_info(dev, PCI_FILL_IDENT); uint16_t device_class; /* libpci before version 2.2.4 does not store class info. */ device_class = pci_read_word(dev, PCI_CLASS_DEVICE); diff --git a/internal.c b/internal.c index cb02753..7628988 100644 --- a/internal.c +++ b/internal.c @@ -30,9 +30,12 @@ { struct pci_dev *temp; - for (temp = pacc->devices; temp; temp = temp->next) - if (pci_filter_match(&filter, temp)) + for (temp = pacc->devices; temp; temp = temp->next) { + if (pci_filter_match(&filter, temp)) { + pci_fill_info(temp, PCI_FILL_IDENT); return temp; + } + } return NULL; } @@ -50,8 +53,10 @@ if (pci_filter_match(&filter, temp)) { /* Read PCI class */ tmp2 = pci_read_word(temp, 0x0a); - if (tmp2 == devclass) + if (tmp2 == devclass) { + pci_fill_info(temp, PCI_FILL_IDENT); return temp; + } } return NULL; @@ -66,9 +71,12 @@ filter.vendor = vendor; filter.device = device; - for (temp = pacc->devices; temp; temp = temp->next) - if (pci_filter_match(&filter, temp)) + for (temp = pacc->devices; temp; temp = temp->next) { + if (pci_filter_match(&filter, temp)) { + pci_fill_info(temp, PCI_FILL_IDENT); return temp; + } + } return NULL; } @@ -88,8 +96,10 @@ if ((card_vendor == pci_read_word(temp, PCI_SUBSYSTEM_VENDOR_ID)) && (card_device == - pci_read_word(temp, PCI_SUBSYSTEM_ID))) + pci_read_word(temp, PCI_SUBSYSTEM_ID))) { + pci_fill_info(temp, PCI_FILL_IDENT); return temp; + } } return NULL; -- To view, visit https://review.coreboot.org/c/flashrom/+/67845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Gerrit-Change-Number: 67845 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Daniel Verkamp <dverkamp(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: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.0.x]: pcidev.c: populate IDs with pci_fill_info()
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Daniel Verkamp, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Daniel Verkamp, Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67844 to review the following change. Change subject: pcidev.c: populate IDs with pci_fill_info() ...................................................................... pcidev.c: populate IDs with pci_fill_info() With pciutils 3.7.0, flashrom is unable to match any PCI devices by vendor/device ID because the vendor_id and device_id fields of struct pci_dev are not filled in. Call pci_fill_info() to request these identifiers before trying to match them against the supported device list. The pciutils ChangeLog for 3.7.0 mentions that the documentation and back-end behavior for pci_fill_info() was updated; it seems that a call to pci_fill_info() was always intended to be required, but some backends (such as the sysfs one used on Linux) would fill the identifier fields even when not requested by the user. The pci_fill_info() function and the PCI_FILL_IDENT flag have been available for all versions of pciutils since at least 2.0 from 1999, so it should be safe to add without any version checks. With this change, reading/writing a nicintel_spi boot ROM is successful. Signed-off-by: Daniel Verkamp <dverkamp(a)chromium.org> Change-Id: Ia011d4d801f8a54160e45a70b14b740e6dcc00ef Reviewed-on: https://review.coreboot.org/c/flashrom/+/46310 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M pcidev.c 1 file changed, 34 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/67844/1 diff --git a/pcidev.c b/pcidev.c index f4e5542..4bc32c1 100644 --- a/pcidev.c +++ b/pcidev.c @@ -211,6 +211,7 @@ for (dev = pacc->devices; dev; dev = dev->next) { if (pci_filter_match(&filter, dev)) { + pci_fill_info(dev, PCI_FILL_IDENT); /* Check against list of supported devices. */ for (i = 0; devs[i].device_name != NULL; i++) if ((dev->vendor_id == devs[i].vendor_id) && -- To view, visit https://review.coreboot.org/c/flashrom/+/67844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: Ia011d4d801f8a54160e45a70b14b740e6dcc00ef Gerrit-Change-Number: 67844 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.0.x]: chipset_enable.c: Validate physmap() return rcrb value
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Sam McNally, Edward O'Callaghan, Angel Pons. Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67843 to review the following change. Change subject: chipset_enable.c: Validate physmap() return rcrb value ...................................................................... chipset_enable.c: Validate physmap() return rcrb value Validate the physical mapping in enable_flash_silvermont(). Change-Id: Icc5a799a06b3f310d9a191fa5eb99b255b20d79d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/48225 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M chipset_enable.c 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/67843/1 diff --git a/chipset_enable.c b/chipset_enable.c index 67ce761..6220416 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -920,6 +920,8 @@ /* Handle GCS (in RCRB) */ void *rcrb = physmap("BYT RCRB", rcba, 4); + if (rcrb == ERROR_PTR) + return ERROR_FATAL; enable_flash_ich_report_gcs(dev, ich_generation, rcrb); physunmap(rcrb, 4); -- To view, visit https://review.coreboot.org/c/flashrom/+/67843 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: Icc5a799a06b3f310d9a191fa5eb99b255b20d79d Gerrit-Change-Number: 67843 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.0.x]: chipset_enable.c: check return value from rphysmap() call
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Sam McNally, Edward O'Callaghan, Angel Pons. Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67842 to review the following change. Change subject: chipset_enable.c: check return value from rphysmap() call ...................................................................... chipset_enable.c: check return value from rphysmap() call Port from the ChromiumOS fork of flashrom. Change-Id: I8075fe5f80ac0da5280d2f0de6829ed3a2496476 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/46444 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M chipset_enable.c 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/42/67842/1 diff --git a/chipset_enable.c b/chipset_enable.c index 3437792..67ce761 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -934,6 +934,8 @@ uint32_t sbase = pci_read_long(dev, 0x54) & 0xfffffe00; msg_pdbg("SPI_BASE_ADDRESS = 0x%x\n", sbase); void *spibar = rphysmap("BYT SBASE", sbase, 512); /* Last defined address on Bay Trail is 0x100 */ + if (spibar == ERROR_PTR) + return ERROR_FATAL; /* Enable Flash Writes. * Silvermont-based: BCR at SBASE + 0xFC (some bits of BCR are also accessible via BC at IBASE + 0x1C). -- To view, visit https://review.coreboot.org/c/flashrom/+/67842 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I8075fe5f80ac0da5280d2f0de6829ed3a2496476 Gerrit-Change-Number: 67842 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.0.x]: it87spi.c: Prevent use-after-free bug
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons. Hello build bot (Jenkins), Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67841 to review the following change. Change subject: it87spi.c: Prevent use-after-free bug ...................................................................... it87spi.c: Prevent use-after-free bug The memory for the `param` string is aliased by `dualbiosindex_suffix`. Moreover, `errno` could have been modified by the call to `free()`. Therefore, only free the former when there are no more uses of either. Change-Id: I79f18f6077c77c0cbb8bfa431e17f9b079f11c95 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/46551 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M it87spi.c 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/67841/1 diff --git a/it87spi.c b/it87spi.c index 0a1e894..a2188fc 100644 --- a/it87spi.c +++ b/it87spi.c @@ -139,12 +139,13 @@ char *dualbiosindex_suffix; errno = 0; long chip_index = strtol(param, &dualbiosindex_suffix, 0); - free(param); if (errno != 0 || *dualbiosindex_suffix != '\0' || chip_index < 0 || chip_index > 1) { msg_perr("DualBIOS: Invalid chip index requested - choose 0 or 1.\n"); + free(param); exit_conf_mode_ite(port); return 1; } + free(param); if (chip_index != (tmp & 1)) { msg_pdbg("DualBIOS: Previous chip index: %d\n", tmp & 1); sio_write(port, 0xEF, (tmp & 0xFE) | chip_index); -- To view, visit https://review.coreboot.org/c/flashrom/+/67841 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I79f18f6077c77c0cbb8bfa431e17f9b079f11c95 Gerrit-Change-Number: 67841 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.0.x]: spi25: Debug flashrom crash when Write Protect is ON
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan, SANTHOSH JANARDHANA HASSAN. Hello build bot (Jenkins), Stefan Reinauer, Edward O'Callaghan, SANTHOSH JANARDHANA HASSAN, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67840 to review the following change. Change subject: spi25: Debug flashrom crash when Write Protect is ON ...................................................................... spi25: Debug flashrom crash when Write Protect is ON When hardware write protect is applied, flashrom crashed and generate coredump. spi_disable_blockprotect_generic() calls flash->chip->printlock() method when disable was failed, but this method is optional, can be NULL depends on type of flashrom chip. NULL pointer check before call is added to avoid crash. BRANCH=none BUG=b:129083894 TEST=Run on Mistral P2 (On CR50 console, run "wp disable") flashrom --wp-range 0 0x400000 flashrom --wp-enable (On CR50 console, run "wp enable") flashrom -r /tmp/test.bin Verify "Block protection could not be disabled!" is shown, but flash read completes. Signed-off-by: Yuji Sasaki <sasakiy(a)chromium.org> Change-Id: I81094ab5f16a85871fc9869a2e285eddbbbdec4e Reviewed-on: https://chromium-review.googlesource.com/1535140 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator(a)appspot.gserviceaccount.com> Tested-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Stefan Reinauer <reinauer(a)google.com> Reviewed-by: SANTHOSH JANARDHANA HASSAN <sahassan(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/40468 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M spi25_statusreg.c 1 file changed, 38 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/67840/1 diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 05c7acf..de629a9 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -186,7 +186,8 @@ status = spi_read_status_register(flash); if ((status & bp_mask) != 0) { msg_cerr("Block protection could not be disabled!\n"); - flash->chip->printlock(flash); + if (flash->chip->printlock) + flash->chip->printlock(flash); return 1; } msg_cdbg("disabled.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/67840 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I81094ab5f16a85871fc9869a2e285eddbbbdec4e Gerrit-Change-Number: 67840 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: SANTHOSH JANARDHANA HASSAN <sahassan(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Yuji Sasaki <sasakiy(a)chromium.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: SANTHOSH JANARDHANA HASSAN <sahassan(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.0.x]: dummyflasher: Add error check for file read
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: David Hendricks, Jacob Garber. Hello build bot (Jenkins), David Hendricks, Jacob Garber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67839 to review the following change. Change subject: dummyflasher: Add error check for file read ...................................................................... dummyflasher: Add error check for file read Print an error message and return if the read from emu_persistent_image fails. Change-Id: Icd1a72f9171e547f2081ba4bc53834a17ef7fcab Signed-off-by: Jacob Garber <jgarber1(a)ualberta.ca> Found-by: Coverity CID 1403912 Reviewed-on: https://review.coreboot.org/c/flashrom/+/34845 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M dummyflasher.c 1 file changed, 23 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/67839/1 diff --git a/dummyflasher.c b/dummyflasher.c index f171128..8c3606e 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -381,8 +381,12 @@ if (image_stat.st_size == emu_chip_size) { msg_pdbg("matches.\n"); msg_pdbg("Reading %s\n", emu_persistent_image); - read_buf_from_file(flashchip_contents, emu_chip_size, - emu_persistent_image); + if (read_buf_from_file(flashchip_contents, emu_chip_size, + emu_persistent_image)) { + msg_perr("Unable to read %s\n", emu_persistent_image); + free(flashchip_contents); + return 1; + } } else { msg_pdbg("doesn't match.\n"); } -- To view, visit https://review.coreboot.org/c/flashrom/+/67839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: Icd1a72f9171e547f2081ba4bc53834a17ef7fcab Gerrit-Change-Number: 67839 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Jacob Garber <jgarber1(a)ualberta.ca> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.0.x]: pickit2_spi: Fix "dead" assignment
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Elyes Haouas. Hello Elyes Haouas, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67838 to review the following change. Change subject: pickit2_spi: Fix "dead" assignment ...................................................................... pickit2_spi: Fix "dead" assignment We never read the first 'ret'. Let's check the first 'ret' and exit if it failed. Also, print the version only when the command succeeded. Found-by: scan-build 7.0.1-8 Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/flashrom/+/34403 Tested-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M pickit2_spi.c 1 file changed, 26 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/67838/1 diff --git a/pickit2_spi.c b/pickit2_spi.c index 102fe37..bae0aa1 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -123,14 +123,20 @@ uint8_t command[CMD_LENGTH] = {CMD_GET_VERSION, CMD_END_OF_BUFFER}; ret = usb_interrupt_write(pickit2_handle, ENDPOINT_OUT, (char *)command, CMD_LENGTH, DFLT_TIMEOUT); + + if (ret != CMD_LENGTH) { + msg_perr("Command Get Firmware Version failed!\n"); + return 1; + } + ret = usb_interrupt_read(pickit2_handle, ENDPOINT_IN, (char *)command, CMD_LENGTH, DFLT_TIMEOUT); - msg_pdbg("PICkit2 Firmware Version: %d.%d\n", (int)command[0], (int)command[1]); if (ret != CMD_LENGTH) { msg_perr("Command Get Firmware Version failed (%s)!\n", usb_strerror()); return 1; } + msg_pdbg("PICkit2 Firmware Version: %d.%d\n", (int)command[0], (int)command[1]); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/67838 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f Gerrit-Change-Number: 67838 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: serial.c: support BAUD rates > 230400 on macOS
by Peter Stuge (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons, Name of user not set #1004601. Peter Stuge has removed a vote from this change. ( https://review.coreboot.org/c/flashrom/+/67822 ) Change subject: serial.c: support BAUD rates > 230400 on macOS ...................................................................... Removed Code-Review+2 by Peter Stuge <peter(a)stuge.se> -- To view, visit https://review.coreboot.org/c/flashrom/+/67822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c67048b9c7bc704d9b0c4ef4771f708fdb3e0f3 Gerrit-Change-Number: 67822 Gerrit-PatchSet: 6 Gerrit-Owner: Name of user not set #1004601 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Peter Stuge <peter(a)stuge.se> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Name of user not set #1004601 Gerrit-MessageType: deleteVote

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022