Attention is currently required from: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
Patchset:
PS4:
I added some reviewers Richard to help try and make progress with libflashrom improvements.
It maybe helpful though to split this into two portions. The first is the framework and test (thanks for writing that!) and the second which hooks up the various programmers.
A little more detail in the commit messages for such a large diff may also invoke shorter review time? Although I'll defer to the communities views on precisely the things they would like to see to help things get reviewed?
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 4
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: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Richard Hughes <hughsient(a)gmail.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 04 May 2021 03:32:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Daniel Campello.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52876 )
Change subject: flashrom_tester: update syntax for --wp-range
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/52876
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If17a40fba1f7d41e09e0163b353d1602c215c8db
Gerrit-Change-Number: 52876
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Comment-Date: Tue, 04 May 2021 03:12:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52364
to look at the new patch set (#3).
Change subject: programmer.h,chipset_enable.c: Make penable cb more descript
......................................................................
programmer.h,chipset_enable.c: Make penable cb more descript
The enable_flash callback function pointer embedded within
the penable struct is used by the chipset_enable entry-point
to dispatch to the correct concrete implementation. The handles
have the canonical form `flash_enable_<chipset-name>()` and
therefore `doit()` is hard to comprehend while groking code
and seeing to what it refers to. It is also a little more
confusing as there use to be another doit() in flashrom
once upon a time.
BUG=none
TEST=builds
Change-Id: I8b3a4ec51ea488c40c20ccafea883f8dea93577d
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M chipset_enable.c
M programmer.h
2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/52364/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/52364
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8b3a4ec51ea488c40c20ccafea883f8dea93577d
Gerrit-Change-Number: 52364
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Daniel Campello, Peter Marheine.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52876 )
Change subject: flashrom_tester: update syntax for --wp-range
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/52876
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If17a40fba1f7d41e09e0163b353d1602c215c8db
Gerrit-Change-Number: 52876
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Tue, 04 May 2021 03:05:03 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52362
to look at the new patch set (#16).
Change subject: cli_classic.c: Make -r/-w/-v argument optional
......................................................................
cli_classic.c: Make -r/-w/-v argument optional
Makes filename optional from -r/-w/-v since the -i parameter allows
building the image to be written from multiple files, and regions to be
read from flash and written to separate image files,
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
---
M cli_classic.c
M flashrom.c
2 files changed, 86 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/52362/16
--
To view, visit https://review.coreboot.org/c/flashrom/+/52362
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41
Gerrit-Change-Number: 52362
Gerrit-PatchSet: 16
Gerrit-Owner: Daniel Campello <campello(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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Simon Glass.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 )
Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
......................................................................
Patch Set 10:
(2 comments)
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/51487/comment/334bd496_cae2d50c
PS10, Line 53: MEC1308_SIO_PORT1
> Well you can move them to a header file
Yes, although that would mean including more header files here ... but maybe this is fine?
In any case, that would be in a separate patch.
What do other people think about it? And if yes, would you prefer do that separate patch before or after this one?
File tests/tests.c:
https://review.coreboot.org/c/flashrom/+/51487/comment/13e1d1b3_8ed2075e
PS10, Line 92: if (current_io && current_io->outb)
> OK, so do you mean to allow current_io to be NULL as well?
Yes! If a test wants io mocking, it sets current_io before executing whatever it is testing, and then at the end cleans up and sets current_io NULL (you can have a look at init_shutdown.c).
So by default when test starts execution, current_io is NULL and if "do nothing" mocking is sufficient then nothing to do, NULL means "do nothing".
--
To view, visit https://review.coreboot.org/c/flashrom/+/51487
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc
Gerrit-Change-Number: 51487
Gerrit-PatchSet: 10
Gerrit-Owner: 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-CC: Simon Glass <sjg(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Simon Glass <sjg(a)chromium.org>
Gerrit-Comment-Date: Tue, 04 May 2021 02:07:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Simon Glass <sjg(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Samir Ibradžić.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 )
Change subject: ft2232_spi.c: Implement spi_send_multicommand()
......................................................................
Patch Set 32:
(1 comment)
Patchset:
PS32:
> * I´m running flashrom on a virutal machine (don´t think this makes any difference)
This explains it (unless you have passed the whole USB controller
into the VM). Emulated USB controllers cause a high latency for
individual packets. The dediprog driver is also optimized to avoid
waiting on packets (at least for reading) because I use it much with
virtualbox.
> NB: yepp, I noticed that with the erase block size, as well and have it on my todo list. I was thinking about something like comandline parameter to select block size or an extra parameter at flashchips.c to mark the preferred block size. May be we should open a new patch for that to discuss ideas... (it was just because of lack of support for this patch for month why I did not step further)
Giving a hint on the command line might work. The "preferred" block
size depends on the writing pattern, i.e. how big are the changed
chunks compared to current flash contents. So that's nothing to
predict.
--
To view, visit https://review.coreboot.org/c/flashrom/+/40477
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
Gerrit-Change-Number: 40477
Gerrit-PatchSet: 32
Gerrit-Owner: Simon Buhrow
Gerrit-Reviewer: Alan Green <avg(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: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-Comment-Date: Mon, 03 May 2021 16:20:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Simon Buhrow
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment