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

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

14 Mar '22

Attention is currently required from: Wonkyu Kim, Anastasia Klimchuk. Hello build bot (Jenkins), Wonkyu Kim, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62783 to look at the new patch set (#3). Change subject: ichspi: Add support for Meteor Lake ...................................................................... ichspi: Add support for Meteor Lake This patch adds Meteor Lake support into flashrom. Additionally, utilize CSSO (CPU Soft Strap Offset) to uniquely detect the chipset when the CSSL (CPU Soft Strap Length) field default value (0x03) on Meteor Lake is the same as Elkhart Lake. BUG=b:224325352 TEST=Flashrom is able to detect MTL SPI DID and show chipset name as below: > flashrom --flash-name .... Found chipset "Intel Meteor Lake-P/M". .... > flashrom - internal --ifd -i fd -i bios -r /tmp/bios.rom .... Reading ich_descriptor... done. Assuming chipset 'Meteor Lake'. Using regions: "bios", "fd". Reading flash... done. SUCCESS Note: Meteor Lake doesn't have PCH die hence, dropped `_POINT` postfix from the chipset name. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 --- 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, 38 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/62783/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

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

14 Mar '22

Attention is currently required from: Wonkyu Kim, Anastasia Klimchuk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 1: (5 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62783/comment/663d6a78_99499377 PS1, Line 12: TEST= > Could you also validate the chip detection logic just as we did in https://review.coreboot. […] Ack https://review.coreboot.org/c/flashrom/+/62783/comment/59119531_69249d21 PS1, Line 18: rom : the chipset name. > line wrap. Ack File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62783/comment/2fa3c9ce_731edd78 PS1, Line 118: ", > missing from here too. Ack https://review.coreboot.org/c/flashrom/+/62783/comment/5cd4f213_bcf84b7f PS1, Line 1048: if (content->CSSL == 0x03) { : if (content->CSSO == 0x70) : return CHIPSET_METEOR_LAKE; > Can you elaborate on this part in the commit message to justify this will not regress Elkhart Lake d […] Ack File util/ich_descriptors_tool/ich_descriptors_tool.c: https://review.coreboot.org/c/flashrom/+/62783/comment/496cce27_4ced15ff PS1, Line 130: "\t- \"gemini\" for Intel's Gemini Lake SoC.\n" > missing line here Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 10:39:10 +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]: ichspi: Add support for Meteor Lake
by Subrata Banik (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Wonkyu Kim, Subrata Banik, Anastasia Klimchuk. Hello Wonkyu Kim, build bot (Jenkins), Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62783 to look at the new patch set (#2). Change subject: ichspi: Add support for Meteor Lake ...................................................................... ichspi: Add support for Meteor Lake This patch adds Meteor Lake support into flashrom. Additionally, utilize CSSO (CPU Soft Strap Offset) to uniquely detect the chipset when the CSSL (CPU Soft Strap Length) field default value (0x03) on Meteor Lake is the same as Elkhart Lake. BUG=b:224325352 TEST=Flashrom is able to detect MTL SPI DID and show chipset name as below: > flashrom --flash-name .... Found chipset "Intel Meteor Lake-P/M". .... > flashrom - internal --ifd -i fd -i bios -r /tmp/bios.rom .... Reading ich_descriptor... done. Assuming chipset 'Meteor Lake'. Using regions: "bios", "fd". Reading flash... done. SUCCESS Note: Meteor Lake doesn't have PCH die hence, dropped `_POINT` postfix from the chipset name. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 --- 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, 38 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/62783/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

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

14 Mar '22

Attention is currently required from: Nico Huber. 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 5: (1 comment) Patchset: PS4: > Sorry, I'm a little confused now. Did you actually read the code and the Jasper […] No that was my fault, my dyslexic brain pattern matched the letters EHL to something else. They probably can be combined however I am not sure about guess_ich_chipset_from_content() just yet. Spooling this question until I can get the board maintainers across this. -- 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: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 14 Mar 2022 10:38:41 +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]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan. Hello 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 (#5). Change subject: ichspi: Add Jasper Lake support ...................................................................... ichspi: Add Jasper Lake support BUG=b:221175960 TEST=dedede with `flashrom -p internal --flash-size`. 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, 31 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/62282/5 -- 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: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

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

14 Mar '22

Attention is currently required from: Wonkyu Kim, Subrata Banik, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 1: (2 comments) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62783/comment/b74c4724_948141e3 PS1, Line 118: ", missing from here too. File util/ich_descriptors_tool/ich_descriptors_tool.c: https://review.coreboot.org/c/flashrom/+/62783/comment/77bd6af7_80615a4f PS1, Line 130: "\t- \"gemini\" for Intel's Gemini Lake SoC.\n" missing line here -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 10:24:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

14 Mar '22

Attention is currently required from: Wonkyu Kim, Subrata Banik, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 1: Code-Review+2 (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62783/comment/828a4470_52ff49a8 PS1, Line 12: TEST= Could you also validate the chip detection logic just as we did in https://review.coreboot.org/c/flashrom/+/62251 so things like the `flashrom -p internal --ifd -i fd -i bios -r /tmp/filename.rom` test. Just don't want to repeat anything learnings from that patch? https://review.coreboot.org/c/flashrom/+/62783/comment/4789b378_a273a71c PS1, Line 18: rom : the chipset name. line wrap. Patchset: PS1: Thank you for the patch and upstreaming this work Subrata. File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62783/comment/d04a8d85_1467358f PS1, Line 1048: if (content->CSSL == 0x03) { : if (content->CSSO == 0x70) : return CHIPSET_METEOR_LAKE; Can you elaborate on this part in the commit message to justify this will not regress Elkhart Lake detection logic? Do we have one of those to test with as well? -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 10:20:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Initialize structures with memset
by Light (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62763 ) Change subject: ich_descriptors.c: Initialize structures with memset ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: Yup thanks :) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62763/comment/6b3d168b_0186f93d PS2, Line 1362: struct ich_descriptors desc; > I think this should work: […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/62763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e5bc84c6199a0731db0a9c8ef56f1215686dab2 Gerrit-Change-Number: 62763 Gerrit-PatchSet: 3 Gerrit-Owner: Light <aarya.chaumal(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: Anastasia Klimchuk <aklm(a)chromium.org> 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: Mon, 14 Mar 2022 10:12:02 +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]: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow ...
by Light (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Angel Pons. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 ) Change subject: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow error ...................................................................... Patch Set 6: (2 comments) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62764/comment/9b813e46_e40d3a6d PS3, Line 507: { > Please put this brace on the previous line Done https://review.coreboot.org/c/flashrom/+/62764/comment/040e55ee_2af139eb PS3, Line 508: >= > nit: Please add spaces around `>=` Done -- To view, visit https://review.coreboot.org/c/flashrom/+/62764 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39 Gerrit-Change-Number: 62764 Gerrit-PatchSet: 6 Gerrit-Owner: Light <aarya.chaumal(a)gmail.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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 14 Mar 2022 10:10:10 +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]: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow ...
by Light (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Light. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62764 to look at the new patch set (#6). Change subject: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow error ...................................................................... ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow error Unsigned types show undefined behaviour if they are subtracted by a value greater than their own (mostly it wraps to the max value). Using this value for left shifting could be even more dangerous. Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M ich_descriptors.c 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/62764/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/62764 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39 Gerrit-Change-Number: 62764 Gerrit-PatchSet: 6 Gerrit-Owner: Light <aarya.chaumal(a)gmail.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: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022