November 2021 - flashrom-gerrit

Change in flashrom[master]: [DONOTMERGE] Buildbot test commit
by Felix Singer (Code Review) 22 Nov '21

22 Nov '21

Felix Singer has removed build bot (Jenkins) from this change. ( https://review.coreboot.org/c/flashrom/+/59536 ) Change subject: [DONOTMERGE] Buildbot test commit ...................................................................... Removed reviewer build bot (Jenkins) with the following votes: * Verified+1 by build bot (Jenkins) <no-reply(a)coreboot.org> -- To view, visit https://review.coreboot.org/c/flashrom/+/59536 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15378c4ff67a0046c2d2857398a5ca29e73321b1 Gerrit-Change-Number: 59536 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: buildbot experiment <no-reply+buildbot(a)coreboot.org> Gerrit-MessageType: deleteReviewer

1 0

Change in flashrom[master]: [DONOTMERGE] Buildbot test commit
by Felix Singer (Code Review) 22 Nov '21

22 Nov '21

Hello build bot (Jenkins), buildbot experiment, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59536 to look at the new patch set (#2). Change subject: [DONOTMERGE] Buildbot test commit ...................................................................... [DONOTMERGE] Buildbot test commit Change-Id: I15378c4ff67a0046c2d2857398a5ca29e73321b1 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M README 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/59536/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59536 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15378c4ff67a0046c2d2857398a5ca29e73321b1 Gerrit-Change-Number: 59536 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: buildbot experiment <no-reply+buildbot(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: [DONOTMERGE] Buildbot test commit
by Felix Singer (Code Review) 22 Nov '21

22 Nov '21

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/59536 ) Change subject: [DONOTMERGE] Buildbot test commit ...................................................................... [DONOTMERGE] Buildbot test commit Change-Id: I15378c4ff67a0046c2d2857398a5ca29e73321b1 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M README 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/59536/1 diff --git a/README b/README index 71e8735..bea661c 100644 --- a/README +++ b/README @@ -1,3 +1,5 @@ +foo + ------------------------------------------------------------------------------- flashrom README ------------------------------------------------------------------------------- -- To view, visit https://review.coreboot.org/c/flashrom/+/59536 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15378c4ff67a0046c2d2857398a5ca29e73321b1 Gerrit-Change-Number: 59536 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: [FIX ME] tests: Add test for extract operation
by Anastasia Klimchuk (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Edward O'Callaghan, Daniel Campello. Hello build bot (Jenkins), Edward O'Callaghan, Daniel Campello, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59532 to look at the new patch set (#2). Change subject: [FIX ME] tests: Add test for extract operation ...................................................................... [FIX ME] tests: Add test for extract operation This patch adds a test for do_extract operation. However test fails because do_extract seems to always return 1. Something needs to be fixed: either do_extract itself, or API how to call it, or maybe that's the test which is doing it wrong? To repro: ninja test Change-Id: I13107c0e095d53184e32a41fa72227cf7dc1d449 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/59532/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13107c0e095d53184e32a41fa72227cf7dc1d449 Gerrit-Change-Number: 59532 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: [FIX ME] tests: Add test for extract operation
by Anastasia Klimchuk (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Edward O'Callaghan, Daniel Campello. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59532 ) Change subject: [FIX ME] tests: Add test for extract operation ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Daniel, Edward I added you because I found your names in commit ce983bccaab450d358854494f15c2d8a1846d56b which added do_extract operation. I am confused because it seems do_extract operation always returns 1. This is because: 1) do_extract calls `do_read(flash, NULL)` and so at this point filename is NULL 2) do_read calls `read_flash_to_file(flash, filename)` 3) then it goes to `write_buf_to_file(buf, size, filename)` and the latter has a block of code: if (!filename) { msg_gerr("No filename specified.\n"); return 1; } There is no way to provide filename into do_extract, so I don't know how to make it work. I would appreciate your advice! As an experiment, I changes two lines in flashrom.c#do_extract: replaced return do_read(flash, NULL); with const char *const filename = "read_chip.test"; return do_read(flash, filename); This replacement makes test running successfully! Maybe I am using API in a wrong way? Or could it be bug in do_extract? Thank you for advice! -- To view, visit https://review.coreboot.org/c/flashrom/+/59532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13107c0e095d53184e32a41fa72227cf7dc1d449 Gerrit-Change-Number: 59532 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Mon, 22 Nov 2021 05:47:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [FIX ME] tests: Add test for extract operation
by Anastasia Klimchuk (Code Review) 22 Nov '21

22 Nov '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/59532 ) Change subject: [FIX ME] tests: Add test for extract operation ...................................................................... [FIX ME] tests: Add test for extract operation This patch adds a test for do_extract operation. However test fails because do_extract, as it is currently written, always returns 1. Something needs to be fixed: either do_extract itself, or API how to call it, or maybe that's the test which is doing it wrong? To repro: ninja test Change-Id: I13107c0e095d53184e32a41fa72227cf7dc1d449 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/59532/1 diff --git a/tests/chip.c b/tests/chip.c index 9c06fe7..cf8441d 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -317,3 +317,21 @@ free(param_dup); } + +void extract_layout_test_success(void **state) +{ + (void) state; /* unused */ + + struct flashrom_flashctx flashctx = { 0 }; + struct flashrom_layout *layout; + struct flashchip mock_chip = chip_8MiB; + const char *param = ""; /* Default values for all params. */ + + setup_chip(&flashctx, &layout, &mock_chip, param); + + printf("Extract layout operation started.\n"); + assert_int_equal(0, do_extract(&flashctx)); + printf("Extract layout operation done.\n"); + + teardown(&layout); +} diff --git a/tests/tests.c b/tests/tests.c index f2547ff..c8bce6a 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -375,6 +375,7 @@ cmocka_unit_test(read_chip_with_dummyflasher_test_success), cmocka_unit_test(write_chip_test_success), cmocka_unit_test(write_chip_with_dummyflasher_test_success), + cmocka_unit_test(extract_layout_test_success), }; ret |= cmocka_run_group_tests_name("chip.c tests", chip_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index 732920a..a6a0df6 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -63,5 +63,6 @@ void read_chip_with_dummyflasher_test_success(void **state); void write_chip_test_success(void **state); void write_chip_with_dummyflasher_test_success(void **state); +void extract_layout_test_success(void **state); #endif /* TESTS_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/59532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13107c0e095d53184e32a41fa72227cf7dc1d449 Gerrit-Change-Number: 59532 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: spi25_statusreg: delete read_status_register()
by Anastasia Klimchuk (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59529 ) Change subject: spi25_statusreg: delete read_status_register() ...................................................................... Patch Set 4: (3 comments) File it87spi.c: https://review.coreboot.org/c/flashrom/+/59529/comment/9571bb9d_3745f02d PS4, Line 136: while (true) I am curious, can this ever become an infinite loop? I understand it was like this before ;) https://review.coreboot.org/c/flashrom/+/59529/comment/650b9665_da59c730 PS4, Line 139: return 1; Can we propagate error code from spi_read_register? https://review.coreboot.org/c/flashrom/+/59529/comment/f7676249_ac4e06b2 PS4, Line 288: return 1; Looks like another opportunity to propagate error code! -- To view, visit https://review.coreboot.org/c/flashrom/+/59529 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I146b4b5439872e66c5d33e156451a729d248c7da Gerrit-Change-Number: 59529 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev(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: build bot (Jenkins) <no-reply(a)coreboot.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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 22 Nov 2021 03:26:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg: inline spi_write_register_flag()
by Anastasia Klimchuk (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59528 ) Change subject: spi25_statusreg: inline spi_write_register_flag() ...................................................................... Patch Set 3: (1 comment) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/59528/comment/7694d171_eb1d4d16 PS3, Line 82: } As per coding style, if braces are needed in one of the branches, they should be in all branches. So you need to add braces (yes even for one statement) for if clause and else if. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Doc… -- To view, visit https://review.coreboot.org/c/flashrom/+/59528 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4996b0848d0ed09032bad2ab13ab1f40bbfc0304 Gerrit-Change-Number: 59528 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(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: build bot (Jenkins) <no-reply(a)coreboot.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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 22 Nov 2021 03:22:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: cli_classic: add writeprotect CLI
by Anastasia Klimchuk (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 ) Change subject: cli_classic: add writeprotect CLI ...................................................................... Patch Set 12: (2 comments) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/ad9b1b25_951130f6 PS12, Line 190: wp_cli I remember there was a conversation about cli vs libflashrom in some other patch in this chain, I have a question in that context: Is `wp_cli` going to live inside cli_classic, it is it just temporarily? Can this be called from libflashrom? There is some logic in here, it would be great not to repeat the same for libflashrom calls? I understand that parsing command line arguments is cli specific, but it seems like at the point of calling `wp_cli` the parsing done? https://review.coreboot.org/c/flashrom/+/58738/comment/6647ccc7_a029f11a PS12, Line 890: ret = wp_cli( : fill_flash, : enable_wp, : disable_wp, : print_wp_status, : print_available_ranges, : set_wp_range, : wp_start, : wp_len, : wp_mode_opt : ); Does this fit into 112 chars? If yes let's have it one line? -- To view, visit https://review.coreboot.org/c/flashrom/+/58738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869 Gerrit-Change-Number: 58738 Gerrit-PatchSet: 12 Gerrit-Owner: Nikolai Artemiev <nartemiev(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 22 Nov 2021 03:14:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_chip_supported()
by Anastasia Klimchuk (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58739 ) Change subject: libflashrom,writeprotect: add flashrom_wp_chip_supported() ...................................................................... Patch Set 10: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/58739 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id64a0789b05c5bc362865f7888f037367d6d566b Gerrit-Change-Number: 58739 Gerrit-PatchSet: 10 Gerrit-Owner: Nikolai Artemiev <nartemiev(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 22 Nov 2021 03:02:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2021