September 2021 - flashrom-gerrit

Change in flashrom[master]: ft2232_spi: reintroduce generic GPIOL control
by Michael Niewöhner (Code Review) 26 Sep '21

26 Sep '21

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/+/57810 ) Change subject: ft2232_spi: reintroduce generic GPIOL control ...................................................................... Patch Set 17: (4 comments) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/57810/comment/b54c6a96_750bd501 PS16, Line 483: msg_perr > As the message says "warning", use msg_pwarn()? Done https://review.coreboot.org/c/flashrom/+/57810/comment/317fcb77_bd3b759f 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 […] Done https://review.coreboot.org/c/flashrom/+/57810/comment/957647d4_5cb08188 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); Yep, nice idea. I took the second one. `char *const arg` doesn't work, since arg is declared globally already. I'm reusing that one instead. > Also just noticed: In the current form, any premature `return` inside > the loop would leak some of these arg_gpiol[] pointers. Sure, any return would need `free(arg)` before. That also applies to the solutions above, doesn't it? https://review.coreboot.org/c/flashrom/+/57810/comment/8616d729_ceb1049e PS16, Line 538: bool format_error = 0; > Just to mention it, don't know how reasonable it would look: For error […] Heh, I thought about using it but feared people (you :P) wouldn't like it :D Done -- 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: 17 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-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

1 0

Change in flashrom[master]: ft2232_spi: reintroduce generic GPIOL control
by Michael Niewöhner (Code Review) 26 Sep '21

26 Sep '21

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Hello Felix Singer, build bot (Jenkins), Alan Green, Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/57810 to look at the new patch set (#17). Change subject: ft2232_spi: reintroduce generic GPIOL control ...................................................................... ft2232_spi: reintroduce generic GPIOL control This reintroduces a reworked version of the GPIOL pin control first introduced in commit 3207844 (CB:49637), which was reverted in commit 6518cf3 (CB:55692) due to breakage. This change introduces a new argument `gpiolX` to allow use of the four GPIOL pins either as generic gpios or as additional CS# signal(s). `X` specifies the GPIOL pin (0-3) to be set to one of [HLC] with the following meaning: * H - set the pin as output high * L - set the pin as output low * C - use the pin as additional CS# signal The third value, `C`, aims to replace the parameter `csgpiol`, that is now marked as deprecated and can be removed at some point in the future. `gpiol` and `csgpiol` are mutually exclusive and use of both results in an error. Multiple pins may be set by specifying the parameter multiple times. Documentation was updated/added accordingly. Test: All pin levels/modes have been verified to behave correctly with a logic analyzer. Change-Id: I3989f0f9596c090de52dca67183b1363dae59d3a Signed-off-by: Alan Green <avg(a)google.com> Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M flashrom.8.tmpl M ft2232_spi.c 2 files changed, 109 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/57810/17 -- 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: 17 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-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom.8: carve out `csgpiol` into its own section
by Michael Niewöhner (Code Review) 26 Sep '21

26 Sep '21

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/+/57893 ) Change subject: flashrom.8: carve out `csgpiol` into its own section ...................................................................... Patch Set 3: (1 comment) File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/57893/comment/fee28e65_d7887ce3 PS2, Line 843: and GPIO-based chip select > Please drop this part. Basically, we'd want to revert the changes of […] oh yes ofc; Done -- 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-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

1 0

Change in flashrom[master]: flashrom.8: carve out `csgpiol` into its own section
by Michael Niewöhner (Code Review) 26 Sep '21

26 Sep '21

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

1 0

Change in flashrom[master]: ft2232_spi: prevent use of reserved pins on some programmers
by Michael Niewöhner (Code Review) 26 Sep '21

26 Sep '21

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

1 0

Change in flashrom[master]: ft2232_spi: prevent use of reserved pins on some programmers
by Michael Niewöhner (Code Review) 26 Sep '21

26 Sep '21

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

1 0

Change in flashrom[master]: ft2232_spi: reintroduce generic GPIOL control
by Alan Green (Code Review) 26 Sep '21

26 Sep '21

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

1 0

Change in flashrom[master]: ch341a_spi: libusb functions can handle all platforms
by Thomas Heijligen (Code Review) 26 Sep '21

26 Sep '21

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

1 0

Change in flashrom[master]: Remove preprocessor closing comments
by Nico Huber (Code Review) 26 Sep '21

26 Sep '21

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

1 0

Change in flashrom[master]: tests: Add NON_ZERO macro and not_null function instead of MOCK_HANDLE
by Nico Huber (Code Review) 26 Sep '21

26 Sep '21

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

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2021