- flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: Makefile: CONFIG_INTERNAL depends on raw mem access
by Fabrice Fontaine (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Alexander Goncharov, Fabrice Fontaine, Thomas Heijligen. Hello Anastasia Klimchuk, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/78930?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Makefile: CONFIG_INTERNAL depends on raw mem access ...................................................................... Makefile: CONFIG_INTERNAL depends on raw mem access CONFIG_INTERNAL depends on raw mem access resulting in the following build failure on sh4 since version 1.3.0: /home/thomas/autobuild/instance-3/output-1/per-package/flashrom/host/bin/../lib/gcc/sh4a-buildroot-linux-gnu/12.3.0/../../../../sh4a-buildroot-linux-gnu/bin/ld: libflashrom.a(internal.o): in function `internal_chip_readn': internal.c:(.text+0x8): undefined reference to `mmio_readn' Fixes the build raised by buildroot autobuilders with the following options: - CONFIG_FT2232_SPI=no - CONFIG_USBBLASTER_SPI=no - CONFIG_ENABLE_LIBUSB1_PROGRAMMERS=yes - CONFIG_ENABLE_LIBPCI_PROGRAMMERS=yes Here is the target information: C compiler found: sh4a-buildroot-linux-gnu-gcc.br_real (Buildroot 2022.08-rc1-5515-gf1a47904b63) 12.3.0 Target arch: sh Target OS: Linux Target endian: little Full build log: - http://autobuild.buildroot.org/results/f74a9d315fb519f284428234713f43fcf4e3… Change-Id: I7610f5f7cc5b114ffa90d5752155acc8b6b7c9f7 Signed-off-by: Fabrice Fontaine <fontaine.fabrice(a)gmail.com> --- M Makefile 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/78930/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/78930?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7610f5f7cc5b114ffa90d5752155acc8b6b7c9f7 Gerrit-Change-Number: 78930 Gerrit-PatchSet: 2 Gerrit-Owner: Fabrice Fontaine <fontaine.fabrice(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Fabrice Fontaine <fontaine.fabrice(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[main]: erase_func_algo: Avoid putting 0xff into buffers
by Anastasia Klimchuk (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Hsuan-ting Chen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79229?usp=email ) Change subject: erase_func_algo: Avoid putting 0xff into buffers ...................................................................... Patch Set 1: Code-Review-1 (1 comment) Patchset: PS1: Hsuan-ting, I appreciate your effort to create a patch, but I don't understand what is the issue you are trying to fix? The test aims to run as close as possible to a real life scenario, and of course in real life there could be 0xff bytes in the chip memory. That's valid and should be tested and handled correctly by erase/write logic. What do you mean by "ambiguous behavior"? The algorithm is deterministic, for a given state of memory on the chip and given image to write there is only one solution. Maybe you can try explain the exact issue you are trying to fix, and I will try to help you? -- To view, visit https://review.coreboot.org/c/flashrom/+/79229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibe159f9f7cf35688cfdbe18245fea2d870386412 Gerrit-Change-Number: 79229 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Thu, 23 Nov 2023 10:01:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: Makefile: CONFIG_INTERNAL depends on raw mem access
by Anastasia Klimchuk (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Alexander Goncharov, Fabrice Fontaine, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78930?usp=email ) Change subject: Makefile: CONFIG_INTERNAL depends on raw mem access ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/78930/comment/b9f4704d_d3048704 : PS1, Line 17: If you could add here in commit message testing info (which environment with which exactly options you tested with) that would be great, thank you! Patchset: PS1: > Thanks for the patch! […] I opened the link from commit message, flashrom is build with the following: > CONFIG_FT2232_SPI=no CONFIG_USBBLASTER_SPI=no CONFIG_ENABLE_LIBUSB1_PROGRAMMERS=yes CONFIG_ENABLE_LIBPCI_PROGRAMMERS=yes which is without internal programmer. I have another question that I am wondering about, it there anything to change in meson.build. I just looked through it, seems all good? -- To view, visit https://review.coreboot.org/c/flashrom/+/78930?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7610f5f7cc5b114ffa90d5752155acc8b6b7c9f7 Gerrit-Change-Number: 78930 Gerrit-PatchSet: 1 Gerrit-Owner: Fabrice Fontaine <fontaine.fabrice(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Fabrice Fontaine <fontaine.fabrice(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 23 Nov 2023 09:47:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: serial: Fix sp_flush_incoming for serprog TCP connections
by Stanislav Ponomarev (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Stanislav Ponomarev. Hello Anastasia Klimchuk, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/79112?usp=email to look at the new patch set (#3). Change subject: serial: Fix sp_flush_incoming for serprog TCP connections ...................................................................... serial: Fix sp_flush_incoming for serprog TCP connections During the development of an esp32 serprog-compatible SPI programmer, and implementation of the TCP over Wi-Fi for serprog, I discovered that sp_flush_incoming() silently fails if the underlying sp_fd descriptor is a TCP socket. This patch adds a check for this case - tcflush returns ENOTTY, meaning tcflush is not supported for not terminal objects, in this case a fallback serialport_read_nonblock loop is used. TESTED=esp32-serprog, TCP-over-WiFi mode, ~90% connection attempts fail to synchronize without patch; no synchronization issues with patch applied. Signed-off-by: Stanislav Ponomarev <me(a)stasponomarev.com> Change-Id: I9724a2fcd4a41dede2c15f83877efa6c3b0b7fae --- M serial.c 1 file changed, 20 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/79112/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/79112?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9724a2fcd4a41dede2c15f83877efa6c3b0b7fae Gerrit-Change-Number: 79112 Gerrit-PatchSet: 3 Gerrit-Owner: Stanislav Ponomarev <me(a)stasponomarev.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stanislav Ponomarev <me(a)stasponomarev.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[main]: erase_func_algo: Avoid putting 0xff into buffers
by Hsuan-ting Chen (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Anastasia Klimchuk. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79229?usp=email ) Change subject: erase_func_algo: Avoid putting 0xff into buffers ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: The build failure is: ``` make: *** [Makefile:1025: man8/flashrom.8] Error 1 make: *** Waiting for unfinished jobs.... Build step 'Execute shell' marked build as failure ``` it is for man, which seems not really related? -- To view, visit https://review.coreboot.org/c/flashrom/+/79229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibe159f9f7cf35688cfdbe18245fea2d870386412 Gerrit-Change-Number: 79229 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 23 Nov 2023 09:07:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: tests: Unit tests for erase function selection algo
by Hsuan-ting Chen (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Anastasia Klimchuk. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67535?usp=email ) Change subject: tests: Unit tests for erase function selection algo ...................................................................... Patch Set 22: (1 comment) File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/67535/comment/5f7ed5b9_3f1e6037 : PS22, Line 270: 0xff Hi Anastasia: I'd like to ask if this test should avoid putting 0xff into either initial_buf or written_buf: If there are some addresses that are already 0xff, will there be multiple choice of erasure? For example, ``` .initial_buf = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, .written_buf = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}, ``` It should be both valid that (1) Erase all, then write (2) Notice that the initial_buf are all 0xff, then skipped IIUC, both cases are valid and it is not able to decide which should be the "golden example". I am not really familiar about erasing algorithm, please let me know if I misunderstood anything. --- I tried to create one: https://review.coreboot.org/c/flashrom/+/79229 -- To view, visit https://review.coreboot.org/c/flashrom/+/67535?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8f3fdefb76e71f6f8dc295d9dead5f38642aace7 Gerrit-Change-Number: 67535 Gerrit-PatchSet: 22 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 23 Nov 2023 08:46:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: erase_func_algo: Avoid putting 0xff into buffers
by Hsuan-ting Chen (Code Review) 23 Nov '23

23 Nov '23

Hsuan-ting Chen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/79229?usp=email ) Change subject: erase_func_algo: Avoid putting 0xff into buffers ...................................................................... erase_func_algo: Avoid putting 0xff into buffers Avoid putting 0xff into either initial_buf or written_buf to prevent ambiguous behaviour. If there are some addresses that are already 0xff, the flash algorithm might skip erasing them as long as they are already 0xff. This might introduce some ambiguities that there will be more than one valid erasing steps. Prevent ambiguous behavior by removing all the 0xffs in initial_buf and written_buf. Change-Id: Ibe159f9f7cf35688cfdbe18245fea2d870386412 Signed-off-by: Hsuan Ting Chen <roccochen(a)google.com> --- M tests/erase_func_algo.c 1 file changed, 54 insertions(+), 32 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/79229/1 diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index 824a03a..4955f80 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -175,6 +175,28 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; g_test_erase_injector = block_erase_chip; + int i = 0; + + /* + * Validity check that all the data in the initial_buf or written_buf + * should not be equal to ERASE_VALUE. This might cause unexpected + * behavior if the flash algorithm tries to skip erasing the addresses + * that are already 0xFF. + */ + int erase_value_address_count = 0; + for (i = 0; i < MOCK_CHIP_SIZE; i++) { + if (current_test_case->initial_buf[i] == ERASE_VALUE) { + printf("initial_buf[%d] == ERASE_VALUE\n", i); + erase_value_address_count++; + } + } + for (i = 0; i < MOCK_CHIP_SIZE; i++) { + if (current_test_case->written_buf[i] == ERASE_VALUE) { + printf("written_buf[%d] == ERASE_VALUE\n", i); + erase_value_address_count++; + } + } + assert_int_equal(0, erase_value_address_count); /* First MOCK_CHIP_SIZE bytes have a meaning and set with given values for this test case. */ memcpy(g_state.buf, current_test_case->initial_buf, MOCK_CHIP_SIZE); @@ -191,7 +213,7 @@ assert_int_equal(0, flashrom_layout_new(layout)); /* Adding regions from test case. */ - int i = 0; + i = 0; while (current_test_case->regions[i].name != NULL) { assert_int_equal(0, flashrom_layout_add_region(*layout, current_test_case->regions[i].start, @@ -266,13 +288,13 @@ */ .chip = &chip_1_2_4_8_16, .regions = {{0, MOCK_CHIP_SIZE/2 - 1, "part1"}, {MOCK_CHIP_SIZE/2, MIN_REAL_CHIP_SIZE - 1, "part2"}}, - .initial_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, - 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff}, + .initial_buf = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, 0xf8, + .written_buf = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f}, .eraseblocks_expected = {{0x8, 0x8}, {0x0, 0x8}}, .eraseblocks_expected_ind = 2, @@ -288,14 +310,14 @@ */ .chip = &chip_1_2_4_8_16, .regions = {{0, 10, "odd1"}, {11, 15, "odd2"}, {MOCK_CHIP_SIZE, MIN_REAL_CHIP_SIZE - 1, "longtail"}}, - .initial_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x0, 0xff, - 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff}, + .initial_buf = {0x1f, 0x1f, 0x0, 0x1f, 0x0, 0x1f, 0x0, 0x1f, + 0x0, 0x1f, 0x0, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x20, 0x2f, - 0x20, 0x2f, 0x0, 0xff, 0xff, 0xff, 0x2f, 0x2f}, + .written_buf = {0x1f, 0x1f, 0x0, 0x1f, 0x0, 0x1f, 0x20, 0x2f, + 0x20, 0x2f, 0x0, 0x1f, 0x1f, 0x1f, 0x2f, 0x2f}, .eraseblocks_expected = {{0xb, 0x1}, {0xc, 0x4}, {0xa, 0x1}, {0x8, 0x2}, {0x0, 0x8}}, .eraseblocks_expected_ind = 5, .erase_test_name = "Erase test case #2", @@ -310,14 +332,14 @@ */ .chip = &chip_1_2_4_8_16, .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, - .initial_buf = {0xff, 0xff, 0xff, 0xff, 0x11, 0x22, 0x33, 0x44, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + .initial_buf = {0x1f, 0x1f, 0x1f, 0x1f, 0x11, 0x22, 0x33, 0x44, + 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0xff, 0xff, 0xff, 0xff, 0x1, 0x2, 0x3, 0x4, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + .written_buf = {0x1f, 0x1f, 0x1f, 0x1f, 0x1, 0x2, 0x3, 0x4, + 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f}, .eraseblocks_expected = {{0x0, 0x10}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #3", @@ -332,13 +354,13 @@ */ .chip = &chip_1_2_4_8_16, .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, - .initial_buf = {0x1, 0x2, 0x3, 0x4, 0xff, 0xff, 0xff, 0xff, + .initial_buf = {0x1, 0x2, 0x3, 0x4, 0x1f, 0x1f, 0x1f, 0x1f, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0x11, 0x22, 0x33, 0x44, 0xff, 0xff, 0xff, 0xff, + .written_buf = {0x11, 0x22, 0x33, 0x44, 0x1f, 0x1f, 0x1f, 0x1f, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, .eraseblocks_expected = {{0x0, 0x10}}, .eraseblocks_expected_ind = 1, @@ -354,13 +376,13 @@ */ .chip = &chip_1_2_4_8_16, .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, - .initial_buf = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0xff, + .initial_buf = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x1f, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0xff, + .written_buf = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x1f, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, .eraseblocks_expected = {{0x0, 0x10}}, .eraseblocks_expected_ind = 1, @@ -444,14 +466,14 @@ .chip = &chip_1_8_16, .regions = {{0, MOCK_CHIP_SIZE/2 - 1, "part1"}, {MOCK_CHIP_SIZE/2, MOCK_CHIP_SIZE - 1, "part2"}, {MOCK_CHIP_SIZE, MIN_REAL_CHIP_SIZE - 1, "longtail"}}, - .initial_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, - 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff}, + .initial_buf = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, - 0xf8, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f}, + .written_buf = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f}, .eraseblocks_expected = {{0x8, 0x8}, {0x0, 0x8}}, .eraseblocks_expected_ind = 2, .erase_test_name = "Erase test case #9", @@ -466,14 +488,14 @@ */ .chip = &chip_1_8_16, .regions = {{0, 10, "odd1"}, {11, 15, "odd2"}, {MOCK_CHIP_SIZE, MIN_REAL_CHIP_SIZE - 1, "longtail"}}, - .initial_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x0, 0xff, - 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff}, + .initial_buf = {0x1f, 0x1f, 0x0, 0x1f, 0x0, 0x1f, 0x0, 0x1f, + 0x0, 0x1f, 0x0, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x20, 0x2f, - 0x20, 0x2f, 0x0, 0xff, 0xff, 0xff, 0x2f, 0x2f}, + .written_buf = {0x1f, 0x1f, 0x0, 0x1f, 0x0, 0x1f, 0x20, 0x2f, + 0x20, 0x2f, 0x0, 0x1f, 0x1f, 0x1f, 0x2f, 0x2f}, .eraseblocks_expected = {{0xb, 0x1}, {0xc, 0x1}, {0xd, 0x1}, {0xe, 0x1}, {0xf, 0x1}, {0x8, 0x1}, {0x9, 0x1}, {0xa, 0x1}, {0x0, 0x8}}, .eraseblocks_expected_ind = 9, @@ -489,14 +511,14 @@ */ .chip = &chip_1_8_16, .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, - .initial_buf = {0xff, 0xff, 0xff, 0xff, 0x11, 0x22, 0x33, 0x44, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + .initial_buf = {0x1f, 0x1f, 0x1f, 0x1f, 0x11, 0x22, 0x33, 0x44, + 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0xff, 0xff, 0xff, 0xff, 0x1, 0x2, 0x3, 0x4, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + .written_buf = {0x1f, 0x1f, 0x1f, 0x1f, 0x1, 0x2, 0x3, 0x4, + 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f}, .eraseblocks_expected = {{0x0, 0x10}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #11", @@ -511,13 +533,13 @@ */ .chip = &chip_1_8_16, .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, - .initial_buf = {0x1, 0x2, 0x3, 0x4, 0xff, 0xff, 0xff, 0xff, + .initial_buf = {0x1, 0x2, 0x3, 0x4, 0x1f, 0x1f, 0x1f, 0x1f, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0x11, 0x22, 0x33, 0x44, 0xff, 0xff, 0xff, 0xff, + .written_buf = {0x11, 0x22, 0x33, 0x44, 0x1f, 0x1f, 0x1f, 0x1f, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, .eraseblocks_expected = {{0x0, 0x10}}, .eraseblocks_expected_ind = 1, @@ -533,13 +555,13 @@ */ .chip = &chip_1_8_16, .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, - .initial_buf = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0xff, + .initial_buf = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x1f, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, - .written_buf = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0xff, + .written_buf = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x1f, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, .eraseblocks_expected = {{0x0, 0x10}}, .eraseblocks_expected_ind = 1, -- To view, visit https://review.coreboot.org/c/flashrom/+/79229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibe159f9f7cf35688cfdbe18245fea2d870386412 Gerrit-Change-Number: 79229 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: serial: Fix sp_flush_incoming for serprog TCP connections
by Anastasia Klimchuk (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Stanislav Ponomarev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79112?usp=email ) Change subject: serial: Fix sp_flush_incoming for serprog TCP connections ...................................................................... Patch Set 2: (10 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/79112/comment/fb1cbbc2_c157a4b1 : PS2, Line 9: pet maybe remove "pet" :) https://review.coreboot.org/c/flashrom/+/79112/comment/d8c511d4_c19fff25 : PS2, Line 9: I was working on an During the development of https://review.coreboot.org/c/flashrom/+/79112/comment/bc61d4c4_ebd18b6a : PS2, Line 10: when i implemented implementation of https://review.coreboot.org/c/flashrom/+/79112/comment/6f375d08_fe6d1e03 : PS2, Line 11: i uppercase https://review.coreboot.org/c/flashrom/+/79112/comment/8b5d3424_7b66fe1c : PS2, Line 14: I added This patch adds https://review.coreboot.org/c/flashrom/+/79112/comment/7667ca81_fcec0ab7 : PS2, Line 18: After that TESTED= Patchset: PS2: Thank you for you patch! I added some comments. I really like your original commit message, it explains everything very well. But I added few comments on it, just because typically commit message is written is more formal style (since it stays in git history forever). Also when I got an email about this patch, it does not have display name (literally email from "Name of user not set"). I think it's probably display name or full name in Gerrit account is empty, or both. You can set it to anything, any nickname is fine. File serial.c: https://review.coreboot.org/c/flashrom/+/79112/comment/67cf5fda_70113442 : PS2, Line 380: int ret = tcflush(sp_fd, TCIFLUSH); : if (ret == 0) : return; We typically use more compact variation. I don't see this exact ret value is used again, so you can do like this: if (!tcflush(sp_fd, TCIFLUSH)) return; https://review.coreboot.org/c/flashrom/+/79112/comment/a4ad7027_d311b8db : PS2, Line 392: ret > 0 A question about return codes: `serialport_read_nonblock` can return positive or negative error codes, I am wondering does positive return code reliably indicates that no more data available to read? I just read through the comment and the code of `serialport_read_nonblock` and I can't decide on that :) If it works reliably, maybe you can add a bit longer comment, like "Positive error code indicates no more data available, which is normal situation and means flush completed successfully". https://review.coreboot.org/c/flashrom/+/79112/comment/9d204200_bfbf6988 : PS2, Line 401: return; I am thinking, maybe change slightly with less returns. It's harder to read when every second line returns. if (errno == ENOTTY) { <... serialport_read_nonblock loop ...> if (ret < 0) msg_perr("Could not flush serial port incoming buffer: read has failed"); } else { msg_perr_strerror("Could not flush serial port incoming buffer: "); } -- To view, visit https://review.coreboot.org/c/flashrom/+/79112?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9724a2fcd4a41dede2c15f83877efa6c3b0b7fae Gerrit-Change-Number: 79112 Gerrit-PatchSet: 2 Gerrit-Owner: Stanislav Ponomarev <me(a)stasponomarev.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stanislav Ponomarev <me(a)stasponomarev.com> Gerrit-Comment-Date: Thu, 23 Nov 2023 08:22:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: Remove dependency on C23 __has_include()
by Anastasia Klimchuk (Code Review) 22 Nov '23

22 Nov '23

Attention is currently required from: Anton Samsonov, Anton Samsonov, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email ) Change subject: Remove dependency on C23 __has_include() ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > > add testing info in commit message. […] Looks good! And you already wrote it, just copy paste into commit message. There is no "golden example" of how testing info should look like - it depends on the patch, the main idea is to give this info just in case. Why I am asking in this patch: it touches different environments, and knowing which environment you tested, might be useful. No strict rules, the paragraph you wrote looks good. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/77089?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic544963ffd29626ae0a21bdddb1c78850cc43ec6 Gerrit-Change-Number: 77089 Gerrit-PatchSet: 4 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Attention: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Comment-Date: Wed, 22 Nov 2023 11:35:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-MessageType: comment

1 0

[XL] Change in flashrom[main]: Abandon Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad
by Angel Pons (Code Review) 21 Nov '23

21 Nov '23

Angel Pons has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/79156?usp=email ) Change subject: Abandon Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad ...................................................................... Abandoned Author cannot abandon for some reason, see CB:79152 instead -- To view, visit https://review.coreboot.org/c/flashrom/+/79156?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad Gerrit-Change-Number: 79156 Gerrit-PatchSet: 3 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-MessageType: abandon

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit