September 2021 - flashrom-gerrit

Change in flashrom[master]: 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. 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 (#9). 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. 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, 111 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/57810/9 -- 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: 9 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-MessageType: newpatchset

1 0

Change in flashrom[master]: pcidev: Move pci device functions out of internal programmer
by Thomas Heijligen (Code Review) 23 Sep '21

23 Sep '21

Attention is currently required from: Nico Huber, Michał Żygowski. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55848 ) Change subject: pcidev: Move pci device functions out of internal programmer ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/55848 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ice40783a05c01cc78b34eb2c4a575554e9d55f8e Gerrit-Change-Number: 55848 Gerrit-PatchSet: 7 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> 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: 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-Comment-Date: Thu, 23 Sep 2021 17:24:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

23 Sep '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/57889 ) Change subject: ch341a_spi: libusb functions can handle all platforms ...................................................................... ch341a_spi: libusb functions can handle all platforms libusb_detach_kernel_driver() returns LIBUSB_ERROR_NOT_SUPPORTED on unsupported platforms. That's fine. There is no need to call this function only on Linux. Change-Id: If6f19744503055ab8e22c863b31e696808e0407d Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M ch341a_spi.c 1 file changed, 3 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/89/57889/1 diff --git a/ch341a_spi.c b/ch341a_spi.c index 4a76286..43acaeb 100644 --- a/ch341a_spi.c +++ b/ch341a_spi.c @@ -20,7 +20,6 @@ #include <string.h> #include <libusb.h> #include "flash.h" -#include "platform.h" #include "programmer.h" /* LIBUSB_CALL ensures the right calling conventions on libusb callbacks. @@ -448,17 +447,13 @@ return -1; } -/* libusb_detach_kernel_driver() and friends basically only work on Linux. We simply try to detach on Linux - * without a lot of passion here. If that works fine else we will fail on claiming the interface anyway. */ -#if IS_LINUX +/* libusb_detach_kernel_driver() is only available on linux. Other platforms return + * LIBUSB_ERROR_NOT_SUPPORTED. That's fine. */ ret = libusb_detach_kernel_driver(handle, 0); - if (ret == LIBUSB_ERROR_NOT_SUPPORTED) { - msg_pwarn("Detaching kernel drivers is not supported. Further accesses may fail.\n"); - } else if (ret != 0 && ret != LIBUSB_ERROR_NOT_FOUND) { + if (ret != 0 && ret != LIBUSB_ERROR_NOT_FOUND && ret != LIBUSB_ERROR_NOT_SUPPORTED) { msg_pwarn("Failed to detach kernel driver: '%s'. Further accesses will probably fail.\n", libusb_error_name(ret)); } -#endif ret = libusb_claim_interface(handle, 0); if (ret != 0) { -- 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-MessageType: newchange

1 0

Change in flashrom[master]: ft2232_spi: prevent use of reserved pins on some programmers
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/+/57809 ) Change subject: ft2232_spi: prevent use of reserved pins on some programmers ...................................................................... Patch Set 6: (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/57809/comment/94b5b8d0_7a5e3d6d PS2, Line 417: /* Flyswatter and Flyswatter-2 require GPIO bits 0x80 : * and 0x40 to be driven low to enable output buffers */ : pindir = 0xcb; : > IMHO, a good argument not to add a redundant variable. Isn't this […] sg, thx -- 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: 6 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 16:36:39 +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. Hello Felix Singer, Alan Green, build bot (Jenkins), 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 (#8). Change subject: [RFC] [WIP] ft2232_spi: reintroduce generic GPIOL control ...................................................................... [RFC] [WIP] 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). Original commit message (will be reworked/corrected later): Adds a new arg "gpiol" to allow the four FT2232 GPIOL pins to be set to any combination of high, low or high-impedance. The existing arg "csgpiol", is intended to function as an additional "cs" signal, allowing pins to be set high but not low. This patch preserves the csgpiol arg for backward compatibility. In the event that both arguments are specified, gpiol is used. TODOs: - discuss merging `csgpiol` into `gpiol` as 'C' (see RFC comment) - reword commit message - man page for both `csgpiol`, `gpiol` Change-Id: I3989f0f9596c090de52dca67183b1363dae59d3a Signed-off-by: Alan Green <avg(a)google.com> Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M ft2232_spi.c 1 file changed, 76 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/57810/8 -- 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: 8 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-MessageType: newpatchset

1 0

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

23 Sep '21

Attention is currently required from: Felix Singer, Angel Pons, Michael Niewöhner. 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 (#6). 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/6 -- 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: 6 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-MessageType: newpatchset

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/074ffefe_36bac161 PS3, Line 297: , > Oh yes, of course it doesn't make sense to provide wrong data at all, so I broke the meaninig. […] Done -- 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 16:27:04 +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]: ft2232_spi: prevent use of reserved pins on some programmers
by Michael Niewöhner (Code Review) 23 Sep '21

23 Sep '21

Attention is currently required from: Felix Singer, Angel Pons, Michael Niewöhner. 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 (#5). 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/5 -- 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: 5 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-MessageType: newpatchset

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: > Hmmm, actually, it took me too long to understand what you mean ;) […] Ack, let's go for the gpiolX=y variant -- 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 16:20:51 +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]: acpi_ec: Implement basic ACPI embedded controller API
by Michał Żygowski (Code Review) 23 Sep '21

23 Sep '21

Attention is currently required from: Angel Pons, Michael Niewöhner, Sergii Dmytruk. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55714 ) Change subject: acpi_ec: Implement basic ACPI embedded controller API ...................................................................... Patch Set 9: (1 comment) Patchset: PS9: > Look good enough to me now. > > However, some remarks: > > I'm really not a fan of avoidable timeouts. I don't mean the code handling > them, but the fact that we risk to run into a timeout. When, for instance, > for one command waiting 100ms is enough but for another we'd want to wait > 300ms, what harm would be done if we always wait 500ms? Of course there > are limits; we don't want a user to become impatient. > True, there is no harm. We may poll in 10us intervals and do 100k checks or so (which gives 1 second). In a good scenario, the EC should be responsive and we shouldn't hit any timeout. > This is why I came up with two classes of timeouts: 1. those we just check > to ensure that a program terminates gracefully; 2. those that we actually > handle and know how to recover and see a chance to continue. In the former > case we usually want to avoid the timeout at all costs but bail out if we > hit it just once. Thus, the timeout can and should be very long. 1, 2, 3 > full seconds, it wouldn't hurt. For the second class of timeouts, we might > hit it more than once, we should find a trade-off between functionality > and responsiveness. > Here we are talking about timeout type 1. If anything times out, then there is probably no reason to continue because there is high chance everything what is going next will fail. > Looking at other implementations I've notice that even coreboot's are > slightly more complex. Those wait for 10us between reads of the status > register. This brings the timeout numbers closer to realtime. But it > might also matter for the EC. Imagine, for instance, an implementation > where every status read triggers an interrupt in the controller. In a > polling loop like the one implemented here, the controller might get > stalled answering those interrupts, leaving less processing time to do > the work we are waiting for. Totally understandable. I might add the 10us intervals. -- To view, visit https://review.coreboot.org/c/flashrom/+/55714 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie3bae8d81c80ae2f286b619e974869e3f2f4545d Gerrit-Change-Number: 55714 Gerrit-PatchSet: 9 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 23 Sep 2021 13:05:50 +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