September 2021 - flashrom-gerrit

Change in flashrom[master]: dediprog.c: Drop dediprog_ prefix for spi data struct members
by Nico Huber (Code Review) 03 Sep '21

03 Sep '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56415 ) Change subject: dediprog.c: Drop dediprog_ prefix for spi data struct members ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File dediprog.c: https://review.coreboot.org/c/flashrom/+/56415/comment/95ce29d3_46dd2e45 PS3, Line 494: dp_data->handle); Might look better without the linebreak now that it's shorter again. -- To view, visit https://review.coreboot.org/c/flashrom/+/56415 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I688d50926b78a6c3f1c5a8ba4ef88a0d5b495bd0 Gerrit-Change-Number: 56415 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 03 Sep 2021 15:07:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dediprog.c: Refactor singleton states into reentrant pattern
by Nico Huber (Code Review) 03 Sep '21

03 Sep '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56414 ) Change subject: dediprog.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: Code-Review+1 (4 comments) File dediprog.c: https://review.coreboot.org/c/flashrom/+/56414/comment/e8c302fb_e41603bd PS3, Line 221: const int finish, Please indent at least after the opening parenthesis (or only two-tabs total, I think I've seen that as well). https://review.coreboot.org/c/flashrom/+/56414/comment/961f98b7_bc18c9e1 PS3, Line 222: struct libusb_context *usb_ctx) Usually such context information is made the first parameter. https://review.coreboot.org/c/flashrom/+/56414/comment/6c22ee53_f55e3a20 PS3, Line 242: size_t size, I don't see a need to waste one line each. For instance, cmd & value & idx and bytes & size could be grouped together. https://review.coreboot.org/c/flashrom/+/56414/comment/e80cab5f_e721d231 PS3, Line 1025: dp_data->dediprog_devicetype = DEV_UNKNOWN; This shouldn't be necessary anymore as the data is discarded. -- To view, visit https://review.coreboot.org/c/flashrom/+/56414 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I72085e750af97b94dfa94f2ebf2a134e41a2ec8d Gerrit-Change-Number: 56414 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 03 Sep 2021 15:04:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add NOT_ZERO and NOT_NULL macros to the header file
by Edward O'Callaghan (Code Review) 03 Sep '21

03 Sep '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57269 ) Change subject: tests: Add NOT_ZERO and NOT_NULL macros to the header file ...................................................................... Patch Set 1: (2 comments) File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/57269/comment/6ae282cb_b652a755 PS1, Line 37: NOT NONZERO ? https://review.coreboot.org/c/flashrom/+/57269/comment/687488af_c3b05cd2 PS1, Line 38: NOT_NULL ((void *)NOT_ZERO) I wonder if this should be a function ptr address so that you can set a break point on the address easily as it has a named symbol associated with the address number. A stub function would be sufficient to serve this purpose. `void * nonnull(void) { return (void *)NONZERO; }` -- To view, visit https://review.coreboot.org/c/flashrom/+/57269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ad6ee4aa9091447c6c9108c92bf7f6e755fca48 Gerrit-Change-Number: 57269 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 03 Sep 2021 04:09:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add NOT_ZERO and NOT_NULL macros to the header file
by Edward O'Callaghan (Code Review) 03 Sep '21

03 Sep '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57269 ) Change subject: tests: Add NOT_ZERO and NOT_NULL macros to the header file ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/57269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ad6ee4aa9091447c6c9108c92bf7f6e755fca48 Gerrit-Change-Number: 57269 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 03 Sep 2021 04:05:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to read from chip
by Edward O'Callaghan (Code Review) 03 Sep '21

03 Sep '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57326 ) Change subject: tests: Add tests to read from chip ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: > This one also extracts setup_for_chip() and teardown() since they are repeating for every test. […] Can the refactor and the new tests be split into two patches to make it slightly easier to review? -- To view, visit https://review.coreboot.org/c/flashrom/+/57326 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia57781ebc670c7bd6197e56fe8a20651a425c756 Gerrit-Change-Number: 57326 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 03 Sep 2021 02:08:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to read from chip
by Anastasia Klimchuk (Code Review) 02 Sep '21

02 Sep '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57326 ) Change subject: tests: Add tests to read from chip ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: This one also extracts setup_for_chip() and teardown() since they are repeating for every test. I wasn't sure, but please tell me if you think this should be in a separate commit. Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/57326 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia57781ebc670c7bd6197e56fe8a20651a425c756 Gerrit-Change-Number: 57326 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-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-Comment-Date: Thu, 02 Sep 2021 01:33:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to read from chip
by Anastasia Klimchuk (Code Review) 02 Sep '21

02 Sep '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/57326 ) Change subject: tests: Add tests to read from chip ...................................................................... tests: Add tests to read from chip Two tests cover the code which performs do_read operation. First one works with fake chip and dummy programmer. Fake chip has all operations defined, and a buffer to emulate chip memory. Second one uses the chip which is closer to the real one, because read/write/unlock/erase operations are real. The tests takes the advantage of dummyflasher's capability of emulating a W25Q128.V chip. BUG=b:181803212 TEST=builds and ninja test Change-Id: Ia57781ebc670c7bd6197e56fe8a20651a425c756 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 5 files changed, 197 insertions(+), 54 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/57326/1 diff --git a/tests/chip.c b/tests/chip.c index 0cb05ad..8b254e4 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -86,6 +86,50 @@ return 0; } +static void reset_global_chip_state() +{ + g_chip_state.unlock_calls = 0; +} + +static void setup_for_chip(struct flashrom_flashctx *flash, struct flashrom_layout **layout, + struct flashchip *chip, const char *programmer_param) +{ + flash->chip = chip; + + reset_global_chip_state(); + + printf("Creating layout with one included region... "); + assert_int_equal(0, flashrom_layout_new(layout)); + /* One region which covers total size of chip. */ + assert_int_equal(0, flashrom_layout_add_region(*layout, 0, chip->total_size * 1024 - 1, "region")); + assert_int_equal(0, flashrom_layout_include_region(*layout, "region")); + + flashrom_layout_set(flash, *layout); + printf("done\n"); + + /* + * We need some programmer (any), and dummy is a very good one, + * because it doesn't need any mocking. So no extra complexity + * from a programmer side, and test can focus on working with the chip. + */ + printf("Dummyflasher initialising with param=\"%s\"... ", programmer_param); + assert_int_equal(0, programmer_init(&programmer_dummy, programmer_param)); + /* Assignment below normally happens while probing, but this test is not probing. */ + flash->mst = &registered_masters[0]; + printf("done\n"); +} + +static void teardown(struct flashrom_layout **layout) +{ + printf("Dummyflasher shutdown... "); + assert_int_equal(0, programmer_shutdown()); + printf("done\n"); + + printf("Releasing layout... "); + flashrom_layout_release(*layout); + printf("done\n"); +} + void erase_chip_test_success(void **state) { (void) state; /* unused */ @@ -108,43 +152,17 @@ .block_erase = block_erase_chip, }}, }; - struct flashrom_flashctx flash = { - .chip = &chip, - }; - + struct flashrom_flashctx flash = { 0 }; struct flashrom_layout *layout; + const char *param = ""; /* Default values for all params. */ - printf("Creating layout with one included region... "); - assert_int_equal(0, flashrom_layout_new(&layout)); - /* One region which covers total size of chip. */ - assert_int_equal(0, flashrom_layout_add_region(layout, 0, chip.total_size * 1024 - 1, "region")); - assert_int_equal(0, flashrom_layout_include_region(layout, "region")); - - flashrom_layout_set(&flash, layout); - printf("done\n"); - - /* - * We need some programmer (any), and dummy is a very good one, - * because it doesn't need any mocking. So no extra complexity - * from a programmer side, and test can focus on the code which - * erases the chip. - */ - const char *param = ""; /* default values for all params */ - printf("Dummyflasher initialising... "); - assert_int_equal(0, programmer_init(&programmer_dummy, param)); - printf("done\n"); + setup_for_chip(&flash, &layout, &chip, param); printf("Erase chip operation started.\n"); assert_int_equal(0, do_erase(&flash)); printf("Erase chip operation done.\n"); - printf("Dummyflasher shutdown... "); - assert_int_equal(0, programmer_shutdown()); - printf("done\n"); - - printf("Releasing layout... "); - flashrom_layout_release(layout); - printf("done\n"); + teardown(&layout); } void erase_chip_with_dummyflasher_test_success(void **state) @@ -184,43 +202,119 @@ } }, }; - struct flashrom_flashctx flash = { - .chip = &chip, - }; + struct flashrom_flashctx flash = { 0 }; struct flashrom_layout *layout; - - printf("Creating layout with one included region... "); - assert_int_equal(0, flashrom_layout_new(&layout)); - /* One region which covers total size of chip. */ - assert_int_equal(0, flashrom_layout_add_region(layout, 0, chip.total_size * 1024 - 1, "region")); - assert_int_equal(0, flashrom_layout_include_region(layout, "region")); - - flashrom_layout_set(&flash, layout); - printf("done\n"); - /* * Dummyflasher is capable to emulate a chip, so we ask it to do this. * Nothing to mock, dummy is taking care of this already. */ char *param_dup = strdup("bus=spi,emulate=W25Q128FV"); - printf("Dummyflasher initialising to emulate W25Q128FV... "); - assert_int_equal(0, programmer_init(&programmer_dummy, param_dup)); - /* Assignment below normally happens while probing, but this test is not probing. */ - flash.mst = &registered_masters[0]; - printf("done\n"); + + setup_for_chip(&flash, &layout, &chip, param_dup); printf("Erase chip operation started.\n"); assert_int_equal(0, do_erase(&flash)); printf("Erase chip operation done.\n"); - printf("Dummyflasher shutdown... "); - assert_int_equal(0, programmer_shutdown()); - printf("done\n"); + teardown(&layout); - printf("Releasing layout... "); - flashrom_layout_release(layout); - printf("done\n"); + free(param_dup); +} + +void read_chip_test_success(void **state) +{ + (void) state; /* unused */ + + struct flashchip chip = { + .vendor = "aklm", + /* + * Total size less than 16 to skip some steps + * in flashrom.c#prepare_flash_access. + */ + .total_size = 8, + .tested = TEST_OK_PREW, + .read = read_chip, + .write = write_chip, + .unlock = unlock_chip, + .block_erasers = + {{ + /* All blocks within total size of the chip. */ + .eraseblocks = { {2 * 1024, 4} }, + .block_erase = block_erase_chip, + }}, + }; + + struct flashrom_flashctx flash = { 0 }; + struct flashrom_layout *layout; + const char *param = ""; /* Default values for all params. */ + + setup_for_chip(&flash, &layout, &chip, param); + + const char *const filename = "read_chip.test"; + + printf("Read chip operation started.\n"); + assert_int_equal(0, do_read(&flash, filename)); + printf("Read chip operation done.\n"); + + teardown(&layout); +} + +void read_chip_with_dummyflasher_test_success(void **state) +{ + (void) state; /* unused */ + + struct flashchip chip = { + .vendor = "aklm&dummyflasher", + /* + * Setup the values for W25Q128.V because we ask dummyflasher + * to emulate this chip. All operations: read/write/unlock/erase + * are real, not mocks, and they are expected to be handled by + * dummyflasher. + */ + .total_size = 16 * 1024, + .tested = TEST_OK_PREW, + .read = spi_chip_read, + .write = spi_chip_write_256, + .unlock = spi_disable_blockprotect, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 4096} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 512} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + }; + + struct flashrom_flashctx flash = { 0 }; + struct flashrom_layout *layout; + /* + * Dummyflasher is capable to emulate a chip, so we ask it to do this. + * Nothing to mock, dummy is taking care of this already. + */ + char *param_dup = strdup("bus=spi,emulate=W25Q128FV"); + + setup_for_chip(&flash, &layout, &chip, param_dup); + + const char *const filename = "read_chip.test"; + + printf("Read chip operation started.\n"); + assert_int_equal(0, do_read(&flash, filename)); + printf("Read chip operation done.\n"); + + teardown(&layout); free(param_dup); } diff --git a/tests/io_mock.h b/tests/io_mock.h index adb5f3b..a285e53 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -53,6 +53,9 @@ /* Linux I2C interface constants, avoiding linux/i2c-dev.h */ #define I2C_SLAVE 0x0703 +/* Always return success for tests. */ +#define S_ISREG(x) 0 + struct io_mock { void *state; diff --git a/tests/meson.build b/tests/meson.build index 53885a8..8019bd3 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -39,8 +39,14 @@ '-Wl,--wrap=write', '-Wl,--wrap=fopen', '-Wl,--wrap=fopen64', + '-Wl,--wrap=fwrite', + '-Wl,--wrap=fflush', '-Wl,--wrap=stat', '-Wl,--wrap=stat64', + '-Wl,--wrap=fstat', + '-Wl,--wrap=fstat64', + '-Wl,--wrap=fileno', + '-Wl,--wrap=fsync', '-Wl,--wrap=fread', '-Wl,--wrap=fgets', '-Wl,--wrap=fclose', diff --git a/tests/tests.c b/tests/tests.c index 4965fe1..46d60c2 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -154,6 +154,18 @@ return 0; } +int __wrap_fstat(int fd, void *buf) +{ + LOG_ME; + return 0; +} + +int __wrap_fstat64(int fd, void *buf) +{ + LOG_ME; + return 0; +} + char *__wrap_fgets(char *buf, int len, FILE *fp) { LOG_ME; @@ -170,6 +182,30 @@ return 0; } +size_t __wrap_fwrite(const void *ptr, size_t size, size_t len, FILE *fp) +{ + LOG_ME; + return size * len; +} + +int __wrap_fflush(FILE *fp) +{ + LOG_ME; + return 0; +} + +int __wrap_fileno(FILE *fp) +{ + LOG_ME; + return MOCK_HANDLE; +} + +int __wrap_fsync(int fd) +{ + LOG_ME; + return 0; +} + int __wrap_setvbuf(FILE *fp, char *buf, int type, size_t size) { LOG_ME; @@ -359,6 +395,8 @@ const struct CMUnitTest chip_tests[] = { cmocka_unit_test(erase_chip_test_success), cmocka_unit_test(erase_chip_with_dummyflasher_test_success), + cmocka_unit_test(read_chip_test_success), + cmocka_unit_test(read_chip_with_dummyflasher_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 df4a41c..70b041f 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -61,5 +61,7 @@ /* chip.c */ void erase_chip_test_success(void **state); void erase_chip_with_dummyflasher_test_success(void **state); +void read_chip_test_success(void **state); +void read_chip_with_dummyflasher_test_success(void **state); #endif /* TESTS_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/57326 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia57781ebc670c7bd6197e56fe8a20651a425c756 Gerrit-Change-Number: 57326 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Add support for Intel Emmitsburg PCH
by David Hendricks (Code Review) 01 Sep '21

01 Sep '21

David Hendricks has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54965 ) Change subject: Add support for Intel Emmitsburg PCH ...................................................................... Add support for Intel Emmitsburg PCH This patch does the following: - Add PCIe ID for Intel Emmitsburg PCH - Based on ICH descriptor content, choose CHIPSET_C620_SERIES_LEWISBURG if ISL/PSL is 80. TESTED=tried on a server with Intel Emmitsburg PCH, flash update was successful. This server, however, does not have flash chip installed, it instead has em100 emulator connected. Change-Id: I2a1bb7467e693d1583aa885fa0e277075edd4a3e Signed-off-by: Jonathan Zhang <jonzhang(a)fb.com> Signed-off-by: David Hendricks <ddaveh(a)amazon.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54965 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Christian Walter <christian.walter(a)9elements.com> Reviewed-by: Johnny Lin <Johnny_Lin(a)wiwynn.com> Reviewed-by: Tim Chu <Tim.Chu(a)quantatw.com> --- M chipset_enable.c M ich_descriptors.c 2 files changed, 3 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Christian Walter: Looks good to me, approved Johnny Lin: Looks good to me, but someone else must approve Tim Chu: Looks good to me, but someone else must approve diff --git a/chipset_enable.c b/chipset_enable.c index cdd51ae..110d7a9 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -2073,6 +2073,7 @@ {0x8086, 0xa247, B_S, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, {0x8086, 0xa248, B_S, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, {0x8086, 0xa249, B_S, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, + {0x8086, 0x1bca, B_S, NT, "Intel", "Emmitsburg Chipset SKU", enable_flash_c620}, {0x8086, 0xa2c4, B_S, NT, "Intel", "H270", enable_flash_pch100}, {0x8086, 0xa2c5, B_S, NT, "Intel", "Z270", enable_flash_pch100}, {0x8086, 0xa2c6, B_S, NT, "Intel", "Q270", enable_flash_pch100}, diff --git a/ich_descriptors.c b/ich_descriptors.c index aba7528..d56ade7 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -939,6 +939,8 @@ warn_peculiar_desc(content->ISL != 23, "Gemini Lake"); return CHIPSET_GEMINI_LAKE; } + if (content->ISL <= 80) + return CHIPSET_C620_SERIES_LEWISBURG; warn_peculiar_desc(content->ISL != 16, "Ibex Peak"); return CHIPSET_5_SERIES_IBEX_PEAK; } else if (upper->MDTBA == 0x00) { -- To view, visit https://review.coreboot.org/c/flashrom/+/54965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a1bb7467e693d1583aa885fa0e277075edd4a3e Gerrit-Change-Number: 54965 Gerrit-PatchSet: 3 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: Add support for Intel Emmitsburg PCH
by Tim Chu (Code Review) 01 Sep '21

01 Sep '21

Attention is currently required from: Arthur Heymans, Jonathan Zhang, David Hendricks, Edward O'Callaghan, Deomid "rojer" Ryabkov. Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54965 ) Change subject: Add support for Intel Emmitsburg PCH ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: Verified on S9S. Can update BIOS successfully. -- To view, visit https://review.coreboot.org/c/flashrom/+/54965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a1bb7467e693d1583aa885fa0e277075edd4a3e Gerrit-Change-Number: 54965 Gerrit-PatchSet: 2 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks Gerrit-Reviewer: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: David Hendricks Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Comment-Date: Wed, 01 Sep 2021 10:03:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add support for Intel Emmitsburg PCH
by Johnny Lin (Code Review) 01 Sep '21

01 Sep '21

Attention is currently required from: Arthur Heymans, Jonathan Zhang, David Hendricks, Edward O'Callaghan, Deomid "rojer" Ryabkov, Tim Chu. Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54965 ) Change subject: Add support for Intel Emmitsburg PCH ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: > Johnny/Tim, please try this out on Craterlake/S9S. Verified on Crater Lake inband update BIOS region successfully. -- To view, visit https://review.coreboot.org/c/flashrom/+/54965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a1bb7467e693d1583aa885fa0e277075edd4a3e Gerrit-Change-Number: 54965 Gerrit-PatchSet: 2 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks Gerrit-Reviewer: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: David Hendricks Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Wed, 01 Sep 2021 08:36:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2021