- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: layout: Factor out flash_region structure from romentry
by Edward O'Callaghan (Code Review) 12 Dec '22

12 Dec '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69196 to look at the new patch set (#7). Change subject: layout: Factor out flash_region structure from romentry ...................................................................... layout: Factor out flash_region structure from romentry The romentry structure is the container ADT with some annotated meta-data such as 'included' or 'file' however the substantive substructure is a 'flash_region'. Therefore factor this out. BUG=b:260440773 BRANCH=none TEST=flashrom_tester Change-Id: I768742b73db901df5b5208fcbcb8a324a06014c2 CoAuthored-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M cli_classic.c M flashrom.c M include/layout.h M layout.c 4 files changed, 73 insertions(+), 34 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/69196/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/69196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I768742b73db901df5b5208fcbcb8a324a06014c2 Gerrit-Change-Number: 69196 Gerrit-PatchSet: 7 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Rename 'internal_delay()' to 'default_delay()'
by Edward O'Callaghan (Code Review) 12 Dec '22

12 Dec '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68855 ) ( 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tree/: Rename 'internal_delay()' to 'default_delay()' ...................................................................... tree/: Rename 'internal_delay()' to 'default_delay()' The non-custom driver programmer delay implementation 'internal_delay()' is unrelated specifically to the 'internal' programmer. The delay implementation is simply a platform-agnostic host delay implementation. Therefore, rename to simply default_delay(). Change-Id: I5e04adf16812ceb1480992c92bca25ed80f8897a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68855 Reviewed-by: Alexander Goncharov <chat(a)joursoir.net> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M amd_imc.c M atavia.c M bitbang_spi.c M ch341a_spi.c M dediprog.c M dummyflasher.c M flashrom.c M ichspi.c M include/flash.h M include/programmer.h M it87spi.c M nicintel_eeprom.c M pony_spi.c M raiden_debug_spi.c M serial.c M serprog.c M udelay.c M wbsio_spi.c 18 files changed, 66 insertions(+), 46 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Alexander Goncharov: Looks good to me, but someone else must approve diff --git a/amd_imc.c b/amd_imc.c index 2ee8332..093d42d 100644 --- a/amd_imc.c +++ b/amd_imc.c @@ -60,7 +60,7 @@ msg_pwarn("IMC MBOX: Timeout!\n"); return 1; } - internal_delay(1000); + default_delay(1000); } return 0; } diff --git a/atavia.c b/atavia.c index ebf2949..8af52d6 100644 --- a/atavia.c +++ b/atavia.c @@ -90,7 +90,7 @@ ready = true; break; } else { - internal_delay(1); + default_delay(1); continue; } } @@ -170,7 +170,7 @@ /* Test if a flash chip is attached. */ pci_write_long(dev, PCI_ROM_ADDRESS, (uint32_t)PCI_ROM_ADDRESS_MASK); - internal_delay(90); + default_delay(90); uint32_t base = pci_read_long(dev, PCI_ROM_ADDRESS); msg_pdbg2("BROM base=0x%08x\n", base); if ((base & PCI_ROM_ADDRESS_MASK) == 0) { diff --git a/bitbang_spi.c b/bitbang_spi.c index 71798dc..fadca70 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -76,10 +76,10 @@ bitbang_spi_set_sck_set_mosi(master, 0, 0, spi_data); else bitbang_spi_set_sck(master, 0, spi_data); - internal_delay(master->half_period); + default_delay(master->half_period); ret <<= 1; ret |= bitbang_spi_set_sck_get_miso(master, 1, spi_data); - internal_delay(master->half_period); + default_delay(master->half_period); } return ret; } @@ -90,9 +90,9 @@ for (i = 7; i >= 0; i--) { bitbang_spi_set_sck_set_mosi(master, 0, (val >> i) & 1, spi_data); - internal_delay(master->half_period); + default_delay(master->half_period); bitbang_spi_set_sck(master, 1, spi_data); - internal_delay(master->half_period); + default_delay(master->half_period); } } @@ -122,9 +122,9 @@ readarr[i] = bitbang_spi_read_byte(master, data->spi_data); bitbang_spi_set_sck(master, 0, data->spi_data); - internal_delay(master->half_period); + default_delay(master->half_period); bitbang_spi_set_cs(master, 1, data->spi_data); - internal_delay(master->half_period); + default_delay(master->half_period); /* FIXME: Run bitbang_spi_release_bus here or in programmer init? */ bitbang_spi_release_bus(master, data->spi_data); diff --git a/ch341a_spi.c b/ch341a_spi.c index 2dc983f..fe18d13 100644 --- a/ch341a_spi.c +++ b/ch341a_spi.c @@ -326,10 +326,10 @@ { /* There is space for 28 bytes instructions of 750 ns each in the CS packet (32 - 4 for the actual CS * instructions), thus max 21 us, but we avoid getting too near to this boundary and use - * internal_delay() for durations over 20 us. */ + * default_delay() for durations over 20 us. */ if ((usecs + stored_delay_us) > 20) { unsigned int inc = 20 - stored_delay_us; - internal_delay(usecs - inc); + default_delay(usecs - inc); usecs = inc; } stored_delay_us += usecs; diff --git a/dediprog.c b/dediprog.c index 6c41f28..723f4a8 100644 --- a/dediprog.c +++ b/dediprog.c @@ -316,7 +316,7 @@ if (voltage_selector == 0) { /* Wait some time as the original driver does. */ - internal_delay(200 * 1000); + default_delay(200 * 1000); } ret = dediprog_write(dediprog_handle, CMD_SET_VCC, voltage_selector, 0, NULL, 0); if (ret != 0x0) { @@ -326,7 +326,7 @@ } if (voltage_selector != 0) { /* Wait some time as the original driver does. */ - internal_delay(200 * 1000); + default_delay(200 * 1000); } return 0; } diff --git a/dummyflasher.c b/dummyflasher.c index f194c10..e5a9c12 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -897,7 +897,7 @@ msg_pspew(" 0x%02x", readarr[i]); msg_pspew("\n"); - internal_delay((writecnt + readcnt) * emu_data->delay_us); + default_delay((writecnt + readcnt) * emu_data->delay_us); return 0; } diff --git a/flashrom.c b/flashrom.c index 18c4864..28201a7 100644 --- a/flashrom.c +++ b/flashrom.c @@ -260,18 +260,18 @@ return; /** - * Drivers should either use internal_delay() directly or their + * Drivers should either use default_delay() directly or their * own custom delay. Only core flashrom logic calls programmer_delay() * which should always have a valid flash context. A NULL context * more than likely indicates a layering violation or BUG however - * for now dispatch a internal_delay() as a safe default for the NULL + * for now dispatch a default_delay() as a safe default for the NULL * base case. */ if (!flash) { msg_perr("%s called with NULL flash context. " "Please report a bug at flashrom(a)flashrom.org\n", __func__); - return internal_delay(usecs); + return default_delay(usecs); } if (flash->mst->buses_supported & BUS_SPI) { @@ -282,7 +282,7 @@ return flash->mst->par.delay(flash, usecs); } - return internal_delay(usecs); + return default_delay(usecs); } int read_memmapped(struct flashctx *flash, uint8_t *buf, unsigned int start, diff --git a/ichspi.c b/ichspi.c index a6587c6..43590d6 100644 --- a/ichspi.c +++ b/ichspi.c @@ -875,7 +875,7 @@ timeout = 100 * 60; /* 60 ms are 9.6 million cycles at 16 MHz. */ while ((REGREAD16(ICH7_REG_SPIS) & SPIS_SCIP) && --timeout) { - internal_delay(10); + default_delay(10); } if (!timeout) { msg_perr("Error: SCIP never cleared!\n"); @@ -951,7 +951,7 @@ /* Wait for Cycle Done Status or Flash Cycle Error. */ while (((REGREAD16(ICH7_REG_SPIS) & (SPIS_CDS | SPIS_FCERR)) == 0) && --timeout) { - internal_delay(10); + default_delay(10); } if (!timeout) { msg_perr("timeout, ICH7_REG_SPIS=0x%04x\n", REGREAD16(ICH7_REG_SPIS)); @@ -991,7 +991,7 @@ timeout = 100 * 60; /* 60 ms are 9.6 million cycles at 16 MHz. */ while ((REGREAD8(swseq_data.reg_ssfsc) & SSFS_SCIP) && --timeout) { - internal_delay(10); + default_delay(10); } if (!timeout) { msg_perr("Error: SCIP never cleared!\n"); @@ -1071,7 +1071,7 @@ /* Wait for Cycle Done Status or Flash Cycle Error. */ while (((REGREAD32(swseq_data.reg_ssfsc) & (SSFS_FDONE | SSFS_FCERR)) == 0) && --timeout) { - internal_delay(10); + default_delay(10); } if (!timeout) { msg_perr("timeout, REG_SSFS=0x%08x\n", REGREAD32(swseq_data.reg_ssfsc)); @@ -1319,7 +1319,7 @@ while ((((hsfs = REGREAD16(ICH9_REG_HSFS)) & (HSFS_FDONE | HSFS_FCERR)) == 0) && --timeout_us) { - internal_delay(8); + default_delay(8); } REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); if (!timeout_us) { diff --git a/include/flash.h b/include/flash.h index 3f78721..aabc785 100644 --- a/include/flash.h +++ b/include/flash.h @@ -60,7 +60,7 @@ const char *descr, uintptr_t phys_addr, size_t len); void master_unmap_flash_region(const struct registered_master *mast, void *virt_addr, size_t len); -/* NOTE: flashctx is not used in internal_delay. In this case, a context should be NULL. */ +/* NOTE: flashctx is not used in default_delay. In this case, a context should be NULL. */ void programmer_delay(const struct flashrom_flashctx *flash, unsigned int usecs); #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) diff --git a/include/programmer.h b/include/programmer.h index 8ba5781..aaa1407 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -211,7 +211,7 @@ void myusec_delay(unsigned int usecs); void myusec_calibrate_delay(void); void internal_sleep(unsigned int usecs); -void internal_delay(unsigned int usecs); +void default_delay(unsigned int usecs); #if CONFIG_INTERNAL == 1 /* board_enable.c */ diff --git a/it87spi.c b/it87spi.c index fc29e51..34dbea0 100644 --- a/it87spi.c +++ b/it87spi.c @@ -146,7 +146,7 @@ if((status & SPI_SR_WIP) == 0) return 0; - internal_delay(1000); + default_delay(1000); } return 0; } diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index 6922fb7..80ccd88 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -213,7 +213,7 @@ eewr |= BIT(EEWR_CMDV); pci_mmio_writel(eewr, eebar + EEWR); - internal_delay(5); + default_delay(5); int i; for (i = 0; i < MAX_ATTEMPTS; i++) if (pci_mmio_readl(eebar + EEWR) & BIT(EEWR_DONE)) @@ -338,7 +338,7 @@ nicintel_ee_bitbang(eebar, 0x00, &rdsr); nicintel_ee_bitset(eebar, EEC, EE_CS, 1); - internal_delay(1); + default_delay(1); if (!(rdsr & SPI_SR_WIP)) { return 0; } @@ -379,7 +379,7 @@ nicintel_ee_bitset(eebar, EEC, EE_CS, 0); nicintel_ee_bitbang(eebar, JEDEC_WREN, NULL); nicintel_ee_bitset(eebar, EEC, EE_CS, 1); - internal_delay(1); + default_delay(1); /* data */ nicintel_ee_bitset(eebar, EEC, EE_CS, 0); @@ -394,7 +394,7 @@ break; } nicintel_ee_bitset(eebar, EEC, EE_CS, 1); - internal_delay(1); + default_delay(1); if (nicintel_ee_ready(eebar)) goto out; } diff --git a/pony_spi.c b/pony_spi.c index 408fa12..0647f91 100644 --- a/pony_spi.c +++ b/pony_spi.c @@ -244,7 +244,7 @@ for (i = 1; i <= 10; i++) { data_out = i & 1; sp_set_pin(PIN_RTS, data_out); - internal_delay(1000); + default_delay(1000); /* If DSR does not change, we are not connected to what we think */ if (data_out != sp_get_pin(PIN_DSR)) { diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 2478133..8260abd 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -892,7 +892,7 @@ /* Reattempting will not result in a recovery. */ return status; } - internal_delay(RETRY_INTERVAL_US); + default_delay(RETRY_INTERVAL_US); continue; } @@ -927,7 +927,7 @@ /* Reattempting will not result in a recovery. */ return status; } - internal_delay(RETRY_INTERVAL_US); + default_delay(RETRY_INTERVAL_US); } } @@ -962,7 +962,7 @@ " config attempt = %d\n" " status = 0x%05x\n", config_attempt + 1, status); - internal_delay(RETRY_INTERVAL_US); + default_delay(RETRY_INTERVAL_US); continue; } @@ -972,7 +972,7 @@ " config attempt = %d\n" " status = 0x%05x\n", config_attempt + 1, status); - internal_delay(RETRY_INTERVAL_US); + default_delay(RETRY_INTERVAL_US); continue; } @@ -1016,7 +1016,7 @@ config_attempt + 1, rsp_config.packet_v2.packet_id, rsp_config.packet_size); - internal_delay(RETRY_INTERVAL_US); + default_delay(RETRY_INTERVAL_US); } return USB_SPI_HOST_INIT_FAILURE; } @@ -1240,7 +1240,7 @@ /* Reattempting will not result in a recovery. */ return status; } - internal_delay(RETRY_INTERVAL_US); + default_delay(RETRY_INTERVAL_US); continue; } for (read_attempt = 0; read_attempt < READ_RETRY_ATTEMPTS; @@ -1277,7 +1277,7 @@ } /* Device needs to reset its transmit index. */ restart_response_v2(ctx_data); - internal_delay(RETRY_INTERVAL_US); + default_delay(RETRY_INTERVAL_US); } } } diff --git a/serial.c b/serial.c index 72f9ef6..a0ef632 100644 --- a/serial.c +++ b/serial.c @@ -403,7 +403,7 @@ if (!tmp) { msg_pdbg2("Empty write\n"); empty_writes--; - internal_delay(500); + default_delay(500); if (empty_writes == 0) { msg_perr("Serial port is unresponsive!\n"); return 1; @@ -510,7 +510,7 @@ ret = 0; break; } - internal_delay(1000); /* 1ms units */ + default_delay(1000); /* 1ms units */ } if (really_read != NULL) *really_read = rd_bytes; @@ -596,7 +596,7 @@ break; } } - internal_delay(1000); /* 1ms units */ + default_delay(1000); /* 1ms units */ } if (really_wrote != NULL) *really_wrote = wr_bytes; diff --git a/serprog.c b/serprog.c index a7796ba..84dfd23 100644 --- a/serprog.c +++ b/serprog.c @@ -157,7 +157,7 @@ goto err_out; } /* A second should be enough to get all the answers to the buffer */ - internal_delay(1000 * 1000); + default_delay(1000 * 1000); sp_flush_incoming(); /* Then try up to 8 times to send syncnop and get the correct special * @@ -577,7 +577,7 @@ msg_pspew("%s usecs=%d\n", __func__, usecs); if (!sp_check_commandavail(S_CMD_O_DELAY)) { msg_pdbg2("serprog_delay used, but programmer doesn't support delays natively - emulating\n"); - internal_delay(usecs); + default_delay(usecs); return; } if ((sp_max_write_n) && (sp_write_n_bytes)) diff --git a/udelay.c b/udelay.c index 6c0efc4..82ddcbd 100644 --- a/udelay.c +++ b/udelay.c @@ -235,7 +235,7 @@ } /* Precise delay. */ -void internal_delay(unsigned int usecs) +void default_delay(unsigned int usecs) { /* If the delay is >1 s, use internal_sleep because timing does not need to be so precise. */ if (usecs > 1000000) { @@ -255,7 +255,7 @@ get_cpu_speed(); } -void internal_delay(unsigned int usecs) +void default_delay(unsigned int usecs) { udelay(usecs); } diff --git a/wbsio_spi.c b/wbsio_spi.c index b643eb4..d635a09 100644 --- a/wbsio_spi.c +++ b/wbsio_spi.c @@ -155,7 +155,7 @@ OUTB(writearr[0], data->spibase); OUTB(mode, data->spibase + 1); - internal_delay(10); + default_delay(10); if (!readcnt) return 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/68855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5e04adf16812ceb1480992c92bca25ed80f8897a Gerrit-Change-Number: 68855 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: 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-MessageType: merged

1 0

[S] Change in flashrom[master]: flashrom.c: Reduce the prog global state machine
by Felix Singer (Code Review) 12 Dec '22

12 Dec '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70603 ) Change subject: flashrom.c: Reduce the prog global state machine ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70603 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I93dfe09fe877acd32c4f22665921544ccd9323f6 Gerrit-Change-Number: 70603 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 12 Dec 2022 22:38:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Reduce the prog global state machine
by Felix Singer (Code Review) 12 Dec '22

12 Dec '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70603 ) Change subject: flashrom.c: Reduce the prog global state machine ...................................................................... Patch Set 1: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/70603/comment/da38df8f_43b2378e PS1, Line 41: g_programmer_type Since this is a bool, I would name this `g_is_internal_programmer`. `_type` sounds like it stores more than two values. Then a comment about what true and false mean isn't needed as well :) -- To view, visit https://review.coreboot.org/c/flashrom/+/70603 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I93dfe09fe877acd32c4f22665921544ccd9323f6 Gerrit-Change-Number: 70603 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 12 Dec 2022 22:36:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: layout: Factor out flash_region structure from romentry
by Anastasia Klimchuk (Code Review) 12 Dec '22

12 Dec '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69196 ) Change subject: layout: Factor out flash_region structure from romentry ...................................................................... Patch Set 6: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/69196/comment/07b32588_217f7588 PS6, Line 16: TEST=builds Have you done any testing with layouts that have multiple regions? Patchset: PS2: > That's a good question since this patch on its own doesn't have any extra context. […] I agree commit message needs more info on why the patch exist, one more paragraph would be great, something around "The patch allows for ABC to be done / XYZ to be improved / OPQ to be fixed later in the chain". As a reminder for Nikolai, b:260440773 is just a magic number, doesn't give any info. Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/69196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I768742b73db901df5b5208fcbcb8a324a06014c2 Gerrit-Change-Number: 69196 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 12 Dec 2022 22:29:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: chipset_enable.c: add PCI ID for TGL-UP3 SPI controller
by Jan Samek (Code Review) 12 Dec '22

12 Dec '22

Attention is currently required from: Angel Pons. Jan Samek has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70388 ) Change subject: chipset_enable.c: add PCI ID for TGL-UP3 SPI controller ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Hi Angel, Could you take a quick look? Thanks a lot. Regards, Jan -- To view, visit https://review.coreboot.org/c/flashrom/+/70388 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie93af14eb5857bfe51964f6565e475b6249dd407 Gerrit-Change-Number: 70388 Gerrit-PatchSet: 1 Gerrit-Owner: Jan Samek <jan.samek(a)siemens.com> 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-Comment-Date: Mon, 12 Dec 2022 18:32:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c:Add function to get a flattened view of the chip erase blocks
by Simon Buhrow (Code Review) 12 Dec '22

12 Dec '22

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Aarya. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65879 ) Change subject: flashrom.c:Add function to get a flattened view of the chip erase blocks ...................................................................... Patch Set 42: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/65879/comment/f3c68b70_e3fc56a1 PS42, Line 1249: index > doesn't `index` and `i` count in tandem? You can just use `i` and drop the `index`? I think `continue` makes `index` and `i` do not count in tandem. -- To view, visit https://review.coreboot.org/c/flashrom/+/65879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4 Gerrit-Change-Number: 65879 Gerrit-PatchSet: 42 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 12 Dec 2022 14:24:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Rename 'internal_delay()' to 'default_delay()'
by Felix Singer (Code Review) 12 Dec '22

12 Dec '22

Attention is currently required from: Edward O'Callaghan, Angel Pons. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68855 ) Change subject: tree/: Rename 'internal_delay()' to 'default_delay()' ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5e04adf16812ceb1480992c92bca25ed80f8897a Gerrit-Change-Number: 68855 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 12 Dec 2022 12:45:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Position heap alloc along side check in compare_range()
by Edward O'Callaghan (Code Review) 12 Dec '22

12 Dec '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69473 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashrom.c: Position heap alloc along side check in compare_range() ...................................................................... flashrom.c: Position heap alloc along side check in compare_range() Change-Id: I0386ac4c09a541cb9a659b2410ce49c3292ecc6e Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/69473 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Alexander Goncharov <chat(a)joursoir.net> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashrom.c 1 file changed, 17 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved Alexander Goncharov: Looks good to me, but someone else must approve diff --git a/flashrom.c b/flashrom.c index 3e75eb3..18c4864 100644 --- a/flashrom.c +++ b/flashrom.c @@ -471,15 +471,16 @@ static int check_erased_range(struct flashctx *flash, unsigned int start, unsigned int len) { int ret; - uint8_t *cmpbuf = malloc(len); const uint8_t erased_value = ERASED_VALUE(flash); + uint8_t *cmpbuf = malloc(len); if (!cmpbuf) { msg_gerr("Out of memory!\n"); return -1; } memset(cmpbuf, erased_value, len); ret = verify_range(flash, cmpbuf, start, len); + free(cmpbuf); return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/69473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0386ac4c09a541cb9a659b2410ce49c3292ecc6e Gerrit-Change-Number: 69473 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> 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-MessageType: merged

1 0

[S] Change in flashrom[master]: flashrom.c: Position heap alloc along side check in compare_range()
by Edward O'Callaghan (Code Review) 12 Dec '22

12 Dec '22

Attention is currently required from: Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69473 ) Change subject: flashrom.c: Position heap alloc along side check in compare_range() ...................................................................... Patch Set 2: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/69473/comment/5129234d_727bc037 PS1, Line 451: uint8_t *cmpbuf = malloc(len); > That's right, I sometimes run djgpp on my personal machine (we don't have it in corp). […] Closing. -- To view, visit https://review.coreboot.org/c/flashrom/+/69473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0386ac4c09a541cb9a659b2410ce49c3292ecc6e Gerrit-Change-Number: 69473 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 12 Dec 2022 12:39:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit