January 2023 - flashrom-gerrit

[L] Change in flashrom[master]: flashrom.c:Add helper functions needed for new erase selction algorithm
by Anastasia Klimchuk (Code Review) 19 Jan '23

19 Jan '23

Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Aarya. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65844 ) Change subject: flashrom.c:Add helper functions needed for new erase selction algorithm ...................................................................... Patch Set 80: Code-Review+1 (8 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/65844/comment/8bf9aead_02a5757b PS80, Line 7: flashrom.c:Add helper functions needed for new erase selction algorithm There are few small things (missing space and one typo), having in mind 72 char limit I suggest to rephrase: flashrom.c: Add helper functions for new erase selection algorithm https://review.coreboot.org/c/flashrom/+/65844/comment/37bc6124_a6f9fbc2 PS80, Line 9: 1)Add a function Add space between number and text (and same for below). Patchset: PS78: > This patch already contains the code from CB:65879. […] Looks like this comment resolved? Edward what do you think? I see this patch is end result of squashing 3 earlier patches. Patchset: PS80: I have few minor style things. Aarya, thank you so much for your work! We are almost there! :) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/65844/comment/c7413505_238178b1 PS80, Line 115: Remove one empty line (there are two empty lines in a row) https://review.coreboot.org/c/flashrom/+/65844/comment/1d93b7b1_ea77511a PS80, Line 173: !! I think one exclamation mark is enough :) (and same below) https://review.coreboot.org/c/flashrom/+/65844/comment/f09ac04e_f2917350 PS80, Line 198: if (ll->start_addr >= rstart && : ll->end_addr <= rend) { This can be one line (will be more readable) https://review.coreboot.org/c/flashrom/+/65844/comment/81d95fe9_9f3b5592 PS80, Line 219: if (ll->start_addr >= rstart && : ll->end_addr <= rend) { Here too: make this one line, for better readability -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 80 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Jan 2023 07:32:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom_tester: Rename lock_test hwwp_locks_swwp_test
by Evan Benn (Code Review) 19 Jan '23

19 Jan '23

Attention is currently required from: Peter Marheine. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71972 ) Change subject: flashrom_tester: Rename lock_test hwwp_locks_swwp_test ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/71972/comment/73dc2068_09cba1f6 PS2, Line 7: simplify > Only renaming it now. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/71972 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6080622755ff16d8fba7044b38f9e09db0c62f97 Gerrit-Change-Number: 71972 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 19 Jan 2023 02:20:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: flashrom_tester: Rewrite IOOpts to support more operations
by Evan Benn (Code Review) 19 Jan '23

19 Jan '23

Attention is currently required from: Peter Marheine. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71973 ) Change subject: flashrom_tester: Rewrite IOOpts to support more operations ...................................................................... Patch Set 6: (1 comment) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/71973/comment/e65b9876_feb878f1 PS6, Line 73: Option > This still seems weird, in that it's required to specify either a file or a region with some file. […] your 3 cases are not quite right, these are the two modes flashrom accepts: ``` a file and an array of regions with optional files a region with a file, and an array of regions with optional files ``` or reworded like yours ``` One file, entire chip Multiple files, one per part of chip (+ optional whole chip sized file with part of chip) (all but one file is optional) ``` I only ever have one region, so: ``` a file and an optional region with optional file a region with a file ``` But we only use these: ``` a file a file and a region a region with a file ``` I will try the last one, it makes the output logic a little more complex -- To view, visit https://review.coreboot.org/c/flashrom/+/71973 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1cb46bb1b26949fd9c19949c43708a8b652e00da Gerrit-Change-Number: 71973 Gerrit-PatchSet: 6 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 19 Jan 2023 02:20:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Add positive check to verify_fail_test
by Evan Benn (Code Review) 19 Jan '23

19 Jan '23

Attention is currently required from: Peter Marheine. Hello build bot (Jenkins), Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71974 to look at the new patch set (#8). Change subject: flashrom_tester: Add positive check to verify_fail_test ...................................................................... flashrom_tester: Add positive check to verify_fail_test In verify_fail_test test that verify works when expected, as well as fails when expected. A verify_region_from_file function is added to support this. BUG=b:235916336 BRANCH=None TEST=None Change-Id: Ibbcc97086466b67cfab4f6c32140bb5f2c456beb Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/src/cmd.rs M util/flashrom_tester/flashrom/src/flashromlib.rs M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/tests.rs 4 files changed, 68 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/71974/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/71974 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibbcc97086466b67cfab4f6c32140bb5f2c456beb Gerrit-Change-Number: 71974 Gerrit-PatchSet: 8 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: flashrom_tester: Rewrite IOOpts to support more operations
by Evan Benn (Code Review) 19 Jan '23

19 Jan '23

Attention is currently required from: Evan Benn. Hello build bot (Jenkins), Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71973 to look at the new patch set (#7). Change subject: flashrom_tester: Rewrite IOOpts to support more operations ...................................................................... flashrom_tester: Rewrite IOOpts to support more operations flashrom cli supports include regions for all of read write and verify, as well as omitting the read/write/verify file if an include region with file is specified. Use an enum to allow only one operation at a time. Unify the read and write region implementations. BUG=b:235916336 BRANCH=None TEST=None Change-Id: I1cb46bb1b26949fd9c19949c43708a8b652e00da Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/src/cmd.rs M util/flashrom_tester/flashrom/src/flashromlib.rs M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/tests.rs 4 files changed, 141 insertions(+), 134 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/71973/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/71973 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1cb46bb1b26949fd9c19949c43708a8b652e00da Gerrit-Change-Number: 71973 Gerrit-PatchSet: 7 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom_tester: Rename lock_test hwwp_locks_swwp_test
by Evan Benn (Code Review) 19 Jan '23

19 Jan '23

Attention is currently required from: Evan Benn. Hello build bot (Jenkins), Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71972 to look at the new patch set (#3). Change subject: flashrom_tester: Rename lock_test hwwp_locks_swwp_test ...................................................................... flashrom_tester: Rename lock_test hwwp_locks_swwp_test Rename lock_test to hwwp_locks_swwp_test. BUG=b:235916336 BRANCH=None TEST=None Change-Id: I6080622755ff16d8fba7044b38f9e09db0c62f97 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/src/tests.rs 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/71972/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/71972 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6080622755ff16d8fba7044b38f9e09db0c62f97 Gerrit-Change-Number: 71972 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
by Nikolai Artemiev (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69517 ) Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking ...................................................................... Patch Set 12: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/69517/comment/d1533372_430f9d59 PS10, Line 1975: ) > ``` […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/69517 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f Gerrit-Change-Number: 69517 Gerrit-PatchSet: 12 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 18 Jan 2023 23:38:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
by Nikolai Artemiev (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Nikolai Artemiev. Nikolai Artemiev has uploaded a new patch set (#12) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/69517 ) Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking ...................................................................... flashrom.c: Supplement `chip->unlock()` calls with wp unlocking The full writeprotect implementation has proper support and ability to unlock flash over spi25_statusreg.c. Therefore if the required bits are available for the given chip prefer proper writeprotect support instead of adhoc spi25_statusreg.c helpers. BUG=b:237485865 BRANCH=none TEST=Tested on grunt DUT (prog: sb600spi, flash: W25Q128.W): `flashrom --wp-range 0x0,0x1000000 \ flashrom --wp-status # Result: range=0x0,0x1000000 \ flashrom -w random.bin # Result: success \ flashrom -v random.bin # Result: success \ flashrom --wp-status # Result: range=0x0,0x1000000` TEST=Tested that chips without WP support can still be unlocked by deleting decode_range for W25Q128.W flashchip and retesting on the grunt DUT. Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f CoAuthored-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 82 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/69517/12 -- To view, visit https://review.coreboot.org/c/flashrom/+/69517 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f Gerrit-Change-Number: 69517 Gerrit-PatchSet: 12 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[1.3.x]: tests: Fix warnings for 32-bit Linux
by Anastasia Klimchuk (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Branden Waldner, Angel Pons, Evan Benn, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71285 ) Change subject: tests: Fix warnings for 32-bit Linux ...................................................................... Patch Set 3: Verified+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71285 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I9ec5d37cc038171afc67a69ea9a6885deb8fa4a8 Gerrit-Change-Number: 71285 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Branden Waldner <scruffy99(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Branden Waldner <scruffy99(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Wed, 18 Jan 2023 23:18:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[1.3.x]: tests: Fix warnings for 32-bit Linux
by Anastasia Klimchuk (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Branden Waldner, Angel Pons, Evan Benn, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71285 ) Change subject: tests: Fix warnings for 32-bit Linux ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71285 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I9ec5d37cc038171afc67a69ea9a6885deb8fa4a8 Gerrit-Change-Number: 71285 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Branden Waldner <scruffy99(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Branden Waldner <scruffy99(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Wed, 18 Jan 2023 23:18:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023