September 2021 - flashrom-gerrit

Change in flashrom[master]: [RFC] [WIP] ft2232_spi: reintroduce generic GPIOL control
by Nico Huber (Code Review) 23 Sep '21

23 Sep '21

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: [RFC] [WIP] ft2232_spi: reintroduce generic GPIOL control ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > Oh, I just had this idea. Wdyt? […] Hmmm, actually, it took me too long to understand what you mean ;) It's more explicit, but seems inconvenient, both for the user who has to encode it and the program that has to decode it. Let's compare these notations directly -p ft2232_spi:gpiol0=C,gpiol3=H -p ft2232_spi:gpiol=0C3H The first is more typing effort, but easier to read. The other way around for the second. In both cases it doesn't look very nice, as the user has to specify everything in a single string without spaces. *shrug* I prefer the first as it should be obvious to any reader (at least for H/L). Especially somebody used to the old `gpiol` option could get confused by a notation that is incompatible. -- 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: 6 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-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: Thu, 23 Sep 2021 11:51:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

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

23 Sep '21

Attention is currently required from: Felix Singer, Alan Green, Nico Huber, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57810 ) Change subject: [RFC] [WIP] ft2232_spi: reintroduce generic GPIOL control ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > Good idea. […] Oh, I just had this idea. Wdyt? `gpiol=<x[CHL]>...` e.g. `gpiol=0C3H` -- 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: 6 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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 23 Sep 2021 11:28:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

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

23 Sep '21

Attention is currently required from: Felix Singer, Alan Green, Nico Huber, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57810 ) Change subject: [RFC] [WIP] ft2232_spi: reintroduce generic GPIOL control ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > Generally, I'd prefer a much more explicit interface, e.g. […] Good idea. That also makes handling reserved pins a bit easier on the user side -- 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: 6 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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 23 Sep 2021 11:26:13 +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]: [RFC] [WIP] ft2232_spi: reintroduce generic GPIOL control
by Nico Huber (Code Review) 23 Sep '21

23 Sep '21

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: [RFC] [WIP] ft2232_spi: reintroduce generic GPIOL control ...................................................................... Patch Set 6: (2 comments) Patchset: PS6: Generally, I'd prefer a much more explicit interface, e.g. gpiol[0-3]=[CHL] File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/57810/comment/913577e3_2a52094a PS2, Line 530: /* * * * * * * * * * * * * * * * * * * * * * * * * * * : * REQUEST FOR COMMENTS: 'C' could replace `csgpiol` * : * * * * * * * * * * * * * * * * * * * * * * * * * * */ : case 'C': : cs_bits |= 1 << gpiol_pin; : pindir |= 1 << gpiol_pin; : break; : > I like it. Sounds good. It would also allow us to make the two interfaces mutually exclusive. We could also show a deprecation warning for `csgpiol` and maybe drop it in the future. -- 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: 6 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-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: Thu, 23 Sep 2021 11:20:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alan Green <avg(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.8: add missing entry for `--flash-contents`
by Michael Niewöhner (Code Review) 23 Sep '21

23 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/+/57807 ) Change subject: flashrom.8: add missing entry for `--flash-contents` ...................................................................... Patch Set 4: (1 comment) File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/57807/comment/a2412ee0_dd4e4b8a PS3, Line 297: , > Might just be me, with this comma it doesn't make much sense to me. In my […] Oh yes, of course it doesn't make sense to provide wrong data at all, so I broke the meaninig. -> Dropped "as" and "then" again -- To view, visit https://review.coreboot.org/c/flashrom/+/57807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I64a8200a86329bd26a2069c5dc39430de9f8ba09 Gerrit-Change-Number: 57807 Gerrit-PatchSet: 4 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-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: Thu, 23 Sep 2021 11:00:32 +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: add missing entry for `--flash-contents`
by Michael Niewöhner (Code Review) 23 Sep '21

23 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/+/57807 to look at the new patch set (#4). Change subject: flashrom.8: add missing entry for `--flash-contents` ...................................................................... flashrom.8: add missing entry for `--flash-contents` Change-Id: I64a8200a86329bd26a2069c5dc39430de9f8ba09 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M flashrom.8.tmpl 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/57807/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/57807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I64a8200a86329bd26a2069c5dc39430de9f8ba09 Gerrit-Change-Number: 57807 Gerrit-PatchSet: 4 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-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 Nico Huber (Code Review) 23 Sep '21

23 Sep '21

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/+/57809 ) Change subject: ft2232_spi: prevent use of reserved pins on some programmers ...................................................................... Patch Set 3: Code-Review+2 (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/57809/comment/abb1ad2c_b625aa75 PS2, Line 417: /* Flyswatter and Flyswatter-2 require GPIO bits 0x80 : * and 0x40 to be driven low to enable output buffers */ : pindir = 0xcb; : > Done IMHO, a good argument not to add a redundant variable. Isn't this pindir & 0xf0 == rsv_bits always true after detecting the specific programmer? If we can use that as a rule to write a patch, we can also just write it into code to avoid human errors. One could still do it with an additional variable, e.g. at the right spot: const uint8_t rsv_bits = pindir & 0xf0; -- 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: 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-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: Thu, 23 Sep 2021 09:05:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: clarify the comment about gpio configuration
by Nico Huber (Code Review) 23 Sep '21

23 Sep '21

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/+/57808 ) Change subject: ft2232_spi: clarify the comment about gpio configuration ...................................................................... Patch Set 3: Code-Review+2 (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/57808/comment/05369707_1cbe8507 PS2, Line 94: GPIOL* pins are : * configured as inputs > mentioned them as well now; recheck, please Done -- To view, visit https://review.coreboot.org/c/flashrom/+/57808 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaceec889a65ead8cdde917f61b2a9695d440f781 Gerrit-Change-Number: 57808 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-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: Thu, 23 Sep 2021 08:59:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.8: add missing entry for `--flash-contents`
by Nico Huber (Code Review) 23 Sep '21

23 Sep '21

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/+/57807 ) Change subject: flashrom.8: add missing entry for `--flash-contents` ...................................................................... Patch Set 3: Code-Review+2 (3 comments) Patchset: PS3: Need a little help with one sentence, maybe I just read it wrong. File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/57807/comment/4d5092ef_35960fa5 PS2, Line 295: .BR <ref\-file> . > Sounds good, thank you! Added a slightly modified version Done File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/57807/comment/ab743f50_878f1ee9 PS3, Line 297: , Might just be me, with this comma it doesn't make much sense to me. In my version the `if` was binding to the last part of the sentence, now it seems to be about the being careful part. At least that's how I read it now. Also, it seems one. One should just avoid wrong data, not be careful when it's wrong. -- To view, visit https://review.coreboot.org/c/flashrom/+/57807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I64a8200a86329bd26a2069c5dc39430de9f8ba09 Gerrit-Change-Number: 57807 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-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: Thu, 23 Sep 2021 08:58:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors: Add explicit checks for all chipsets
by David Hendricks (Code Review) 23 Sep '21

23 Sep '21

Attention is currently required from: Nico Huber, Michał Żygowski, Jonathan Zhang, Angel Pons. David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57793 ) Change subject: ich_descriptors: Add explicit checks for all chipsets ...................................................................... Patch Set 3: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/57793/comment/c40a41b3_3ff2c415 PS1, Line 9: CB:55651 > Please reference merged commits by their commit hash, e.g. […] Done File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/57793/comment/ce10add0_f3071c18 PS1, Line 962: warn_peculiar_desc(content->ICCRIBA > 0x34, "C620 series"); > Another case, Done https://review.coreboot.org/c/flashrom/+/57793/comment/03a33642_4c10e4c1 PS1, Line 965: warn_peculiar_desc(content->ICCRIBA != 0x31, "100 series"); > and another one. Basically, if there is an expression given as the first […] Good point about `warn_if` - I've removed that as well. https://review.coreboot.org/c/flashrom/+/57793/comment/d9ee735d_504a9f51 PS1, Line 968: if (content->ICCRIBA != 0x34) : msg_pwarn("Unknown flash descriptor, assuming 300 series compatibility.\n"); here too -- To view, visit https://review.coreboot.org/c/flashrom/+/57793 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ica49477492876810a6fa212768b1ab9e8c12001f Gerrit-Change-Number: 57793 Gerrit-PatchSet: 3 Gerrit-Owner: David Hendricks Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.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: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 23 Sep 2021 05:09: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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2021