September 2022 - flashrom-gerrit

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

28 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 5: (5 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/66373/comment/62e953a7_54530904 PS4, Line 10: This is an important step to : remove the global state because it'll allow the programmer's data : to be used in delay functions. > Maybe just drop part of this sentence after 'because' as its not what this patch does specifically f […] Ack https://review.coreboot.org/c/flashrom/+/66373/comment/d17f1455_0debfdb0 PS4, Line 13: : Programmers that use internal delay can provide NULL as a context. > Move this up to the last paragraph. Ack https://review.coreboot.org/c/flashrom/+/66373/comment/53449ebd_dd2d77b5 PS4, Line 16: This is one of the steps on the way to move master's data : memory management behind the initialisation API, for more : context see other patches under the same topic specified below. > Drop this paragraph. Ack File flashrom.c: https://review.coreboot.org/c/flashrom/+/66373/comment/3037cd62_b484e4e3 PS5, Line 222: if (!usecs) : return; : : if (!programmer->delay) : return internal_delay(flash, usecs); // maybe don't pass context into internal? or pass NULL? : : if (!flash) { : msg_perr("%s called but flashctx is not valid!\n" : "Please report a bug at flashrom(a)flashrom.org\n", __func__); : exit(1); : } : programmer->delay(flash, usecs); : } : leave all this diff out this patch and have it purely just about plumbing flashctx though the control flow graph (CFG) just as is the case in https://review.coreboot.org/c/flashrom/+/67392/ File ichspi.c: https://review.coreboot.org/c/flashrom/+/66373/comment/0105d16a_e5e9db9d PS5, Line 860: truct flashctx *flas Is it maybe sufficient to just use `NULL` in the case of ichspi as it is the internal timer anyway. -- 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: 5 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: Wed, 28 Sep 2022 10:27:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: tree: provide flashrom context into programmer's delay
by Alexander Goncharov (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66373 to look at the new patch set (#6). Change subject: tree: provide flashrom context into programmer's delay ...................................................................... tree: provide flashrom context into programmer's delay Add the flashrom context to the delay function declaration, which located in `struct programmer_entry`. This is an important step to remove the global state. Programmers that use internal delay can provide NULL as a context. TOPIC=register_master_api TEST=builds Change-Id: Ibb0bce26ce2052853ee52158d7ba742967a9e229 Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Ticket: https://ticket.coreboot.org/issues/391 --- M 82802ab.c M amd_imc.c M at45db.c M atavia.c M bitbang_spi.c M ch341a_spi.c M cli_classic.c M dediprog.c M dummyflasher.c M edi.c M en29lv640b.c M flashrom.c M ichspi.c M include/flash.h M include/programmer.h M it87spi.c M jedec.c M nicintel_eeprom.c M pony_spi.c M raiden_debug_spi.c M s25f.c M serial.c M serprog.c M spi25.c M spi25_statusreg.c M sst28sf040.c M stm50.c M udelay.c M w29ee011.c M w39.c M wbsio_spi.c 31 files changed, 184 insertions(+), 141 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/66373/6 -- 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: 6 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-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: tree: provide flashrom context into programmer's delay
by Alexander Goncharov (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66373 to look at the new patch set (#5). Change subject: tree: provide flashrom context into programmer's delay ...................................................................... tree: provide flashrom context into programmer's delay Add the flashrom context to the delay function declaration, which located in `struct programmer_entry`. This is an important step to remove the global state. Programmers that use internal delay can provide NULL as a context. TOPIC=register_master_api TEST=builds Change-Id: Ibb0bce26ce2052853ee52158d7ba742967a9e229 Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Ticket: https://ticket.coreboot.org/issues/391 --- M 82802ab.c M amd_imc.c M at45db.c M atavia.c M bitbang_spi.c M ch341a_spi.c M cli_classic.c M dediprog.c M dummyflasher.c M edi.c M en29lv640b.c M flashrom.c M ichspi.c M include/flash.h M include/programmer.h M it87spi.c M jedec.c M nicintel_eeprom.c M pony_spi.c M raiden_debug_spi.c M s25f.c M serial.c M serprog.c M spi25.c M spi25_statusreg.c M sst28sf040.c M stm50.c M udelay.c M w29ee011.c M w39.c M wbsio_spi.c 31 files changed, 184 insertions(+), 141 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/66373/5 -- 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: 5 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-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: stlinkv3_spi: fix uninitialized pointer
by Angel Pons (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67700 ) Change subject: stlinkv3_spi: fix uninitialized pointer ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > Huh? Huh? -- 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Wed, 28 Sep 2022 09:45:46 +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

[M] Change in flashrom[1.2.x]: Makefile: Fix building on AArch64 NixOS
by Nico Huber (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Thomas Heijligen, Pyry Kontio, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67902 ) Change subject: Makefile: Fix building on AArch64 NixOS ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Give me a look on it at a large Monitor. […] It's a cherry-pick from master. So in a sense, the old implementation on master was affected. New one should be fine :) -- To view, visit https://review.coreboot.org/c/flashrom/+/67902 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Iaa4477a71e758cf9ecad2c22f3b77bc6508a3510 Gerrit-Change-Number: 67902 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Pyry Kontio <pyry.kontio(a)drasa.eu> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Pyry Kontio <pyry.kontio(a)drasa.eu> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 28 Sep 2022 09:32:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: flashrom.c: Add wrapper function to use the erase algorithm
by Aarya (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: 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/+/66104 to look at the new patch set (#59). 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 1 file changed, 150 insertions(+), 290 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/66104/59 -- 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: 59 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: 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

[S] Change in flashrom[master]: stlinkv3_spi: fix uninitialized pointer
by Felix Singer (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk, Alexander Goncharov. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67700 ) Change subject: stlinkv3_spi: fix uninitialized pointer ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > Done Huh? -- 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Wed, 28 Sep 2022 07:05:11 +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

[M] Change in flashrom[1.2.x]: Makefile: Fix building on AArch64 NixOS
by Thomas Heijligen (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Nico Huber, Pyry Kontio, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67902 ) Change subject: Makefile: Fix building on AArch64 NixOS ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Give me a look on it at a large Monitor. Is the new implementation on the master brach also affected? -- To view, visit https://review.coreboot.org/c/flashrom/+/67902 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Iaa4477a71e758cf9ecad2c22f3b77bc6508a3510 Gerrit-Change-Number: 67902 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Pyry Kontio <pyry.kontio(a)drasa.eu> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Pyry Kontio <pyry.kontio(a)drasa.eu> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 28 Sep 2022 06:56:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: stlinkv3_spi: fix uninitialized pointer
by Thomas Heijligen (Code Review) 28 Sep '22

28 Sep '22

Attention is currently required from: Felix Singer, Angel Pons, Anastasia Klimchuk, Alexander Goncharov. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67700 ) Change subject: stlinkv3_spi: fix uninitialized pointer ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS1: > > My suggestion would be to add a comment libusb_device_handle *stlinkv3_handle = NULL; /* NULL for […] Done -- 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Wed, 28 Sep 2022 06:52:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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

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

28 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Alexander Goncharov. Jean THOMAS has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 5: (26 comments) Patchset: PS5: Hi Joursoir & Felix, thans for your reviews. I tried to address your coding style remarks, let me know if I missed something. File dirtyjtag_spi.c: https://review.coreboot.org/c/flashrom/+/67878/comment/8ad863d5_5cf9bf76 PS4, Line 34: DJTAGUNK > It seems this is unused? […] Yes UNK stands for UNKNOWN. I won't fix this, I will remove the enum instead since this isn't used at the moment. I'll add it back when we settle on all the protocol changes for DJTAG2. https://review.coreboot.org/c/flashrom/+/67878/comment/8c176dde_2eaee656 PS4, Line 39: 0xC0CA > Please use lower case Done https://review.coreboot.org/c/flashrom/+/67878/comment/f0e0e7f9_da669637 PS4, Line 47: dirytjtag_command_identifier > typo: dir*ty*jtag Done https://review.coreboot.org/c/flashrom/+/67878/comment/2abaa0bc_b8dc8e9b PS4, Line 48: CMD_STOP = 0x00, > Please use TAB instead of two spaces. Done https://review.coreboot.org/c/flashrom/+/67878/comment/b67edd36_1f834b8e PS4, Line 57: dirytjtag_signal_identifier > typo: dir*ty*jtag Done https://review.coreboot.org/c/flashrom/+/67878/comment/d4a8b3c9_36d13b6f PS4, Line 58: SIG_TCK = 1 << 1, > TABs here too. Done https://review.coreboot.org/c/flashrom/+/67878/comment/e4417da3_3dd15146 PS4, Line 66: char* data > `char *data` Done https://review.coreboot.org/c/flashrom/+/67878/comment/ef20f8da_efd9ac38 PS4, Line 87: char* data > `char *data` Done https://review.coreboot.org/c/flashrom/+/67878/comment/8723baed_ced95366 PS4, Line 120: static inline unsigned int _min(unsigned int a, unsigned int b) : { : return (a < b) ? a : b; : } > Seems to be unused? Done https://review.coreboot.org/c/flashrom/+/67878/comment/02c682d7_9b8681d0 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. Done https://review.coreboot.org/c/flashrom/+/67878/comment/a4aba6d3_13c49d2b PS4, Line 128: writecnt+readcnt > spaces Done https://review.coreboot.org/c/flashrom/+/67878/comment/eac1a34e_a04cb603 PS4, Line 129: size_t num_xfer = (len+max_xfer_size-1)/max_xfer_size; // ceil(len/max_xfer_size) > spaces Done https://review.coreboot.org/c/flashrom/+/67878/comment/4ebaec91_4acde234 PS4, Line 131: char *tx_buf = malloc(max_xfer_size*num_xfer); > spaces Done https://review.coreboot.org/c/flashrom/+/67878/comment/6d9a88d6_8bb76d7d PS4, Line 133: msg_perr("%s: Failed tx_buf allocation", __func__); > Missing \n Done https://review.coreboot.org/c/flashrom/+/67878/comment/968e8ebe_3cd91f06 PS4, Line 136: char *rx_buf = malloc(max_xfer_size*num_xfer); > spaces Done https://review.coreboot.org/c/flashrom/+/67878/comment/05bb1699_a77fb9a7 PS4, Line 138: msg_perr("%s: Failed rx_buf allocation", __func__); > Missing \n Done https://review.coreboot.org/c/flashrom/+/67878/comment/e54f0d8e_a4a31f8a PS4, Line 146: len%max_xfer_size > add spaces before and after % Done https://review.coreboot.org/c/flashrom/+/67878/comment/e72cba73_543a13be PS4, Line 147: len%max_xfer_size > same here Done https://review.coreboot.org/c/flashrom/+/67878/comment/9ee7a852_a707f71a PS4, Line 149: txn_size = 30; > How about setting this value during declaration in line 145? So it would be the "default" and if app […] Done https://review.coreboot.org/c/flashrom/+/67878/comment/970e365c_b720cca4 PS4, Line 152: tx_buf+30*i > same here Done https://review.coreboot.org/c/flashrom/+/67878/comment/3be44f64_5f81025c PS4, Line 155: rx_buf+i*30 > same here Done https://review.coreboot.org/c/flashrom/+/67878/comment/e8e9d571_40433bda 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. […] Done https://review.coreboot.org/c/flashrom/+/67878/comment/0fa67328_5d72adb2 PS4, Line 248: tmp, strerror(errno)); > nit: I would use one line here. It's not that long. Done https://review.coreboot.org/c/flashrom/+/67878/comment/b1453bbf_25bc2cd9 PS4, Line 289: 0xFF > lower case Done https://review.coreboot.org/c/flashrom/+/67878/comment/61d2b698_d50d983f PS4, Line 290: 0xFF > make this lower case Done -- 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: 5 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: 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: Wed, 28 Sep 2022 06:39:05 +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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022