April 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: [WIP] meson: rework the programmer selection
by Thomas Heijligen (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63724 ) Change subject: [WIP] meson: rework the programmer selection ...................................................................... Patch Set 7: (3 comments) File meson.build: https://review.coreboot.org/c/flashrom/+/63724/comment/e4982333_0e97ac5a PS7, Line 212: 'internal' : { : 'systems' : systems_raw_hwaccess + ['linux'], : 'cpu_families' : (host_machine.system() == 'linux' ? [host_machine.cpu_family()] : ['x86', 'x86_64']), : 'deps' : [ libpci ], : 'groups' : [ ], : 'srcs' : (host_machine.cpu_family() in ['x86', 'x86_64'] ? files( : 'processor_enable.c', : 'chipset_enable.c', : 'board_enable.c', : 'cbtable.c', : 'internal.c', : 'it87spi.c', : 'it85spi.c', : 'sb600spi.c', : 'amd_imc.c', : 'wbsio_spi.c', : 'mcp6x_spi.c', : 'ichspi.c', : 'dmi.c', : 'pcidev.c', : ) : files( : 'board_enable.c', : 'cbtable.c', : 'chipset_enable.c', : 'internal.c', : 'processor_enable.c', : 'pcidev.c', : )), : 'flags' : [ : '-DCONFIG_INTERNAL=1', : '-DCONFIG_INTERNAL_DMI=' + (get_option('use_internal_dmi') ? '1' : '0'), : ] : }, > I remember there was a conversation about internal, that it needs a refactoring to make the array id […] This should work. I only discovered this possibility while working on the code. I still prefer to split the internal programmer. But now it is not important any longer for the build system change. https://review.coreboot.org/c/flashrom/+/63724/comment/a7a79a9d_2ac35f10 PS7, Line 484: srcs += 'bitbang_spi.c' > oh... […] No, it just was guarded with `need_bitbang_spi`. I've just put it to the common code. File meson_options.txt: https://review.coreboot.org/c/flashrom/+/63724/comment/635985df_3b6e7173 PS7, Line 6: choices > Just checking, this is an array, so it allows having combinations like […] yes, the choices ensure at least one programmer is enabled, there can't be a string which is no programmer, so typos will get detected here, you can select one and more programmer. E.g. `-Dprogrammer=auto,ch341a_spi` will select all available programmer, but forces to compile the ch341a_spi or fail. -- To view, visit https://review.coreboot.org/c/flashrom/+/63724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib44b26e3748fc71f116184082b4aed0bb208b4c1 Gerrit-Change-Number: 63724 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 27 Apr 2022 07:50:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Unify timeouts across all SPI operations to 30s
by Anastasia Klimchuk (Code Review) 27 Apr '22

27 Apr '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/62867 ) Change subject: ichspi: Unify timeouts across all SPI operations to 30s ...................................................................... ichspi: Unify timeouts across all SPI operations to 30s `ich_hwseq_wait_for_cycle_complete()` drops taking `timeout` as argument in favor of a fixed timeout of `30 seconds` for any given SPI operation as recommended by the SPI programming guide. Document: Alder Lake-P Client Platform SPI Programming Guide Rev 1.30 (supporting document for multi-master accessing the SPI Flash device.) Refer to below section to understand the problem in more detail and SPI operation timeout recommendation from Intel in multi-master scenarios. On Intel Chipsets that support multi-mastering access of the SPI flash may run into a timeout failure when the operation initiated from a single master just follows the SPI operational timeout recommendation as per the vendor datasheet (example: winbond spiflash W25Q256JV-DTR specification, table 9.7). In the multi-master SPI accessing scenario using hardware sequencing operation, it's impossible to know the actual status of the SPI bus prior to individual master starting the operation (SPI Cycle In Progress a.k.a SCIP bit represents the status of SPI operation on individual master). Thus, any SPI operation triggered in multi-master environment might need to account a worst case scenario where the most time consuming operation might have occupied the SPI bus from a master and an operation initiated by another master just timed out. Here is the timeout calculation for any hardware sequencing operation: Worst Case Operational Delay = (Maximum Time consumed by a SPI operation + Any marginal adjustment) Timeout Recommendation for Hardware Sequencing Operation = ((Worst Case Operational Delay) * (#No. Of SPI Master - 1) + Current Operational latency) Assume, on Intel platform with 6 SPI master like, Host CPU, CSE, EC, GbE and other reserved etc, hence, the Timeout Calculation for SPI erase Operation would look like as below: Maximum Time consumed by a SPI Operation = 5 seconds Worst Case Operational Delay = 5 seconds Timeout Recommendation for Hardware Seq Operation = 5 seconds * (6 - 1) + 5 seconds = 30 seconds BUG=b:223630977 TEST=Able to perform read/write/erase operation on PCH 600 series chipset (board name: Brya). Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b Reviewed-on: https://review.coreboot.org/c/flashrom/+/62867 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M ichspi.c 1 file changed, 12 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index c25303e..5d4d4d7 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1275,21 +1275,24 @@ Resets all error flags in HSFS. Returns 0 if the cycle completes successfully without errors within timeout us, 1 on errors. */ -static int ich_hwseq_wait_for_cycle_complete(unsigned int timeout, - unsigned int len, - enum ich_chipset ich_gen) +static int ich_hwseq_wait_for_cycle_complete(unsigned int len, enum ich_chipset ich_gen) { + /* + * The SPI bus may be busy due to performing operations from other masters, hence + * introduce the long timeout of 30s to cover the worst case scenarios as well. + */ + unsigned int timeout_us = 30 * 1000 * 1000; uint16_t hsfs; uint32_t addr; - timeout /= 8; /* scale timeout duration to counter */ + timeout_us /= 8; /* scale timeout duration to counter */ while ((((hsfs = REGREAD16(ICH9_REG_HSFS)) & (HSFS_FDONE | HSFS_FCERR)) == 0) && - --timeout) { + --timeout_us) { programmer_delay(8); } REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); - if (!timeout) { + if (!timeout_us) { addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask; msg_perr("Timeout error between offset 0x%08x and " "0x%08x (= 0x%08x + %d)!\n", @@ -1371,7 +1374,6 @@ { uint32_t erase_block; uint16_t hsfc; - uint32_t timeout = 5000 * 1000; /* 5 s for max 64 kB */ erase_block = ich_hwseq_get_erase_block_size(addr); if (len != erase_block) { @@ -1415,7 +1417,7 @@ prettyprint_ich9_reg_hsfc(hsfc, ich_generation); REGWRITE16(ICH9_REG_HSFC, hsfc); - if (ich_hwseq_wait_for_cycle_complete(timeout, len, ich_generation)) + if (ich_hwseq_wait_for_cycle_complete(len, ich_generation)) return -1; return 0; } @@ -1424,7 +1426,6 @@ unsigned int addr, unsigned int len) { uint16_t hsfc; - uint16_t timeout = 100 * 60; uint8_t block_len; if (addr + len > flash->chip->total_size * 1024) { @@ -1458,7 +1459,7 @@ hsfc |= HSFC_FGO; /* start */ REGWRITE16(ICH9_REG_HSFC, hsfc); - if (ich_hwseq_wait_for_cycle_complete(timeout, block_len, ich_generation)) + if (ich_hwseq_wait_for_cycle_complete(block_len, ich_generation)) return 1; ich_read_data(buf, block_len, ICH9_REG_FDATA0); addr += block_len; @@ -1471,7 +1472,6 @@ static int ich_hwseq_write(struct flashctx *flash, const uint8_t *buf, unsigned int addr, unsigned int len) { uint16_t hsfc; - uint16_t timeout = 100 * 60; uint8_t block_len; if (addr + len > flash->chip->total_size * 1024) { @@ -1506,7 +1506,7 @@ hsfc |= HSFC_FGO; /* start */ REGWRITE16(ICH9_REG_HSFC, hsfc); - if (ich_hwseq_wait_for_cycle_complete(timeout, block_len, ich_generation)) + if (ich_hwseq_wait_for_cycle_complete(block_len, ich_generation)) return -1; addr += block_len; buf += block_len; -- To view, visit https://review.coreboot.org/c/flashrom/+/62867 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b Gerrit-Change-Number: 62867 Gerrit-PatchSet: 17 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: meson: outsource platform specific code to `platform/meson.build`
by Thomas Heijligen (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Anastasia Klimchuk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63826 ) Change subject: meson: outsource platform specific code to `platform/meson.build` ...................................................................... Patch Set 2: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/63826/comment/ec1acbb2_0153a10a PS2, Line 80: host_is_x86 = ['x86', 'x86_64'].contains(host_machine.cpu_family()) > I have a overall question about the idea: what is counted as "platform specific code" ? If in future […] The term "outsource platform specific code" was maybe a bit to much for this commit message. I've only outsourced the build instructions for the code in `platform/`. With a full realization of the `platform/` directory the statements left in the top level `meson.build` should be vanish. I've created the `platform/` directory to abstract functionality on different platforms. E.g. the different endiannesses. The code in `platform/` should not interact with code outside from this directory to have a clear separation between platform abstraction and common flashrom code. Good candidates to put there where the `mmio_(read|write)` functins, but without the `register_shutdown` variants. Also the `hwaccess_x86_*` functions in a slightly different way. The support for serial devices on unix and windows is currently realized with macros. This can be put in `platform/serial_unix.c`, `platform_serial_windows.c` and some helper file outside of `platform/` for code just shared between the programmers. We should document where to put what but what when starting to create directories. I haven't found a good place for it yet. -- To view, visit https://review.coreboot.org/c/flashrom/+/63826 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I88044a3f903f316138483dd872a6d95f8686ae69 Gerrit-Change-Number: 63826 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 27 Apr 2022 07:31:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom: initialize restore func count in correct place
by Nikolai Artemiev (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Edward O'Callaghan, Angel Pons. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63881 ) Change subject: flashrom: initialize restore func count in correct place ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/63881/comment/f97323f3_946a4ae8 PS1, Line 12: Previously the count was reset after calling chip->unlock(), causing the : restore handler that is registered by spi_disable_blockprotect_generic() : to be lost. > The following maybe a little more clear: […] Done File flashrom.c: https://review.coreboot.org/c/flashrom/+/63881/comment/279699ed_5620df97 PS1, Line 1865: flash->chip_restore_fn_count = 0; > Maybe add a short comment here? […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/63881 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf Gerrit-Change-Number: 63881 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 27 Apr 2022 04:45:54 +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]: flashrom: initialize restore func count in correct place
by Nikolai Artemiev (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/63881 to look at the new patch set (#3). Change subject: flashrom: initialize restore func count in correct place ...................................................................... flashrom: initialize restore func count in correct place Set `flash->chip_restore_fn_count` to zero before calling the chip's unlock funciton in `prepare_flash_access()`. Previously `flash->chip_restore_fn_count` was uninitialized before calling `chip->unlock()` and subsequently reset after the dispatch by initializing it. This caused the restore handler that is registered within `spi_disable_blockprotect_generic()` to be lost. BUG=b:228945411 BRANCH=none TEST=enable wp; flashrom -w; check wp still enabled. Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/63881/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/63881 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf Gerrit-Change-Number: 63881 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom: Drop read_flash_to_file() usage
by Edward O'Callaghan (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Daniel Campello, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59291 to look at the new patch set (#14). Change subject: flashrom: Drop read_flash_to_file() usage ...................................................................... flashrom: Drop read_flash_to_file() usage Aspire towards a goal of making cli_classic more of just a user of libflashrom than having quasi-parallel paths in flashrom.c This converts remaining read_flash_to_file() usage to the do_read() provider wrapper around libflashrom. BUG=b:208132085 TEST=` sudo ./flashrom -p ft2232_spi:type=232H,divisor=1000 -f -r out -c W25X05 Flashrom output: No EEPROM/flash device found. Force read (-f -r -c) requested, pretending the chip is there: Assuming Winbond flash chip "W25X05" (64 kB, SPI) on ft2232_spi. Please note that forced reads most likely contain garbage. Block protection could not be disabled! Reading flash... done. Data read: xxd out-1khz 00000000: 0000 07ff ffff e000 0000 7fff fffe 0000 ................ 00000010: 0007 ffff ffe0 0000 007f ffff fe00 0000 ................ 00000020: 07ff ffff e000 0000 7fff fffe 0000 0007 ................ 00000030: ffff ffe0 0000 007f ffff fe00 0000 0fff ................ xxd out-100khz 00000000: b6db 6db6 db6d b6db 6db6 db6d b6db 6db6 ..m..m..m..m..m. 00000010: db6d b6db 6db6 db6d b6db 6db6 db6d b6db .m..m..m..m..m.. 00000020: 6db6 db6d b6db 6db6 db24 9249 2492 4924 m..m..m..$.I$.I$ 00000030: 9249 2492 4924 9249 2492 4924 9249 2492 .I$.I$.I$.I$.I$. ` Change-Id: I4b690b688acf9d5deb46e8642a252a2132ea8c73 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Tested-by: Nikolai Artemiev <nartemiev(a)google.com> --- M cli_classic.c M flash.h M flashrom.c 3 files changed, 2 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/59291/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/59291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4b690b688acf9d5deb46e8642a252a2132ea8c73 Gerrit-Change-Number: 59291 Gerrit-PatchSet: 14 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: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom: Drop read_flash_to_file() usage
by Nikolai Artemiev (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59291 ) Change subject: flashrom: Drop read_flash_to_file() usage ...................................................................... Patch Set 13: Code-Review+1 (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/59291/comment/acabc496_810a2cb1 PS4, Line 635: if (map_flash(&flashes[0]) != 0) { : free(flashes[0].chip); : ret = 1; : goto out_shutdown; : } : msg_cinfo("Please note that forced reads most likely contain garbage.\n"); : ret = do_read(&flashes[0], filename); : unmap_flash(&flashes[0]); > 100% makes sense, I put the easy part out into https://review.coreboot. […] Tested with a FT232H adapter: Command: ``` sudo ./flashrom -p ft2232_spi:type=232H,divisor=1000 -f -r out -c W25X05 ``` Flashrom output: ``` No EEPROM/flash device found. Force read (-f -r -c) requested, pretending the chip is there: Assuming Winbond flash chip "W25X05" (64 kB, SPI) on ft2232_spi. Please note that forced reads most likely contain garbage. Block protection could not be disabled! Reading flash... done. ``` Data read: ``` xxd out-1khz 00000000: 0000 07ff ffff e000 0000 7fff fffe 0000 ................ 00000010: 0007 ffff ffe0 0000 007f ffff fe00 0000 ................ 00000020: 07ff ffff e000 0000 7fff fffe 0000 0007 ................ 00000030: ffff ffe0 0000 007f ffff fe00 0000 0fff ................ ``` ``` xxd out-100khz 00000000: b6db 6db6 db6d b6db 6db6 db6d b6db 6db6 ..m..m..m..m..m. 00000010: db6d b6db 6db6 db6d b6db 6db6 db6d b6db .m..m..m..m..m.. 00000020: 6db6 db6d b6db 6db6 db24 9249 2492 4924 m..m..m..$.I$.I$ 00000030: 9249 2492 4924 9249 2492 4924 9249 2492 .I$.I$.I$.I$.I$. ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/59291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4b690b688acf9d5deb46e8642a252a2132ea8c73 Gerrit-Change-Number: 59291 Gerrit-PatchSet: 13 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: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 27 Apr 2022 04:16:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [WIP] meson: rework the programmer selection
by Anastasia Klimchuk (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63724 ) Change subject: [WIP] meson: rework the programmer selection ...................................................................... Patch Set 7: (9 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/63724/comment/efb69e21_ba157f39 PS7, Line 12: get mandetory get mandetory -> gets mandatory https://review.coreboot.org/c/flashrom/+/63724/comment/d6346d9f_2a2cce87 PS7, Line 20: get's selectet get's selectet -> gets selected https://review.coreboot.org/c/flashrom/+/63724/comment/fb9a8825_fbf78a18 PS7, Line 21: determin determin -> determine Patchset: PS7: Thomas this is really really awesome! File meson.build: https://review.coreboot.org/c/flashrom/+/63724/comment/c0749f20_8c4045f2 PS7, Line 109: get edded does it mean "gets added"? https://review.coreboot.org/c/flashrom/+/63724/comment/682f5938_b03d23fb PS7, Line 212: 'internal' : { : 'systems' : systems_raw_hwaccess + ['linux'], : 'cpu_families' : (host_machine.system() == 'linux' ? [host_machine.cpu_family()] : ['x86', 'x86_64']), : 'deps' : [ libpci ], : 'groups' : [ ], : 'srcs' : (host_machine.cpu_family() in ['x86', 'x86_64'] ? files( : 'processor_enable.c', : 'chipset_enable.c', : 'board_enable.c', : 'cbtable.c', : 'internal.c', : 'it87spi.c', : 'it85spi.c', : 'sb600spi.c', : 'amd_imc.c', : 'wbsio_spi.c', : 'mcp6x_spi.c', : 'ichspi.c', : 'dmi.c', : 'pcidev.c', : ) : files( : 'board_enable.c', : 'cbtable.c', : 'chipset_enable.c', : 'internal.c', : 'processor_enable.c', : 'pcidev.c', : )), : 'flags' : [ : '-DCONFIG_INTERNAL=1', : '-DCONFIG_INTERNAL_DMI=' + (get_option('use_internal_dmi') ? '1' : '0'), : ] : }, I remember there was a conversation about internal, that it needs a refactoring to make the array idea work. Is it still true? https://review.coreboot.org/c/flashrom/+/63724/comment/63f8f9d1_5b949a76 PS7, Line 484: srcs += 'bitbang_spi.c' oh... was it missing all the time? File meson_options.txt: https://review.coreboot.org/c/flashrom/+/63724/comment/47b1d178_18e47212 PS7, Line 6: choices Just checking, this is an array, so it allows having combinations like all+auto+programmer1+programmer2+... Is there any concerns? The code looks like it would work fine with any combination, which is great! https://review.coreboot.org/c/flashrom/+/63724/comment/bb1e843a_f8ba063a PS7, Line 9: 'atahpt', 'atapromise', 'buspirate_spi', 'ch341a_spi', 'dediprog', 'developerbox_spi', : 'digilent_spi', 'jlink_spi', 'drkaiser', 'dummy', 'ft2232_spi', 'gfxnvidia', 'raiden_debug_spi', : 'internal', 'it8212', 'linux_mtd', 'linux_spi', 'mstarddc_spi', 'nic3com', 'nicintel_eeprom', : 'nicintel', 'nicintel_spi', 'nicnatsemi', 'nicrealtek', 'ogp_spi', 'pickit2_spi', 'pony_spi', : 'rayer_spi', 'satamv', 'satasii', 'serprog', 'usbblaster_spi', 'stlinkv3_spi', : 'lspcon_i2c_spi', 'mediatek_i2c_spi', 'realtek_mst_i2c_spi', For the list of individual programmers, do you think maybe we can order them alphabetically? It wasn't ordered before, but I don't know why. -- To view, visit https://review.coreboot.org/c/flashrom/+/63724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib44b26e3748fc71f116184082b4aed0bb208b4c1 Gerrit-Change-Number: 63724 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 27 Apr 2022 03:21:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom: initialize restore func count in correct place
by Edward O'Callaghan (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63881 ) Change subject: flashrom: initialize restore func count in correct place ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/63881/comment/8091050a_053ef57a PS1, Line 12: Previously the count was reset after calling chip->unlock(), causing the : restore handler that is registered by spi_disable_blockprotect_generic() : to be lost. The following maybe a little more clear: ``` Previously `flash->chip_restore_fn_count` was uninitialised before calling `chip->unlock()` and subsequently reset after the dispatch by initialising it. This caused the restore handler that is registered within `spi_disable_blockprotect_generic()` to be lost. ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/63881 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c7df424bd2ae2b5fb2a2ab6b47a3c9ff3233acf Gerrit-Change-Number: 63881 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 27 Apr 2022 01:32:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: Add optimisation level s and disable debug in meson build
by Anastasia Klimchuk (Code Review) 27 Apr '22

27 Apr '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63606 ) Change subject: meson: Add optimisation level s and disable debug in meson build ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > I think this approach is fine. […] Great thanks! The last thing left, is for someone to approve :) -- To view, visit https://review.coreboot.org/c/flashrom/+/63606 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4ca96a866529cac320e66516ef280d5100ceefab Gerrit-Change-Number: 63606 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 27 Apr 2022 01:18:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2022