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

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) April 2, 2022

April 2, 2022

Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons. Hello Sam McNally, build bot (Jenkins), Subrata Banik, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62282 to look at the new patch set (#9). Change subject: ichspi: Add Jasper Lake support ...................................................................... ichspi: Add Jasper Lake support Additionally, utilize CSSO (CPU Soft Strap Offset) to uniquely detect the chipset when the CSSL (CPU Soft Strap Length) field default value (0x03) on Jasper Lake is the same as Elkhart Lake. BUG=b:221175960 TEST=dedede with `flashrom -p internal --flash-size`. ``` $ flashrom -VVV -p internal --ifd -i fd -i bios -r /tmp/filename.rom <snip> Enabling hardware sequencing by default for 100+ series PCH. OK. No board enable found matching coreboot IDs vendor="Google", model="Magolor". The following protocols are supported: Programmer-specific. Probing for Programmer Opaque flash chip, 0 kB: Chip identified: GD25Q127C/GD25Q128C Hardware sequencing reports 1 attached SPI flash chip with a density of 16384 kB. There is only one partition containing the whole address space (0x000000 - 0xffffff). There are 4096 erase blocks with 4096 B each. Added layout entry 00000000 - 00ffffff named complete flash Found GigaDevice flash chip "GD25Q127C/GD25Q128C" (16384 kB, Programmer-specific) mapped at physical address 0x0000000000000000. Found GigaDevice flash chip "GD25Q127C/GD25Q128C" (16384 kB, Programmer-specific). This chip may contain one-time programmable memory. flashrom cannot read and may never be able to write it, hence it may not be able to completely clone the contents of this chip (see man page for details). Reading Status register Block protection is disabled. Reading ich descriptor... Reading 4096 bytes starting at 0x000000. done. Assuming chipset 'Jasper Lake'. Added layout entry 00000000 - 00000fff named fd Added layout entry 00381000 - 00ffffff named bios Added layout entry 00001000 - 00380fff named me restore_power_management: Re-enabling power management. Using regions: "bios", "fd". Reading Status register Block protection is disabled. Reading flash... 0x381000-0xffffff:R Reading 13103104 bytes starting at 0x381000. 000000-0x0fff:R Reading 4096 bytes starting at 0x000000. done. restore_power_management: Re-enabling power management. SUCCESS Restoring PCI config space for 00:1f:5 reg 0xdc restore_power_management: Re-enabling power management. ``` Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M chipset_enable.c M ich_descriptors.c M ichspi.c M programmer.h M util/ich_descriptors_tool/ich_descriptors_tool.c 5 files changed, 37 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/62282/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 9 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Nico Huber (Code Review) April 1, 2022

April 1, 2022

Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 8: (2 comments) Patchset: PS4: > Given the subtle differences such as `freq_str` it looks like keeping them separate was the right id […] Well it's different from EHL, but is it really new? It looks closer to the big core PCHs, looking at Figure 1 of the datasheet. The masters and regions in the SPI guide also don't look like what we print for APL/GLK. And from the CSE version it should be somewhere between Ice Point and Comet Point. I haven't looked at every single bit, but it seems 300-series compatible (renaming the enum entry is also an option, if the code matches). File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62282/comment/c55662d0_05398e49 PS8, Line 1061: CHIPSET_ELKHART_LAKE > I do not but this preserves the current behavior and that sounds like a another patch. […] EHL has CSSO 0x58. It's in the updated SPI Guide and was confirmed with a real-world descriptor. IIRC, David wants us to have all paths with explicit checks plus a fallback default that repeats the most recent known chipset. That would be a fixup for the ADL/MTL patches, though. Have a look how things were before your patches and you can get the idea. -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 8 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 18:09:13 +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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) April 1, 2022

April 1, 2022

Attention is currently required from: Nico Huber, Subrata Banik, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 8: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62282/comment/a720aef7_ce52143b PS8, Line 1061: CHIPSET_ELKHART_LAKE > Do you know what is the CSSO value for EHL by any chance ? […] I do not but this preserves the current behavior and that sounds like a another patch. Keeping the `else` or not feels stylistically subjective? Let me know if you feel strongly? -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 8 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 07:29:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Subrata Banik (Code Review) April 1, 2022

April 1, 2022

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 8: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 8 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.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-Comment-Date: Fri, 01 Apr 2022 05:37:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Subrata Banik (Code Review) April 1, 2022

April 1, 2022

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 8: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62282/comment/4449d214_53c7ae86 PS8, Line 1061: CHIPSET_ELKHART_LAKE Do you know what is the CSSO value for EHL by any chance ? in case both JSL and EHL has same CSSO value then the `else` is meaningless now 😊 -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 8 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.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-Comment-Date: Fri, 01 Apr 2022 05:37:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) April 1, 2022

April 1, 2022

Attention is currently required from: Nico Huber, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 8: (1 comment) Patchset: PS4: > Not sure what is actionable here Nico? The guess_ich_chipset_from_content() has now been implemented […] Given the subtle differences such as `freq_str` it looks like keeping them separate was the right idea so marking as resolved. -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 8 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Apr 2022 04:12:15 +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]: tests: use MOCK_FD instead of NON_ZERO
by Nikolai Artemiev (Code Review) April 1, 2022

April 1, 2022

Attention is currently required from: Daniel Campello. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63193 ) Change subject: tests: use MOCK_FD instead of NON_ZERO ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/63193 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib6bac051852aea2465665a6fd669b7f5e3772985 Gerrit-Change-Number: 63193 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Fri, 01 Apr 2022 01:52:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0