- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: ch341a_spi: Refactor singleton states into reentrant pattern
by Alexander Goncharov (Code Review) 09 Feb '23

09 Feb '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72807 to look at the new patch set (#3). Change subject: ch341a_spi: Refactor singleton states into reentrant pattern ...................................................................... ch341a_spi: Refactor singleton states into reentrant pattern Move global singleton states into a struct and store within the spi_master data field for the life-time of the driver. This patchset also includes stdlib.h to be able to work with memory allocation. Pass programmer's data as a parameter to functions that need it. This is one of the steps on the way to move spi_master data memory management behind the initialisation API, for more context see other patches under the same topic specified below. TOPIC=register_master_api TEST=Tested on flash W25Q128JVSQ flashrom -E # Result: success flashrom -v ff.bin # Result: verified flashrom -w firmware.bin # Result: success flashrom -v firmware.bin # Result: verified Change-Id: I9fe72bff88b7f8778c1199bdab8ba43bf32e1c8c Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Ticket: https://ticket.coreboot.org/issues/391 --- M ch341a_spi.c 1 file changed, 113 insertions(+), 78 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/72807/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/72807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fe72bff88b7f8778c1199bdab8ba43bf32e1c8c Gerrit-Change-Number: 72807 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: ch341a: Refactor singleton states into reentrant pattern
by Alexander Goncharov (Code Review) 09 Feb '23

09 Feb '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72807 ) Change subject: ch341a: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 2: This change is ready for review. -- To view, visit https://review.coreboot.org/c/flashrom/+/72807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fe72bff88b7f8778c1199bdab8ba43bf32e1c8c Gerrit-Change-Number: 72807 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 09 Feb 2023 14:21:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dummyflasher: use new API to register shutdown function
by Alexander Goncharov (Code Review) 09 Feb '23

09 Feb '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72408 ) Change subject: dummyflasher: use new API to register shutdown function ...................................................................... Patch Set 4: (1 comment) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/72408/comment/5033b603_7182cc2e PS3, Line 966: .probe = probe_variable_size, : .read = dummy_opaque_read, : .write = dummy_opaque_write, : .erase = dummy_opaque_erase, : .shutdown = dummy_shutdown, : .delay = dummy_nop_delay, > You need to align indentation (look at `=` sign), shutdown is the longest word and breaks the alignm […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/72408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Gerrit-Change-Number: 72408 Gerrit-PatchSet: 4 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 09 Feb 2023 13:36:27 +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[master]: dummyflasher: use new API to register shutdown function
by Alexander Goncharov (Code Review) 09 Feb '23

09 Feb '23

Attention is currently required from: Edward O'Callaghan, Alexander Goncharov. Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72408 to look at the new patch set (#4). Change subject: dummyflasher: use new API to register shutdown function ...................................................................... dummyflasher: use new API to register shutdown function This allows masters to register shutdown function in *_master struct, which means there is no need to call register_shutdown in init function, since this call is now a part of register_*_master. A dummy programmer can register masters for multiple buses that share a programmer's data (a pointer to struct emu_data) with each other. To avoid unexpected memory freeing by shutdown function, we need to keep track of how many buses are using the shared resource. Use the reference counting technique to achieve this. TEST=ninja test Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Ticket: https://ticket.coreboot.org/issues/391 --- M dummyflasher.c 1 file changed, 49 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/72408/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/72408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Gerrit-Change-Number: 72408 Gerrit-PatchSet: 4 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: flashrom.c: Add functions for new erase selection algorithm
by Thomas Heijligen (Code Review) 09 Feb '23

09 Feb '23

Attention is currently required from: Simon Buhrow, Aarya. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65844 ) Change subject: flashrom.c: Add functions for new erase selection algorithm ...................................................................... Patch Set 88: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 88 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-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 09 Feb 2023 09:30:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dummyflasher: use new API to register shutdown function
by Anastasia Klimchuk (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Edward O'Callaghan, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72408 ) Change subject: dummyflasher: use new API to register shutdown function ...................................................................... Patch Set 3: Code-Review+1 (2 comments) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/72408/comment/4466b9fc_d5a9d205 PS2, Line 1416: data->refs_cnt += ((dummy_buses_supported & BUS_PROG) != BUS_NONE); : data->refs_cnt += ((dummy_buses_supported & BUS_NONSPI) != BUS_NONE); : data->refs_cnt += ((dummy_buses_supported & BUS_SPI) != BUS_NONE); : : #if 0 : data->refs_cnt += (dummy_buses_supported & BUS_PROG) ? 1 : 0; : data->refs_cnt += (dummy_buses_supported & BUS_NONSPI) ? 1 : 0; : data->refs_cnt += (dummy_buses_supported & BUS_SPI) ? 1 : 0; : #endif > Your last idea is exactly what we need. […] Done File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/72408/comment/11849093_8a4ab32e PS3, Line 966: .probe = probe_variable_size, : .read = dummy_opaque_read, : .write = dummy_opaque_write, : .erase = dummy_opaque_erase, : .shutdown = dummy_shutdown, : .delay = dummy_nop_delay, You need to align indentation (look at `=` sign), shutdown is the longest word and breaks the alignment. -- To view, visit https://review.coreboot.org/c/flashrom/+/72408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Gerrit-Change-Number: 72408 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 09 Feb 2023 07:23:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: add bus coverage test for dummy
by Anastasia Klimchuk (Code Review) 08 Feb '23

08 Feb '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/72665 ) Change subject: tests: add bus coverage test for dummy ...................................................................... tests: add bus coverage test for dummy Dummy programmer has a shared data between *_masters. To make sure the dummy has no memory leakage, we need a test that will covers initialization and shutdown of the programmer with different bus types, i.e. programmer specific, non-SPI and SPI. TEST=ninja test Change-Id: Iafe715feb5f5c0b5efd6827cdb2c3a314f542319 Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Reviewed-on: https://review.coreboot.org/c/flashrom/+/72665 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/dummyflasher.c M tests/tests.c M tests/tests.h 3 files changed, 41 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/tests/dummyflasher.c b/tests/dummyflasher.c index c41f9df..052938b 100644 --- a/tests/dummyflasher.c +++ b/tests/dummyflasher.c @@ -123,6 +123,24 @@ run_basic_lifecycle(state, &dummy_io, &programmer_dummy, NULL); } +void dummy_all_buses_test_success(void **state) +{ + struct io_mock_fallback_open_state dummy_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock dummy_io = { + .fallback_open_state = &dummy_fallback_open_state, + }; + + run_basic_lifecycle(state, &dummy_io, &programmer_dummy, "bus=lpc+fwh"); + run_basic_lifecycle(state, &dummy_io, &programmer_dummy, "bus=spi"); + run_basic_lifecycle(state, &dummy_io, &programmer_dummy, "bus=prog"); + run_basic_lifecycle(state, &dummy_io, &programmer_dummy, "bus=parallel+fwh+prog"); + run_basic_lifecycle(state, &dummy_io, &programmer_dummy, "bus=spi+prog"); + run_basic_lifecycle(state, &dummy_io, &programmer_dummy, "bus=parallel+lpc+spi"); +} + #else SKIP_TEST(dummy_basic_lifecycle_test_success) SKIP_TEST(dummy_probe_lifecycle_test_success) @@ -131,4 +149,5 @@ SKIP_TEST(dummy_init_success_invalid_param_test_success) SKIP_TEST(dummy_init_success_unhandled_param_test_success) SKIP_TEST(dummy_null_prog_param_test_success) + SKIP_TEST(dummy_all_buses_test_success) #endif /* CONFIG_DUMMY */ diff --git a/tests/tests.c b/tests/tests.c index 0912f35..0de0168 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -447,6 +447,7 @@ cmocka_unit_test(dummy_init_success_invalid_param_test_success), cmocka_unit_test(dummy_init_success_unhandled_param_test_success), cmocka_unit_test(dummy_null_prog_param_test_success), + cmocka_unit_test(dummy_all_buses_test_success), cmocka_unit_test(nicrealtek_basic_lifecycle_test_success), cmocka_unit_test(raiden_debug_basic_lifecycle_test_success), cmocka_unit_test(dediprog_basic_lifecycle_test_success), diff --git a/tests/tests.h b/tests/tests.h index bdcfdae..8f63f91 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -51,6 +51,7 @@ void dummy_init_success_invalid_param_test_success(void **state); void dummy_init_success_unhandled_param_test_success(void **state); void dummy_null_prog_param_test_success(void **state); +void dummy_all_buses_test_success(void **state); void nicrealtek_basic_lifecycle_test_success(void **state); void raiden_debug_basic_lifecycle_test_success(void **state); void dediprog_basic_lifecycle_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/72665 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafe715feb5f5c0b5efd6827cdb2c3a314f542319 Gerrit-Change-Number: 72665 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[L] Change in flashrom[master]: flashrom.c: Add functions for new erase selection algorithm
by Anastasia Klimchuk (Code Review) 08 Feb '23

08 Feb '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/+/65844 ) Change subject: flashrom.c: Add functions for new erase selection algorithm ...................................................................... Patch Set 88: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 88 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-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 09 Feb 2023 06:05:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: flashrom.c: Add functions for new erase selection algorithm
by Aarya (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Simon Buhrow, Thomas Heijligen, Anastasia Klimchuk. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65844 ) Change subject: flashrom.c: Add functions for new erase selection algorithm ...................................................................... Patch Set 88: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/65844/comment/7922906f_cc85188d PS87, Line 7: selction > Just found one typo: selction -> selection Done Patchset: PS87: > * The files are named `erasure_layout.h`. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 88 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-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 09 Feb 2023 05:36:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: flashrom.c: Add functions for new erase selection algorithm
by Aarya (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Simon Buhrow, Aarya, Anastasia Klimchuk. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/65844 to look at the new patch set (#88). Change subject: flashrom.c: Add functions for new erase selection algorithm ...................................................................... flashrom.c: Add functions for new erase selection algorithm 1) Add function to flatten out the addresses of the flash chip as per the different erase functions. This function will return a list of layouts which is dynamically allocated. So after use all the layouts as well as the list itself should be freed. The free_erase_layout function does that. 2) Add function to align start and end address of the region (in struct walk_info) to some erase sector boundaries and modify the region start and end addresses to match nearest erase sector boundaries. This function will be used in the new algorithm for erase function selection. 3) Add function that returns a list of sectors (as seen by the first erase function) that need erasing. 4) Add a function to call the erase algorithm. Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M Makefile A erasure_layout.c A include/erasure_layout.h M meson.build 4 files changed, 475 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/65844/88 -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 88 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-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit