- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[main]: Add Idaville platform into chipset enable list and add update IRC reg...
by Anastasia Klimchuk (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Angel Pons, David Hendricks, Kevin Yu. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79340?usp=email ) Change subject: Add Idaville platform into chipset enable list and add update IRC region support ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: David, may I ask your help to review this patch? That would be huge help, thank you so much! -- To view, visit https://review.coreboot.org/c/flashrom/+/79340?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: Icc526b22a9efc8071e972bb1d8cfc51d6a83651b Gerrit-Change-Number: 79340 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Kevin Yu <ykevin(a)celestica.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 03:23:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: Add Idaville platform into chipset enable list and add update IRC reg...
by Anastasia Klimchuk (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Angel Pons, Kevin Yu. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79340?usp=email ) Change subject: Add Idaville platform into chipset enable list and add update IRC region support ...................................................................... Patch Set 6: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/79340/comment/cbf2f644_ad8ed0ae : PS6, Line 7: Add Idaville platform into chipset enable list and add update IRC region support Commit title and message are limited by 72 chars width, so you need to shorten your commit title. For example "Add support for Idaville HCC platform and iRC regions" And then the rest of commit description can be provide full details of the commit. https://review.coreboot.org/c/flashrom/+/79340/comment/2c8bc947_ef80464e : PS6, Line 11: You need to add full testing info into commit message (which platforms you tested on, which commands etc). Patchset: PS6: Hello Kevin! Sorry there was some delay in reply! It's because by coincidence your patch fell into the holidays season here, and most people would be away (between mid-December and mid-January that is). I am looking at it now, and I have few bigger questions, before diving into details: 1) This seems platform-specific feature (Intel), so flashrom.c is probably not the right place for the code, I think the code should live in ichspi.c 2) If you could give me more details on what is IRC region, any docs you can share? I want to understand what that is. There are few other comments that I have, you can have a look. Also I think we need few more reviewers on the patch, but that's fine. Thank you for patch! And let's start working on it :) File flashrom.c: https://review.coreboot.org/c/flashrom/+/79340/comment/c6310cbe_b2b19c43 : PS6, Line 527: }else{ A generic comment to all of your code (applies to all the files): code should follow the coding style which is described in our dev guidelines: https://www.flashrom.org/dev_guide/development_guide.html#coding-style (please don't miss to click the link and open Linux kernel style guide). You need to read the code style first, and then go through your code and make sure all of it follows the style. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/79340?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: Icc526b22a9efc8071e972bb1d8cfc51d6a83651b Gerrit-Change-Number: 79340 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Kevin Yu <ykevin(a)celestica.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 03:16:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashchips: Mark FM25Q16 as probe/read tested
by Stefan Reinauer (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79975?usp=email ) Change subject: flashchips: Mark FM25Q16 as probe/read tested ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/79975?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: I8e55cd6eeb693bf33e580d662da08e50fb46809d Gerrit-Change-Number: 79975 Gerrit-PatchSet: 1 Gerrit-Owner: 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 03:01:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashchips: Mark FM25Q16 as probe/read tested
by Anastasia Klimchuk (Code Review) 15 Jan '24

15 Jan '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/79975?usp=email ) Change subject: flashchips: Mark FM25Q16 as probe/read tested ...................................................................... flashchips: Mark FM25Q16 as probe/read tested As reported on the mailing list: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/I6H7… Change-Id: I8e55cd6eeb693bf33e580d662da08e50fb46809d Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M flashchips.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/79975/1 diff --git a/flashchips.c b/flashchips.c index 663b01b..f580a36 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6198,7 +6198,7 @@ /* OTP: 1024B total; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ /* QPI enable 0x38, disable 0xFF */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PR, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = { -- To view, visit https://review.coreboot.org/c/flashrom/+/79975?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: I8e55cd6eeb693bf33e580d662da08e50fb46809d Gerrit-Change-Number: 79975 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[main]: tests: Add test cases for erasing/writing unaligned layout regions
by Anastasia Klimchuk (Code Review) 15 Jan '24

15 Jan '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78985?usp=email ) ( 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tests: Add test cases for erasing/writing unaligned layout regions ...................................................................... tests: Add test cases for erasing/writing unaligned layout regions The test cases from #16 onwards cover the case when layout region is not aligned with eraseblock region, and therefore either layout start boundary or end boundary needs to be extended to align with eraseblock. Ticket: https://ticket.coreboot.org/issues/494 Change-Id: I726a30b0e47a966e8093ddc19abf4a65fb1d70ce Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78985 Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M tests/erase_func_algo.c 1 file changed, 118 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index 824a03a..ed9eb04 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -27,8 +27,6 @@ #define ERASE_VALUE 0xff #define MOCK_CHIP_SIZE 16 -/* How many small chip test cases. */ -#define TEST_CASES_NUM 16 #define MIN_BUF_SIZE 1024 /* Minimum buffer size flashrom operates for chip operations. */ #define MIN_REAL_CHIP_SIZE 1024 /* Minimum chip size that can be defined for real chip in flashchips */ @@ -169,6 +167,26 @@ }, }; +static struct flashchip chip_8_16 = { + .vendor = "aklm", + /* See comment on previous chip. */ + .total_size = 1, + .tested = TEST_OK_PREW, + .gran = WRITE_GRAN_1BYTE, + .read = TEST_READ_INJECTOR, + .write = TEST_WRITE_INJECTOR, + .block_erasers = + { + { + .eraseblocks = { {8, MIN_REAL_CHIP_SIZE / 8} }, + .block_erase = TEST_ERASE_INJECTOR, + }, { + .eraseblocks = { {16, MIN_REAL_CHIP_SIZE / 16} }, + .block_erase = TEST_ERASE_INJECTOR, + } + }, +}; + static void setup_chip(struct flashrom_flashctx *flashctx, struct flashrom_layout **layout, const char *programmer_param, struct test_case *current_test_case) { @@ -230,8 +248,9 @@ /* * Setup all test cases. * - * First half of test cases (0 - (size(testcases)/2-1)) set up for a chip with erasers: 1, 2, 4, 8, 16 bytes. - * Second half (size(testcases)/2 - size(testcases)) repeates the same test cases for a chip with erasers: 1, 8, 16 bytes. + * First half of test cases is set up for a chip with erasers: 1, 2, 4, 8, 16 bytes. + * Second half repeates the same test cases for a chip with erasers: 1, 8, 16 bytes. + * Tests from #16 onwards use the chip with erasers: 8, 16 bytes, to test unaligned layout regions. */ static struct test_case test_cases[] = { { @@ -593,6 +612,94 @@ .eraseblocks_expected_ind = 16, .erase_test_name = "Erase test case #15", .write_test_name = "Write test case #15", + }, { + /* + * Test case #16 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned regions 2+4+9+1b which are smaller than the smallest eraseblock. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 1, "reg2"}, {2, 5, "reg4"}, {6, 14, "reg9"}, + {15, MIN_REAL_CHIP_SIZE - 1, "reg1"}}, + .initial_buf = {0x4, 0x4, 0x5, 0x5, 0x5, 0x5, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x7}, + .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 = {0x14, 0x14, 0x15, 0x15, 0x15, 0x15, 0x16, 0x16, + 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x17}, + .eraseblocks_expected = {{0x8, 0x8}, {0x0, 0x10}, {0x0, 0x8}, {0x0, 0x8}}, + .eraseblocks_expected_ind = 4, + .erase_test_name = "Erase test case #16", + .write_test_name = "Write test case #16", + }, { + /* + * Test case #17 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned region 3+13b which are smaller than the smallest eraseblock. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 2, "reg3"}, {3, MIN_REAL_CHIP_SIZE - 1, "tail"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .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 = {0x14, 0x14, 0x14, 0x16, 0x16, 0x16, 0x16, 0x16, + 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16}, + .eraseblocks_expected = {{0x0, 0x10}, {0x0, 0x8}}, + .eraseblocks_expected_ind = 2, + .erase_test_name = "Erase test case #17", + .write_test_name = "Write test case #17", + }, { + /* + * Test case #18 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned region 9+7b. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 8, "reg9"}, {9, MIN_REAL_CHIP_SIZE - 1, "tail"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x4, 0x4, 0x4, 0x4, 0x4, + 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .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 = {0x14, 0x14, 0x14, 0x14, 0x14, 0x14, 0x14, 0x14, + 0x14, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16}, + .eraseblocks_expected = {{0x8, 0x8}, {0x0, 0x10}}, + .eraseblocks_expected_ind = 2, + .erase_test_name = "Erase test case #18", + .write_test_name = "Write test case #18", + }, { + /* + * Test case #19 + * + * Initial vs written: 3 bytes of the logical layout are different, rest is the same. + * Layout with unaligned region 3 bytes. Layout does not cover the whole chip memory. + * Chip memory outside of logical layout is skipped by both erase and write ops. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 2, "reg3"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + .written_buf = {0x14, 0x14, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase test case #19", + .write_test_name = "Write test case #19", }, }; @@ -612,11 +719,13 @@ * The caller needs to free the allocated memory. */ struct CMUnitTest *get_erase_func_algo_tests(size_t *num_tests) { - // Twice the number of test cases, because each test case is run twice: for erase and write. - struct CMUnitTest *all_cases = calloc(TEST_CASES_NUM * 2, sizeof(struct CMUnitTest)); - *num_tests = TEST_CASES_NUM * 2; + const size_t test_cases_num = ARRAY_SIZE(test_cases); - for (size_t i = 0; i < TEST_CASES_NUM; i++) { + // Twice the number of test cases, because each test case is run twice: for erase and write. + struct CMUnitTest *all_cases = calloc(test_cases_num * 2, sizeof(struct CMUnitTest)); + *num_tests = test_cases_num * 2; + + for (size_t i = 0; i < test_cases_num; i++) { all_cases[i] = (struct CMUnitTest) { .name = test_cases[i].erase_test_name, .setup_func = setup, @@ -624,7 +733,7 @@ .initial_state = &test_cases[i], .test_func = erase_function_algo_test_success, }; - all_cases[i + TEST_CASES_NUM] = (struct CMUnitTest) { + all_cases[i + test_cases_num] = (struct CMUnitTest) { .name = test_cases[i].write_test_name, .setup_func = setup, .teardown_func = teardown, -- To view, visit https://review.coreboot.org/c/flashrom/+/78985?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: I726a30b0e47a966e8093ddc19abf4a65fb1d70ce Gerrit-Change-Number: 78985 Gerrit-PatchSet: 6 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: 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-MessageType: merged

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix unaligned region end offset by 1
by Anastasia Klimchuk (Code Review) 15 Jan '24

15 Jan '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78984?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: erasure_layout: Fix unaligned region end offset by 1 ...................................................................... erasure_layout: Fix unaligned region end offset by 1 In the case when layout region is not aligned with eraseblock, end region boundary is extended to match the eraseblock. There is a special handling of this extended area (between original end of region and extended one). Fix the offset of this extended area by +1 so that it covers the extended area and not the original region. Before the patch, the last byte of the original region was failed to write since it was treated as if an extended area, while it was the last byte of the normal layout region. Ticket: https://ticket.coreboot.org/issues/494 Change-Id: I7f78a0090065cd2a952cba1a5d28179483ba4c55 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78984 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M erasure_layout.c 1 file changed, 4 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/erasure_layout.c b/erasure_layout.c index c60305f..1dd6b60 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -279,9 +279,10 @@ memcpy(newcontents + region_start, curcontents + region_start, old_start - region_start); } if (region_end - old_end) { - read_flash(flashctx, curcontents + old_end, old_end, region_end - old_end); - memcpy(old_end_buf, newcontents + old_end, region_end - old_end); - memcpy(newcontents + old_end, curcontents + old_end, region_end - old_end); + chipoff_t end_offset = old_end + 1; + read_flash(flashctx, curcontents + end_offset, end_offset, region_end - old_end); + memcpy(old_end_buf, newcontents + end_offset, region_end - old_end); + memcpy(newcontents + end_offset, curcontents + end_offset, region_end - old_end); } // select erase functions -- To view, visit https://review.coreboot.org/c/flashrom/+/78984?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: I7f78a0090065cd2a952cba1a5d28179483ba4c55 Gerrit-Change-Number: 78984 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix unaligned region end offset by 1
by Anastasia Klimchuk (Code Review) 15 Jan '24

15 Jan '24

Attention is currently required from: Aarya, Alexander Goncharov, Carly Zlabek, Vincent Fazio. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78984?usp=email ) Change subject: erasure_layout: Fix unaligned region end offset by 1 ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > After some investigation I came to conclusion that no fix is needed for start boundary extension (se […] Alright, resolving this to unblock the patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/78984?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: I7f78a0090065cd2a952cba1a5d28179483ba4c55 Gerrit-Change-Number: 78984 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Mon, 15 Jan 2024 01:58:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashrom_tester: Fix partial_lock_test on libflashrom
by Hsuan-ting Chen (Code Review) 09 Jan '24

09 Jan '24

Attention is currently required from: Anastasia Klimchuk, David Wu, Edward O'Callaghan, Evan Benn, Hsuan Ting Chen. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79304?usp=email ) Change subject: flashrom_tester: Fix partial_lock_test on libflashrom ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Hi Anastasia, Ev and Ed have already finished one round of their code reviews. Could you please help me with the next round? Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/79304?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: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Gerrit-Change-Number: 79304 Gerrit-PatchSet: 3 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Evan Benn <evanbenn(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 09 Jan 2024 11:10:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix unaligned region end offset by 1
by Anastasia Klimchuk (Code Review) 08 Jan '24

08 Jan '24

Attention is currently required from: Aarya, Alexander Goncharov, Carly Zlabek, Vincent Fazio. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78984?usp=email ) Change subject: erasure_layout: Fix unaligned region end offset by 1 ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Carly, Vincent, Happy New Year! I left this patch for the holidays, but now I am gradually returning back. I thought to add you to this patch because you might be interested to have a look (or maybe test), I remember you are using this functionality a lot. I tested on some images (added comments in the ticket) and also wrote tests in the following patch CB:78985 Thank you so much for your help! -- To view, visit https://review.coreboot.org/c/flashrom/+/78984?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: I7f78a0090065cd2a952cba1a5d28179483ba4c55 Gerrit-Change-Number: 78984 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Mon, 08 Jan 2024 12:28:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashchips: Remove Macronix MX25U25635F from chip list
by Anastasia Klimchuk (Code Review) 06 Jan '24

06 Jan '24

Attention is currently required from: DZ, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79633?usp=email ) Change subject: flashchips: Remove Macronix MX25U25635F from chip list ...................................................................... Patch Set 1: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/79633/comment/e3440f1b_798cb553 : PS1, Line 7: Remove Macronix MX25U25635F from chip list If the patch goes through (see my other comment), then commit title need to mention both chips, for example: > Update definitions for MX25U25635F and MX25U25643G Patchset: PS1: Daniel, thank you for the patch! Sorry there was some delay in review, we had holidays here. Happy New Year! File flashchips.c: https://review.coreboot.org/c/flashrom/+/79633/comment/7ad9014e_951f3633 : PS1, Line 10310: .wps = {SECURITY, 7, OTP}, /* This bit is set by WPSEL command */ I looked through both datasheets, and found there is unfortunately this one difference! For MX25U25643G: Table 12. Security Register Definitions has bit7 set by WPSEL command (as it is in the existing definition) For MX25U25635F: Table 8. Security Register Definitions has bit7 marked as reserved So this one `.wps` definition in `.reg_bits` only applies to newer model it seems. In general, this was the only difference between two chip definitions, the older one had no support for write-protect operation defined, the newer one has it defined as marked as tested. Let's also check with Nikolai, but in general, if there is any difference between two chip definitions, we will need to leave both of them (people might still have old chips used). Your other idea about renaming model id macro is valid though, maybe it can be done even if both definitions stay. -- To view, visit https://review.coreboot.org/c/flashrom/+/79633?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: Ief3fd7641bed817066692c4abffff6d3b0df16b9 Gerrit-Change-Number: 79633 Gerrit-PatchSet: 1 Gerrit-Owner: DZ 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: DZ Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 06 Jan 2024 09:23:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit