Attention is currently required from: Felix Singer.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/67886
to look at the new patch set (#5).
Change subject: jlink_spi.c: Move parameters into struct
......................................................................
jlink_spi.c: Move parameters into struct
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: I7af29e4bd107bfcda929155bbe603798d092fa79
---
M jlink_spi.c
1 file changed, 45 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/67886/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/67886
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7af29e4bd107bfcda929155bbe603798d092fa79
Gerrit-Change-Number: 67886
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer.
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/67881
to look at the new patch set (#3).
Change subject: jlink_spi.c: Set `use_serial_number` to false by default
......................................................................
jlink_spi.c: Set `use_serial_number` to false by default
Set `use_serial_number` to false by default removing the else branch
which is entered when its related programmer argument `serial` is not
found.
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: I83b5e651097d1a248baf5ddb6b09e5cff161317d
---
M jlink_spi.c
1 file changed, 15 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/67881/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/67881
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I83b5e651097d1a248baf5ddb6b09e5cff161317d
Gerrit-Change-Number: 67881
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer.
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/67883
to look at the new patch set (#4).
Change subject: jlink_spi.c: Move function argument into previous line
......................................................................
jlink_spi.c: Move function argument into previous line
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: I13b7f37adb242abbbd3a2961e7cdc4cf7ec578d8
---
M jlink_spi.c
1 file changed, 11 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/67883/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/67883
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I13b7f37adb242abbbd3a2961e7cdc4cf7ec578d8
Gerrit-Change-Number: 67883
Gerrit-PatchSet: 4
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen, Angel Pons.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67908 )
Change subject: ft2232_spi.c: Split out most programmer param parsing
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Sry, this should have gone to the first patch
Then also see CB:67886 ;)
Though, patches need to be squashed and I need to push the rest rest of the programmers I have only locally.
--
To view, visit https://review.coreboot.org/c/flashrom/+/67908
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I25faac1060fc2bcd6042e34802e5bc493c936377
Gerrit-Change-Number: 67908
Gerrit-PatchSet: 1
Gerrit-Owner: 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-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 02 Oct 2022 06:38:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67908 )
Change subject: ft2232_spi.c: Split out most programmer param parsing
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> On the first view, still from a mobile device, I like this patch. […]
Sry, this should have gone to the first patch
--
To view, visit https://review.coreboot.org/c/flashrom/+/67908
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I25faac1060fc2bcd6042e34802e5bc493c936377
Gerrit-Change-Number: 67908
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 02 Oct 2022 06:21:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67908 )
Change subject: ft2232_spi.c: Split out most programmer param parsing
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
On the first view, still from a mobile device, I like this patch.
what do you think of creating an const instance of ft2232_parameters with the default values instead of passing them in the code? So you have `struct ft2232_parameters programmer_parameters = ft2232_default_parameters;`
--
To view, visit https://review.coreboot.org/c/flashrom/+/67908
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I25faac1060fc2bcd6042e34802e5bc493c936377
Gerrit-Change-Number: 67908
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 02 Oct 2022 06:21:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov.
Felix Singer 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 6:
(1 comment)
File tests/libusb_wraps.c:
PS5:
> Why? In all previous tests, wraps were added in the same patch with the test that needed those wraps […]
Commits should only do one thing and not multiple things. I think this commit does two things:
* Introducing lots of functions in common code
* Introducing tests which are based on the new common code
Speaking generally, I think it's a bad practice to submit a commit of this kind, because it's not atomic. The commit title doesn't even give me an idea that new common code is introduced, but many comments are related to the common code.
Assuming that future commits get merged, which depend on the newly introduced common code, and we decide or we would like to revert this commit (for whatever reason, everything can happen), then we also have to revert all others that depend on it.
Don't get me wrong. I'm not saying that this will be the case for this commit. I just would like to move to this practice, since it gives us more possibilities in the future when we try or even have to fix something. Like a revert might be faster and easier than trying to fix the specific issue.
Other advantages: People are able to concentrate on the specific changes during the review and the comment history stays focused.
--
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: 6
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: 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: Sat, 01 Oct 2022 23:31:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Shreeyash .
Thomas Heijligen 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:
(2 comments)
Patchset:
PS2:
> Yes... I can't seem to understand the next step? […]
This patch addresses the symptoms of two issues. One, with the libusb_context, was caused by a wrong use of the libusb api. This part I've fixed.
The second part, the libusb_device_handle, is an false positive of the analyzers. It is still open and now handled by multiple open patches.
I'm sorry if this was not clear.
File stlinkv3_spi.c:
https://review.coreboot.org/c/flashrom/+/66225/comment/b75aaa93_5514ef4b
PS1, Line 501: stlinkv3_handle = usb_dev_get_by_vid_pid_serial(usb_ctx,
> So I think this is a false positive. […]
Sorry, this has gone under. But it's important to make clear that the NULL is for a false positive in the analyser not a big in flashrom
--
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Shreeyash <shreeyash335(a)gmail.com>
Gerrit-Comment-Date: Sat, 01 Oct 2022 20:29:27 +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: Shreeyash <shreeyash335(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer.
Shreeyash 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?
Yes... I can't seem to understand the next step?
My change, I believe was overruled by another change suggested by Thomas that I agree with. What do I have to do?
--
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Sat, 01 Oct 2022 13:50:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
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 9: Code-Review+1
(1 comment)
Patchset:
PS9:
Looks good, no more comments from me :)
--
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: 9
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: Sat, 01 Oct 2022 12:40:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment