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

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Nico Huber (Code Review) 26 Mar '22

26 Mar '22

Attention is currently required from: Felix Singer, Light, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62747 ) Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc ...................................................................... Patch Set 12: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62747/comment/779a76fd_4f8b23f6 PS9, Line 11: when later used in need_erase could result in undefined behaviour. > Maybe intitializing them with zeroes using calloc and then onlycopying "flashrom bug!" once should b […] That would depend much on the kind of bug we are trying to catch, i.e. do we expect flashrom could accidentally write the first bytes of a chip or accidentally write any bytes? -- To view, visit https://review.coreboot.org/c/flashrom/+/62747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab Gerrit-Change-Number: 62747 Gerrit-PatchSet: 12 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> 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: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 26 Mar 2022 19:28:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow ...
by Nico Huber (Code Review) 26 Mar '22

26 Mar '22

Attention is currently required from: Angel Pons, Light, Anastasia Klimchuk. Nico Huber 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 12: Code-Review+1 -- 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: 12 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 26 Mar 2022 19:24:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow ...
by Nico Huber (Code Review) 26 Mar '22

26 Mar '22

Attention is currently required from: Angel Pons, Light, Anastasia Klimchuk. Nico Huber 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 12: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62764/comment/8a2a5728_fe226634 PS6, Line 507: assert(j >= 12); > One difference that I see, is for Sunrise Point: `num_regions` is 10 (assigned at line 492 in curren […] If `num_regions` is 10, the second loop never runs. This doesn't change because `j = 12;` can only increase `j`. -- 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: 12 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 26 Mar 2022 19:24:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips.c: Fix voltage ranges for MX23L1654 and MX23L6454
by Felix Singer (Code Review) 26 Mar '22

26 Mar '22

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/63124 ) Change subject: flashchips.c: Fix voltage ranges for MX23L1654 and MX23L6454 ...................................................................... flashchips.c: Fix voltage ranges for MX23L1654 and MX23L6454 An user reported incorrect voltage ranges for the following flash chips: * MX23L1654 * MX23L6454 Currently, their low end is at 3.0V, while 2.7V is allowed according to their datasheets. Thus, fix their voltage ranges. Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: I91dd6058d5951868b154c8bc8818cb0c412f2499 --- M flashchips.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/63124/1 diff --git a/flashchips.c b/flashchips.c index acb7718..806d010 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8279,7 +8279,7 @@ .probe_timing = TIMING_ZERO, .write = NULL, /* MX23L1654 is a mask ROM, so it is read-only */ .read = spi_chip_read, /* Fast read (0x0B) supported */ - .voltage = {3000, 3600}, + .voltage = {2700, 3600}, }, { @@ -8311,7 +8311,7 @@ .probe_timing = TIMING_ZERO, .write = NULL, /* MX23L6454 is a mask ROM, so it is read-only */ .read = spi_chip_read, /* Fast read (0x0B) supported */ - .voltage = {3000, 3600}, + .voltage = {2700, 3600}, }, { -- To view, visit https://review.coreboot.org/c/flashrom/+/63124 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I91dd6058d5951868b154c8bc8818cb0c412f2499 Gerrit-Change-Number: 63124 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow ...
by Anastasia Klimchuk (Code Review) 26 Mar '22

26 Mar '22

Attention is currently required from: Nico Huber, Angel Pons, Light. Anastasia Klimchuk 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 12: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62764/comment/9496c0c9_d2e02f1e PS12, Line 7: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow error We need to fit this into 72 char, and it is 74 at the moment :) First of all, the latest version does not assert anything, so maybe say "ensure" instead of "assert"? Also maybe drop the last word (error). I think "to prevent underflow" is enough. And then it will fit into char limit. -- 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: 12 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Sat, 26 Mar 2022 08:21:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow ...
by Anastasia Klimchuk (Code Review) 26 Mar '22

26 Mar '22

Attention is currently required from: Nico Huber, Angel Pons, Light. Anastasia Klimchuk 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 12: Code-Review+1 (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62764/comment/716ae958_c48dbf18 PS6, Line 507: assert(j >= 12); > Yeah I am okay with making it explicit :) One difference that I see, is for Sunrise Point: `num_regions` is 10 (assigned at line 492 in current patchset). So in initial version the loop starts with j=10, after this patch it starts with j=12. I understand about underflow and undefined behaviour, looks like it's correct to start loop with j=12. I am curious, what was printed for Sunrise Point for j=10,11 , was that printing garbage? Otherwise, looks good! What do you think Nico? -- 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: 12 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Sat, 26 Mar 2022 06:50:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: board_enable.c: Remove unnecessary assignment
by Anastasia Klimchuk (Code Review) 26 Mar '22

26 Mar '22

Attention is currently required from: Felix Singer, Idwer Vollering, Angel Pons, Light. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 ) Change subject: board_enable.c: Remove unnecessary assignment ...................................................................... Patch Set 10: Code-Review+2 (1 comment) Patchset: PS10: Keith, would you like your name added as Tested-by tag? (sorry I only now thought about it, should have asked earlier). -- To view, visit https://review.coreboot.org/c/flashrom/+/62761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc Gerrit-Change-Number: 62761 Gerrit-PatchSet: 10 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Sat, 26 Mar 2022 04:32:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add support for Meteor Lake
by Wonkyu Kim (Code Review) 25 Mar '22

25 Mar '22

Attention is currently required from: Wonkyu Kim, Subrata Banik, Angel Pons, Anastasia Klimchuk. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 8: Code-Review+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: 8 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: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 25 Mar 2022 20:57:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.8.tmpl: document lspcon_i2c_spi
by Edward O'Callaghan (Code Review) 25 Mar '22

25 Mar '22

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63104 ) Change subject: flashrom.8.tmpl: document lspcon_i2c_spi ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/63104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9ccb9694fdea29e68f062cc049efc0204917a139 Gerrit-Change-Number: 63104 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 25 Mar 2022 06:03:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips.c: add writeprotect support for more chips
by Nikolai Artemiev (Code Review) 25 Mar '22

25 Mar '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62214 ) Change subject: flashchips.c: add writeprotect support for more chips ...................................................................... Patch Set 26: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62214/comment/2706778b_f4f909b7 PS26, Line 10: Chips I wasn't able to test were just : checked against the datasheets. > Could a EM100 be leveraged here? I just tried it, it would have been a great way to test but it appears that SR2 commands are being intrepreted incorrectly, e.g. 0x35 should be read sr2 and 0x31 should be write sr2. ``` em100 -c GD25Q64 --stop -t --start` ... 0x35 - enter quad I/O mode ... 0x31 - unknown command ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/62214 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7f3d4c4148056098a845b5c64308b0333ebda395 Gerrit-Change-Number: 62214 Gerrit-PatchSet: 26 Gerrit-Owner: Nikolai Artemiev <nartemiev(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: 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 25 Mar 2022 05:51:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022