September 2022 - flashrom-gerrit

[S] Change in flashrom[master]: serial.c: support BAUD rates > 230400 on macOS
by Peter Stuge (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons, Name of user not set #1004601. Peter Stuge has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67822 ) Change subject: serial.c: support BAUD rates > 230400 on macOS ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c67048b9c7bc704d9b0c4ef4771f708fdb3e0f3 Gerrit-Change-Number: 67822 Gerrit-PatchSet: 6 Gerrit-Owner: Name of user not set #1004601 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Peter Stuge <peter(a)stuge.se> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Name of user not set #1004601 Gerrit-Comment-Date: Mon, 26 Sep 2022 11:36:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: Add WP settings for Flash Chip `W25Q512NW`
by Edward O'Callaghan (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Subrata Banik, Nikolai Artemiev, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67827 ) Change subject: flashchips: Add WP settings for Flash Chip `W25Q512NW` ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67827 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5148f71404466dcf7772e3eb6e1800eb8666696 Gerrit-Change-Number: 67827 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Sep 2022 11:18:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: serial.c: support BAUD rates > 230400 on macOS
by Name of user not set (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67822 to look at the new patch set (#6). Change subject: serial.c: support BAUD rates > 230400 on macOS ...................................................................... serial.c: support BAUD rates > 230400 on macOS use the apple way like demonstrated in the IOSerialTestLib.c example code to set BAUD rates > 230400 on macOS. Signed-off-by: Denis Ahrens <denis(a)h3q.com> Change-Id: I9c67048b9c7bc704d9b0c4ef4771f708fdb3e0f3 --- M serial.c 1 file changed, 32 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/67822/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/67822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c67048b9c7bc704d9b0c4ef4771f708fdb3e0f3 Gerrit-Change-Number: 67822 Gerrit-PatchSet: 6 Gerrit-Owner: Name of user not set #1004601 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: serial.c: support BAUD rates > 230400 on macOS
by Name of user not set (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67822 to look at the new patch set (#5). Change subject: serial.c: support BAUD rates > 230400 on macOS ...................................................................... serial.c: support BAUD rates > 230400 on macOS use the apple way like demonstrated in the IOSerialTestLib.c example code to set BAUD rates > 230400 on macOS. Signed-off-by: Denis Ahrens <denis(a)h3q.com> Change-Id: I9c67048b9c7bc704d9b0c4ef4771f708fdb3e0f3 --- M serial.c 1 file changed, 32 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/67822/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/67822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c67048b9c7bc704d9b0c4ef4771f708fdb3e0f3 Gerrit-Change-Number: 67822 Gerrit-PatchSet: 5 Gerrit-Owner: Name of user not set #1004601 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: add basic lifecycle test for ch341a_spi
by Anastasia Klimchuk (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67664 ) Change subject: tests: add basic lifecycle test for ch341a_spi ...................................................................... Patch Set 5: (10 comments) Patchset: PS5: Nice work, thanks for the patch! File tests/ch341a_spi.c: https://review.coreboot.org/c/flashrom/+/67664/comment/afbcd384_b54474da PS5, Line 22: transfer->status = LIBUSB_TRANSFER_COMPLETED; : transfer->actual_length = transfer->length; : transfer->callback(transfer); Are you sure these lines needed? What happens if you remove them, and just return 0? Maybe I am missing something, but I looked through the programmer code and I can't find where status, actual_length and callback are used? Especially, calling callback in a mock should not be needed, since return value is not used (or maybe it's void). https://review.coreboot.org/c/flashrom/+/67664/comment/78fa1389_087fe5ce PS5, Line 43: mediatek_i2c_spi_basic_lifecycle_test_success This is a different test probably a copy-paste ;) You need to use your test method name here, `ch341a_spi_basic_lifecycle_test_success` The meaning behind this is: if the programmer is disabled in the build, the corresponding test should not run either (since there is nothing to test). File tests/libusb_wraps.c: PS5: > The new added wraps should go into a separate patch pointing to this one. Why? In all previous tests, wraps were added in the same patch with the test that needed those wraps. https://review.coreboot.org/c/flashrom/+/67664/comment/14496f0e_a3359de5 PS5, Line 39: /* https://libusb.sourceforge.io/api-1.0/group__libusb__lib.html#ga5f8376b7a86… */ > I'm going to delete such links after everyone says they're OK with the patch. […] Alright, thank you! I will just mark this unresolved so that you won't forget to remove links at the last moment :) https://review.coreboot.org/c/flashrom/+/67664/comment/d8eeb5b5_1f1d8d48 PS5, Line 75: You need 2 tabs here as indentation (see below __wrap_libusb_get_config_descriptor in the same situation) https://review.coreboot.org/c/flashrom/+/67664/comment/b12c66dd_5a56331a PS5, Line 196: calloc(1, sizeof(struct libusb_transfer)); This is a custom logic (a small one but still), so it should go into custom mocks. Same as you did for __wrap_libusb_submit_transfer. https://review.coreboot.org/c/flashrom/+/67664/comment/0f3672bb_6fd2ef67 PS5, Line 219: free(transfer); Same here, custom logic, please move to io_mock, and implement in tests/ch341a_spi.c File tests/usb_unittests.h: https://review.coreboot.org/c/flashrom/+/67664/comment/2409520b_9d2edd1c PS5, Line 28: CONFIG_CH341A_SPI == 1 You can run tests with this option enabled and disabled. When you run with option disabled, the test will be SKIPPED, but everything should be built and tests run (so, no errors). To try that: In your builddir you can set option disabled, run meson configure -Dprogrammer=<list some programmers, but not ch341a> for example meson configure -Dprogrammer=dediprog,linux_mtd,linux_spi Then run tests, and check that tests for the rest of programmers are skipped as expected Then to return back with everything enabled you can run meson configure -Dprogrammer=auto or meson configure -Dprogrammer=all depending on what was you previous setting This will also verify that SKIP_TEST macro is set up correctly. https://review.coreboot.org/c/flashrom/+/67664/comment/277c46a5_62332897 PS5, Line 58: struct libusb_transfer; If you add typedef like the others, you can avoid repeating `struct` in wraps. -- 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: 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-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: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Mon, 26 Sep 2022 10:02:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: serial.c: support BAUD rates > 230400 on macOS
by Name of user not set (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons. Name of user not set #1004601 has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67822 ) Change subject: serial.c: support BAUD rates > 230400 on macOS ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/flashrom/+/67822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c67048b9c7bc704d9b0c4ef4771f708fdb3e0f3 Gerrit-Change-Number: 67822 Gerrit-PatchSet: 4 Gerrit-Owner: Name of user not set #1004601 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 26 Sep 2022 09:34:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: mediatek_i2c_spi: document programmer
by Angel Pons (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67825 ) Change subject: mediatek_i2c_spi: document programmer ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/67825/comment/4bb57c21_e5b8abb1 PS1, Line 1589: Flashrom flashrom is not capitalized, there's multiple instances of this -- To view, visit https://review.coreboot.org/c/flashrom/+/67825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia63df470170fbadcabadcdad8e5acc0cde3a274b Gerrit-Change-Number: 67825 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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: 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-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Sep 2022 08:20:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: spi: Make 'default_spi_send_multicommand' the default unless defined
by Thomas Heijligen (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67481 ) Change subject: spi: Make 'default_spi_send_multicommand' the default unless defined ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67481/comment/53f3b181_4958a1e8 PS1, Line 9: Drop the explicit need to specify the default Please mention the struct where you perform the changes on Patchset: PS1: The same as in CB:67480 applies here. -- To view, visit https://review.coreboot.org/c/flashrom/+/67481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6cc24bf982da3d5251d391eb397db43dd10280e8 Gerrit-Change-Number: 67481 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Sep 2022 07:59:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: spi: Make 'default_spi_send_command' the default unless defined
by Thomas Heijligen (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67480 ) Change subject: spi: Make 'default_spi_send_command' the default unless defined ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67480/comment/2f080737_f2c409fb PS1, Line 9: Drop the explicit need to specify the default Please mention the struct where you perform the changes on Patchset: PS1: I think setting optionally one of `.command` or `.multicommand` to `NULL` will not help to simplify the code development. Simplification would be to have a singe code path for sending spi commands. -- To view, visit https://review.coreboot.org/c/flashrom/+/67480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I63abcb8c64f233cdbf58a149a31051fa648305a2 Gerrit-Change-Number: 67480 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Sep 2022 07:58:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: spi: Make 'default_spi_write_aai' the default unless defined
by Thomas Heijligen (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67479 ) Change subject: spi: Make 'default_spi_write_aai' the default unless defined ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67479/comment/5617b5f3_434b5056 PS1, Line 9: Drop the explicit need to specify the default 'default_spi_write_aai' Please mention the struct where you perform the changes on Patchset: PS1: Two things: First, from the commit message I can't get the conclusion that, from this patch on, the control flow code looks up if there is a custom function for the programmer and if not uses the default one. Second, as I mentioned already in the email, IMO it would be god to combine this with a rename to make it clear that the functions are optional and there is an implicit default behavior. -- To view, visit https://review.coreboot.org/c/flashrom/+/67479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a Gerrit-Change-Number: 67479 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Sep 2022 07:48:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022