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

Change in flashrom[master]: dmi: Correctly check for ERROR_PTR
by Nico Huber (Code Review) 05 Mar '22

05 Mar '22

Attention is currently required from: Angel Pons. Hello Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/62611 to review the following change. Change subject: dmi: Correctly check for ERROR_PTR ...................................................................... dmi: Correctly check for ERROR_PTR For the physmap*() functions, NULL is considered valid return value. Fixes a segmentation fault when DMI tables can't be mapped. Tested on intel/eblake board with broken coreboot. Change-Id: Ic403c2940c2b91acbd113f0acfa3ce9ef6c6bb6c Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M dmi.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/62611/1 diff --git a/dmi.c b/dmi.c index 16d350d..a7b300d 100644 --- a/dmi.c +++ b/dmi.c @@ -164,7 +164,7 @@ unsigned int i = 0, j = 0; uint8_t *dmi_table_mem = physmap_ro("DMI Table", base, len); - if (dmi_table_mem == NULL) { + if (dmi_table_mem == ERROR_PTR) { msg_perr("Unable to access DMI Table\n"); return; } -- To view, visit https://review.coreboot.org/c/flashrom/+/62611 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic403c2940c2b91acbd113f0acfa3ce9ef6c6bb6c Gerrit-Change-Number: 62611 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ichspi: Add Alder Lake support
by Subrata Banik (Code Review) 05 Mar '22

05 Mar '22

Attention is currently required from: Nico Huber, Rizwan Qureshi, Edward O'Callaghan, Angel Pons. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 ) Change subject: ichspi: Add Alder Lake support ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf Gerrit-Change-Number: 62251 Gerrit-PatchSet: 14 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> 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: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 05 Mar 2022 13:13:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops
by Nico Huber (Code Review) 05 Mar '22

05 Mar '22

Attention is currently required from: Felix Singer, Michał Żygowski, Paul Menzel, Michael Niewöhner, Sergii Dmytruk. Nico Huber 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: (1 comment) File ite_ecfw.c: https://review.coreboot.org/c/flashrom/+/55715/comment/ec3651a4_0504d635 PS29, Line 411: } > Yes, correct. […] I trust you that we need to do this. The thing is, we need to ensure flashrom actually does it. ;) Normally, there is no order enforced of the blocks to be written. But in case of `write_mode == 0` this function must be called with the last block last (and it mustn't be skipped) and before that it must be called with the first block (so we know the first KiB). Currently this happens by coincidence because the whole flash is erased, the way the core flashrom code is written, we expect that the last block of the contents to be flashed are not empty, and we expect that the changes in contents need an erase. We could leave comments everywhere about this (still with the risk that it breaks because of changes somewhere else). Or we could try to find a way to make it work by definition. A hunch tells me that the write granularity could help there. -- 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: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Sat, 05 Mar 2022 13:06:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Alder Lake support
by Edward O'Callaghan (Code Review) 05 Mar '22

05 Mar '22

Attention is currently required from: Nico Huber, Rizwan Qureshi, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 ) Change subject: ichspi: Add Alder Lake support ...................................................................... Patch Set 14: (1 comment) Patchset: PS12: > Sorry I think I didn't write well. We are saying the same thing. […] Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/62251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf Gerrit-Change-Number: 62251 Gerrit-PatchSet: 14 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> 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: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 05 Mar 2022 12:03:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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]: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops
by Michael Niewöhner (Code Review) 05 Mar '22

05 Mar '22

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Sergii Dmytruk. Michael Niewöhner 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: (1 comment) File ite_ecfw.c: https://review.coreboot.org/c/flashrom/+/55715/comment/67f6d3e1_b65487b4 PS29, Line 411: } > When we think about flashrom as an application to simply replace flash content, these problems are o […] Yes, correct. Flashing the ec directly on hardware level is possible but not a good idea at runtime, since flashing would interfere with the currently running firmware. Thus, the ec firmware implements flashing with help of the scratch region, as Michal described. We have to comply with the logic implemented there. -- 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: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Sat, 05 Mar 2022 09:37:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: comment

1 0

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

04 Mar '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: (2 comments) Patchset: PS29: > I mean it should be fully included in `-p internal` eventually. The `internal` […] Understood. It may be a good idea to merge it with internal programmer indeed. File ite_ecfw.c: https://review.coreboot.org/c/flashrom/+/55715/comment/ffb5fb1a_704ee98d PS29, Line 411: } > AFAICS, if we'd allow a partial erase, flashrom could decide to only write […] When we think about flashrom as an application to simply replace flash content, these problems are only theoretical. But if we look at it hardware-wise, it is a different story. The main problem with this first kbyte is that it contains the code responsible for writing to the flash. This is a special scratch memory which is doublemapped to 8051 External Memory and ROM space. When the flashing process begins, the flash access routines are copied to the scratch ROM space 4KB where EC can safely execute them without worrying about the flash content (EC wouldn't be able to fetch code when the SPI flash is in WIP/busy state). The command that is sent when writing the first kilobyte probably indicates to the EC that: 1. The flash access is finished after it takes the first kilobyte to be flashed. 2. EC can switch back from scratch ROM to the flash ROM with the execution. That is why it must be done at the end of flashing process. I hope I didn't make any mistake here, Michael can confirm -- 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: Fri, 04 Mar 2022 15:53:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: layout: Change signature for prepare_layout_for_extraction
by Nico Huber (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62554 ) Change subject: layout: Change signature for prepare_layout_for_extraction ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Thanks for reply! :) > > > I'm not sure if I mentioned this somewhere already, the topic seems familiar. I kind of see this extract feature as CLI code. > > Sorry if I missed that! I don't remember, but maybe because extract feature was upstreamed earlier when I just started and I wasn't fully aware of all everything that going on. Don't worry, if we discussed this somewhere else already, I lost track of it anyway. > My reasoning for this patch chain was to align with larger movement to make cli a libflashrom user, which means libflashrom is used from everywhere and needs to be tested. Yes, I got that :) we need to adapt libflashrom API for this. Making existing functions public isn't always the best way, though. > > > So far, we have kept libflashrom free from file i/o and some target environments just don't support it > > Maybe I am missing something, but I don't see `prepare_layout_for_extraction` doing file i/o at the moment? It is populating file names, which is prep step for i/o, but the i/o itself is done next in read operation. That's right. But for prepare_layout_for_extraction() to be useful in the libflashrom API, we'd need another function there that probably does what write_buf_to_include_args() currently does. And that's doing file i/o. Also, the description in your next patch says "Fill in file name". So I'd say it's part of a file i/o concept. It's why I generally prefer to discuss the coarse direction of a topic on the mailing list first. Here on Gerrit, people are often focused to bring patches in step-by-step, but sometimes the bigger picture is only visible at the end of a patch train. It can be frustrating for both authors and reviewers to see that the effort on prior reviews doesn't pan out. Or worse, they might want to merge something that they consider wrong just because they put all the effort into it. > Maybe `prepare_layout_for_extraction` can be renamed into something more specific? I would much prefer to discuss first if the whole topic belongs into libflashrom or the CLI. > I agree with keeping libflashrom free from file i/o. -- To view, visit https://review.coreboot.org/c/flashrom/+/62554 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7d3874d1097bb0d7bb8d9fa8e639cc1e71407627 Gerrit-Change-Number: 62554 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 04 Mar 2022 13:55:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq
by Rizwan Qureshi (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Patrick Georgi, Stefan Reinauer, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Nico Huber, Martin Roth, Subrata Banik, Caveh Jalali, David Hendricks, Tim Wawrzynczak, Nick Vaccaro, Boris Mittelberg. Rizwan Qureshi 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 13: Code-Review+2 -- 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 <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: 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 <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: Fri, 04 Mar 2022 13:42:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes 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) 04 Mar '22

04 Mar '22

Attention is currently required from: Patrick Georgi, Rizwan Qureshi, Stefan Reinauer, Angel Pons, Sridhar Siricilla, Alex Levin, YH Lin, Nico Huber, Martin Roth, 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 13: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/61854/comment/3f5e8bb5_db48dc19 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. > > > > Ping! > > Ping! Ping! https://review.coreboot.org/c/flashrom/+/61854/comment/b72f2717_64fa01e5 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. > > > > Ping! > > Ping! Ping! -- 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 <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 <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: Fri, 04 Mar 2022 08:44:58 +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: Add Alder Lake support
by Edward O'Callaghan (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Nico Huber, Rizwan Qureshi, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 ) Change subject: ichspi: Add Alder Lake support ...................................................................... Patch Set 13: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62251/comment/49b621cb_131210a5 PS12, Line 1041: return CHIPSET_600_SERIES_ALDER_POINT; > Done For completeness here, Sam reported back: ``` $ ./util/ich_descriptors_tool/ich_descriptors_tool -d -c "chipset" -f /tmp/image-nivviks.bin The flash image has a size of 33554432 [0x2000000] bytes. Assuming chipset '600 series Alder Point'. === Content Section === FLVALSIG 0x0ff0a55a FLMAP0 0x00040003 FLMAP1 0x46100208 FLMAP2 0x001401b0 --- Details --- NR (Number of Regions): 16 FRBA (Flash Region Base Address): 0x040 NC (Number of Components): 1 FCBA (Flash Component Base Address): 0x030 ISL (ICH/PCH/SoC Strap Length): 70 FISBA/FPSBA (Flash ICH/PCH/SoC Strap Base Addr): 0x100 NM (Number of Masters): 2 FMBA (Flash Master Base Address): 0x080 MSL/PSL (MCH/PROC Strap Length): 1 FMSBA (Flash MCH/PROC Strap Base Address): 0xb00 === Component Section === FLCOMP 0x093000f6 FLILL 0xad604221 FLILL1 0xc7c4b9b7 --- Details --- Component 1 density: 32 MB Component 2 is not used. Read Clock Frequency: 100 MHz Read ID and Status Clock Freq.: 50 MHz Write and Erase Clock Freq.: 50 MHz Fast Read is supported. Fast Read Clock Frequency: 50 MHz Dual Output Fast Read Support: disabled Invalid instruction 0: 0x21 Invalid instruction 1: 0x42 Invalid instruction 2: 0x60 Invalid instruction 3: 0xad Invalid instruction 4: 0xb7 Invalid instruction 5: 0xb9 Invalid instruction 6: 0xc4 Invalid instruction 7: 0xc7 === Region Section === FLREG0 0x00000000 FLREG1 0x1fff0500 FLREG2 0x04ff0001 FLREG3 0x00007fff FLREG4 0x00007fff FLREG5 0x00007fff FLREG6 0x00007fff FLREG7 0x00007fff FLREG8 0x00007fff FLREG9 0x00007fff FLREG10 0x00007fff FLREG11 0x00007fff FLREG12 0x00007fff FLREG13 0x00007fff FLREG14 0x00007fff FLREG15 0x00007fff --- Details --- Region 0 (Descr. ) 0x00000000 - 0x00000fff Region 1 (BIOS ) 0x00500000 - 0x01ffffff Region 2 (ME ) 0x00001000 - 0x004fffff Region 3 (GbE ) is unused. Region 4 (Platf. ) is unused. Region 5 (DevExp ) is unused. Region 6 (BIOS2 ) is unused. Region 7 (unknown) is unused. Region 8 (EC/BMC ) is unused. Region 9 (unknown) is unused. Region 10 (IE ) is unused. Region 11 (10GbE ) is unused. Region 12 (unknown) is unused. Region 13 (unknown) is unused. Region 14 (unknown) is unused. Region 15 (unknown) is unused. === Master Section === FLMSTR1 0xffffffff FLMSTR2 0xffffffff --- Details --- FD BIOS ME GbE Pltf Reg5 Reg6 Reg7 EC Reg9 RegA RegB RegC RegD RegE RegF BIOS rw rw rw rw rw rw rw rw rw rw rw rw rw rw rw rw ME rw rw rw rw rw rw rw rw rw rw rw rw rw rw rw rw === Upper Map Section === FLUMAP1 0xc0ff02df --- Details --- VTL (length in DWORDS) = 2 VTBA (base address) = 0x000df0 VSCC Table: 1 entries JID0 = 0x0000471f VSCC0 = 0x20152015 Manufacturer ID 0x1f, Device ID 0x4700 BES=0x1, WG=1, WSR=0, WEWS=1, EO=0x20 === Softstraps === --- North/MCH/PROC (1 entries) --- STRP0 = 0xffffffff ISL (70) is greater than the current maximum of 23 entries. Only the first 23 entries will be printed. --- South/ICH/PCH (23 entries) --- STRP0 = 0x00000000 STRP1 = 0x00000001 STRP2 = 0x0100000d STRP3 = 0x022c4000 STRP4 = 0x0001087f STRP5 = 0x00000000 STRP6 = 0x00000018 STRP7 = 0x00070003 STRP8 = 0x48030008 STRP9 = 0x00000000 STRP10 = 0x0eff0001 STRP11 = 0x0481fbf0 STRP12 = 0x00000000 STRP13 = 0x000f000c STRP14 = 0x22224222 STRP15 = 0x22424222 STRP16 = 0x00000000 STRP17 = 0x00000000 STRP18 = 0x00ff0000 STRP19 = 0xc8800060 STRP20 = 0x36008645 STRP21 = 0x00000000 STRP22 = 0x00580e20 The meaning of the descriptor straps are unknown yet. === Dumping region files === Dumping 4096 bytes of the Descriptor region from 0x00000000-0x00000fff to /tmp/image-nivviks.bin.Descriptor.bin... done. Dumping 28311552 bytes of the BIOS region from 0x00500000-0x01ffffff to /tmp/image-nivviks.bin.BIOS.bin... done. Dumping 5238784 bytes of the ME region from 0x00001000-0x004fffff to /tmp/image-nivviks.bin.ME.bin... done. The GbE region is unused and thus not dumped. The Platform region is unused and thus not dumped. The Region5 region is unused and thus not dumped. The BIOS2 region is unused and thus not dumped. The Region7 region is unused and thus not dumped. The EC/BMC region is unused and thus not dumped. The Region9 region is unused and thus not dumped. The IE region is unused and thus not dumped. The 10GbE region is unused and thus not dumped. The Region12 region is unused and thus not dumped. The Region13 region is unused and thus not dumped. The Region14 region is unused and thus not dumped. The Region15 region is unused and thus not dumped. ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/62251 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf Gerrit-Change-Number: 62251 Gerrit-PatchSet: 13 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> 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: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 04 Mar 2022 02:40:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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 March 2022