May 2023 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: flashrom.c:Enable erase path optimisation
by Aarya (Code Review) 02 May '23

02 May '23

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Anastasia Klimchuk. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 ) Change subject: flashrom.c:Enable erase path optimisation ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > How else can we check if the region is write protected from this erase path. […] Currently we are checking for wp from region->write_prot but that only works for BUS_PROG. For all else we attempt to erase and the failure of the erase function tells us something is wrong. This makes it difficult to figure out the reason for failure especially if its because the region is wp since the code already does the checks for writable (the first case) before attempting to erase. I feel there should be get_region for all programmers, if possible :) -- To view, visit https://review.coreboot.org/c/flashrom/+/74872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Gerrit-Change-Number: 74872 Gerrit-PatchSet: 3 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 02 May 2023 17:51:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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]: erasure_layout.c: Fix incorrect return value in erase_write
by Aarya (Code Review) 02 May '23

02 May '23

Aarya has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74882 ) Change subject: erasure_layout.c: Fix incorrect return value in erase_write ...................................................................... erasure_layout.c: Fix incorrect return value in erase_write On failure of erasefn in erase_write it didn't set the error value in ret which caused send success status as return value. Change-Id: Ia3bd5fd250dcd0a03f0281c478b9bacb71872f31 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M erasure_layout.c 1 file changed, 17 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/74882/1 diff --git a/erasure_layout.c b/erasure_layout.c index a802ae4..108fea4 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -328,9 +328,12 @@ addr, addr + len - 1); free(region.name); - if (erasefn(flashctx, addr, len)) + if (erasefn(flashctx, addr, len)) { + ret = -1; goto _end; + } if (check_erased_range(flashctx, addr, len)) { + ret = - 1; msg_cerr("ERASE FAILED!\n"); goto _end; } -- To view, visit https://review.coreboot.org/c/flashrom/+/74882 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia3bd5fd250dcd0a03f0281c478b9bacb71872f31 Gerrit-Change-Number: 74882 Gerrit-PatchSet: 1 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom.c:Enable erase path optimisation
by Aarya (Code Review) 02 May '23

02 May '23

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Anastasia Klimchuk. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 ) Change subject: flashrom.c:Enable erase path optimisation ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > I don't know what the bug is however it seems to indicate something is wrong in the new path `get_re […] How else can we check if the region is write protected from this erase path. The region->write_prot was supposed to do that I guess. -- To view, visit https://review.coreboot.org/c/flashrom/+/74872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Gerrit-Change-Number: 74872 Gerrit-PatchSet: 2 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 02 May 2023 15:48:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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]: doc: Add doc how to add docs
by Alexander Goncharov (Code Review) 02 May '23

02 May '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74225 ) Change subject: doc: Add doc how to add docs ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: I understand that this is only an initial version of the document, but I think that it's important to write about the next things: * copyright (Under what license will the documentation be published? What sources of information can be used? etc) * writing style - formal or informal? jokes? are pictures allowed? * only flashrom related documentation? or doc about general things (ROM/EEPROM, bitbang, chip packages and so on) can be placed there? -- To view, visit https://review.coreboot.org/c/flashrom/+/74225 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I356a286ad2f3334392efadda366b0ca0f8042752 Gerrit-Change-Number: 74225 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 02 May 2023 15:38:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Convert README to sphinx
by Alexander Goncharov (Code Review) 02 May '23

02 May '23

Attention is currently required from: Thomas Heijligen, Stefan Reinauer, Edward O'Callaghan, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74216 ) Change subject: doc: Convert README to sphinx ...................................................................... Patch Set 5: (19 comments) Patchset: PS5: The "Build Instructions" section looks like a wall of text. It's difficult to read and navigate. Plain README used indentation for formatting, but in our case, let's use _real_ formatting - my comments suggest using **bold** format because I found it most appropriate File README.rst: https://review.coreboot.org/c/flashrom/+/74216/comment/85c08d22_600fedee PS5, Line 26: flashrom supports building with make and meson. ``` flashrom supports building with **make** and **meson**. ``` https://review.coreboot.org/c/flashrom/+/74216/comment/4d487ef7_fc5d9408 PS5, Line 38: To build flashrom you need to install the following software: ``` **To build flashrom you need to install the following software:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/4ccfd0e0_43513562 PS5, Line 48: Linux et al: ``` **Linux et al:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/1fa460ba_6d34d4ff PS5, Line 54: On FreeBSD, you need the following ports: ``` **On FreeBSD, you need the following ports:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/89e35e1a_9270c587 PS5, Line 59: On OpenBSD, you need the following ports: ``` **On OpenBSD, you need the following ports:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/0708c7be_0b5f155a PS5, Line 64: To compile on Linux, use:: ``` **To compile on Linux, use**:: ``` https://review.coreboot.org/c/flashrom/+/74216/comment/326ed1d3_f4523392 PS5, Line 68: To compile on FreeBSD, OpenBSD or DragonFly BSD, use:: ``` **To compile on FreeBSD, OpenBSD or DragonFly BSD, use**:: ``` https://review.coreboot.org/c/flashrom/+/74216/comment/a451613b_57e8a47b PS5, Line 72: To compile on Nexenta, use:: ``` **To compile on Nexenta, use**:: ``` https://review.coreboot.org/c/flashrom/+/74216/comment/ce212de2_40046553 PS5, Line 76: To compile on Solaris, use:: ``` **To compile on Solaris, use**:: ``` https://review.coreboot.org/c/flashrom/+/74216/comment/837a4add_ee3fdcf5 PS5, Line 80: To compile on NetBSD (with pciutils, libftdi, libusb installed in /usr/pkg/), use:: ``` **To compile on NetBSD (with pciutils, libftdi, libusb installed in /usr/pkg/), use**:: ``` https://review.coreboot.org/c/flashrom/+/74216/comment/0b66e236_6ae5b8ed PS5, Line 84: To compile and run on Darwin/Mac OS X: ``` **To compile and run on Darwin/Mac OS X:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/c9982ba4_2a1b99ba PS5, Line 89: To cross-compile on Linux for DOS: ``` **To cross-compile on Linux for DOS**: ``` https://review.coreboot.org/c/flashrom/+/74216/comment/f25539d6_0e12f19b PS5, Line 138: To cross-compile on Linux for Windows: ``` **To cross-compile on Linux for Windows:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/b47b73ef_f1482d5f PS5, Line 152: Processor architecture dependent features: ``` **Processor architecture dependent features:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/e036847c_1d69538a PS5, Line 158: Compiler quirks: ``` **Compiler quirks:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/ff14ad42_0950dc7e PS5, Line 164: Bindings: ``` **Bindings:** ``` https://review.coreboot.org/c/flashrom/+/74216/comment/1ab6671d_0f854486 PS5, Line 197: 'make export' ``` ``make export`` ``` https://review.coreboot.org/c/flashrom/+/74216/comment/62ccec29_1a2015f5 PS5, Line 203: 'make tarball' ``` ``make tarball`` ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/74216 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e0debc0e15a9c4866f6d16fb010cd3f13171ff3 Gerrit-Change-Number: 74216 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 02 May 2023 15:12:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: libflashrom: Allow getting the progress_state from the flashctx
by Richard Hughes (Code Review) 02 May '23

02 May '23

Attention is currently required from: Nico Huber, Richard Hughes, Edward O'Callaghan, Angel Pons, Daniel Campello. Hello build bot (Jenkins), Nico Huber, Sean Rhodes, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Daniel Campello, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/64663 to look at the new patch set (#3). Change subject: libflashrom: Allow getting the progress_state from the flashctx ...................................................................... libflashrom: Allow getting the progress_state from the flashctx External users of the libflashrom API cannot peek into the abstract flashrom_flashctx structure, unlike the local CLI. Provide some API to get the flashrom_progress struct from a flashrom_flashctx. This allows fwupd to use the new flashrom_set_progress_callback() API without using a global variable with the callback data. Tested with fwupd in https://github.com/fwupd/fwupd/pull/4675 Change-Id: I322bf56ff92f7b4d0ffc92768e9f0cdf7cb82010 Signed-off-by: Richard Hughes <richard(a)hughsie.com> --- M cli_output.c M include/libflashrom.h M libflashrom.c M libflashrom.map 4 files changed, 36 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/64663/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/64663 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I322bf56ff92f7b4d0ffc92768e9f0cdf7cb82010 Gerrit-Change-Number: 64663 Gerrit-PatchSet: 3 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.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: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c:Enable erase path optimisation
by Edward O'Callaghan (Code Review) 02 May '23

02 May '23

Attention is currently required from: Aarya, Nikolai Artemiev, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 ) Change subject: flashrom.c:Enable erase path optimisation ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > This change breaks the unit test for full chip erase when write protect is enabled (https://ticket. […] I don't know what the bug is however it seems to indicate something is wrong in the new path `get_region` not being set I don't think is the root cause of anything, I would expect that to be `NULL` for most drivers. -- To view, visit https://review.coreboot.org/c/flashrom/+/74872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Gerrit-Change-Number: 74872 Gerrit-PatchSet: 2 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 02 May 2023 02:00:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c:Enable erase path optimisation
by Aarya (Code Review) 01 May '23

01 May '23

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Anastasia Klimchuk. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 ) Change subject: flashrom.c:Enable erase path optimisation ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: This change breaks the unit test for full chip erase when write protect is enabled (https://ticket.coreboot.org/issues/488). I tracked the reason to be get_flash_region in flashrom.c:372 where flash->mst->spi.get_region has no value assigned so the final else block is being executed instead of the BUS_SPI one. -- To view, visit https://review.coreboot.org/c/flashrom/+/74872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Gerrit-Change-Number: 74872 Gerrit-PatchSet: 2 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 01 May 2023 13:00:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c:Enable erase path optimisation
by Aarya (Code Review) 01 May '23

01 May '23

Attention is currently required from: Aarya. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/74872 to look at the new patch set (#2). Change subject: flashrom.c:Enable erase path optimisation ...................................................................... flashrom.c:Enable erase path optimisation Set the use_legacy_erase_path flag to flase to enable erase path optmisatoptimisation by default. Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c 1 file changed, 14 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/74872/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/74872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Gerrit-Change-Number: 74872 Gerrit-PatchSet: 2 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c: Enable erase path optimisation
by Aarya (Code Review) 01 May '23

01 May '23

Aarya has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74872 ) Change subject: flashrom.c: Enable erase path optimisation ...................................................................... flashrom.c: Enable erase path optimisation Set the use_legacy_erase_path flag to flase to enable erase path optmisatoptimisation by default. Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c 1 file changed, 14 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/74872/1 diff --git a/flashrom.c b/flashrom.c index 07ed0df..7b0ceed 100644 --- a/flashrom.c +++ b/flashrom.c @@ -35,7 +35,7 @@ #include "chipdrivers.h" #include "erasure_layout.h" -static bool use_legacy_erase_path = true; +static bool use_legacy_erase_path = false; const char flashrom_version[] = FLASHROM_VERSION; -- To view, visit https://review.coreboot.org/c/flashrom/+/74872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Gerrit-Change-Number: 74872 Gerrit-PatchSet: 1 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2023