August 2024 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[main]: tests: Check verify op completed in full if chip memory modified
by Peter Marheine (Code Review) Aug. 26, 2024

Aug. 26, 2024

Attention is currently required from: Anastasia Klimchuk. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84078?usp=email ) Change subject: tests: Check verify op completed in full if chip memory modified ...................................................................... Patch Set 1: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/84078/comment/df163ae7_0d9344df?us… : PS1, Line 18: Note: at the moment the test is not fully working, the following : write test cases fail (all erase test cases pass): Looks like it might be a fencepost error (possibly treating the upper bound of a region as exclusive rather than inclusive), since it looks like the unverified bytes might always fall at the end of a region. File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/84078/comment/b54fe4e7_102f0525?us… : PS1, Line 60: uint8_t Since this is true/false, just use `bool`. https://review.coreboot.org/c/flashrom/+/84078/comment/a966e2b2_5bc432f5?us… : PS1, Line 77: == 0x1 I'd rather use implicit boolean conversion throughout (`if (g_state.was_modified[start])`). -- To view, visit https://review.coreboot.org/c/flashrom/+/84078?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I3c5d55a0deb20f23f4072caac8c0dce04cc98fd4 Gerrit-Change-Number: 84078 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 25 Aug 2024 23:39:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in flashrom[main]: flashchips: add GD25B512MF and GD25R512MF
by Victor Lim (Code Review) Aug. 25, 2024

Aug. 25, 2024

Victor Lim has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84083?usp=email ) Change subject: flashchips: add GD25B512MF and GD25R512MF ...................................................................... flashchips: add GD25B512MF and GD25R512MF GD25B512MF: 3V 512Mbit, QE = 1 GD25R512MF: GD25B512MF feature + RPMC These two part share the same datasheet on the flash side. https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20240412/DS-0… Tested both models on ch347 with erase, write, read, and protection. Change-Id: I9821efb34fb4abb806ad52acec46aad186888c07 Signed-off-by: Victor Lim <vlim(a)gigadevice.com> --- M flashchips.c M include/flashchips.h 2 files changed, 57 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/84083/1 diff --git a/flashchips.c b/flashchips.c index 5ca02c7..96e8760 100644 --- a/flashchips.c +++ b/flashchips.c @@ -7505,6 +7505,62 @@ .decode_range = DECODE_RANGE_SPI25, }, +{ + .vendor = "GigaDevice", + .name = "GD25B512MF/GD25R512MF", + .bustype = BUS_SPI, + .manufacture_id = GIGADEVICE_ID, + .model_id = GIGADEVICE_GD25B512MF, + .total_size = 65536, + .page_size = 256, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR2 | FEATURE_WRSR3 | FEATURE_4BA, + .tested = TEST_OK_PREWB, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = SPI_BLOCK_ERASE_21, + }, { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = SPI_BLOCK_ERASE_5C, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_DC, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, + .unlock = SPI_DISABLE_BLOCKPROTECT_BP4_SRWD, /* TODO: 2nd status reg (read with 0x35) */ + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, + .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .srl = {STATUS2, 6, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like TB */ + .cmp = {STATUS3, 3, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, + }, + { .vendor = "GigaDevice", .name = "GD25Q32(B)", diff --git a/include/flashchips.h b/include/flashchips.h index e4d0bca..be83a6f 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -393,6 +393,7 @@ #define GIGADEVICE_GD25Q64 0x4017 /* Same as GD25Q64B */ #define GIGADEVICE_GD25Q128 0x4018 /* Same as GD25Q128B, GD25Q127C, GD25Q128C,and GD25Q128E, GD25B128E, GD25R128E can be distinguished by SFDP */ #define GIGADEVICE_GD25Q256D 0x4019 +#define GIGADEVICE_GD25B512MF 0x401A /* Same as GD25R512MF */ #define GIGADEVICE_GD25VQ21B 0x4212 #define GIGADEVICE_GD25VQ41B 0x4213 /* Same as GD25VQ40C, can be distinguished by SFDP */ #define GIGADEVICE_GD25VQ80C 0x4214 -- To view, visit https://review.coreboot.org/c/flashrom/+/84083?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9821efb34fb4abb806ad52acec46aad186888c07 Gerrit-Change-Number: 84083 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com>

1 0

[S] Change in flashrom[main]: flashchips: redo add GD25B256E and GD25R256E
by Victor Lim (Code Review) Aug. 25, 2024

Aug. 25, 2024

Victor Lim has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84082?usp=email ) Change subject: flashchips: redo add GD25B256E and GD25R256E ...................................................................... flashchips: redo add GD25B256E and GD25R256E Redo this commit because the previous commit did not trigger Jenkins removed FEATURE_WRSR_EXT2 from the model after datasheet review. replace printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, .unlock = SPI_DISABLE_BLOCKPROTECT, with .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, .unlock = SPI_DISABLE_BLOCKPROTECT_BP4_SRWD, GD25B256E: 3V 256Mbit, Quad enabled. GD25R256E: GD25B256E features + RPMC, so they share the same datasheet on flash side https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20230627/DS-0… Tested both models on ch347 with erase, write, read and protection. Change-Id: I1596628211d508d3057893c049f1a1c95a123073 Signed-off-by: Victor Lim <vlim(a)gigadevice.com> --- M flashchips.c 1 file changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/84082/1 diff --git a/flashchips.c b/flashchips.c index a498ded..5ca02c7 100644 --- a/flashchips.c +++ b/flashchips.c @@ -7451,14 +7451,14 @@ { .vendor = "GigaDevice", - .name = "GD25Q256D/GD25Q256E", + .name = "GD25Q256E/GD25B256E/GD25R256E/GD25Q256D", .bustype = BUS_SPI, .manufacture_id = GIGADEVICE_ID, .model_id = GIGADEVICE_GD25Q256D, .total_size = 32768, .page_size = 256, .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA | - FEATURE_WRSR_EXT2 | FEATURE_WRSR2 | FEATURE_WRSR3, + FEATURE_WRSR2 | FEATURE_WRSR3, .tested = TEST_OK_PREWB, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, @@ -7490,8 +7490,8 @@ .block_erase = SPI_BLOCK_ERASE_C7, } }, - .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, - .unlock = SPI_DISABLE_BLOCKPROTECT, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, + .unlock = SPI_DISABLE_BLOCKPROTECT_BP4_SRWD, /* TODO: 2nd status reg (read with 0x35) */ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, @@ -7500,7 +7500,7 @@ .srp = {STATUS1, 7, RW}, .srl = {STATUS2, 6, RW}, .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, - .tb = {STATUS1, 6, RW}, + .tb = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like TB */ }, .decode_range = DECODE_RANGE_SPI25, }, -- To view, visit https://review.coreboot.org/c/flashrom/+/84082?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1596628211d508d3057893c049f1a1c95a123073 Gerrit-Change-Number: 84082 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com>

1 0

[S] Change in flashrom[main]: flashchips: add GD25B256E and GD25R256E
by Anastasia Klimchuk (Code Review) Aug. 24, 2024

Aug. 24, 2024

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim. Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/83998?usp=email ) Change subject: flashchips: add GD25B256E and GD25R256E ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: For some reasons, Jenkins did not run on the latest version of the patch (it would give Verified+1 vote). If you could rebase the patch and upload again, that should trigger Jenkins again? Or, you can click "Rebase" button on the top right. Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/83998?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie733e0c2e35fa4797f5198f2c8334469b65f402c Gerrit-Change-Number: 83998 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 24 Aug 2024 13:42:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: flashchips: add GD25B256E and GD25R256E
by Anastasia Klimchuk (Code Review) Aug. 24, 2024

Aug. 24, 2024

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim. Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/83998?usp=email ) Change subject: flashchips: add GD25B256E and GD25R256E ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: Thank you so much for checking all the datasheets and testing! -- To view, visit https://review.coreboot.org/c/flashrom/+/83998?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie733e0c2e35fa4797f5198f2c8334469b65f402c Gerrit-Change-Number: 83998 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 24 Aug 2024 13:38:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[M] Change in flashrom[main]: tests: Check verify op completed in full if chip memory modified
by Anastasia Klimchuk (Code Review) Aug. 24, 2024

Aug. 24, 2024

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84078?usp=email ) Change subject: tests: Check verify op completed in full if chip memory modified ...................................................................... tests: Check verify op completed in full if chip memory modified The patch adds new functionality to the test: tracking the areas of chip memory that were modified (i.e. by erase or write operation), and then checking those areas were completely covered by verify operation. The test operates over the mock chip memory of 16 bytes, so it is possible to track each byte which was modified, and assert that is has been verified afterwards. Note: at the moment the test is not fully working, the following write test cases fail (all erase test cases pass): [ FAILED ] Write test case #1 Error: byte 15, modified: 1, verified: 0 [ FAILED ] Write test case #2 1353 Error: byte 7, modified: 1, verified: 0 1354 Error: byte 9, modified: 1, verified: 0 1355 Error: byte 14, modified: 1, verified: 0 1356 Error: byte 15, modified: 1, verified: 0 [ FAILED ] Write test case #10 1494 Error: byte 7, modified: 1, verified: 0 1495 Error: byte 9, modified: 1, verified: 0 1496 Error: byte 14, modified: 1, verified: 0 1497 Error: byte 15, modified: 1, verified: 0 As usual, there are two options: either the test is not fully working or there is a bug in the code. Change-Id: I3c5d55a0deb20f23f4072caac8c0dce04cc98fd4 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M tests/erase_func_algo.c 1 file changed, 51 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/84078/1 diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index 576923f..9dce6ca 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -57,6 +57,8 @@ struct all_state { uint8_t buf[MIN_REAL_CHIP_SIZE]; /* Buffer emulating the memory of the mock chip. */ + uint8_t was_modified[MIN_REAL_CHIP_SIZE]; /* Which bytes were modified, 0x1 if byte was modified. */ + uint8_t was_verified[MIN_REAL_CHIP_SIZE]; /* Which bytes were verified, 0x1 if byte was verified. */ struct erase_invoke eraseblocks_actual[MOCK_CHIP_SIZE]; /* The actual order of eraseblocks invocations. */ unsigned int eraseblocks_actual_ind; /* Actual number of eraseblocks invocations. */ const struct test_case* current_test_case; /* Currently executed test case. */ @@ -70,6 +72,11 @@ assert_in_range(start + len, 0, MIN_REAL_CHIP_SIZE); memcpy(buf, &g_state.buf[start], len); + + /* If these bytes were modified before => current read op is verify op, track it */ + if (g_state.was_modified[start] == 0x1) + memset(&g_state.was_verified[start], 0x1, len); + return 0; } @@ -81,6 +88,10 @@ assert_in_range(start + len, 0, MIN_REAL_CHIP_SIZE); memcpy(&g_state.buf[start], buf, len); + + /* Track the bytes were written */ + memset(&g_state.was_modified[start], 0x1, len); + return 0; } @@ -101,6 +112,10 @@ assert_in_range(blockaddr + blocklen, 0, MIN_REAL_CHIP_SIZE); memset(&g_state.buf[blockaddr], ERASE_VALUE, blocklen); + + /* Track the bytes were erased */ + memset(&g_state.was_modified[blockaddr], 0x1, blocklen); + return 0; } @@ -223,6 +238,11 @@ memset(g_state.eraseblocks_actual, 0, MOCK_CHIP_SIZE * sizeof(struct erase_invoke)); g_state.eraseblocks_actual_ind = 0; + /* Clear the tracking of each byte modified. */ + memset(g_state.was_modified, 0, MIN_REAL_CHIP_SIZE); + /* Clear the tracking of each byte verified. */ + memset(g_state.was_verified, 0, MIN_REAL_CHIP_SIZE); + flashctx->chip = current_test_case->chip; printf("Creating layout ... "); @@ -771,7 +791,7 @@ /* * Setup all test cases with protected region. - * Protected region is the same for all test cases, between bytes 8 - 15. + * Protected region is the same for all test cases, between bytes START_PROTECTED_REGION and up to END_PROTECTED_REGION. */ static struct test_case test_cases_protected_region[] = { { @@ -1060,6 +1080,13 @@ int eraseblocks_invocations = (g_state.eraseblocks_actual_ind == current_test_case->eraseblocks_expected_ind); + int chip_verified = 1; + for (int i = 0; i < MOCK_CHIP_SIZE; i++) + if (g_state.was_modified[i] == 0x1 && g_state.was_verified[i] != 0x1) { + chip_verified = 0; /* byte was modified, but not verified after */ + printf("Error: byte %d, modified: %d, verified: %d\n", i, g_state.was_modified[i], g_state.was_verified[i]); + } + if (chip_erased) printf("Erased chip memory state for %s is CORRECT\n", current_test_case->erase_test_name); @@ -1083,10 +1110,18 @@ current_test_case->eraseblocks_expected_ind, g_state.eraseblocks_actual_ind); + if (chip_verified) + printf("Erased chip memory state for %s was verified successfully\n", + current_test_case->erase_test_name); + else + printf("Erased chip memory state for %s was NOT verified completely\n", + current_test_case->erase_test_name); + all_erase_tests_result |= ret; all_erase_tests_result |= !chip_erased; all_erase_tests_result |= !eraseblocks_in_order; all_erase_tests_result |= !eraseblocks_invocations; + all_erase_tests_result |= !chip_verified; teardown_chip(&layout); @@ -1117,6 +1152,13 @@ int chip_written = !memcmp(g_state.buf, current_test_case->written_buf, MOCK_CHIP_SIZE); + int chip_verified = 1; + for (int i = 0; i < MOCK_CHIP_SIZE; i++) + if (g_state.was_modified[i] == 0x1 && g_state.was_verified[i] != 0x1) { + chip_verified = 0; /* the byte was modified, but not verified after */ + printf("Error: byte %d, modified: %d, verified: %d\n", i, g_state.was_modified[i], g_state.was_verified[i]); + } + if (chip_written) printf("Written chip memory state for %s is CORRECT\n", current_test_case->write_test_name); @@ -1124,8 +1166,16 @@ printf("Written chip memory state for %s is WRONG\n", current_test_case->write_test_name); + if (chip_verified) + printf("Written chip memory state for %s was verified successfully\n", + current_test_case->write_test_name); + else + printf("Written chip memory state for %s was NOT verified completely\n", + current_test_case->write_test_name); + all_write_test_result |= ret; all_write_test_result |= !chip_written; + all_write_test_result |= !chip_verified; teardown_chip(&layout); -- To view, visit https://review.coreboot.org/c/flashrom/+/84078?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I3c5d55a0deb20f23f4072caac8c0dce04cc98fd4 Gerrit-Change-Number: 84078 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: flashchips: add GD25B256E and GD25R256E
by Victor Lim (Code Review) Aug. 24, 2024

Aug. 24, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Victor Lim has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/83998?usp=email ) Change subject: flashchips: add GD25B256E and GD25R256E ...................................................................... Patch Set 1: (3 comments) Patchset: PS1: completed File flashchips.c: https://review.coreboot.org/c/flashrom/+/83998/comment/c7687e4a_b04eec05?us… : PS1, Line 7461: FEATURE_WRSR_EXT2 > Agree, will remove it and upload again Done https://review.coreboot.org/c/flashrom/+/83998/comment/55bbc987_86d887a8?us… : PS1, Line 7493: SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD > agree, will change it and upload again. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/83998?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie733e0c2e35fa4797f5198f2c8334469b65f402c Gerrit-Change-Number: 83998 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 24 Aug 2024 04:23:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Victor Lim <vlim(a)gigadevice.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips: add GD25B256E and GD25R256E
by Victor Lim (Code Review) Aug. 24, 2024

Aug. 24, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/83998?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: flashchips: add GD25B256E and GD25R256E ...................................................................... flashchips: add GD25B256E and GD25R256E removed FEATURE_WRSR_EXT2 from the model after datasheet review. replace printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, .unlock = SPI_DISABLE_BLOCKPROTECT, with .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, .unlock = SPI_DISABLE_BLOCKPROTECT_BP4_SRWD, GD25B256E: 3V 256Mbit, Quad enabled. GD25R256E: GD25B256E features + RPMC, so they share the same datasheet on flash side https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20230627/DS-0… Tested both models on ch347 with erase, write, read and protection. Change-Id: Ie733e0c2e35fa4797f5198f2c8334469b65f402c Signed-off-by: Victor Lim <vlim(a)gigadevice.com> --- M flashchips.c M include/flashchips.h 2 files changed, 6 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/83998/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/83998?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie733e0c2e35fa4797f5198f2c8334469b65f402c Gerrit-Change-Number: 83998 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>

1 0

[XS] Change in flashrom[main]: flashchips: add GD25B256E and GD25R256E
by Victor Lim (Code Review) Aug. 23, 2024

Aug. 23, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Victor Lim has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/83998?usp=email ) Change subject: flashchips: add GD25B256E and GD25R256E ...................................................................... Patch Set 1: (2 comments) File flashchips.c: https://review.coreboot.org/c/flashrom/+/83998/comment/94c32829_a58e3c9e?us… : PS1, Line 7461: FEATURE_WRSR_EXT2 > From the datasheet you linked, I think `FEATURE_WRSR_EXT2` not needed. […] Agree, will remove it and upload again https://review.coreboot.org/c/flashrom/+/83998/comment/b366e3e3_113c7a56?us… : PS1, Line 7493: SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD > I think this needs SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD if all the models have BP4 agree, will change it and upload again. -- To view, visit https://review.coreboot.org/c/flashrom/+/83998?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie733e0c2e35fa4797f5198f2c8334469b65f402c Gerrit-Change-Number: 83998 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 23 Aug 2024 15:53:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: stlinkv3_spi: Mark STLinkV3-Mini not working
by Anastasia Klimchuk (Code Review) Aug. 23, 2024

Aug. 23, 2024

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/83921?usp=email ) Change subject: stlinkv3_spi: Mark STLinkV3-Mini not working ...................................................................... stlinkv3_spi: Mark STLinkV3-Mini not working The STLinkV3 Mini does not support the bridge API, it return LIBUSB_IO_ERROR when querying the bridge version. The official ST updater does not lists the bridge version in the info screen. Due to it's construction (additional connector on the bottom) it is likely that ST disabled the bridge functions on the castellated pads. Change-Id: Ic1ea4ca7acedca5d374cff8fcef450c18e55a9e8 Signed-off-by: Miklos Marton <martonmiklosqdev(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/83921 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M stlinkv3_spi.c 1 file changed, 8 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index f9046df..29aa8d3 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -115,7 +115,7 @@ #define USB_TIMEOUT_IN_MS 5000 static const struct dev_entry devs_stlinkv3_spi[] = { - {0x0483, 0x374E, NT, "STMicroelectronics", "STLINK-V3E"}, + {0x0483, 0x374E, BAD, "STMicroelectronics", "STLINK-V3E"}, {0x0483, 0x374F, OK, "STMicroelectronics", "STLINK-V3S"}, {0x0483, 0x3753, OK, "STMicroelectronics", "STLINK-V3 dual VCP"}, {0x0483, 0x3754, NT, "STMicroelectronics", "STLINK-V3 no MSD"}, @@ -498,8 +498,14 @@ devs_stlinkv3_spi[devIndex].vendor_id, devs_stlinkv3_spi[devIndex].device_id, param_str); - if (stlinkv3_handle) + if (stlinkv3_handle) { + if (devs_stlinkv3_spi[devIndex].status == BAD) { + msg_perr("The STLINK-V3 Mini/MiniE does not support the bridge interface\n"); + free(param_str); + goto init_err_exit; + } break; + } devIndex++; } -- To view, visit https://review.coreboot.org/c/flashrom/+/83921?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic1ea4ca7acedca5d374cff8fcef450c18e55a9e8 Gerrit-Change-Number: 83921 Gerrit-PatchSet: 5 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0