November 2022 - flashrom-gerrit

[S] Change in flashrom[master]: spi25.c: Rename spi_get_erasefn_from_opcode to spi25_get_erasefn_from...
by Aarya (Code Review) Nov. 29, 2022

Nov. 29, 2022

Attention is currently required from: Simon Buhrow, Aarya. Aarya has uploaded a new patch set (#5) to the change originally created by Thomas Heijligen. ( https://review.coreboot.org/c/flashrom/+/67716 ) Change subject: spi25.c: Rename spi_get_erasefn_from_opcode to spi25_get_erasefn_from_opcode ...................................................................... spi25.c: Rename spi_get_erasefn_from_opcode to spi25_get_erasefn_from_opcode This function works only with spi25 chips Change-Id: Ie054160b0fdd34bcb128285c6a047e3a3fa8be0c Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M include/chipdrivers.h M sfdp.c M spi25.c 3 files changed, 19 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/67716/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/67716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie054160b0fdd34bcb128285c6a047e3a3fa8be0c Gerrit-Change-Number: 67716 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: spi.c: Add AT45 & SF25F erasefn opcode mapping
by Aarya (Code Review) Nov. 29, 2022

Nov. 29, 2022

Attention is currently required from: Simon Buhrow, Aarya. Aarya has uploaded a new patch set (#5) to the change originally created by Thomas Heijligen. ( https://review.coreboot.org/c/flashrom/+/67717 ) Change subject: spi.c: Add AT45 & SF25F erasefn opcode mapping ...................................................................... spi.c: Add AT45 & SF25F erasefn opcode mapping Change-Id: I798a91f1e20b63662715c68e6d43d03fc6005d51 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M include/chipdrivers.h M spi.c 2 files changed, 56 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/67717/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/67717 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I798a91f1e20b63662715c68e6d43d03fc6005d51 Gerrit-Change-Number: 67717 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c: Update check_block_eraser function to use probe opcode
by Aarya (Code Review) Nov. 29, 2022

Nov. 29, 2022

Attention is currently required from: Simon Buhrow, Nico Huber, Aarya. Hello build bot (Jenkins), Simon Buhrow, Nico Huber, Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66717 to look at the new patch set (#18). Change subject: flashrom.c: Update check_block_eraser function to use probe opcode ...................................................................... flashrom.c: Update check_block_eraser function to use probe opcode Update the check_block_eraser function to use probe_opcode to see if the given block_eraser is supported by the spi master. This will help to get a real count of usable block_erasers. Change-Id: I6591a84ae1fe5bc1648051cc30b9393450033852 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c 1 file changed, 26 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/66717/18 -- To view, visit https://review.coreboot.org/c/flashrom/+/66717 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6591a84ae1fe5bc1648051cc30b9393450033852 Gerrit-Change-Number: 66717 Gerrit-PatchSet: 18 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c:Add function to get a flattened view of the chip erase blocks
by Aarya (Code Review) Nov. 29, 2022

Nov. 29, 2022

Attention is currently required from: Felix Singer, Simon Buhrow, Nico Huber, Paul Menzel, Aarya. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/65879 to look at the new patch set (#37). Change subject: flashrom.c:Add function to get a flattened view of the chip erase blocks ...................................................................... flashrom.c:Add function to get a flattened view of the chip erase blocks Add a 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. Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c M include/flash.h 2 files changed, 111 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/65879/37 -- To view, visit https://review.coreboot.org/c/flashrom/+/65879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4 Gerrit-Change-Number: 65879 Gerrit-PatchSet: 37 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: flashrom.c: Add wrapper function to use the erase algorithm
by Aarya (Code Review) Nov. 29, 2022

Nov. 29, 2022

Attention is currently required from: Nico Huber, Thomas Heijligen, Aarya, Anastasia Klimchuk. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66104 to look at the new patch set (#68). Change subject: flashrom.c: Add wrapper function to use the erase algorithm ...................................................................... flashrom.c: Add wrapper function to use the erase algorithm Add a function to call the erase algorithm. Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c M include/flash.h 2 files changed, 149 insertions(+), 291 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/66104/68 -- To view, visit https://review.coreboot.org/c/flashrom/+/66104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8 Gerrit-Change-Number: 66104 Gerrit-PatchSet: 68 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Add selfcheck to unit tests
by Anastasia Klimchuk (Code Review) Nov. 29, 2022

Nov. 29, 2022

Attention is currently required from: Felix Singer, Thomas Heijligen, Evan Benn. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69620 ) Change subject: tests: Add selfcheck to unit tests ...................................................................... Patch Set 8: Code-Review+1 (2 comments) File tests/selfcheck.c: https://review.coreboot.org/c/flashrom/+/69620/comment/7c28333d_2b2f58be PS7, Line 68: assert_table(chip->bustype != BUS_NONE, "chip bustype is BUS_NONE", i, : chip->name); The full story is here: https://www.flashrom.org/Development_Guidelines#Coding_style > 80-columns soft limit for most code and comments. This is to encourage simple design and concise naming. > 112-columns hard limit. Use this to reduce line breaks in cases where they harm grep-ability or overall readability, such as print statements and function signatures. Don't abuse this for long variable/function names or deep nesting. The reason I asked you is because I think this falls under "Use this to reduce line breaks in cases where they harm grep-ability or overall readability". In general, of course not all the lines should be long! :) If a line is nice and readable within 80 chars, that's perfect. File tests/selfcheck.c: https://review.coreboot.org/c/flashrom/+/69620/comment/4d4fcf37_14e9ced5 PS8, Line 25: #define assert_table(assertion, message, index, name) \ : do { \ : if (!(assertion)) \ : fail_msg(message " for index:%zu name:%s", (index), (name) ? (name) : "unknown"); \ : } while (0) The macro has changed with lot more new lines and \ why? -- To view, visit https://review.coreboot.org/c/flashrom/+/69620 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I41cd014d9bf909296b6c28e3e00548e6883ff41a Gerrit-Change-Number: 69620 Gerrit-PatchSet: 8 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 22:30:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashchips: Add support for IS25WP016
by Felix Singer (Code Review) Nov. 29, 2022

Nov. 29, 2022

Attention is currently required from: Alex Mikhalev. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70140 ) Change subject: flashchips: Add support for IS25WP016 ...................................................................... Patch Set 2: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/70140/comment/4274a8bb_d938c95d PS2, Line 9: Tested using `linux_spi` programmer on Raspberry Pi CM4. Please test reading, writing and erasing. https://review.coreboot.org/c/flashrom/+/70140/comment/8957d127_9adecf60 PS2, Line 11: Datasheet: https://www.issi.com/WW/pdf/25LP-WP016D.pdf Vendors often remove datasheets from their websites. I suggest using a caching service, e.g. `https://web.archive.org/web/20221129211027/https://www.issi.com/WW/pdf/25LP-WP016D.pdf`. Patchset: PS2: Thanks for contributing. Just did a quick review, didn't check the values yet. -- To view, visit https://review.coreboot.org/c/flashrom/+/70140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2b8caea229ffda72f1b04183c31715faccb64ad5 Gerrit-Change-Number: 70140 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Mikhalev <alex(a)corvus-robotics.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Alex Mikhalev <alex(a)corvus-robotics.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 21:31:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: add probe lifecycle test for ch341a_spi
by Anastasia Klimchuk (Code Review) Nov. 29, 2022

Nov. 29, 2022

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68755 ) Change subject: tests: add probe lifecycle test for ch341a_spi ...................................................................... tests: add probe lifecycle test for ch341a_spi This test upgrades mocks to simulate a read request. Read buffer is populated with chip manufacture id and chip model id to emulate successful probing. TEST=ninja test Change-Id: I0a2d5591d097435fc69719e1d9bd153433425821 Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68755 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/ch341a_spi.c M tests/tests.c M tests/tests.h 3 files changed, 66 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/tests/ch341a_spi.c b/tests/ch341a_spi.c index c706171..1cb2652 100644 --- a/tests/ch341a_spi.c +++ b/tests/ch341a_spi.c @@ -19,9 +19,15 @@ /* Same macro as in ch341a_spi.c programmer. */ #define WRITE_EP 0x02 +#define READ_EP 0x82 struct ch341a_spi_io_state { struct libusb_transfer *transfer_out; + /* + * Since the test transfers a data that fits in one CH341 packet, we + * don't need an array of these transfers (as is done in the driver code). + */ + struct libusb_transfer *transfer_in; }; static struct libusb_transfer *ch341a_libusb_alloc_transfer(void *state, int iso_packets) @@ -39,10 +45,15 @@ { struct ch341a_spi_io_state *io_state = state; - assert_true(transfer->endpoint == WRITE_EP); + assert_true(transfer->endpoint == WRITE_EP || transfer->endpoint == READ_EP); - assert_null(io_state->transfer_out); - io_state->transfer_out = transfer; + if (transfer->endpoint == WRITE_EP) { + assert_null(io_state->transfer_out); + io_state->transfer_out = transfer; + } else if (transfer->endpoint == READ_EP) { + assert_null(io_state->transfer_in); + io_state->transfer_in = transfer; + } return 0; } @@ -67,6 +78,17 @@ io_state->transfer_out = NULL; } + if (io_state->transfer_in) { + io_state->transfer_in->buffer[1] = reverse_byte(0xEF); /* WINBOND_NEX_ID */ + io_state->transfer_in->buffer[2] = reverse_byte(0x40); /* WINBOND_NEX_W25Q128_V left byte */ + io_state->transfer_in->buffer[3] = reverse_byte(0x18); /* WINBOND_NEX_W25Q128_V right byte */ + + io_state->transfer_in->status = LIBUSB_TRANSFER_COMPLETED; + io_state->transfer_in->actual_length = io_state->transfer_in->length; + io_state->transfer_in->callback(io_state->transfer_in); + io_state->transfer_in = NULL; + } + return 0; } @@ -89,6 +111,26 @@ run_basic_lifecycle(state, &ch341a_spi_io, &programmer_ch341a_spi, ""); } +void ch341a_spi_probe_lifecycle_test_success(void **state) +{ + struct ch341a_spi_io_state ch341a_spi_io_state = { 0 }; + struct io_mock_fallback_open_state ch341a_spi_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock ch341a_spi_io = { + .state = &ch341a_spi_io_state, + .libusb_alloc_transfer = &ch341a_libusb_alloc_transfer, + .libusb_submit_transfer = &ch341a_libusb_submit_transfer, + .libusb_free_transfer = &ch341a_libusb_free_transfer, + .libusb_handle_events_timeout = &ch341a_libusb_handle_events_timeout, + .fallback_open_state = &ch341a_spi_fallback_open_state, + }; + + run_probe_lifecycle(state, &ch341a_spi_io, &programmer_ch341a_spi, "", "W25Q128.V"); +} + #else SKIP_TEST(ch341a_spi_basic_lifecycle_test_success) + SKIP_TEST(ch341a_spi_probe_lifecycle_test_success) #endif /* CONFIG_CH341A_SPI */ diff --git a/tests/tests.c b/tests/tests.c index 2461ff6..fe70204 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -421,6 +421,7 @@ cmocka_unit_test(realtek_mst_basic_lifecycle_test_success), cmocka_unit_test(realtek_mst_no_allow_brick_test_success), cmocka_unit_test(ch341a_spi_basic_lifecycle_test_success), + cmocka_unit_test(ch341a_spi_probe_lifecycle_test_success), }; ret |= cmocka_run_group_tests_name("lifecycle.c tests", lifecycle_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index 4d91e4d..bcca9e8 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -61,6 +61,7 @@ void realtek_mst_basic_lifecycle_test_success(void **state); void realtek_mst_no_allow_brick_test_success(void **state); void ch341a_spi_basic_lifecycle_test_success(void **state); +void ch341a_spi_probe_lifecycle_test_success(void **state); /* layout.c */ void included_regions_dont_overlap_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/68755 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2d5591d097435fc69719e1d9bd153433425821 Gerrit-Change-Number: 68755 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: tests: add basic lifecycle test for ch341a_spi
by Anastasia Klimchuk (Code Review) Nov. 29, 2022

Nov. 29, 2022

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67664 ) Change subject: tests: add basic lifecycle test for ch341a_spi ...................................................................... tests: add basic lifecycle test for ch341a_spi TEST=the following scenarios run tests successfully 1) ch341a_spi is enabled result: all tests run and pass, including ch341a 2) ch341a_spi is disabled result: ch341a_spi test is skipped, the rest of tests run and pass 3) libusb isn't presented in the system result: tests for usb programmers are skipped, the rest of tests run normally Change-Id: If28fbe09ad685082152aa3a7e8d5a150169aee9e Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67664 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M meson.build A tests/ch341a_spi.c M tests/tests.c M tests/tests.h M tests/usb_unittests.h 5 files changed, 123 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/meson.build b/meson.build index d4a8ad1..14a84ac 100644 --- a/meson.build +++ b/meson.build @@ -182,6 +182,7 @@ 'deps' : [ libusb1 ], 'groups' : [ group_usb, group_external ], 'srcs' : files('ch341a_spi.c'), + 'test_srcs' : files('tests/ch341a_spi.c'), 'flags' : [ '-DCONFIG_CH341A_SPI=1' ], }, 'dediprog' : { diff --git a/tests/ch341a_spi.c b/tests/ch341a_spi.c new file mode 100644 index 0000000..c706171 --- /dev/null +++ b/tests/ch341a_spi.c @@ -0,0 +1,94 @@ +/* + * This file is part of the flashrom project. + * + * Copyright 2022 Alexander Goncharov <chat(a)joursoir.net> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "lifecycle.h" + +#if CONFIG_CH341A_SPI == 1 + +/* Same macro as in ch341a_spi.c programmer. */ +#define WRITE_EP 0x02 + +struct ch341a_spi_io_state { + struct libusb_transfer *transfer_out; +}; + +static struct libusb_transfer *ch341a_libusb_alloc_transfer(void *state, int iso_packets) +{ + return calloc(1, sizeof(struct libusb_transfer)); +} + +/* + * The libusb code stores submitted transfers in their own context. But this + * function doesn't require a context pointer because libusb stores context + * pointers in libusb_transfer instances. Since our ch341 driver is using + * the default context, we store the transfer in our own. + */ +static int ch341a_libusb_submit_transfer(void *state, struct libusb_transfer *transfer) +{ + struct ch341a_spi_io_state *io_state = state; + + assert_true(transfer->endpoint == WRITE_EP); + + assert_null(io_state->transfer_out); + io_state->transfer_out = transfer; + + return 0; +} + +static void ch341a_libusb_free_transfer(void *state, struct libusb_transfer *transfer) +{ + free(transfer); +} + +/* + * Handle submitted transfers by pretending that a transfer is completed and + * invoking its callback (that is the flashrom code). + */ +static int ch341a_libusb_handle_events_timeout(void *state, libusb_context *ctx, struct timeval *tv) +{ + struct ch341a_spi_io_state *io_state = state; + + if (io_state->transfer_out) { + io_state->transfer_out->status = LIBUSB_TRANSFER_COMPLETED; + io_state->transfer_out->actual_length = io_state->transfer_out->length; + io_state->transfer_out->callback(io_state->transfer_out); + io_state->transfer_out = NULL; + } + + return 0; +} + +void ch341a_spi_basic_lifecycle_test_success(void **state) +{ + struct ch341a_spi_io_state ch341a_spi_io_state = { 0 }; + struct io_mock_fallback_open_state ch341a_spi_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock ch341a_spi_io = { + .state = &ch341a_spi_io_state, + .libusb_alloc_transfer = &ch341a_libusb_alloc_transfer, + .libusb_submit_transfer = &ch341a_libusb_submit_transfer, + .libusb_free_transfer = &ch341a_libusb_free_transfer, + .libusb_handle_events_timeout = &ch341a_libusb_handle_events_timeout, + .fallback_open_state = &ch341a_spi_fallback_open_state, + }; + + run_basic_lifecycle(state, &ch341a_spi_io, &programmer_ch341a_spi, ""); +} + +#else + SKIP_TEST(ch341a_spi_basic_lifecycle_test_success) +#endif /* CONFIG_CH341A_SPI */ diff --git a/tests/tests.c b/tests/tests.c index ea416b8..2461ff6 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -420,6 +420,7 @@ cmocka_unit_test(mediatek_i2c_no_allow_brick_test_success), cmocka_unit_test(realtek_mst_basic_lifecycle_test_success), cmocka_unit_test(realtek_mst_no_allow_brick_test_success), + cmocka_unit_test(ch341a_spi_basic_lifecycle_test_success), }; ret |= cmocka_run_group_tests_name("lifecycle.c tests", lifecycle_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index 7d84101..4d91e4d 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -60,6 +60,7 @@ void mediatek_i2c_no_allow_brick_test_success(void **state); void realtek_mst_basic_lifecycle_test_success(void **state); void realtek_mst_no_allow_brick_test_success(void **state); +void ch341a_spi_basic_lifecycle_test_success(void **state); /* layout.c */ void included_regions_dont_overlap_test_success(void **state); diff --git a/tests/usb_unittests.h b/tests/usb_unittests.h index f4d83f1..c81e181 100644 --- a/tests/usb_unittests.h +++ b/tests/usb_unittests.h @@ -25,7 +25,7 @@ #ifndef _USB_UNITTESTS_H_ #define _USB_UNITTESTS_H_ -#if CONFIG_RAIDEN_DEBUG_SPI == 1 || CONFIG_DEDIPROG == 1 +#if CONFIG_RAIDEN_DEBUG_SPI == 1 || CONFIG_DEDIPROG == 1 || CONFIG_CH341A_SPI == 1 #include <libusb.h> -- To view, visit https://review.coreboot.org/c/flashrom/+/67664 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If28fbe09ad685082152aa3a7e8d5a150169aee9e Gerrit-Change-Number: 67664 Gerrit-PatchSet: 11 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: tests: add mocks for libusb's asynchronous API
by Anastasia Klimchuk (Code Review) Nov. 29, 2022

Nov. 29, 2022

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69873 ) Change subject: tests: add mocks for libusb's asynchronous API ...................................................................... tests: add mocks for libusb's asynchronous API This patch adds mocks for several libusb functions are introduced in one of the previous commits. Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Change-Id: I5a316687ab39a112d968eeaedb71f7b4b659d8d5 Reviewed-on: https://review.coreboot.org/c/flashrom/+/69873 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/io_mock.h M tests/libusb_wraps.c 2 files changed, 28 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/tests/io_mock.h b/tests/io_mock.h index 644da74..3af5de4 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -99,6 +99,10 @@ uint8_t config_index, struct libusb_config_descriptor **); void (*libusb_free_config_descriptor)(void *state, struct libusb_config_descriptor *); + struct libusb_transfer* (*libusb_alloc_transfer)(void *state, int iso_packets); + int (*libusb_submit_transfer)(void *state, struct libusb_transfer *transfer); + void (*libusb_free_transfer)(void *state, struct libusb_transfer *transfer); + int (*libusb_handle_events_timeout)(void *state, libusb_context *ctx, struct timeval *tv); /* POSIX File I/O */ int (*iom_open)(void *state, const char *pathname, int flags); diff --git a/tests/libusb_wraps.c b/tests/libusb_wraps.c index a05cbe9..be990a1 100644 --- a/tests/libusb_wraps.c +++ b/tests/libusb_wraps.c @@ -188,23 +188,31 @@ struct libusb_transfer *__wrap_libusb_alloc_transfer(int iso_packets) { LOG_ME; + if (get_io() && get_io()->libusb_alloc_transfer) + return get_io()->libusb_alloc_transfer(get_io()->state, iso_packets); return not_null(); } int __wrap_libusb_submit_transfer(struct libusb_transfer *transfer) { LOG_ME; + if (get_io() && get_io()->libusb_submit_transfer) + return get_io()->libusb_submit_transfer(get_io()->state, transfer); return 0; } void __wrap_libusb_free_transfer(struct libusb_transfer *transfer) { LOG_ME; + if (get_io() && get_io()->libusb_free_transfer) + get_io()->libusb_free_transfer(get_io()->state, transfer); } int __wrap_libusb_handle_events_timeout(libusb_context *ctx, struct timeval *tv) { LOG_ME; + if (get_io() && get_io()->libusb_handle_events_timeout) + get_io()->libusb_handle_events_timeout(get_io()->state, ctx, tv); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/69873 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5a316687ab39a112d968eeaedb71f7b4b659d8d5 Gerrit-Change-Number: 69873 Gerrit-PatchSet: 2 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