September 2022 - flashrom-gerrit

[S] Change in flashrom[1.0.x]: pickit2_spi: Fix "dead" assignment
by Edward O'Callaghan (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Nico Huber. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67838 ) Change subject: pickit2_spi: Fix "dead" assignment ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67838 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f Gerrit-Change-Number: 67838 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 28 Sep 2022 00:23:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: dirtyjtag: Add DirtyJTAG programmer
by Felix Singer (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Thomas Heijligen, Jean THOMAS. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 4: (3 comments) File dirtyjtag_spi.c: https://review.coreboot.org/c/flashrom/+/67878/comment/a253dc55_4cbe494d PS4, Line 120: static inline unsigned int _min(unsigned int a, unsigned int b) : { : return (a < b) ? a : b; : } Seems to be unused? https://review.coreboot.org/c/flashrom/+/67878/comment/551fc13b_0b284884 PS4, Line 133: msg_perr("%s: Failed tx_buf allocation", __func__); Missing \n https://review.coreboot.org/c/flashrom/+/67878/comment/ffc9b933_c42fb4b0 PS4, Line 138: msg_perr("%s: Failed rx_buf allocation", __func__); Missing \n -- To view, visit https://review.coreboot.org/c/flashrom/+/67878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic43e9a014ed7d04e429e73b30c9dcfdde1a78913 Gerrit-Change-Number: 67878 Gerrit-PatchSet: 4 Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Comment-Date: Wed, 28 Sep 2022 00:17:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: writeprotect: Add some debug logging if wp_verify fails
by Evan Benn (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Nikolai Artemiev. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67720 ) Change subject: writeprotect: Add some debug logging if wp_verify fails ...................................................................... Patch Set 1: (1 comment) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/67720/comment/196f8be1_20a2c385 PS1, Line 168: wp_verify > nit: change to `%s:` and add `__func__` as a format arg […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/67720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5fcaf767570418f90ae44826a1135d9b49653033 Gerrit-Change-Number: 67720 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 28 Sep 2022 00:09:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: writeprotect: Add some debug logging if wp_verify fails
by Evan Benn (Code Review) 28 Sep '22

28 Sep '22

Hello build bot (Jenkins), Anastasia Klimchuk, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67720 to look at the new patch set (#2). Change subject: writeprotect: Add some debug logging if wp_verify fails ...................................................................... writeprotect: Add some debug logging if wp_verify fails Change-Id: I5fcaf767570418f90ae44826a1135d9b49653033 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M writeprotect.c 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/67720/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5fcaf767570418f90ae44826a1135d9b49653033 Gerrit-Change-Number: 67720 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: dirtyjtag: Add DirtyJTAG programmer
by Felix Singer (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Thomas Heijligen, Jean THOMAS. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/67878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic43e9a014ed7d04e429e73b30c9dcfdde1a78913 Gerrit-Change-Number: 67878 Gerrit-PatchSet: 4 Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Comment-Date: Wed, 28 Sep 2022 00:02:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: dirtyjtag: Add DirtyJTAG programmer
by Felix Singer (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Thomas Heijligen, Jean THOMAS. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 4: (21 comments) Patchset: PS4: Looks like a lot, but actually just some minor issues :) Looks good to me else. Thanks for contributing! File dirtyjtag_spi.c: https://review.coreboot.org/c/flashrom/+/67878/comment/9bef9d82_cb1905c7 PS4, Line 34: DJTAGUNK It seems this is unused? What does UNK mean? Unknown? If so, I would write that out. https://review.coreboot.org/c/flashrom/+/67878/comment/05073c0d_d4896e32 PS4, Line 39: 0xC0CA Please use lower case https://review.coreboot.org/c/flashrom/+/67878/comment/5251a3f7_63b700cf PS4, Line 47: dirytjtag_command_identifier typo: dir*ty*jtag https://review.coreboot.org/c/flashrom/+/67878/comment/7e1ca032_b7993262 PS4, Line 57: dirytjtag_signal_identifier typo: dir*ty*jtag https://review.coreboot.org/c/flashrom/+/67878/comment/678b77b2_0fefabed PS4, Line 66: char* data `char *data` https://review.coreboot.org/c/flashrom/+/67878/comment/32da85ef_593223f5 PS4, Line 87: char* data `char *data` https://review.coreboot.org/c/flashrom/+/67878/comment/a05769b0_221f2eb7 PS4, Line 125: static int dirtyjtag_djtag1_spi_send_command(struct dirtyjtag_spi_data *context, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) This is also pretty long. Same as with line 174. https://review.coreboot.org/c/flashrom/+/67878/comment/bc3a430e_2baa6234 PS4, Line 128: writecnt+readcnt spaces https://review.coreboot.org/c/flashrom/+/67878/comment/d0f30199_bc5e7460 PS4, Line 129: size_t num_xfer = (len+max_xfer_size-1)/max_xfer_size; // ceil(len/max_xfer_size) spaces https://review.coreboot.org/c/flashrom/+/67878/comment/993d73f3_1e5dcf3a PS4, Line 131: char *tx_buf = malloc(max_xfer_size*num_xfer); spaces https://review.coreboot.org/c/flashrom/+/67878/comment/982455fd_be9956be PS4, Line 136: char *rx_buf = malloc(max_xfer_size*num_xfer); spaces https://review.coreboot.org/c/flashrom/+/67878/comment/c08ef3a9_999bc155 PS4, Line 146: len%max_xfer_size add spaces before and after % https://review.coreboot.org/c/flashrom/+/67878/comment/b3b09043_a682afe8 PS4, Line 147: len%max_xfer_size same here https://review.coreboot.org/c/flashrom/+/67878/comment/71c3c872_37d30806 PS4, Line 149: txn_size = 30; How about setting this value during declaration in line 145? So it would be the "default" and if applicable it gets overridden. This way we can remove the else-block then. https://review.coreboot.org/c/flashrom/+/67878/comment/5e6efbce_6cb7d232 PS4, Line 152: tx_buf+30*i same here https://review.coreboot.org/c/flashrom/+/67878/comment/17d90845_5d3ae69c PS4, Line 155: rx_buf+i*30 same here https://review.coreboot.org/c/flashrom/+/67878/comment/84c2e349_13165043 PS4, Line 174: static int dirtyjtag_spi_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) That line is pretty long. Please move each argument into a separate line or group them somehow. For example `writecnt` and `readcnt` could go into one line. Same with `writearr` and `readarr`. https://review.coreboot.org/c/flashrom/+/67878/comment/b6fa893c_9d5865f6 PS4, Line 248: tmp, strerror(errno)); nit: I would use one line here. It's not that long. https://review.coreboot.org/c/flashrom/+/67878/comment/faaf1c1a_8f2d907f PS4, Line 289: 0xFF lower case https://review.coreboot.org/c/flashrom/+/67878/comment/a05ec946_7576f67c PS4, Line 290: 0xFF make this lower case -- To view, visit https://review.coreboot.org/c/flashrom/+/67878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic43e9a014ed7d04e429e73b30c9dcfdde1a78913 Gerrit-Change-Number: 67878 Gerrit-PatchSet: 4 Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Comment-Date: Wed, 28 Sep 2022 00:01:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: tree: provide flashrom context into programmer's delay
by Edward O'Callaghan (Code Review) 27 Sep '22

27 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66373 ) Change subject: tree: provide flashrom context into programmer's delay ...................................................................... Patch Set 4: Code-Review+2 (1 comment) Patchset: PS4: > IMPORTANT: We've decided that now is not the time for this chain of patches. […] Can we rebase this one and have it go in? I ended up with basically the same patch here https://review.coreboot.org/c/flashrom/+/67392/ while dealing with singleton patterns too. -- To view, visit https://review.coreboot.org/c/flashrom/+/66373 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibb0bce26ce2052853ee52158d7ba742967a9e229 Gerrit-Change-Number: 66373 Gerrit-PatchSet: 4 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: 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: Felix Singer <felixsinger(a)posteo.net> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Tue, 27 Sep 2022 22:20:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: dirtyjtag: Add DirtyJTAG programmer
by Alexander Goncharov (Code Review) 27 Sep '22

27 Sep '22

Attention is currently required from: Thomas Heijligen, Jean THOMAS. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 4: (3 comments) Patchset: PS4: Thanks for the patch! Just some formatting fixes. File dirtyjtag_spi.c: https://review.coreboot.org/c/flashrom/+/67878/comment/b8684a7e_8be68ccc PS4, Line 48: CMD_STOP = 0x00, Please use TAB instead of two spaces. https://review.coreboot.org/c/flashrom/+/67878/comment/2a8f59df_e42984c1 PS4, Line 58: SIG_TCK = 1 << 1, TABs here too. -- To view, visit https://review.coreboot.org/c/flashrom/+/67878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic43e9a014ed7d04e429e73b30c9dcfdde1a78913 Gerrit-Change-Number: 67878 Gerrit-PatchSet: 4 Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Comment-Date: Tue, 27 Sep 2022 21:44:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: stlinkv3_spi: fix uninitialized pointer
by Alexander Goncharov (Code Review) 27 Sep '22

27 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67700 ) Change subject: stlinkv3_spi: fix uninitialized pointer ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > My suggestion would be to add a comment libusb_device_handle *stlinkv3_handle = NULL; /* NULL for suppressing scan-build false positive core.uninitialized.Branch */ This line is much longer than 112-columns hard limit. I hope you will not mind if I add this comment before the variable. Please mark it as resolved if everything is OK. Patchset: PS2: I've updated the commit message -- To view, visit https://review.coreboot.org/c/flashrom/+/67700 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibaf25f67186724d9045ade849026782c3eac4952 Gerrit-Change-Number: 67700 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Sep 2022 20:02:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> 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]: stlinkv3_spi: fix uninitialized pointer
by Alexander Goncharov (Code Review) 27 Sep '22

27 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Miklós Márton, Thomas Heijligen, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67700 to look at the new patch set (#2). Change subject: stlinkv3_spi: fix uninitialized pointer ...................................................................... stlinkv3_spi: fix uninitialized pointer `stlinkv3_handle` declared without an initial value. The variable is initialized in a branch which can only be accessed if `devs_stlinkv3_spi[0].vendor_id != 0`. Otherwise, the variable contains a garbage value. We can consider this case as a false positive because `devs_stlinkv3_spi` holds as a minimum one device entry (otherwise we wouldn't need a driver). This issue was found by setting compiler flag `-Og`, which optimizes debugging experience, and running scan-build. So, we have to fix it to allow the compiler to use the flag and remove the warning from the scan-build list. Change-Id: Ibaf25f67186724d9045ade849026782c3eac4952 Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> --- M stlinkv3_spi.c 1 file changed, 24 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/00/67700/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67700 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibaf25f67186724d9045ade849026782c3eac4952 Gerrit-Change-Number: 67700 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022