September 2022 - flashrom-gerrit

[S] Change in flashrom[1.0.x]: test_build.sh: Run `make clean` before compiling
by Nico Huber (Code Review) 30 Sep '22

30 Sep '22

Attention is currently required from: Felix Singer. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67997 ) Change subject: test_build.sh: Run `make clean` before compiling ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Does this fix anything? `make` should know what to do. It could only fail if there are bigger changes to the build system that we shouldn't do on a release branch anyway. -- To view, visit https://review.coreboot.org/c/flashrom/+/67997 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: I6a6f5a3483941148330b95936e1445f21a34061b Gerrit-Change-Number: 67997 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Fri, 30 Sep 2022 20:07:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: stlinkv3_spi: Bug fix, Initialize uninitialized pointers
by Felix Singer (Code Review) 30 Sep '22

30 Sep '22

Attention is currently required from: Shreeyash . Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66225 ) Change subject: stlinkv3_spi: Bug fix, Initialize uninitialized pointers ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: @Shreeyash Are you still interested in getting this done? -- To view, visit https://review.coreboot.org/c/flashrom/+/66225 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ce156a9f7455434ea461560320c84660c93c02f Gerrit-Change-Number: 66225 Gerrit-PatchSet: 2 Gerrit-Owner: Shreeyash <shreeyash335(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Shreeyash <shreeyash335(a)gmail.com> Gerrit-Comment-Date: Fri, 30 Sep 2022 17:45:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: dirtyjtag: Add DirtyJTAG programmer
by Nico Huber (Code Review) 30 Sep '22

30 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Jean THOMAS, Alexander Goncharov. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67878 ) Change subject: dirtyjtag: Add DirtyJTAG programmer ...................................................................... Patch Set 7: Code-Review+1 (4 comments) Patchset: PS7: Thanks for your update. Alas, I missed to check the resource usage/cleanup in the init function. Everything else looks good to me :) Also something else I kept forgetting to mention: We have some additional, annoying programmer lists in `test_build.sh`. You should add yours there too. File dirtyjtag_spi.c: https://review.coreboot.org/c/flashrom/+/67878/comment/321761d6_c989c674 PS7, Line 154: 32 Just curious, is there anything in the last two bytes? https://review.coreboot.org/c/flashrom/+/67878/comment/d45918d2_18f25575 PS7, Line 212: NULL Should use the `djtag_data->libusb_ctx`. Many more below, too. https://review.coreboot.org/c/flashrom/+/67878/comment/be295205_640c7791 PS7, Line 297: return -1; Sorry, I always forget to check such cleanup paths ._. There are some steps missing (I guess it should do about the same as the shutdown function). However, not everything should be called on every error. For instance, calling libusb_close() before `handle` is set might be bad. Hence we'll need more labels. Also, all early returns should go through a cleanup path. Except the first maybe, because there no resources have been allocated yet. -- 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: 7 Gerrit-Owner: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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: Jean THOMAS <virgule(a)jeanthomas.me> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 30 Sep 2022 13:35:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[1.1.x]: Makefile: Fix dependencies for developerbox_spi
by Angel Pons (Code Review) 30 Sep '22

30 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68011 ) Change subject: Makefile: Fix dependencies for developerbox_spi ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/68011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: Ic0fe589ffdccede0fbf6360c2bebe58a36654f10 Gerrit-Change-Number: 68011 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Comment-Date: Fri, 30 Sep 2022 12:28:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: stlinkv3_spi: fix uninitialized pointer
by Nico Huber (Code Review) 30 Sep '22

30 Sep '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons, Anastasia Klimchuk, Alexander Goncharov. Nico Huber 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: > > I spent some time trying to understand this very eloquent conversation :D […] AFAICT, everybody agrees that this is a false-positive, is that right? If so, please make sure the commit message is 100% correct, whatever you choose to merge. Because then it's not a fix for anything in flashrom, certainly not a bug fix, but a workaround to assist limited path analysis. Btw. also under the assumption that this is a false-positive, is there no way to make the compiler/scan-build run through without changing the code? -- 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: Angel Pons <th3fanbus(a)gmail.com> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> 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-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 30 Sep 2022 10:31:53 +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

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

30 Sep '22

Attention is currently required from: Simon Buhrow, Nico Huber, Thomas Heijligen. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 ) Change subject: flashrom.c: Add wrapper function to use the erase algorithm ...................................................................... Patch Set 61: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/66104/comment/8acaf8a4_002378eb PS52, Line 1057: flashctx->chip->page_size > I´m fine with this implementation: Using page_size and making sure that all (test) chips have a vali […] Now its based on gran -- 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: 61 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 10:01:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Buhrow Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: drivers: Move (un)map_flash_region from programmer to par/spi_master
by Angel Pons (Code Review) 30 Sep '22

30 Sep '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Jonathon Hall, Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67695 ) Change subject: drivers: Move (un)map_flash_region from programmer to par/spi_master ...................................................................... Patch Set 7: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/67695/comment/5cba4a1e_13a21af8 PS2, Line 1865: &mapper_phys > I wonder if this is the part that is the second patch here and if the parallel framework part of the […] +1 regarding divide and conquer. One strategy I employ quite often is to separate "refactoring changes that (ideally) do not have an impact on current behavior" and "changes that actually change behavior on purpose (e.g. to fix and/or improve something)". -- To view, visit https://review.coreboot.org/c/flashrom/+/67695 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c3df6ae260bcdb246dfb0cd8e043919609b014b Gerrit-Change-Number: 67695 Gerrit-PatchSet: 7 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 30 Sep 2022 07:39:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-MessageType: comment

1 0

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

30 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) Commit Message: https://review.coreboot.org/c/flashrom/+/67700/comment/e13972cb_459e825e PS2, Line 9: declared is declared -- 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: Angel Pons <th3fanbus(a)gmail.com> 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: 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: Fri, 30 Sep 2022 07:26:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

30 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: Code-Review+1 (1 comment) Patchset: PS1: > I spent some time trying to understand this very eloquent conversation :D Thanks for putting in the effort! 😅 > I looked into alternative patch CB:66225 and I see that Felix pushed the latest patchset which makes it almost the same as this patch (that's probably what the first huh means). This patch, however has a comment, which I think is good, good to have a comment (that's probably the second huh). My "Huh?" simply means that I don't understand what Felix's "Huh?" means. > So technically there are two options what to do: > 1) Click Submit for this patch, abandon CB:66225 > 2) Felix uploads one more patchset to CB:66225, with a comment, which would make CB:66225 identical to this one. Then click submit on CB:66225 and abandon this one. > > Just from purely technical perspective 1) looks easier. If CB:66225 would be mine, I would just say yeah let's abandon. But it is not mine, started by another contributor, and yes it was pushed earlier. > I vote for 1) and I can write a nice comment to CB:66225 explaining what has happened. > What do other people think? I too vote for 1) as it seems more straightforward. -- 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: Angel Pons <th3fanbus(a)gmail.com> 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: 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: Fri, 30 Sep 2022 07:24:59 +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]: flashrom.c: Add wrapper function to use the erase algorithm
by Simon Buhrow (Code Review) 30 Sep '22

30 Sep '22

Attention is currently required from: Nico Huber, Thomas Heijligen. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 ) Change subject: flashrom.c: Add wrapper function to use the erase algorithm ...................................................................... Patch Set 61: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/66104/comment/6b7c445e_70a727a0 PS52, Line 1057: flashctx->chip->page_size > Done I´m fine with this implementation: Using page_size and making sure that all (test) chips have a valid page_size. However there might be use cases I do not have in mind? -- 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: 61 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-Comment-Date: Fri, 30 Sep 2022 06:57:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022