Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/57893
to look at the new patch set (#3).
Change subject: flashrom.8: carve out `csgpiol` into its own section
......................................................................
flashrom.8: carve out `csgpiol` into its own section
Documentation for `csgpiol` was put into the generic programmer options
section. Move it to its own section.
Change-Id: Ic7379331d36b3068eacde5a983b4ccb3afc56c51
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M flashrom.8.tmpl
1 file changed, 11 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/57893/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/57893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic7379331d36b3068eacde5a983b4ccb3afc56c51
Gerrit-Change-Number: 57893
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57809 )
Change subject: ft2232_spi: prevent use of reserved pins on some programmers
......................................................................
Patch Set 7:
(1 comment)
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/57809/comment/b8803d1b_19ef0c48
PS6, Line 424: gets
> drop one `gets` please
lol who wrote that?
--
To view, visit https://review.coreboot.org/c/flashrom/+/57809
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ied450fa5ef358153adefec3beabc63a62c9f60cd
Gerrit-Change-Number: 57809
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 26 Sep 2021 19:21:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/57809
to look at the new patch set (#7).
Change subject: ft2232_spi: prevent use of reserved pins on some programmers
......................................................................
ft2232_spi: prevent use of reserved pins on some programmers
On some programmers a output buffer needs to be enabled by pulling a
gpio high/low. This gpio can not be used for `csgpiol`. Prevent this by
printing an error.
Change-Id: Ied450fa5ef358153adefec3beabc63a62c9f60cd
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M ft2232_spi.c
1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/57809/7
--
To view, visit https://review.coreboot.org/c/flashrom/+/57809
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ied450fa5ef358153adefec3beabc63a62c9f60cd
Gerrit-Change-Number: 57809
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Angel Pons, Michael Niewöhner.
Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57810 )
Change subject: ft2232_spi: reintroduce generic GPIOL control
......................................................................
Patch Set 16:
(1 comment)
Patchset:
PS16:
> Alan, would the new syntax suit your needs?
Yes, this new syntax works for us. We no longer have the setup that required this, but, if we were still working with that board, then this patch would allow us to use flashrom with it.
Thank you for checking.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57810
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3989f0f9596c090de52dca67183b1363dae59d3a
Gerrit-Change-Number: 57810
Gerrit-PatchSet: 16
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Alan Green <avg(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Sun, 26 Sep 2021 19:17:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57889 )
Change subject: ch341a_spi: libusb functions can handle all platforms
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> This would change the behavior on non-Linux platforms where libusb implements […]
Yes, this will detach an unsuitable kernel driver an all systems where these function is available. The claiming of the interface will then attach a suitable one. There is no uniform behavior of this across the usb programmer. The ch341a_spi is the only one where it's only for Linux.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57889
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If6f19744503055ab8e22c863b31e696808e0407d
Gerrit-Change-Number: 57889
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Sun, 26 Sep 2021 19:05:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57800 )
Change subject: Remove preprocessor closing comments
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
There was at least one case where I had to ask Gerrit for additional
context to see the matching #if (and my context setting is above the
default already). So there is definitely some value.
Also, I'd much prefer to focus on getting rid of the #if mess.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57800
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I74288afde108e764adc651039aae81857abffedf
Gerrit-Change-Number: 57800
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Sun, 26 Sep 2021 16:16:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57269 )
Change subject: tests: Add NON_ZERO macro and not_null function instead of MOCK_HANDLE
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> Build fails because "curl: (7) Failed to connect to cmocka.org port 443: Connection refused" […]
Generally, if there is hope that it is fixed, one can just hit the rebase
button. Works only if something else is merged to master in the meantime, ofc.
I'll just try that.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57269
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5ad6ee4aa9091447c6c9108c92bf7f6e755fca48
Gerrit-Change-Number: 57269
Gerrit-PatchSet: 3
Gerrit-Owner: 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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sun, 26 Sep 2021 16:11:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57889 )
Change subject: ch341a_spi: libusb functions can handle all platforms
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
This would change the behavior on non-Linux platforms where libusb implements
this API. Without us being able to test things right away. (Hate to say it)
maybe something to post-pone after the next release?
--
To view, visit https://review.coreboot.org/c/flashrom/+/57889
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If6f19744503055ab8e22c863b31e696808e0407d
Gerrit-Change-Number: 57889
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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: Sun, 26 Sep 2021 15:56:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Alan Green, Angel Pons, Michael Niewöhner.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57810 )
Change subject: ft2232_spi: reintroduce generic GPIOL control
......................................................................
Patch Set 16: Code-Review+1
(5 comments)
Patchset:
PS16:
Alan, would the new syntax suit your needs?
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/57810/comment/310892f1_87318f3a
PS16, Line 483: msg_perr
As the message says "warning", use msg_pwarn()?
https://review.coreboot.org/c/flashrom/+/57810/comment/9d2ecba1_f5c3f5d0
PS16, Line 484: "in the future. Use `gpiolX=C` instead.\n");
We try to keep output within 80 columns. Could add a \n after the
first sentence for instance.
https://review.coreboot.org/c/flashrom/+/57810/comment/4586c346_7eaeb773
PS16, Line 513: arg_gpiol[3] = extract_programmer_param("gpiol3");
This looks a little odd. How about moving the call into the loop?
We could still use an array for the string literals, e.g.
const char *const gpiol_params[] = { "gpiol0", "gpiol1", ...
char *const arg = extract_programmer_param(gpiol_params[pin]);
Alternatively, a single buffer could be used like this:
char gpiol_param[7];
snprintf(gpiol_param, sizeof(gpiol_param), "gpiol%d", pin);
char *const arg = extract_programmer_param(gpiol_param);
Also just noticed: In the current form, any premature `return` inside
the loop would leak some of these arg_gpiol[] pointers.
https://review.coreboot.org/c/flashrom/+/57810/comment/8d311a6c_c4f6ed80
PS16, Line 538: bool format_error = 0;
Just to mention it, don't know how reasonable it would look: For error
paths, using `goto` is common. The end of the loop body could look like
this:
free(arg);
continue;
format_error:
msg_perr("Error: ...");
free(arg);
return -2;
}
It would save us an indentation level.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57810
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3989f0f9596c090de52dca67183b1363dae59d3a
Gerrit-Change-Number: 57810
Gerrit-PatchSet: 16
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Alan Green <avg(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Alan Green <avg(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Sun, 26 Sep 2021 15:29:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Angel Pons, Michael Niewöhner.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57893 )
Change subject: flashrom.8: carve out `csgpiol` into its own section
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File flashrom.8.tmpl:
https://review.coreboot.org/c/flashrom/+/57893/comment/d2d2cc60_ab1d0594
PS2, Line 843: and GPIO-based chip select
Please drop this part. Basically, we'd want to revert the changes of
CB:28911 made here.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic7379331d36b3068eacde5a983b4ccb3afc56c51
Gerrit-Change-Number: 57893
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Sun, 26 Sep 2021 15:05:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment