Attention is currently required from: Sam McNally, Daniel Campello.
Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52450
to look at the new patch set (#11).
Change subject: cli_classic.c: add -x option for do_extract()
......................................................................
cli_classic.c: add -x option for do_extract()
This change introduces a new option to extract all layout regions to
files with the name of each region (or with the provided filename via
-i region:file). It is implemented by mutating the flash layout to
include all regions and backfilling the entry->file with entry->name
(replacing spaces with underscores)
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff
---
M cli_classic.c
M flash.h
M flashrom.c
M layout.c
M layout.h
5 files changed, 39 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/52450/11
--
To view, visit https://review.coreboot.org/c/flashrom/+/52450
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff
Gerrit-Change-Number: 52450
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Sam McNally, Daniel Campello.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52450 )
Change subject: cli_classic.c: add -x option for do_extract()
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/52450
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff
Gerrit-Change-Number: 52450
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Comment-Date: Fri, 30 Apr 2021 01:24:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Daniel Campello.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52531 )
Change subject: cli_classic.c: implement set_wp_region operation
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/52531
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibad68a038ab38b9986b0d8b5f5eb6c73b20ef381
Gerrit-Change-Number: 52531
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Comment-Date: Fri, 30 Apr 2021 01:23:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Daniel Campello.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52530 )
Change subject: cli_classic.c: reorder writeprotect operation processing
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/52530
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7480d3f947aaaf30093d056226fe0c402763efdc
Gerrit-Change-Number: 52530
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Comment-Date: Fri, 30 Apr 2021 01:22:05 +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 (#15).
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/15
--
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: 15
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: Sam McNally, Daniel Campello.
Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52450
to look at the new patch set (#10).
Change subject: cli_classic.c: add -x option for do_extract()
......................................................................
cli_classic.c: add -x option for do_extract()
This change introduces a new option to extract all layout regions to
files with the name of each region (or with the provided filename via
-i region:file). It is implemented by mutating the flash layout to
include all regions and backfilling the entry->file with entry->name
(replacing spaces with underscores)
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff
---
M cli_classic.c
M flash.h
M flashrom.c
M layout.c
M layout.h
5 files changed, 39 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/52450/10
--
To view, visit https://review.coreboot.org/c/flashrom/+/52450
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff
Gerrit-Change-Number: 52450
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: newpatchset
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 (#14).
Change subject: cli_classic: Make -r/-w/-v argument optional
......................................................................
cli_classic: 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/14
--
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: 14
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: Sam McNally, Daniel Campello.
Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52450
to look at the new patch set (#9).
Change subject: cli_classic.c: add -x option for do_extract()
......................................................................
cli_classic.c: add -x option for do_extract()
This change introduces a new option to extract all layout regions to
files with the name of each region (or with the provided filename via
-i region:file). It is implemented by mutating the flash layout to
include all regions and backfilling the entry->file with entry->name
(replacing spaces with underscores)
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff
---
M cli_classic.c
M flash.h
M flashrom.c
M layout.c
M layout.h
5 files changed, 39 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/52450/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/52450
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8c69223fa92cf5b50abe070f1ab9f19d3b42f6ff
Gerrit-Change-Number: 52450
Gerrit-PatchSet: 9
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.
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:
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/51487/comment/895ea4b0_adbc9c8d
PS6, Line 9: hwaccess_io.h
: because those are needed to test lifecycle of mec1308.c
> Do you mean you would like to see more tests that are using mocked i/o, to ensure the approach works […]
I added test for ene_lpc.c, to verify the io mocking framework is working for different drivers, and yes it is! Moreover, ene's state is more complicated, it needs to track two values. I picked ene almost at random, but it is actually a good example that framework can deal with different states.
Patchset:
PS1:
> flashbuses_to_text_test_success() was only introduced to get the ball rolling on starting to think a […]
I would keep them until I have a better replacement, what do you think?
Patchset:
PS10:
And now we have very cool io mocking framework, and two tests that are using it!
This definitely needs to be split into 3 patches (framework, mec test, ene test) but I thought it would be good to have a first look at the whole picture.
Also thank you to everyone!
File tests/tests.c:
https://review.coreboot.org/c/flashrom/+/51487/comment/d2e996c9_8f09c4c2
PS6, Line 88: /*
: * Uncomment LOG_ME below for verbose logging.
: * It is commented by default because of too many calls.
: */
: /* LOG_ME; */
> So far I was able to print everything or nothing, but I will look more into this. […]
I haven't done anything with verbosity yet, a general question: is it possible to leave this for the next patch? I fully understand this comment is not a final solution!
what do you think?
https://review.coreboot.org/c/flashrom/+/51487/comment/1f7c7fef_d1b58928
PS6, Line 102: return ((port == 0x2e /* MEC1308_SIO_PORT1 */
: || port == 0x4e /* MEC1308_SIO_PORT2 */)
: ? 0x20 /* MEC1308_DEVICE_ID_REG */
: : ((outb_val == 0x84 /* MEC1308_MBX_DATA_START */)
: ? 0xaa /* MEC1308_CMD_PASSTHRU_SUCCESS */
: : 0));
> So, CB:52604 is what I had in mind. Feel free to take it over if […]
This is awesome, and it works! Thank you!
--
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-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-Comment-Date: Fri, 30 Apr 2021 00:23:14 +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>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
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/+/51487
to look at the new patch set (#10).
Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
......................................................................
tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
This patch includes mocks for io operations in hwaccess_x86_io.h
because those are needed to test lifecycle of mec1308.c and
ene_lpc.c
BUG=b:181803212
TEST=builds and ninja test
Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
A hwaccess_x86_io_unittest.h
M meson.build
M tests/init_shutdown.c
A tests/io_mock.h
M tests/meson.build
M tests/tests.c
M tests/tests.h
7 files changed, 246 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/51487/10
--
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-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