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

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Nico Huber (Code Review) 04 May '21

04 May '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Simon Glass. Nico Huber 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: (1 comment) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/51487/comment/7845e73b_4e58eb98 PS10, Line 53: MEC1308_SIO_PORT1 > Just to add one comment, adding tests to code that has not been written with testing in mind is ofte […] It depends much on what one wants to test. Sharing the numbers with the tests implies _not_ testing if flashrom (still) uses the correct numbers. I don't know much about designing code for testing. Pulling such definitions out of their compilation unit makes it easier to write tests I suppose. But it also makes it harder to read/review the code. -- 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Simon Glass <sjg(a)chromium.org> Gerrit-Comment-Date: Tue, 04 May 2021 16:06:55 +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

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Daniel Campello (Code Review) 04 May '21

04 May '21

Attention is currently required from: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Daniel Campello 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 6: Code-Review+1 -- 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: 6 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 15:52:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom_tester: update syntax for --wp-range
by Daniel Campello (Code Review) 04 May '21

04 May '21

Attention is currently required from: Paul Menzel. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52876 ) Change subject: flashrom_tester: update syntax for --wp-range ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52876/comment/a03cfab9_39a64984 PS1, Line 12: > Nit: Please remove the blank line. Done -- 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: 2 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 04 May 2021 15:50:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom_tester: update syntax for --wp-range
by Daniel Campello (Code Review) 04 May '21

04 May '21

Attention is currently required from: Daniel Campello. Hello build bot (Jenkins), Edward O'Callaghan, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52876 to look at the new patch set (#2). Change subject: flashrom_tester: update syntax for --wp-range ...................................................................... flashrom_tester: update syntax for --wp-range TEST=cargo test Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: If17a40fba1f7d41e09e0163b353d1602c215c8db --- M util/flashrom_tester/flashrom/src/cmd.rs 1 file changed, 4 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/52876/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: 2 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Simon Glass (Code Review) 04 May '21

04 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Simon Glass 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: (1 comment) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/51487/comment/9b9d3675_6c5fbf9c PS10, Line 53: MEC1308_SIO_PORT1 > Yes, although that would mean including more header files here ... but maybe this is fine? […] Just to add one comment, adding tests to code that has not been written with testing in mind is often going to involve refactoring, I think. In general, I suggest doing a 'refactoring' commit to get things in the right place then another commit to add your test (i.e. in that order). I don't think we should be afraid to modify the code as needed for tests. -- 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 04 May 2021 15:43:03 +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

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Richard Hughes (Code Review) 04 May '21

04 May '21

Attention is currently required from: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, Daniel Campello, Anastasia Klimchuk, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/49643 to look at the new patch set (#6). Change subject: libflashrom: Return progress state to the library user ...................................................................... libflashrom: Return progress state to the library user Projects using libflashrom like fwupd expect the user to wait for the operation to complete. To avoid the user thinking the process has "hung" or "got stuck" report back the progress complete of the erase, write and read operations. Include a test for the dummy spi25 device. Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Signed-off-by: Richard Hughes <richard(a)hughsie.com> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M en29lv640b.c M flash.h M it87spi.c M jedec.c M libflashrom.c M libflashrom.h M linux_mtd.c M lspcon_i2c_spi.c M realtek_mst_i2c_spi.c M spi.c M spi25.c M sst28sf040.c M tests/spi25.c M tests/tests.c M tests/tests.h 20 files changed, 145 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/49643/6 -- 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: 6 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-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom_tester: update syntax for --wp-range
by Paul Menzel (Code Review) 04 May '21

04 May '21

Attention is currently required from: Daniel Campello. Paul Menzel 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: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52876/comment/ee305479_596bcdaa PS1, Line 12: Nit: Please remove the blank line. -- 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Tue, 04 May 2021 15:27:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Daniel Campello (Code Review) 04 May '21

04 May '21

Attention is currently required from: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Daniel Campello 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 5: (1 comment) File lspcon_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/49643/comment/6b1e91c5_070d715f PS5, Line 355: PAGE_SIZE LSPCON_PAGE_SIZE ? commit cbe8a39ece4c228ffb23736da3f477ef3b82358c -- 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: 5 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 14:33:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Richard Hughes (Code Review) 04 May '21

04 May '21

Attention is currently required from: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, Daniel Campello, Anastasia Klimchuk, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/49643 to look at the new patch set (#5). Change subject: libflashrom: Return progress state to the library user ...................................................................... libflashrom: Return progress state to the library user Projects using libflashrom like fwupd expect the user to wait for the operation to complete. To avoid the user thinking the process has "hung" or "got stuck" report back the progress complete of the erase, write and read operations. Include a test for the dummy spi25 device. Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Signed-off-by: Richard Hughes <richard(a)hughsie.com> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M en29lv640b.c M flash.h M it87spi.c M jedec.c M libflashrom.c M libflashrom.h M linux_mtd.c M lspcon_i2c_spi.c M realtek_mst_i2c_spi.c M spi.c M spi25.c M sst28sf040.c M tests/spi25.c M tests/tests.c M tests/tests.h 20 files changed, 145 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/49643/5 -- 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: 5 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-MessageType: newpatchset

1 0

Change in flashrom[master]: RFC: flashchips.c: merge GD25B128B/GD25Q128B and GD25Q127C/GD25Q128C
by Angel Pons (Code Review) 04 May '21

04 May '21

Attention is currently required from: Nico Huber, Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52883 ) Change subject: RFC: flashchips.c: merge GD25B128B/GD25Q128B and GD25Q127C/GD25Q128C ...................................................................... Patch Set 1: Code-Review-1 (1 comment) Patchset: PS1: > Further explanation of the reasons for this change: […] Sorry, I'm not convinced. This looks like a workaround for something only downstream uses. However, I agree that having to deal with multiple flash chip definitions is annoying. If what downstream needs is to ignore one of the flashchip definitions, why not add a command-line argument to specify flashchip entries to ignore? It would have the same semantics as `-C`. This feature would be useful for everyone, not just downstream. -- To view, visit https://review.coreboot.org/c/flashrom/+/52883 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I688f2e15aef61afbec728a9a81094bee56d6fbfa Gerrit-Change-Number: 52883 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 04 May 2021 10:09:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021