January 2022 - flashrom-gerrit

Change in flashrom[master]: hwaccess: fix build on non-x86 targets
by Thomas Heijligen (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Paul Menzel, Edward O'Callaghan, Peter Marheine. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61194 ) Change subject: hwaccess: fix build on non-x86 targets ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61194 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I20f122679c30340b2c73afd7419e79644ddc3c4e Gerrit-Change-Number: 61194 Gerrit-PatchSet: 8 Gerrit-Owner: Peter Marheine <pmarheine(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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: 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: Tue, 25 Jan 2022 16:36:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add Elkhart Lake support
by Werner Zeh (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/60711 to look at the new patch set (#3). Change subject: Add Elkhart Lake support ...................................................................... Add Elkhart Lake support Elkhart Lake has a chipset called Mule Creek Canyon which is quite compatible with 300 series chipsets. There are a few differences though, e.g. different encoding for the SPI clock values for read and write in the FLCOMP register. In addition Elkhart Lake has a new PCI device ID for the SPI controller which is added, too. TEST=Read and flash complete flash on Siemens MC EHL1 Change-Id: I711e39a3ec9cd7098389231eaa1cb864d615a475 Signed-off-by: Werner Zeh <werner.zeh(a)siemens.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, 47 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/60711/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/60711 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I711e39a3ec9cd7098389231eaa1cb864d615a475 Gerrit-Change-Number: 60711 Gerrit-PatchSet: 3 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Add Elkhart Lake support
by Werner Zeh (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Nico Huber. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60711 ) Change subject: Add Elkhart Lake support ...................................................................... Patch Set 2: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/60711/comment/2b446403_aa04df79 PS1, Line 1033: return CHIPSET_500_SERIES_TIGER_POINT; > You nailed it, Nico. […] Turned out the SPI programming guide is wrong and will be updated. But we can rely on the values in the descriptor. I will change this patch accordingly. -- To view, visit https://review.coreboot.org/c/flashrom/+/60711 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I711e39a3ec9cd7098389231eaa1cb864d615a475 Gerrit-Change-Number: 60711 Gerrit-PatchSet: 2 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Tue, 25 Jan 2022 11:08:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c Invert the meaning of 'dual_output' bit
by Werner Zeh (Code Review) 25 Jan '22

25 Jan '22

Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61362 to look at the new patch set (#2). Change subject: ich_descriptors.c Invert the meaning of 'dual_output' bit ...................................................................... ich_descriptors.c Invert the meaning of 'dual_output' bit In the Flash Component description register (FLCOMP) bit 30 reports the capability of using dual output for fast read operation on the flash component. According to various SPI Programming Guides (checked for Panther Point, Lewisburg C620, Apollo Lake and Elkhart Lake) the dual output is enabled when this bit is set and disabled if not. Currently the logic displays it the other way around when parsing the descriptor. This patch changes this so now if bit 30 in FLCOMP is not set, dual read support for fast read operation is shown as disabled. Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com> --- M ich_descriptors.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/61362/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f Gerrit-Change-Number: 61362 Gerrit-PatchSet: 2 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: util/ich_descriptors_tool: Invert the meaning of 'dual_output' bit
by Werner Zeh (Code Review) 25 Jan '22

25 Jan '22

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61362 ) Change subject: util/ich_descriptors_tool: Invert the meaning of 'dual_output' bit ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/61362/comment/527a52c5_999fd5e8 PS1, Line 7: util/ich_descriptors_tool > nit: I'd use `ich_descriptors.c` as file path, the code is also used by flashrom itself. Ack https://review.coreboot.org/c/flashrom/+/61362/comment/a0b240ab_b57e9b06 PS1, Line 11: According to the SPI Programming Guide the dual output is : enabled when this bit is set and disabled if not (checked for Panther : Point, Lewisburg C620, Apollo Lake and Elkhart Lake). > Starting with "According to the SPI Programming Guide" makes it seem like you only checked one SPI p […] Fine with me, will adapt. -- To view, visit https://review.coreboot.org/c/flashrom/+/61362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f Gerrit-Change-Number: 61362 Gerrit-PatchSet: 1 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 25 Jan 2022 11:07:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: util/ich_descriptors_tool: Invert the meaning of 'dual_output' bit
by Angel Pons (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Werner Zeh. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61362 ) Change subject: util/ich_descriptors_tool: Invert the meaning of 'dual_output' bit ...................................................................... Patch Set 1: Code-Review+2 (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/61362/comment/15303ce1_a9734f79 PS1, Line 7: util/ich_descriptors_tool nit: I'd use `ich_descriptors.c` as file path, the code is also used by flashrom itself. https://review.coreboot.org/c/flashrom/+/61362/comment/82c4ced2_4120f43a PS1, Line 11: According to the SPI Programming Guide the dual output is : enabled when this bit is set and disabled if not (checked for Panther : Point, Lewisburg C620, Apollo Lake and Elkhart Lake). Starting with "According to the SPI Programming Guide" makes it seem like you only checked one SPI programming guide. I'd suggest rephrasing the sentence as follows: According to the SPI Programming Guides for Panther Point, Lewisburg C620, Apollo Lake and Elkhart Lake, the dual output is enabled when this bit is set and disabled if not. Another option would be to move the part in parentheses closer to "SPI Programming Guide": According to various SPI Programming Guides (checked for Panther Point, Lewisburg C620, Apollo Lake and Elkhart Lake), the dual output is enabled when this bit is set and disabled if not. Patchset: PS1: I checked the SPI programming guides for Lynx Point, Wildcat Point, Bay Trail, Skylake-H and Gemini Lake; all agree with this change. -- To view, visit https://review.coreboot.org/c/flashrom/+/61362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f Gerrit-Change-Number: 61362 Gerrit-PatchSet: 1 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Tue, 25 Jan 2022 10:15:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Anastasia Klimchuk (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Nicholas Chin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: > > nic3-14159 from #flashrom […] Thank you so much Nicolas for help! -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 25 Jan 2022 07:59:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: util/ich_descriptors_tool: Invert the meaning of 'dual_output' bit
by Werner Zeh (Code Review) 25 Jan '22

25 Jan '22

Werner Zeh has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/61362 ) Change subject: util/ich_descriptors_tool: Invert the meaning of 'dual_output' bit ...................................................................... util/ich_descriptors_tool: Invert the meaning of 'dual_output' bit In the Flash Component description register (FLCOMP) bit 30 reports the capability of using dual output for fast read operation on the flash component. According to the SPI Programming Guide the dual output is enabled when this bit is set and disabled if not (checked for Panther Point, Lewisburg C620, Apollo Lake and Elkhart Lake). Currently the logic displays it the other way around when parsing the descriptor. This patch changes this so now if bit 30 in FLCOMP is not set, dual read support for fast read operation is shown as disabled. Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com> --- M ich_descriptors.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/61362/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 56a3c67..c54e819 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -365,7 +365,7 @@ pprint_freq(cs, desc->component.modes.freq_fastread)); if (cs > CHIPSET_6_SERIES_COUGAR_POINT) msg_pdbg2("Dual Output Fast Read Support: %sabled\n", - desc->component.modes.dual_output ? "dis" : "en"); + desc->component.modes.dual_output ? "en" : "dis"); int has_forbidden_opcode = 0; if (desc->component.FLILL != 0) { -- To view, visit https://review.coreboot.org/c/flashrom/+/61362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6282ac8326ab0b92e9c70c09dba0299bf0deb6f Gerrit-Change-Number: 61362 Gerrit-PatchSet: 1 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Nicholas Chin (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: > nic3-14159 from #flashrom I'm also here on Gerrit :) -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nicholas Chin Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 25 Jan 2022 06:00:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Anastasia Klimchuk (Code Review) 25 Jan '22

25 Jan '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 7: Code-Review-1 (2 comments) Patchset: PS1: > Oh alright I got it. All good, I appreciate your help anyway! […] asked for help on #flashrom and #coreboot Patchset: PS7: nic3-14159 from #flashrom kindly helped me and tested this patch on a machine with ich7 chipset, and found a bug: > "@aklm I ran it through GDB and found that ich_spibar is never set to spibar in the init_ich7_spi codepath, leaving it with the initial NULL value that leads to the segfault. .... Looks like ich_generation is also never changed from CHIPSET_ICH_UNKNOWN in the init_ich7_spi codepath" I give this -1 until I fix. -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 25 Jan 2022 05:29:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2022