February 2022 - flashrom-gerrit

Change in flashrom[master]: pcidev: Move pci_get_dev() logic into canonical place
by Edward O'Callaghan (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Subrata Banik, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59276 ) Change subject: pcidev: Move pci_get_dev() logic into canonical place ...................................................................... Patch Set 5: (1 comment) File board_enable.c: https://review.coreboot.org/c/flashrom/+/59276/comment/15c01ce1_0f5b61b9 PS4, Line 1097: pcidev_getdev1 > Thanks Subrata, I had a similar though to use the signature `struct pci_dev *pcidev_getdevfn(struct […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/59276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id9ce055d5e5d347520ec5002b8c6548e60eaa0a7 Gerrit-Change-Number: 59276 Gerrit-PatchSet: 5 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 28 Feb 2022 23:38:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> 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

Change in flashrom[master]: pcidev: Move pci_get_dev() logic into canonical place
by Edward O'Callaghan (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Subrata Banik, Angel Pons. Hello build bot (Jenkins), Nico Huber, Subrata Banik, Angel Pons, Anastasia Klimchuk, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59276 to look at the new patch set (#5). Change subject: pcidev: Move pci_get_dev() logic into canonical place ...................................................................... pcidev: Move pci_get_dev() logic into canonical place BUG=b:220950271 TEST=```sudo ./flashrom -p internal --flash-size <snip> Found Programmer flash chip "Opaque flash chip" (16384 kB, Programmer-specific) mapped at physical address 0x0000000000000000. 16777216 ``` Change-Id: Id9ce055d5e5d347520ec5002b8c6548e60eaa0a7 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M pcidev.c M programmer.h 3 files changed, 15 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/59276/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/59276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id9ce055d5e5d347520ec5002b8c6548e60eaa0a7 Gerrit-Change-Number: 59276 Gerrit-PatchSet: 5 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: pcidev: Move scandev_inclass logic from internal to pcidev
by Edward O'Callaghan (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine. Hello build bot (Jenkins), Angel Pons, Anastasia Klimchuk, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59277 to look at the new patch set (#7). Change subject: pcidev: Move scandev_inclass logic from internal to pcidev ...................................................................... pcidev: Move scandev_inclass logic from internal to pcidev BUG=b:220950271 TEST=```sudo ./flashrom -p internal -r /tmp/bios <snip> Found Programmer flash chip "Opaque flash chip" (16384 kB, Programmer-specific) mapped at physical address 0x0000000000000000. Reading flash... done. ``` Change-Id: I1978e178fb73485f1c5c7e732853522847267cee Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M chipset_enable.c M internal.c M mcp6x_spi.c M pcidev.c M programmer.h 6 files changed, 26 insertions(+), 26 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/59277/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/59277 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1978e178fb73485f1c5c7e732853522847267cee Gerrit-Change-Number: 59277 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: Peter Marheine <pmarheine(a)chromium.org> 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: pcidev: Move pci_get_dev() logic into canonical place
by Edward O'Callaghan (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Subrata Banik, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59276 ) Change subject: pcidev: Move pci_get_dev() logic into canonical place ...................................................................... Patch Set 4: (1 comment) File board_enable.c: https://review.coreboot.org/c/flashrom/+/59276/comment/cb7754d6_d2e67956 PS4, Line 1097: pcidev_getdev1 > This was discussed before, see https://review.coreboot.org/c/flashrom/+/59276/3..4/board_enable. […] Thanks Subrata, I had a similar though to use the signature `struct pci_dev *pcidev_getdevfn(struct pci_dev *dev, const int func)` that's a good name! Thank you for the detailed suggestion. While there is only one call site but I think this is a good compromise to get this patch moving again. -- To view, visit https://review.coreboot.org/c/flashrom/+/59276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id9ce055d5e5d347520ec5002b8c6548e60eaa0a7 Gerrit-Change-Number: 59276 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 28 Feb 2022 23:29:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq
by Subrata Banik (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Patrick Georgi, Rizwan Qureshi, Stefan Reinauer, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Nico Huber, Martin Roth - Personal, Caveh Jalali, David Hendricks, Tim Wawrzynczak, Nick Vaccaro, Boris Mittelberg. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61854 ) Change subject: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq ...................................................................... Patch Set 12: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/61854/comment/cad602cb_90184d3c PS7, Line 17: Without this synchronisation being implemented, flashrom is running : into below error: : : Erasing and writing flash chip... Timeout error between offset : 0x0061c000 and 0x0061c03f (= 0x0061c000 + 63)! FAILED! : Uh oh. Erase/write failed. Checking if anything has changed. > > Please let me know if anything we need to help for moving this CL? > > What was said in the first message of the comment thread. I have reattempted to update the commit msg with setup details and replication steps. Please suggest if you need better wordings. https://review.coreboot.org/c/flashrom/+/61854/comment/6d032c44_38e54812 PS7, Line 25: TEST=Concurrent flashrom access is not throwing timeout. > > Please let me know if anything we need to help for moving this CL? > > What was said in the first message of the comment thread. I'm able to verify this change on eve device. Updated the same in the commit msg. File ichspi.c: https://review.coreboot.org/c/flashrom/+/61854/comment/8fa304e8_c069934d PS12, Line 1412: } > Please move out of the read-modify-write. […] Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/61854 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484 Gerrit-Change-Number: 61854 Gerrit-PatchSet: 12 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Alex Levin <levinale(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Martin Roth - Personal <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Alex Levin <levinale(a)chromium.org> Gerrit-Attention: YH Lin <yueherngl(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth - Personal <martinroth(a)google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-Comment-Date: Mon, 28 Feb 2022 14:02:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq
by Subrata Banik (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Patrick Georgi, Rizwan Qureshi, Stefan Reinauer, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Martin Roth - Personal, Subrata Banik, Caveh Jalali, David Hendricks, Tim Wawrzynczak, Nick Vaccaro, Boris Mittelberg. Hello build bot (Jenkins), Patrick Georgi, Stefan Reinauer, Rizwan Qureshi, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Nico Huber, Martin Roth - Personal, Caveh Jalali, Tim Wawrzynczak, Edward O'Callaghan, Nick Vaccaro, Boris Mittelberg, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61854 to look at the new patch set (#13). Change subject: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq ...................................................................... ichspi.c: Check SPI Cycle In-Progress prior start HW Seq As per EDS, SPI controller sets the HSFSTS.bit5 (SCIP) when software sets the Flash Cycle Go (FGO) bit in the Hardware Sequencing Flash Control register. This bit remains set until the cycle completes on the SPI interface. Hardware automatically sets and clears this bit. Software must initiate the next SPI transaction when this bit is 0. Platform Setup: Alder Lake based ChromeOS devices (Brya variants) Replication Steps: Accepting and running firmware Auto Update (AU) on the Brya variants (dogfooder system) is seeing `flashrom` getting timed out. Problem Statement: Evidencing AU (Auto Update) failure while performing firmware update on the Alder Lake based ChromeOS devices. Observation: Based on the initial understanding from the failure log/pattern, it seems like the platform is evidencing multiple `flashrom` access from different source, for example: `futility` accesses flashrom for erase, write and read operation, `crossystem` uses flashrom for updating VBNV, additionally, `set_fw_good` script also uses `crossystem` to update the fw status. Solution: Without this SCIP check being implemented in flashrom, there is no way to ensure multiple instances of flashrom performing different SPI operations are not cancelling each other and running into below error: Erasing and writing flash chip... Timeout error between offset 0x0061c000 and 0x0061c03f (= 0x0061c000 + 63)! FAILED! Uh oh. Erase/write failed. Checking if anything has changed. TEST=Able to flash coreboot image on Alder Lake, Brya variants, Tiger Lake, Volteer variants, Kaby Lake, Eve system and Comet Lake, Hatch variants without any failure. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484 --- M ichspi.c 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/61854/13 -- To view, visit https://review.coreboot.org/c/flashrom/+/61854 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484 Gerrit-Change-Number: 61854 Gerrit-PatchSet: 13 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Alex Levin <levinale(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Martin Roth - Personal <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Alex Levin <levinale(a)chromium.org> Gerrit-Attention: YH Lin <yueherngl(a)chromium.org> Gerrit-Attention: Martin Roth - Personal <martinroth(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops
by Michał Żygowski (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Sergii Dmytruk. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... Patch Set 29: (12 comments) Patchset: PS29: > > Now I probe only for ITE Super I/Os. […] You mean compile the files when CONFIG_INTERNAL is selected? Michael also wanted to add more interfaces to program ITE ECs (using different methods). I assume the "-p " would not have to change? PS29: > Splitting an old comment thread, might make things easier to follow. […] The internal DMI decoder checks the legacy region for SMBIOS entry. This will simply fail on UEFI firmware which do not populate SMBIOS entry in the legacy range, thus external decoder is the only viable option. We could also rely on the OS to expose the SMBIOS tables as a file in sysfs, but that would not apply to all build targets. There is also the raw memory access problem, not all memory can be mapped to the application. File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/55715/comment/33da8698_0df2d28f PS29, Line 1423: .B " flashrom \-p ite_ecfw:portpair=pairnum" > Isn't this board specific? If so the information should come from […] We can add a field that would specify the default port pair for given board. Could be board specific or OEM specific, depending on UEFI/EC configuration https://review.coreboot.org/c/flashrom/+/55715/comment/fe968320_006f643a PS29, Line 1471: .B " flashrom \-p ite_ecfw:boardmismatch=force" > I don't think flashrom supports such a force yet nor that it should. […] There is a System76 EC firmware which serves as a replacement for the original firmware being used here. One could use the force switch to flash the replacement anyway. S76 EC embeds different strings into EC firmware which would result in a mismatch. Secondly it can override board PCI matching, e.g. we have a board that we know is supported (but not yet added to the board support table) and would like to flash the EC anyways. There are two usecases for this flag so I have to split it somehow, e.g. projectmismatch should enable forcing the use of a file despite the strings in the file don't match. I understand that forcing a flash operation is questionable and generally shouldn't be allowed, but we still permit it for internal programmer with boardmismatch. The results also may be catastrophic. File it87spi.c: https://review.coreboot.org/c/flashrom/+/55715/comment/3dd6d7e2_c4b24a36 PS29, Line 105: case 0x89: > How does this affect the internal programmer? Would be nice to split this […] We can split it, however I have no means to answer your question (no hardware with ITE of ID starting with 0x89) File ite_ecfw.c: https://review.coreboot.org/c/flashrom/+/55715/comment/276535b8_8f155fff PS29, Line 411: } > Doesn't this mean we always have to flash the whole chip? and all blocks in […] We replay the behaviour of the original EFI file doing the update. We only have to erase the whole chip in one sway. Write operations may be of one block size (64K) and flashrom would write and verify per block. Writing order is not relevant, just the first kbyte has to be written at the end (if the first block has been modified). https://review.coreboot.org/c/flashrom/+/55715/comment/6c0d12ab_6f23f3c0 PS29, Line 493: ctx_data->ite_string_offset >= start && : ctx_data->ite_string_offset <= (start + len - sizeof(struct ite_string)) > Shouldn't this check `start == 0` instead? How would ite_ecfw_patch_autoload() […] The code should skip the autoload patching if offset is 0, I have to fix it. The offset is obtained in ite_ecfw_get_ite_string_offset and is supposed to be in the first 256 bytes of the firmware file (with 16 bytes interval). Offset is stored in the ctx_data https://review.coreboot.org/c/flashrom/+/55715/comment/cc3b3f9c_7b63b4b6 PS29, Line 495: (uint8_t *) > Ah, I was too tired yesterday when I read this and didn't […] The autoload option breaks the verification indeed. Such cast allows modifying the initial content of the firmware file to be flashed for the autoload purpose. How would you see such modification to be done on the fly in flashrom? https://review.coreboot.org/c/flashrom/+/55715/comment/073a87e3_1180a413 PS29, Line 603: > What about ite_ecfw_chunkwise_erase() for ITE5570? Probably we can set the erase size to 256 bytes or 1024 bytes, this needs testing. EC_CMD_WRITE_BLOCK may also have some parameter to specify which chunk to write (it can select the block nimber with second parameter and starting chunk with third parameter, but it always consumes the number of chunks needed to flash full block). Maybe Michael could tell more if this is possible. Then we wouldn't need any 64k granularity. https://review.coreboot.org/c/flashrom/+/55715/comment/1511842e_eb8542ad PS29, Line 696: } > Are the IDs from RDID? I guess we should also check that the database entry […] It looks like RDID, but the EC doesn't return reliable data. It starts with 0xef (Winbond, which is true for the usd chip) but the rest of ID is either 0xef or 0x00. So we use this function for informative purposes only (until we find out what is going wrong here). https://review.coreboot.org/c/flashrom/+/55715/comment/82626999_7b0e5b5f PS29, Line 812: Probing > Probing might be the wrong word. AFAICS, the code just starts writing to EC RAM. No longer true. If the board is not on the list of supported boards (matched by PIC and PCI subsystem of the host bridge), the code won't get to writing to EC RAM, unless we force it. We probe the Super I/O later but yeah it starts with writing to EC RAM (after board matching). We could start with probing for Super I/O first and see if anything is detected. https://review.coreboot.org/c/flashrom/+/55715/comment/3f3db44d_23a77c62 PS29, Line 905: int ite_ecfw_verify_file_project(uint8_t *const newcontents, > Is it still call somewhere? Currently not, but I wish it would be at some point. We haven't decided yet where a programmer hook could be placed before the flash process begins (we have only the bad model of internal programmer and cb_check_image function). Could struct programmer_entry be extended with some pre flash access hook? -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 29 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 28 Feb 2022 12:47:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq
by Nico Huber (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Patrick Georgi, Rizwan Qureshi, Stefan Reinauer, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Martin Roth - Personal, Subrata Banik, Caveh Jalali, David Hendricks, Tim Wawrzynczak, Nick Vaccaro, Boris Mittelberg. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61854 ) Change subject: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq ...................................................................... Patch Set 12: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/61854/comment/2ab3b24c_3c4e044c PS12, Line 1412: } Please move out of the read-modify-write. (same for the other occurences) -- To view, visit https://review.coreboot.org/c/flashrom/+/61854 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484 Gerrit-Change-Number: 61854 Gerrit-PatchSet: 12 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Alex Levin <levinale(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Martin Roth - Personal <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Alex Levin <levinale(a)chromium.org> Gerrit-Attention: YH Lin <yueherngl(a)chromium.org> Gerrit-Attention: Martin Roth - Personal <martinroth(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-Comment-Date: Mon, 28 Feb 2022 11:43:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq
by Nico Huber (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Patrick Georgi, Rizwan Qureshi, Stefan Reinauer, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Martin Roth - Personal, Subrata Banik, Caveh Jalali, David Hendricks, Tim Wawrzynczak, Nick Vaccaro, Boris Mittelberg. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61854 ) Change subject: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq ...................................................................... Patch Set 12: Code-Review-1 (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/61854/comment/7d4618b6_ffea2f7a PS7, Line 17: Without this synchronisation being implemented, flashrom is running : into below error: : : Erasing and writing flash chip... Timeout error between offset : 0x0061c000 and 0x0061c03f (= 0x0061c000 + 63)! FAILED! : Uh oh. Erase/write failed. Checking if anything has changed. > Please let me know if anything we need to help for moving this CL? What was said in the first message of the comment thread. https://review.coreboot.org/c/flashrom/+/61854/comment/382b595c_283d6679 PS7, Line 25: TEST=Concurrent flashrom access is not throwing timeout. > Please let me know if anything we need to help for moving this CL? What was said in the first message of the comment thread. Patchset: PS12: Please don't submit with a commit message that risks to repeat the confusion of this havoc review. -- To view, visit https://review.coreboot.org/c/flashrom/+/61854 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484 Gerrit-Change-Number: 61854 Gerrit-PatchSet: 12 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Alex Levin <levinale(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Martin Roth - Personal <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Alex Levin <levinale(a)chromium.org> Gerrit-Attention: YH Lin <yueherngl(a)chromium.org> Gerrit-Attention: Martin Roth - Personal <martinroth(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-Comment-Date: Mon, 28 Feb 2022 11:42:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: writeprotect: add {get,set}_wp_mode()
by Angel Pons (Code Review) 28 Feb '22

28 Feb '22

Attention is currently required from: Nikolai Artemiev, Sergii Dmytruk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58483 ) Change subject: writeprotect: add {get,set}_wp_mode() ...................................................................... Patch Set 64: Code-Review+1 (1 comment) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58483/comment/7cc5d34f_c3ba7f73 PS58, Line 366: } > General thought: That the positive path comes first in such an if and ?: […] s/int/enum flashrom_wp_mode -- To view, visit https://review.coreboot.org/c/flashrom/+/58483 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7b68e940f0e1359281806c98e1da119b4caf8405 Gerrit-Change-Number: 58483 Gerrit-PatchSet: 64 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 28 Feb 2022 09:50:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2022