- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: flash.h: Make functions global that will be used for new erase algorithm
by Anastasia Klimchuk (Code Review) 15 Jan '23

15 Jan '23

Attention is currently required from: Simon Buhrow, Thomas Heijligen, Aarya. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71174 ) Change subject: flash.h: Make functions global that will be used for new erase algorithm ...................................................................... Patch Set 4: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/71174/comment/93bfba90_12744877 PS4, Line 10: make these functions non global "make these functions global" -> remove "non" :) commit title and the patch itself makes functions global. -- To view, visit https://review.coreboot.org/c/flashrom/+/71174 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ee7e208948337b88467935fd2861b5f9ad6af9d Gerrit-Change-Number: 71174 Gerrit-PatchSet: 4 Gerrit-Owner: 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: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Sun, 15 Jan 2023 06:54:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c: Plumb 'all_skipped' global state into func param
by Anastasia Klimchuk (Code Review) 15 Jan '23

15 Jan '23

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Aarya. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67093 ) Change subject: flashrom.c: Plumb 'all_skipped' global state into func param ...................................................................... Patch Set 38: Code-Review+2 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67093/comment/7ba11910_07656af5 PS4, Line 11: > Done Just last thing: add the testing info into commit message. Something like: running <command with dummy> twice, displays a message <message about same content>. Thank you! changing commit message should not affect approvals. Patchset: PS38: Thanks so much for your work and patience! I think this needs one last rebase and add testing info in commit message (see my other comment), and that would be it. and then let's merge :) -- To view, visit https://review.coreboot.org/c/flashrom/+/67093 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2346c869c47b48604360b0facf9313aae086c8dd Gerrit-Change-Number: 67093 Gerrit-PatchSet: 38 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Sun, 15 Jan 2023 06:46:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: MAINTAINERS: Add Sergii Dmytruk for write-protect
by Anastasia Klimchuk (Code Review) 15 Jan '23

15 Jan '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71881 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: MAINTAINERS: Add Sergii Dmytruk for write-protect ...................................................................... MAINTAINERS: Add Sergii Dmytruk for write-protect Change-Id: Ia730fcff863b5590d76cfaabe76badd503d4a60c Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/71881 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M MAINTAINERS 1 file changed, 15 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/MAINTAINERS b/MAINTAINERS index 2406952..6e4c40e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -98,6 +98,7 @@ WRITEPROTECT M: Nikolai Artemiev <nartemiev(a)google.com> +M: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> S: Supported F: writeprotect* F: include/writeprotect* -- To view, visit https://review.coreboot.org/c/flashrom/+/71881 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia730fcff863b5590d76cfaabe76badd503d4a60c Gerrit-Change-Number: 71881 Gerrit-PatchSet: 3 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: tests/: Assert on NULL heap allocations in tests
by Edward O'Callaghan (Code Review) 15 Jan '23

15 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71919 ) Change subject: tests/: Assert on NULL heap allocations in tests ...................................................................... tests/: Assert on NULL heap allocations in tests Change-Id: Id2adcfe859fb25d2a7f0734655c6b9a58c0890b6 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip.c M tests/chip_wp.c M tests/helpers.c M tests/raiden_debug_spi.c 4 files changed, 23 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/71919/1 diff --git a/tests/chip.c b/tests/chip.c index 194e114..5b025d3 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -279,6 +279,7 @@ const char *const filename = "read_chip.test"; unsigned long size = mock_chip.total_size * 1024; unsigned char *buf = calloc(size, sizeof(unsigned char)); + assert_non_null(buf); printf("Read chip operation started.\n"); assert_int_equal(0, flashrom_image_read(&flashctx, buf, size)); @@ -316,6 +317,7 @@ const char *const filename = "read_chip.test"; unsigned long size = mock_chip.total_size * 1024; unsigned char *buf = calloc(size, sizeof(unsigned char)); + assert_non_null(buf); printf("Read chip operation started.\n"); assert_int_equal(0, flashrom_image_read(&flashctx, buf, size)); @@ -365,6 +367,7 @@ const char *const filename = "-"; unsigned long size = mock_chip.total_size * 1024; uint8_t *const newcontents = malloc(size); + assert_non_null(newcontents); printf("Write chip operation started.\n"); assert_int_equal(0, read_buf_from_file(newcontents, size, filename)); @@ -403,6 +406,7 @@ const char *const filename = "-"; unsigned long size = mock_chip.total_size * 1024; uint8_t *const newcontents = malloc(size); + assert_non_null(newcontents); printf("Write chip operation started.\n"); assert_int_equal(0, read_buf_from_file(newcontents, size, filename)); @@ -452,6 +456,7 @@ const char *const filename = "-"; unsigned long size = mock_chip.total_size * 1024; uint8_t *const newcontents = malloc(size); + assert_non_null(newcontents); printf("Verify chip operation started.\n"); assert_int_equal(0, read_buf_from_file(newcontents, size, filename)); @@ -491,6 +496,7 @@ const char *const filename = "-"; unsigned long size = mock_chip.total_size * 1024; uint8_t *const newcontents = malloc(size); + assert_non_null(newcontents); /* * Dummyflasher controls chip state and fully emulates reads and writes, diff --git a/tests/chip_wp.c b/tests/chip_wp.c index f20b500..cb9b3ab 100644 --- a/tests/chip_wp.c +++ b/tests/chip_wp.c @@ -269,6 +269,7 @@ * be optimized away. */ unsigned long size = flashrom_flash_getsize(&flash); uint8_t *const contents = malloc(size); + assert_non_null(contents); memset(contents, UNERASED_VALUE(&flash), size); assert_int_equal(0, flashrom_image_write(&flash, contents, size, NULL)); free(contents); diff --git a/tests/helpers.c b/tests/helpers.c index e2e652e..271fb48 100644 --- a/tests/helpers.c +++ b/tests/helpers.c @@ -45,6 +45,7 @@ const char src0[] = "hello"; const char src1[] = " world"; char *dest = calloc(1, 1); + assert_non_null(dest); dest = strcat_realloc(dest, src0); dest = strcat_realloc(dest, src1); assert_string_equal("hello world", dest); diff --git a/tests/raiden_debug_spi.c b/tests/raiden_debug_spi.c index f5bf185..5c79a90 100644 --- a/tests/raiden_debug_spi.c +++ b/tests/raiden_debug_spi.c @@ -19,6 +19,7 @@ static ssize_t raiden_debug_libusb_get_device_list(void *state, libusb_context *ctx, libusb_device ***list) { *list = calloc(1, sizeof(**list)); + assert_non_null(*list); /* * libusb_device is opaque type, it is tossed around between libusb functions but always @@ -48,10 +49,14 @@ void *state, libusb_device *dev, uint8_t config_index, struct libusb_config_descriptor **config) { *config = calloc(1, sizeof(**config)); + assert_non_null(*config); struct libusb_endpoint_descriptor *tmp_endpoint = calloc(2, sizeof(*tmp_endpoint)); + assert_non_null(tmp_endpoint); struct libusb_interface_descriptor *tmp_interface_desc = calloc(1, sizeof(*tmp_interface_desc)); + assert_non_null(tmp_interface_desc); struct libusb_interface *tmp_interface = calloc(1, sizeof(*tmp_interface)); + assert_non_null(tmp_interface); /* in endpoint */ tmp_endpoint[0].bEndpointAddress = 0x80; -- To view, visit https://review.coreboot.org/c/flashrom/+/71919 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id2adcfe859fb25d2a7f0734655c6b9a58c0890b6 Gerrit-Change-Number: 71919 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: tests/lifecycle.c: Avoid unnecessary heap allocations
by Edward O'Callaghan (Code Review) 15 Jan '23

15 Jan '23

Attention is currently required from: Anastasia Klimchuk. Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71918 ) Change subject: tests/lifecycle.c: Avoid unnecessary heap allocations ...................................................................... tests/lifecycle.c: Avoid unnecessary heap allocations Just use a static string on the stack. Change-Id: I0414ab9a63867fc58b04ad62ed3ec4f221448a58 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/lifecycle.c 1 file changed, 15 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/18/71918/1 diff --git a/tests/lifecycle.c b/tests/lifecycle.c index 0c79699..95c4249 100644 --- a/tests/lifecycle.c +++ b/tests/lifecycle.c @@ -40,10 +40,8 @@ struct flashrom_programmer *flashprog; - char *param_dup = param ? strdup(param) : NULL; - printf("Testing flashrom_programmer_init for programmer=%s ...\n", prog->name); - assert_int_equal(0, flashrom_programmer_init(&flashprog, prog->name, param_dup)); + assert_int_equal(0, flashrom_programmer_init(&flashprog, prog->name, param)); printf("... flashrom_programmer_init for programmer=%s successful\n", prog->name); if (action) @@ -53,8 +51,6 @@ assert_int_equal(0, flashrom_programmer_shutdown(flashprog)); printf("... flashrom_programmer_shutdown for programmer=%s successful\n", prog->name); - free(param_dup); - io_mock_register(NULL); } @@ -82,10 +78,9 @@ io_mock_register(io); struct flashrom_programmer *flashprog; - char *param_dup = strdup(param); - printf("Testing init error path for programmer=%s with params: %s ...\n", prog->name, param_dup); - assert_int_equal(error_code, flashrom_programmer_init(&flashprog, prog->name, param_dup)); + printf("Testing init error path for programmer=%s with params: %s ...\n", prog->name, param); + assert_int_equal(error_code, flashrom_programmer_init(&flashprog, prog->name, param)); printf("... init failed with error code %i as expected\n", error_code); /* @@ -99,7 +94,5 @@ assert_int_equal(0, flashrom_programmer_shutdown(flashprog)); printf("... completed\n"); - free(param_dup); - io_mock_register(NULL); } -- To view, visit https://review.coreboot.org/c/flashrom/+/71918 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0414ab9a63867fc58b04ad62ed3ec4f221448a58 Gerrit-Change-Number: 71918 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: tests/chip{_wp}.c: Avoid unnecessary heap allocations
by Edward O'Callaghan (Code Review) 15 Jan '23

15 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71917 ) Change subject: tests/chip{_wp}.c: Avoid unnecessary heap allocations ...................................................................... tests/chip{_wp}.c: Avoid unnecessary heap allocations Just use a static string on the stack. Change-Id: Ic6cb4f32094ae5868912ebcffc8ab21026c48d32 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip.c M tests/chip_wp.c 2 files changed, 22 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/71917/1 diff --git a/tests/chip.c b/tests/chip.c index 1d9cdc9..194e114 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -243,7 +243,7 @@ * Dummyflasher is capable to emulate W25Q128.V, 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"); + const char *param_dup = "bus=spi,emulate=W25Q128FV"; setup_chip(&flashctx, &layout, &mock_chip, param_dup, &chip_io); @@ -252,8 +252,6 @@ printf("Erase chip operation done.\n"); teardown(&layout); - - free(param_dup); } void read_chip_test_success(void **state) @@ -311,7 +309,7 @@ * Dummyflasher is capable to emulate W25Q128.V, 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"); + const char *param_dup = "bus=spi,emulate=W25Q128FV"; setup_chip(&flashctx, &layout, &mock_chip, param_dup, &chip_io); @@ -326,7 +324,6 @@ teardown(&layout); - free(param_dup); free(buf); } @@ -398,7 +395,7 @@ * Dummyflasher is capable to emulate W25Q128.V, 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"); + const char *param_dup = "bus=spi,emulate=W25Q128FV"; setup_chip(&flashctx, &layout, &mock_chip, param_dup, &chip_io); @@ -414,7 +411,6 @@ teardown(&layout); - free(param_dup); free(newcontents); } @@ -487,7 +483,7 @@ * Dummyflasher is capable to emulate W25Q128.V, 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"); + const char *param_dup = "bus=spi,emulate=W25Q128FV"; setup_chip(&flashctx, &layout, &mock_chip, param_dup, &verify_chip_io); @@ -514,6 +510,5 @@ teardown(&layout); - free(param_dup); free(newcontents); } diff --git a/tests/chip_wp.c b/tests/chip_wp.c index 40303ff..f20b500 100644 --- a/tests/chip_wp.c +++ b/tests/chip_wp.c @@ -106,7 +106,7 @@ { (void) state; /* unused */ - char *param_dup = strdup("bus=spi,emulate=W25Q128FV,hwwp=no"); + const char *param_dup = "bus=spi,emulate=W25Q128FV,hwwp=no"; struct flashrom_flashctx flash = { 0 }; struct flashchip mock_chip = chip_W25Q128_V; @@ -123,7 +123,6 @@ teardown(NULL); flashrom_wp_cfg_release(wp_cfg); - free(param_dup); } /* Enabling hardware WP with a valid range succeeds */ @@ -131,7 +130,7 @@ { (void) state; /* unused */ - char *param_dup = strdup("bus=spi,emulate=W25Q128FV,hwwp=no"); + const char *param_dup = "bus=spi,emulate=W25Q128FV,hwwp=no"; struct flashrom_flashctx flash = { 0 }; struct flashchip mock_chip = chip_W25Q128_V; @@ -158,7 +157,6 @@ teardown(NULL); flashrom_wp_cfg_release(wp_cfg); - free(param_dup); } /* Enable hardware WP and verify that it can not be unset */ @@ -166,7 +164,7 @@ { (void) state; /* unused */ - char *param_dup = strdup("bus=spi,emulate=W25Q128FV,hwwp=yes"); + const char *param_dup = "bus=spi,emulate=W25Q128FV,hwwp=yes"; struct flashrom_flashctx flash = { 0 }; struct flashchip mock_chip = chip_W25Q128_V; @@ -198,7 +196,6 @@ teardown(NULL); flashrom_wp_cfg_release(wp_cfg); - free(param_dup); } /* WP state is decoded correctly from status registers */ @@ -220,7 +217,7 @@ * Multiplaying that by base unit gives 16 KiB protected region at the * bottom (start of the chip), which is then complemented. */ - char *param_dup = strdup("bus=spi,emulate=W25Q128FV,spi_status=0x41ec"); + const char *param_dup = "bus=spi,emulate=W25Q128FV,spi_status=0x41ec"; struct flashrom_flashctx flash = { 0 }; struct flashchip mock_chip = chip_W25Q128_V; @@ -243,7 +240,6 @@ teardown(NULL); flashrom_wp_cfg_release(wp_cfg); - free(param_dup); } /* Enabled WP makes full chip erasure fail */ @@ -256,7 +252,7 @@ struct flashchip mock_chip = chip_W25Q128_V; struct flashrom_wp_cfg *wp_cfg; - char *param_dup = strdup("bus=spi,emulate=W25Q128FV,hwwp=yes"); + const char *param_dup = "bus=spi,emulate=W25Q128FV,hwwp=yes"; setup_chip(&flash, &layout, &mock_chip, param_dup); /* Layout regions are created by setup_chip(). */ @@ -293,7 +289,6 @@ teardown(&layout); flashrom_wp_cfg_release(wp_cfg); - free(param_dup); } /* Enabled WP does not block erasing unprotected parts of the chip */ @@ -306,7 +301,7 @@ struct flashchip mock_chip = chip_W25Q128_V; struct flashrom_wp_cfg *wp_cfg; - char *param_dup = strdup("bus=spi,emulate=W25Q128FV,hwwp=yes"); + const char *param_dup = "bus=spi,emulate=W25Q128FV,hwwp=yes"; setup_chip(&flash, &layout, &mock_chip, param_dup); /* Layout region is created by setup_chip(). */ @@ -330,5 +325,4 @@ teardown(&layout); flashrom_wp_cfg_release(wp_cfg); - free(param_dup); } -- To view, visit https://review.coreboot.org/c/flashrom/+/71917 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic6cb4f32094ae5868912ebcffc8ab21026c48d32 Gerrit-Change-Number: 71917 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: writeprotect.c: differentiate lack of WP from lack of implementation
by Edward O'Callaghan (Code Review) 15 Jan '23

15 Jan '23

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68221 ) Change subject: writeprotect.c: differentiate lack of WP from lack of implementation ...................................................................... Patch Set 7: (2 comments) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/68221/comment/b05c8681_2b73ded6 PS7, Line 425: verify_chip_support `chip_supported()` makes more sense imho. https://review.coreboot.org/c/flashrom/+/68221/comment/88885bfc_8b8d2929 PS7, Line 427: flash->chip == NULL `!ptr` is canonical. -- To view, visit https://review.coreboot.org/c/flashrom/+/68221 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e75b124c21ccab51a9b5e1cd344b5310d384187 Gerrit-Change-Number: 68221 Gerrit-PatchSet: 7 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Sun, 15 Jan 2023 01:30:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Reduce the prog global state machine
by Edward O'Callaghan (Code Review) 15 Jan '23

15 Jan '23

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70603 ) Change subject: flashrom.c: Reduce the prog global state machine ...................................................................... Patch Set 2: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/70603/comment/834d955a_16b10c6b PS1, Line 41: g_programmer_type > Since this is a bool, I would name this `g_is_internal_programmer`. […] Well this is ephemeral, I wouldn't wrangle over the perfect name, the `_type` suffix is meant to give disambiguation from the `g_programmer`. -- To view, visit https://review.coreboot.org/c/flashrom/+/70603 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I93dfe09fe877acd32c4f22665921544ccd9323f6 Gerrit-Change-Number: 70603 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 15 Jan 2023 01:24:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal,board_enable: Remove force_boardenable from globals
by Edward O'Callaghan (Code Review) 15 Jan '23

15 Jan '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70031 ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: internal,board_enable: Remove force_boardenable from globals ...................................................................... internal,board_enable: Remove force_boardenable from globals Make `force_boardenable` stack local to the internal_init() entry-point. It's life-time should not exceed that of the internal's init entry function. Change-Id: I3324681f024003694a5531d9d35bb13d2c583eb0 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70031 Reviewed-by: Sam McNally <sammc(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M board_enable.c M include/programmer.h M internal.c 3 files changed, 34 insertions(+), 17 deletions(-) Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved diff --git a/board_enable.c b/board_enable.c index bf26173..d35b151 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2683,7 +2683,7 @@ return NULL; } -static int board_enable_safetycheck(const struct board_match *board) +static int board_enable_safetycheck(const struct board_match *board, bool force_boardenable) { if (!board) return 1; @@ -2705,7 +2705,7 @@ } /* FIXME: Should this be identical to board_flash_enable? */ -static int board_handle_phase(enum board_match_phase phase) +static int board_handle_phase(enum board_match_phase phase, bool force_boardenable) { const struct board_match *board = NULL; @@ -2714,7 +2714,7 @@ if (!board) return 0; - if (board_enable_safetycheck(board)) + if (board_enable_safetycheck(board, force_boardenable)) return 0; if (!board->enable) { @@ -2726,17 +2726,18 @@ return board->enable(); } -void board_handle_before_superio(void) +void board_handle_before_superio(bool force_boardenable) { - board_handle_phase(P1); + board_handle_phase(P1, force_boardenable); } -void board_handle_before_laptop(void) +void board_handle_before_laptop(bool force_boardenable) { - board_handle_phase(P2); + board_handle_phase(P2, force_boardenable); } -int board_flash_enable(const char *vendor, const char *model, const char *cb_vendor, const char *cb_model) +int board_flash_enable(const char *vendor, const char *model, const char *cb_vendor, const char *cb_model, + bool force_boardenable) { const struct board_match *board = NULL; int ret = 0; @@ -2762,7 +2763,7 @@ return 0; } - if (board_enable_safetycheck(board)) + if (board_enable_safetycheck(board, force_boardenable)) return 1; /* limit the maximum size of the parallel bus */ diff --git a/include/programmer.h b/include/programmer.h index ad3c502..9e706d5 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -224,9 +224,9 @@ uint8_t sio_read(uint16_t port, uint8_t reg); void sio_write(uint16_t port, uint8_t reg, uint8_t data); void sio_mask(uint16_t port, uint8_t reg, uint8_t data, uint8_t mask); -void board_handle_before_superio(void); -void board_handle_before_laptop(void); -int board_flash_enable(const char *vendor, const char *model, const char *cb_vendor, const char *cb_model); +void board_handle_before_superio(bool force_boardenable); +void board_handle_before_laptop(bool force_boardenable); +int board_flash_enable(const char *vendor, const char *model, const char *cb_vendor, const char *cb_model, bool force_boardenable); /* chipset_enable.c */ int chipset_flash_enable(const struct programmer_cfg *cfg); @@ -263,7 +263,6 @@ #if CONFIG_INTERNAL == 1 extern int is_laptop; extern bool laptop_ok; -extern bool force_boardenable; extern bool force_boardmismatch; void probe_superio(void); int register_superio(struct superio s); diff --git a/internal.c b/internal.c index f21c4fc..808039d 100644 --- a/internal.c +++ b/internal.c @@ -30,7 +30,6 @@ int is_laptop = 0; bool laptop_ok = false; -bool force_boardenable = false; bool force_boardmismatch = false; enum chipbustype internal_buses_supported = BUS_NONE; @@ -202,6 +201,7 @@ const char *cb_vendor = NULL; const char *cb_model = NULL; #endif + bool force_boardenable = false; ret = get_params(cfg, &force_boardenable, &force_boardmismatch, @@ -261,7 +261,7 @@ dmi_init(); /* In case Super I/O probing would cause pretty explosions. */ - board_handle_before_superio(); + board_handle_before_superio(force_boardenable); /* Probe for the Super I/O chip and fill global struct superio. */ probe_superio(); @@ -274,7 +274,7 @@ #endif /* Check laptop whitelist. */ - board_handle_before_laptop(); + board_handle_before_laptop(force_boardenable); /* * Disable all internal buses by default if we are not sure @@ -300,7 +300,7 @@ * parallel writes on IT8705F. Also, this handles the manual chip select for Gigabyte's DualBIOS. */ init_superio_ite(cfg); - if (board_flash_enable(board_vendor, board_model, cb_vendor, cb_model)) { + if (board_flash_enable(board_vendor, board_model, cb_vendor, cb_model, force_boardenable)) { msg_perr("Aborting to be safe.\n"); ret = 1; goto internal_init_exit; -- To view, visit https://review.coreboot.org/c/flashrom/+/70031 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3324681f024003694a5531d9d35bb13d2c583eb0 Gerrit-Change-Number: 70031 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: sfdp.c : Initializing hbuf to avoid warnings generated by scan-build
by Name of user not set (Code Review) 14 Jan '23

14 Jan '23

Attention is currently required from: Alexander Goncharov. eshankelkar(a)galorithm.com has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71791 ) Change subject: sfdp.c : Initializing hbuf to avoid warnings generated by scan-build ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: > Oops, I wanted to point out CB:62747. Thank you for directing me there. -- To view, visit https://review.coreboot.org/c/flashrom/+/71791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6815e246b4fd225d1837cae6e7d2aa0236b48b1b Gerrit-Change-Number: 71791 Gerrit-PatchSet: 5 Gerrit-Owner: eshankelkar(a)galorithm.com Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 14 Jan 2023 15:15:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit