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

Change in flashrom[master]: ft2232_spi.c: Add FTDI FT4233H
by Charles Parent (Code Review) 03 Mar '22

03 Mar '22

Charles Parent has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62480 ) Change subject: ft2232_spi.c: Add FTDI FT4233H ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Thank you! You're welcome ! Do you need something else to finish the Pull request ? -- To view, visit https://review.coreboot.org/c/flashrom/+/62480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73cee8fd2a6613a8fbc26508d99bbe67da2b4f72 Gerrit-Change-Number: 62480 Gerrit-PatchSet: 2 Gerrit-Owner: Charles Parent <cha.parent(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Comment-Date: Thu, 03 Mar 2022 12:02:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Charles Parent <cha.parent(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Alder Lake support
by Sam McNally (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Nico Huber, Rizwan Qureshi, Edward O'Callaghan, Angel Pons. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 ) Change subject: ichspi: Add Alder Lake support ...................................................................... Patch Set 12: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62251/comment/f2e4b91c_1b362f84 PS12, Line 12: TEST=```localhost ~ # flashrom --flash-name > > This tests the changes to `guess_ich_chipset_from_content()`, please check the flashrom log to ver […] Thanks for that detail. With Nivviks/ADL-N and Brya/ADL-P: ``` Reading ich descriptor... Reading 4096 bytes starting at 0x000000. done. Assuming chipset '600 series Alder Point'. Added layout entry 00000000 - 00000fff named fd Added layout entry 00500000 - 01ffffff named bios Added layout entry 00001000 - 004fffff named me ``` File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62251/comment/eddcfd4c_5ad9d2ee PS12, Line 307: case CHIPSET_600_SERIES_ALDER_POINT: > Looking at the ADL-P SPI Guide Rev. 1.24, it shows the same numbers as […] From the values available in the FIT tool, I think you're right. -- 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: 12 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: Thu, 03 Mar 2022 11:05:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sam McNally <sammc(a)google.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> 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) 03 Mar '22

03 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/53800e7d_3c5ef805 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! https://review.coreboot.org/c/flashrom/+/61854/comment/c92fbac5_a48a3a68 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! -- 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: Thu, 03 Mar 2022 06:37:23 +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]: tests: Add test for extract operation
by Anastasia Klimchuk (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Daniel Campello, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59532 ) Change subject: tests: Add test for extract operation ...................................................................... Patch Set 4: (3 comments) This change is ready for review. File tests/chip.c: https://review.coreboot.org/c/flashrom/+/59532/comment/9ea6647b_623bee71 PS3, Line 462: mock_chip.total_size * 1024; > flashrom_flash_getsize() ? That's a good idea, and gives more coverage :) https://review.coreboot.org/c/flashrom/+/59532/comment/47ec7e9c_08d74bb8 PS3, Line 464: > trim Done https://review.coreboot.org/c/flashrom/+/59532/comment/c0be707c_f73d21d1 PS3, Line 468: printf("Extract layout operation done.\n"); > you probably want to validate buf now contains the read content? […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/59532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13107c0e095d53184e32a41fa72227cf7dc1d449 Gerrit-Change-Number: 59532 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> 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: Peter Marheine <pmarheine(a)chromium.org> 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: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 03 Mar 2022 06:21:26 +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]: layout: Hoist prepare_layout_for_extraction into libflashrom API
by Anastasia Klimchuk (Code Review) 03 Mar '22

03 Mar '22

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62555 ) Change subject: layout: Hoist prepare_layout_for_extraction into libflashrom API ...................................................................... layout: Hoist prepare_layout_for_extraction into libflashrom API Currently extract operation is not available via libflashrom API. It consists of two calls: prepare_layout_for_extraction and read operation. The latter is already a part of libflashrom API. So prepare_layout_for_extraction is remaining piece to make extract operation available in libflashrom. TEST ME ON DEVICE BUG=b:222384032 Change-Id: I3f5b4079528ddd4d1ca9acfa6d107e8c3d6814ff Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M layout.c M layout.h M libflashrom.h M libflashrom.map 5 files changed, 26 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/62555/1 diff --git a/cli_classic.c b/cli_classic.c index f044e87..573d28f 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -184,7 +184,7 @@ static int do_extract(struct flashctx *const flash) { const struct flashrom_layout *const layout = get_layout(flash); - prepare_layout_for_extraction(layout); + flashrom_layout_prepare_for_extraction(layout); return do_read(flash, NULL); } diff --git a/layout.c b/layout.c index b4ccdaa..50a2900 100644 --- a/layout.c +++ b/layout.c @@ -299,24 +299,6 @@ return ret; } -void prepare_layout_for_extraction(const struct flashrom_layout *const layout) -{ - struct romentry *entry = NULL; - unsigned int i; - - while ((entry = mutable_layout_next(layout, entry))) { - entry->included = true; - - if (!entry->file) - entry->file = strdup(entry->name); - - for (i = 0; entry->file[i]; ++i) { - if (isspace(entry->file[i])) - entry->file[i] = '_'; - } - } -} - const struct romentry *layout_next_included_region( const struct flashrom_layout *const l, const chipoff_t where) { @@ -455,6 +437,29 @@ } /** + * @brief Fill in file name for each included region. + * + * @param layout The layout to prepare. + */ +void flashrom_layout_prepare_for_extraction(const struct flashrom_layout *const layout) +{ + struct romentry *entry = NULL; + unsigned int i; + + while ((entry = mutable_layout_next(layout, entry))) { + entry->included = true; + + if (!entry->file) + entry->file = strdup(entry->name); + + for (i = 0; entry->file[i]; ++i) { + if (isspace(entry->file[i])) + entry->file[i] = '_'; + } + } +} + +/** * @brief Free a layout. * * @param layout Layout to free. diff --git a/layout.h b/layout.h index ce8386a..6ab42d8 100644 --- a/layout.h +++ b/layout.h @@ -63,7 +63,6 @@ const struct romentry *layout_next_included(const struct flashrom_layout *, const struct romentry *); const struct romentry *layout_next(const struct flashrom_layout *, const struct romentry *); int included_regions_overlap(const struct flashrom_layout *); -void prepare_layout_for_extraction(const struct flashrom_layout *const); int layout_sanity_checks(const struct flashrom_flashctx *); #endif /* !__LAYOUT_H__ */ diff --git a/libflashrom.h b/libflashrom.h index 557978d..eaaa480 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -116,6 +116,7 @@ int flashrom_layout_include_region(struct flashrom_layout *, const char *name); int flashrom_layout_get_region_range(struct flashrom_layout *, const char *name, unsigned int *start, unsigned int *len); +void flashrom_layout_prepare_for_extraction(const struct flashrom_layout *const); void flashrom_layout_release(struct flashrom_layout *); void flashrom_layout_set(struct flashrom_flashctx *, const struct flashrom_layout *); diff --git a/libflashrom.map b/libflashrom.map index 2249d35..fce24cc 100644 --- a/libflashrom.map +++ b/libflashrom.map @@ -16,6 +16,7 @@ flashrom_init; flashrom_layout_get_region_range; flashrom_layout_include_region; + flashrom_layout_prepare_for_extraction; flashrom_layout_read_fmap_from_buffer; flashrom_layout_read_fmap_from_rom; flashrom_layout_read_from_ifd; -- To view, visit https://review.coreboot.org/c/flashrom/+/62555 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f5b4079528ddd4d1ca9acfa6d107e8c3d6814ff Gerrit-Change-Number: 62555 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: layout: Change signature for prepare_layout_for_extraction
by Anastasia Klimchuk (Code Review) 03 Mar '22

03 Mar '22

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62554 ) Change subject: layout: Change signature for prepare_layout_for_extraction ...................................................................... layout: Change signature for prepare_layout_for_extraction This function used to accept flash context as an argument, but inside it is getting layout from context and then working with this layout. The patch changes the signature to accept layout instead of flash context to align with other layout functions and to be able to add prepare_layout_for_extraction to libflashrom API. The reason for adding prepare_layout_for_extraction to libflashrom API is that it allows using extract functionality from libflashrom. At the moment there is a do_extract function in cli_classic. It has two calls: prepare_layout_for_extraction and do_read. Read operation is already available from libflashrom API, so one thing remaining is prepare_layout_for_extraction. Adding prepare_layout_for_extraction to libflashrom happens in the next patch in this chain. TEST ME ON DEVICE BUG=b:222384032 Change-Id: I7d3874d1097bb0d7bb8d9fa8e639cc1e71407627 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M layout.c M layout.h 3 files changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/62554/1 diff --git a/cli_classic.c b/cli_classic.c index 8c337eb..f044e87 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -183,7 +183,8 @@ static int do_extract(struct flashctx *const flash) { - prepare_layout_for_extraction(flash); + const struct flashrom_layout *const layout = get_layout(flash); + prepare_layout_for_extraction(layout); return do_read(flash, NULL); } diff --git a/layout.c b/layout.c index 05bee19..b4ccdaa 100644 --- a/layout.c +++ b/layout.c @@ -299,13 +299,12 @@ return ret; } -void prepare_layout_for_extraction(struct flashctx *flash) +void prepare_layout_for_extraction(const struct flashrom_layout *const layout) { - const struct flashrom_layout *const l = get_layout(flash); struct romentry *entry = NULL; unsigned int i; - while ((entry = mutable_layout_next(l, entry))) { + while ((entry = mutable_layout_next(layout, entry))) { entry->included = true; if (!entry->file) diff --git a/layout.h b/layout.h index abbdc22..ce8386a 100644 --- a/layout.h +++ b/layout.h @@ -63,7 +63,7 @@ const struct romentry *layout_next_included(const struct flashrom_layout *, const struct romentry *); const struct romentry *layout_next(const struct flashrom_layout *, const struct romentry *); int included_regions_overlap(const struct flashrom_layout *); -void prepare_layout_for_extraction(struct flashrom_flashctx *); +void prepare_layout_for_extraction(const struct flashrom_layout *const); int layout_sanity_checks(const struct flashrom_flashctx *); #endif /* !__LAYOUT_H__ */ -- 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: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: pcidev: Move scandev_inclass logic from internal to pcidev
by Anastasia Klimchuk (Code Review) 03 Mar '22

03 Mar '22

Attention is currently required from: Edward O'Callaghan, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59277 ) Change subject: pcidev: Move scandev_inclass logic from internal to pcidev ...................................................................... Patch Set 8: Code-Review+2 -- 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: 8 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 03 Mar 2022 01:02:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

03 Mar '22

Attention is currently required from: Edward O'Callaghan, 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 (#8). 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/8 -- 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: 8 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: 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) 02 Mar '22

02 Mar '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/59276 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/59276 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M board_enable.c M pcidev.c M programmer.h 3 files changed, 15 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved diff --git a/board_enable.c b/board_enable.c index 94cfc9d..194357a 100644 --- a/board_enable.c +++ b/board_enable.c @@ -1094,15 +1094,7 @@ return -1; } -#if !defined(OLD_PCI_GET_DEV) - dev = pci_get_dev(pacc, dev->domain, dev->bus, dev->dev, 1); -#else - /* pciutils/libpci before version 2.2 is too old to support - * PCI domains. Such old machines usually don't have domains - * besides domain 0, so this is not a problem. - */ - dev = pci_get_dev(pacc, dev->bus, dev->dev, 1); -#endif + dev = pcidev_getdevfn(dev, 1); if (!dev) { msg_perr("MCP SMBus controller could not be found\n"); return -1; diff --git a/pcidev.c b/pcidev.c index f323851..0f8c4e5 100644 --- a/pcidev.c +++ b/pcidev.c @@ -157,6 +157,19 @@ return NULL; } +struct pci_dev *pcidev_getdevfn(struct pci_dev *dev, const int func) +{ +#if !defined(OLD_PCI_GET_DEV) + return pci_get_dev(pacc, dev->domain, dev->bus, dev->dev, func); +#else + /* pciutils/libpci before version 2.2 is too old to support + * PCI domains. Such old machines usually don't have domains + * besides domain 0, so this is not a problem. + */ + return pci_get_dev(pacc, dev->bus, dev->dev, func); +#endif +} + static int pcidev_shutdown(void *data) { if (pacc == NULL) { diff --git a/programmer.h b/programmer.h index 4024966..d0e86a4 100644 --- a/programmer.h +++ b/programmer.h @@ -126,6 +126,7 @@ uintptr_t pcidev_readbar(struct pci_dev *dev, int bar); struct pci_dev *pcidev_init(const struct dev_entry *devs, int bar); struct pci_dev *pcidev_scandev(struct pci_filter *filter, struct pci_dev *start); +struct pci_dev *pcidev_getdevfn(struct pci_dev *dev, const int func); /* rpci_write_* are reversible writes. The original PCI config space register * contents will be restored on shutdown. * To clone the pci_dev instances internally, the `pacc` global -- 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: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: 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-MessageType: merged

1 0

Change in flashrom[master]: ichspi: Add Alder Lake support
by Nico Huber (Code Review) 02 Mar '22

02 Mar '22

Attention is currently required from: Sam McNally, Rizwan Qureshi, Edward O'Callaghan, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 ) Change subject: ichspi: Add Alder Lake support ...................................................................... Patch Set 12: Code-Review+1 (5 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62251/comment/064d449f_450b512d PS12, Line 12: TEST=```localhost ~ # flashrom --flash-name > This tests the changes to `guess_ich_chipset_from_content()`, please check the flashrom log to verify the correct chipset is detected. This is only visible in a very verbose log. For `--ifd` the chipset is detected separately so it also works with external programmers. The line would start with "Assuming chipset ", `ich_descriptors_tool` can also be used to test. Patchset: PS12: > Same question as for Jasper Lake, does there anything differ from the […] So far I've seen no difference beside the number of straps in the descriptor. File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62251/comment/0fa2d6e6_180adf91 PS12, Line 307: case CHIPSET_600_SERIES_ALDER_POINT: Looking at the ADL-P SPI Guide Rev. 1.24, it shows the same numbers as for TGL. What document/version was used to write this? (please also mention in the commit message) If your version of the documentation differs, we could comare the output to the settings in FIT tool (it usually has the last word at Intel). https://review.coreboot.org/c/flashrom/+/62251/comment/6521362b_8069828f PS12, Line 1041: return CHIPSET_600_SERIES_ALDER_POINT; Please test the detection (easy to do with ich_descriptors_tool). For EHL the SPI guide needed to be fixed. File ichspi.c: https://review.coreboot.org/c/flashrom/+/62251/comment/2e5c09a7_137548ce PS12, Line 1818: case CHIPSET_300_SERIES_CANNON_POINT: : case CHIPSET_400_SERIES_COMET_POINT: : case CHIPSET_500_SERIES_TIGER_POINT: : case CHIPSET_600_SERIES_ALDER_POINT: I'm not sure if all of this was guess work, I can't find more than 6 regs in datasheets. Can somebody confirm? It probably doesn't matter much, as there are no conflicting registers documented and we don't write to them anyway. If the datasheet turns out to be incomplete, please mention which was used in the commit message and what is missing. -- 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: 12 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: Sam McNally <sammc(a)google.com> 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: Wed, 02 Mar 2022 17:18:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Sam McNally <sammc(a)google.com> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022