- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: flashrom.c: Reduce the prog global state machine
by Edward O'Callaghan (Code Review) 11 Dec '22

11 Dec '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70603 ) Change subject: flashrom.c: Reduce the prog global state machine ...................................................................... flashrom.c: Reduce the prog global state machine programmer_init() no longer needs to set a full ref to the programmer handle. A few cases of needing the programmer name and if it is a internal type are required however a 'const char *' and 'bool' type are much easier to reason about as well as mitigates going backwards with patches that attempt to use the 'programmer' state via the global. Change-Id: I93dfe09fe877acd32c4f22665921544ccd9323f6 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 25 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/70603/1 diff --git a/flashrom.c b/flashrom.c index 4f9bf7d..59533d2 100644 --- a/flashrom.c +++ b/flashrom.c @@ -36,7 +36,9 @@ const char flashrom_version[] = FLASHROM_VERSION; -static const struct programmer_entry *programmer = NULL; +// TODO: Remove global state. +static const char *g_programmer_name = NULL; +static bool g_programmer_type = false; /* * Programmers supporting multiple buses can have differing size limits on @@ -132,7 +134,10 @@ msg_perr("Invalid programmer specified!\n"); return -1; } - programmer = prog; + g_programmer_name = prog->name; +#if CONFIG_INTERNAL == 1 + g_programmer_type = prog == &programmer_internal; +#endif /* Initialize all programmer specific data. */ /* Default to unlimited decode sizes. */ max_rom_decode = (const struct decode_sizes) { @@ -996,7 +1001,7 @@ msg_cinfo("mapped at physical address 0x%0*" PRIxPTR ".\n", PRIxPTR_WIDTH, flash->physical_memory); else - msg_cinfo("on %s.\n", programmer->name); + msg_cinfo("on %s.\n", g_programmer_name); /* Flash registers may more likely not be mapped if the chip was forced. * Lock info may be stored in registers, so avoid lock info printing. */ @@ -1480,11 +1485,7 @@ static bool is_internal_programmer() { -#if CONFIG_INTERNAL == 1 - return programmer == &programmer_internal; -#else - return false; -#endif + return g_programmer_type; } static void nonfatal_help_message(void) -- 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-MessageType: newchange

1 0

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

11 Dec '22

Attention is currently required from: Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68855 to look at the new patch set (#4). 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> --- 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, 62 insertions(+), 46 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/68855/4 -- 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests/selfcheck.c: Fix on non-x86 machines
by Evan Benn (Code Review) 11 Dec '22

11 Dec '22

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70554 ) Change subject: tests/selfcheck.c: Fix on non-x86 machines ...................................................................... Patch Set 2: (1 comment) File tests/selfcheck.c: https://review.coreboot.org/c/flashrom/+/70554/comment/6b332367_ebb879fe PS2, Line 142: assert_true(board_matches_size > 1); Maybe it should be `> 0` unconditionally then? https://source.chromium.org/chromiumos/chromiumos/codesearch/+/HEAD:src/thi… I guess I added the check to assert that we aren't accessing invalid memory on the next line, I dont know why I used 1 instead of zero though. -- To view, visit https://review.coreboot.org/c/flashrom/+/70554 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f Gerrit-Change-Number: 70554 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 12 Dec 2022 00:02:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Replace NULL-case of programmer_delay() with internal_delay
by Edward O'Callaghan (Code Review) 11 Dec '22

11 Dec '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68434 ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tree/: Replace NULL-case of programmer_delay() with internal_delay ...................................................................... tree/: Replace NULL-case of programmer_delay() with internal_delay Replace `programmer_delay(NULL, [..])` calls with direct `internal_delay([..])` dispatches explicitly. Custom driver delays remain hooked as well as core flashrom logic. The NULL base case of 'programmer_delay()' then becomes a condition to validate for layering violations or invalid flash contexts. Change-Id: I1da230804d5e8f47a6e281feb66f381514dc6861 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68434 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M amd_imc.c M atavia.c M bitbang_spi.c M dediprog.c M dummyflasher.c M flashrom.c M ichspi.c M it87spi.c M nicintel_eeprom.c M pony_spi.c M raiden_debug_spi.c M wbsio_spi.c 12 files changed, 63 insertions(+), 32 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/amd_imc.c b/amd_imc.c index b11eca3..2ee8332 100644 --- a/amd_imc.c +++ b/amd_imc.c @@ -60,7 +60,7 @@ msg_pwarn("IMC MBOX: Timeout!\n"); return 1; } - programmer_delay(NULL, 1000); + internal_delay(1000); } return 0; } diff --git a/atavia.c b/atavia.c index 1eebc97..ebf2949 100644 --- a/atavia.c +++ b/atavia.c @@ -90,7 +90,7 @@ ready = true; break; } else { - programmer_delay(NULL, 1); + internal_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); - programmer_delay(NULL, 90); + internal_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 f7ded1f..71798dc 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); - programmer_delay(NULL, master->half_period); + internal_delay(master->half_period); ret <<= 1; ret |= bitbang_spi_set_sck_get_miso(master, 1, spi_data); - programmer_delay(NULL, master->half_period); + internal_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); - programmer_delay(NULL, master->half_period); + internal_delay(master->half_period); bitbang_spi_set_sck(master, 1, spi_data); - programmer_delay(NULL, master->half_period); + internal_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); - programmer_delay(NULL, master->half_period); + internal_delay(master->half_period); bitbang_spi_set_cs(master, 1, data->spi_data); - programmer_delay(NULL, master->half_period); + internal_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/dediprog.c b/dediprog.c index b4279d8..6c41f28 100644 --- a/dediprog.c +++ b/dediprog.c @@ -316,7 +316,7 @@ if (voltage_selector == 0) { /* Wait some time as the original driver does. */ - programmer_delay(NULL, 200 * 1000); + internal_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. */ - programmer_delay(NULL, 200 * 1000); + internal_delay(200 * 1000); } return 0; } diff --git a/dummyflasher.c b/dummyflasher.c index da4efc7..f194c10 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -897,7 +897,7 @@ msg_pspew(" 0x%02x", readarr[i]); msg_pspew("\n"); - programmer_delay(NULL, (writecnt + readcnt) * emu_data->delay_us); + internal_delay((writecnt + readcnt) * emu_data->delay_us); return 0; } diff --git a/flashrom.c b/flashrom.c index d0f0d2d..3e75eb3 100644 --- a/flashrom.c +++ b/flashrom.c @@ -259,8 +259,20 @@ if (usecs == 0) return; - if (!flash) + /** + * Drivers should either use internal_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 + * 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); + } if (flash->mst->buses_supported & BUS_SPI) { if (flash->mst->spi.delay) diff --git a/ichspi.c b/ichspi.c index 2b6c7ae..a6587c6 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) { - programmer_delay(NULL, 10); + internal_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) { - programmer_delay(NULL, 10); + internal_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) { - programmer_delay(NULL, 10); + internal_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) { - programmer_delay(NULL, 10); + internal_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) { - programmer_delay(NULL, 8); + internal_delay(8); } REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); if (!timeout_us) { diff --git a/it87spi.c b/it87spi.c index a022041..fc29e51 100644 --- a/it87spi.c +++ b/it87spi.c @@ -146,7 +146,7 @@ if((status & SPI_SR_WIP) == 0) return 0; - programmer_delay(NULL, 1000); + internal_delay(1000); } return 0; } diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index 4d5ab6b..6922fb7 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -213,7 +213,7 @@ eewr |= BIT(EEWR_CMDV); pci_mmio_writel(eewr, eebar + EEWR); - programmer_delay(NULL, 5); + internal_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); - programmer_delay(NULL, 1); + internal_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); - programmer_delay(NULL, 1); + internal_delay(1); /* data */ nicintel_ee_bitset(eebar, EEC, EE_CS, 0); @@ -394,7 +394,7 @@ break; } nicintel_ee_bitset(eebar, EEC, EE_CS, 1); - programmer_delay(NULL, 1); + internal_delay(1); if (nicintel_ee_ready(eebar)) goto out; } diff --git a/pony_spi.c b/pony_spi.c index fc27fc4..408fa12 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); - programmer_delay(NULL, 1000); + internal_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 517b173..2478133 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -892,7 +892,7 @@ /* Reattempting will not result in a recovery. */ return status; } - programmer_delay(NULL, RETRY_INTERVAL_US); + internal_delay(RETRY_INTERVAL_US); continue; } @@ -927,7 +927,7 @@ /* Reattempting will not result in a recovery. */ return status; } - programmer_delay(NULL, RETRY_INTERVAL_US); + internal_delay(RETRY_INTERVAL_US); } } @@ -962,7 +962,7 @@ " config attempt = %d\n" " status = 0x%05x\n", config_attempt + 1, status); - programmer_delay(NULL, RETRY_INTERVAL_US); + internal_delay(RETRY_INTERVAL_US); continue; } @@ -972,7 +972,7 @@ " config attempt = %d\n" " status = 0x%05x\n", config_attempt + 1, status); - programmer_delay(NULL, RETRY_INTERVAL_US); + internal_delay(RETRY_INTERVAL_US); continue; } @@ -1016,7 +1016,7 @@ config_attempt + 1, rsp_config.packet_v2.packet_id, rsp_config.packet_size); - programmer_delay(NULL, RETRY_INTERVAL_US); + internal_delay(RETRY_INTERVAL_US); } return USB_SPI_HOST_INIT_FAILURE; } @@ -1240,7 +1240,7 @@ /* Reattempting will not result in a recovery. */ return status; } - programmer_delay(NULL, RETRY_INTERVAL_US); + internal_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); - programmer_delay(NULL, RETRY_INTERVAL_US); + internal_delay(RETRY_INTERVAL_US); } } } diff --git a/wbsio_spi.c b/wbsio_spi.c index 1aa8729..b643eb4 100644 --- a/wbsio_spi.c +++ b/wbsio_spi.c @@ -155,7 +155,7 @@ OUTB(writearr[0], data->spibase); OUTB(mode, data->spibase + 1); - programmer_delay(NULL, 10); + internal_delay(10); if (!readcnt) return 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/68434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1da230804d5e8f47a6e281feb66f381514dc6861 Gerrit-Change-Number: 68434 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

[M] Change in flashrom[master]: tree/: Move programmer_delay() out of programmer state machine
by Edward O'Callaghan (Code Review) 11 Dec '22

11 Dec '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67393 ) Change subject: tree/: Move programmer_delay() out of programmer state machine ...................................................................... tree/: Move programmer_delay() out of programmer state machine Handle the special cases of both serprog and ch341a_spi. Also rewrite programmer_delay() to handle the two base cases of zero time and no valid flashctx yet before handling per master branching. Additionally, modify the custom delay function pointer signature to allow closure over the flashctx. This allows driver specific delay implementations to recover programmer specific opaque data within their delay implementations. Therefore programmer specific delay functions can avoid programmer specific globals. Change-Id: Id059abb58b31a066a408009073912da2b224d40c Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67393 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ch341a_spi.c M flashrom.c M include/programmer.h M serprog.c 4 files changed, 68 insertions(+), 31 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/ch341a_spi.c b/ch341a_spi.c index 294a6a6..2dc983f 100644 --- a/ch341a_spi.c +++ b/ch341a_spi.c @@ -322,7 +322,7 @@ *ptr++ = CH341A_CMD_UIO_STM_END; } -static void ch341a_spi_delay(unsigned int usecs) +static void ch341a_spi_delay(const struct flashctx *flash, unsigned int usecs) { /* 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 @@ -419,6 +419,7 @@ .write_aai = default_spi_write_aai, .shutdown = ch341a_spi_shutdown, .probe_opcode = default_spi_probe_opcode, + .delay = ch341a_spi_delay, }; static int ch341a_spi_init(const struct programmer_cfg *cfg) @@ -525,5 +526,4 @@ .type = USB, .devs.dev = devs_ch341a_spi, .init = ch341a_spi_init, - .delay = ch341a_spi_delay, }; diff --git a/flashrom.c b/flashrom.c index 9c22eb6..d0f0d2d 100644 --- a/flashrom.c +++ b/flashrom.c @@ -256,12 +256,21 @@ void programmer_delay(const struct flashctx *flash, unsigned int usecs) { - if (usecs > 0) { - if (programmer->delay) - programmer->delay(usecs); - else - internal_delay(usecs); + if (usecs == 0) + return; + + if (!flash) + return internal_delay(usecs); + + if (flash->mst->buses_supported & BUS_SPI) { + if (flash->mst->spi.delay) + return flash->mst->spi.delay(flash, usecs); + } else if (flash->mst->buses_supported & BUS_PARALLEL) { + if (flash->mst->par.delay) + return flash->mst->par.delay(flash, usecs); } + + return internal_delay(usecs); } int read_memmapped(struct flashctx *flash, uint8_t *buf, unsigned int start, diff --git a/include/programmer.h b/include/programmer.h index 7bcef02..8ba5781 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -49,8 +49,6 @@ } devs; int (*init) (const struct programmer_cfg *cfg); - - void (*delay) (unsigned int usecs); }; extern const struct programmer_entry *const programmer_table[]; @@ -314,6 +312,7 @@ int (*write_aai)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*shutdown)(void *data); bool (*probe_opcode)(const struct flashctx *flash, uint8_t opcode); + void (*delay) (const struct flashctx *flash, unsigned int usecs); void *data; }; @@ -435,6 +434,7 @@ uint32_t (*chip_readl) (const struct flashctx *flash, const chipaddr addr); void (*chip_readn) (const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len); int (*shutdown)(void *data); + void (*delay) (const struct flashctx *flash, unsigned int usecs); void *data; }; int register_par_master(const struct par_master *mst, const enum chipbustype buses, void *data); diff --git a/serprog.c b/serprog.c index a3a3db3..a7796ba 100644 --- a/serprog.c +++ b/serprog.c @@ -453,6 +453,8 @@ return NULL; } +static void serprog_delay(const struct flashctx *flash, unsigned int usecs); + static struct spi_master spi_master_serprog = { .map_flash_region = serprog_map, .features = SPI_MASTER_4BA, @@ -464,6 +466,7 @@ .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, .probe_opcode = default_spi_probe_opcode, + .delay = serprog_delay, }; static int sp_check_opbuf_usage(int bytes_to_be_added) @@ -568,6 +571,27 @@ sp_do_read_n(&(buf[addrm-addr]), addrm, lenm); // FIXME: return error } +static void serprog_delay(const struct flashctx *flash, unsigned int usecs) +{ + unsigned char buf[4]; + 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); + return; + } + if ((sp_max_write_n) && (sp_write_n_bytes)) + sp_pass_writen(); + sp_check_opbuf_usage(5); + buf[0] = ((usecs >> 0) & 0xFF); + buf[1] = ((usecs >> 8) & 0xFF); + buf[2] = ((usecs >> 16) & 0xFF); + buf[3] = ((usecs >> 24) & 0xFF); + sp_stream_buffer_op(S_CMD_O_DELAY, 4, buf); + sp_opbuf_usage += 5; + sp_prev_was_write = 0; +} + static const struct par_master par_master_serprog = { .map_flash_region = serprog_map, .chip_readb = serprog_chip_readb, @@ -578,6 +602,7 @@ .chip_writew = fallback_chip_writew, .chip_writel = fallback_chip_writel, .chip_writen = fallback_chip_writen, + .delay = serprog_delay, }; static enum chipbustype serprog_buses_supported = BUS_NONE; @@ -941,32 +966,10 @@ return 1; } -static void serprog_delay(unsigned int usecs) -{ - unsigned char buf[4]; - 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); - return; - } - if ((sp_max_write_n) && (sp_write_n_bytes)) - sp_pass_writen(); - sp_check_opbuf_usage(5); - buf[0] = ((usecs >> 0) & 0xFF); - buf[1] = ((usecs >> 8) & 0xFF); - buf[2] = ((usecs >> 16) & 0xFF); - buf[3] = ((usecs >> 24) & 0xFF); - sp_stream_buffer_op(S_CMD_O_DELAY, 4, buf); - sp_opbuf_usage += 5; - sp_prev_was_write = 0; -} - const struct programmer_entry programmer_serprog = { .name = "serprog", .type = OTHER, /* FIXME */ .devs.note = "All programmer devices speaking the serprog protocol\n", .init = serprog_init, - .delay = serprog_delay, }; -- To view, visit https://review.coreboot.org/c/flashrom/+/67393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c Gerrit-Change-Number: 67393 Gerrit-PatchSet: 11 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: 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-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: tests/selfcheck.c: Fix on non-x86 machines
by Edward O'Callaghan (Code Review) 11 Dec '22

11 Dec '22

Attention is currently required from: Angel Pons, Anastasia Klimchuk, Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70554 ) Change subject: tests/selfcheck.c: Fix on non-x86 machines ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > ``` […] sorted. -- To view, visit https://review.coreboot.org/c/flashrom/+/70554 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f Gerrit-Change-Number: 70554 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Sun, 11 Dec 2022 23:57:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests/selfcheck.c: Fix on non-x86 machines
by Edward O'Callaghan (Code Review) 11 Dec '22

11 Dec '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Evan Benn. Hello build bot (Jenkins), Anastasia Klimchuk, Evan Benn, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70554 to look at the new patch set (#2). Change subject: tests/selfcheck.c: Fix on non-x86 machines ...................................................................... tests/selfcheck.c: Fix on non-x86 machines The global const of `board_matches_size` has value `1` on non-x86 machines. Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/selfcheck.c 1 file changed, 17 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/70554/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70554 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f Gerrit-Change-Number: 70554 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[1.3.x]: layout: Check return values for strdup in register_include_arg
by Anastasia Klimchuk (Code Review) 11 Dec '22

11 Dec '22

Attention is currently required from: David Hendricks, Angel Pons, Evan Benn. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70322 ) Change subject: layout: Check return values for strdup in register_include_arg ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70322 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: I6c22196be6847a8c9704f1de936604a51b4b8a28 Gerrit-Change-Number: 70322 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Sun, 11 Dec 2022 23:29:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[1.3.x]: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence
by Anastasia Klimchuk (Code Review) 11 Dec '22

11 Dec '22

Attention is currently required from: David Hendricks, Angel Pons, Evan Benn. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70323 ) Change subject: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70323 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: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Gerrit-Change-Number: 70323 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Sun, 11 Dec 2022 23:21:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[1.3.x]: cbtables.c/search_lb_records: Drop unused variable `count`
by Anastasia Klimchuk (Code Review) 11 Dec '22

11 Dec '22

Attention is currently required from: David Hendricks, Angel Pons, Evan Benn. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70324 ) Change subject: cbtables.c/search_lb_records: Drop unused variable `count` ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70324 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: I340208f513bed57a9cc2bba880a2400352c5cc42 Gerrit-Change-Number: 70324 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Sun, 11 Dec 2022 23:17:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit