Change in flashrom[master]: ft2232_spi: Enhance csgpiol parameter for FT2232
Samir Ibradžić has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... ft2232_spi: Enhance csgpiol parameter for FT2232 This allows multiple 'csgpiol' bits to be set to active state at the same time. Previously, only one GPIOL could be activated. I have an use-case such that FT4232H is wired to two different SPI chips, and in order to select one of them two GPIOLs have to be set. Now, one can enable any particular GPIOL, for example: csgpiol=01 would activate GPIOL0 and GPIOL1 at the same time. The change is backward-compatible with previous csgpiol formatting. Signed-off-by: Samir Ibradzic <sibradzic@gmail.com> Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b --- M ft2232_spi.c 1 file changed, 26 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/38705/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index 5bdc2a7..947050f 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -86,10 +86,17 @@ /* The variables cs_bits and pindir store the values for the "set data bits low byte" MPSSE command that * sets the initial state and the direction of the I/O pins. The pin offsets are as follows: - * SCK is bit 0. - * DO is bit 1. - * DI is bit 2. - * CS is bit 3. + * TCK/SK is bit 0. + * TDI/DO is bit 1. + * TDO/DI is bit 2. + * TMS/CS is bit 3. + * GPIOL0 is bit 4. + * GPIOL1 is bit 5. + * GPIOL2 is bit 6. + * GPIOL3 is bit 7. + * + * The pin signal direction bit offsets follow the same order; 0 means that + * pin at the matching bit index is an input, 1 means pin is an output. * * The default values (set below) are used for most devices: * value: 0x08 CS=high, DI=low, DO=low, SK=low @@ -331,19 +338,24 @@ } free(arg); + /* Allows setting multiple GPIOL states, for example: csgpiol=012 */ arg = extract_programmer_param("csgpiol"); if (arg) { - char *endptr; - unsigned int temp = strtoul(arg, &endptr, 10); - if (*endptr || endptr == arg || temp > 3) { - msg_perr("Error: Invalid GPIOL specified: \"%s\".\n" - "Valid values are between 0 and 3.\n", arg); - free(arg); - return -2; + unsigned int ngpios = strlen(arg); + for (unsigned int i=0; i<=ngpios; i++) { + int temp = arg[i] - '0'; + if (ngpios == 0 || (ngpios != i && (temp < 0 || temp > 3))) { + msg_perr("Error: Invalid GPIOLs specified: \"%s\".\n" + "Valid values are numbers between 0 and 3. " + "Multiple GPIOLs can be specified.\n", arg); + free(arg); + return -2; + } else { + unsigned int pin = temp + 4; + cs_bits |= 1 << pin; + pindir |= 1 << pin; + } } - unsigned int pin = temp + 4; - cs_bits |= 1 << pin; - pindir |= 1 << pin; } free(arg); -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 1 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-MessageType: newchange
Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... Patch Set 1: Hi, can someone please review this? -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 1 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Tue, 04 Feb 2020 18:48:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... Patch Set 1: Code-Review+2
Patch Set 1:
Hi, can someone please review this?
Sorry this took so much time for review. Thanks very much for the patch! -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 1 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Wed, 05 Feb 2020 00:50:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... Patch Set 1: Who's attention do we need in order to get this merged? -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 1 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 12 Feb 2020 03:39:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... Patch Set 1: Code-Review-1 (1 comment) Only giving a -1 to draw attention. It's a minor thing, but should be fixed before submitting. https://review.coreboot.org/c/flashrom/+/38705/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/38705/1/ft2232_spi.c@345 PS1, Line 345: i=0; i<=ngpios; There should be spaces around these operators. -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 1 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 12 Feb 2020 21:20:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello Edward O'Callaghan, Angel Pons, David Hendricks, Stefan Reinauer, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/38705 to look at the new patch set (#2). Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... ft2232_spi: Enhance csgpiol parameter for FT2232 This allows multiple 'csgpiol' bits to be set to active state at the same time. Previously, only one GPIOL could be activated. I have an use-case such that FT4232H is wired to two different SPI chips, and in order to select one of them two GPIOLs have to be set. Now, one can enable any particular GPIOL, for example: csgpiol=01 would activate GPIOL0 and GPIOL1 at the same time. The change is backward-compatible with previous csgpiol formatting. Signed-off-by: Samir Ibradzic <sibradzic@gmail.com> Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b --- M ft2232_spi.c 1 file changed, 26 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/38705/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 2 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Thank you! https://review.coreboot.org/c/flashrom/+/38705/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/38705/1/ft2232_spi.c@345 PS1, Line 345: i=0; i<=ngpios;
There should be spaces around these operators. Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 2 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 13 Feb 2020 09:54:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... Patch Set 2: (1 comment) Hi there. Can we have this merged please? https://review.coreboot.org/c/flashrom/+/38705/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/38705/1/ft2232_spi.c@345 PS1, Line 345: i=0; i<=ngpios;
There should be spaces around these operators. Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 2 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 14 Feb 2020 08:54:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Samir Ibradžić has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... Patch Set 2: David, Edward, Stefan, Gentlemen, can we have this merged please? -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 2 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 09 Mar 2020 03:38:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... ft2232_spi: Enhance csgpiol parameter for FT2232 This allows multiple 'csgpiol' bits to be set to active state at the same time. Previously, only one GPIOL could be activated. I have an use-case such that FT4232H is wired to two different SPI chips, and in order to select one of them two GPIOLs have to be set. Now, one can enable any particular GPIOL, for example: csgpiol=01 would activate GPIOL0 and GPIOL1 at the same time. The change is backward-compatible with previous csgpiol formatting. Signed-off-by: Samir Ibradzic <sibradzic@gmail.com> Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Reviewed-on: https://review.coreboot.org/c/flashrom/+/38705 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> --- M ft2232_spi.c 1 file changed, 26 insertions(+), 14 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/ft2232_spi.c b/ft2232_spi.c index 5bdc2a7..3e4dc9e 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -86,10 +86,17 @@ /* The variables cs_bits and pindir store the values for the "set data bits low byte" MPSSE command that * sets the initial state and the direction of the I/O pins. The pin offsets are as follows: - * SCK is bit 0. - * DO is bit 1. - * DI is bit 2. - * CS is bit 3. + * TCK/SK is bit 0. + * TDI/DO is bit 1. + * TDO/DI is bit 2. + * TMS/CS is bit 3. + * GPIOL0 is bit 4. + * GPIOL1 is bit 5. + * GPIOL2 is bit 6. + * GPIOL3 is bit 7. + * + * The pin signal direction bit offsets follow the same order; 0 means that + * pin at the matching bit index is an input, 1 means pin is an output. * * The default values (set below) are used for most devices: * value: 0x08 CS=high, DI=low, DO=low, SK=low @@ -331,19 +338,24 @@ } free(arg); + /* Allows setting multiple GPIOL states, for example: csgpiol=012 */ arg = extract_programmer_param("csgpiol"); if (arg) { - char *endptr; - unsigned int temp = strtoul(arg, &endptr, 10); - if (*endptr || endptr == arg || temp > 3) { - msg_perr("Error: Invalid GPIOL specified: \"%s\".\n" - "Valid values are between 0 and 3.\n", arg); - free(arg); - return -2; + unsigned int ngpios = strlen(arg); + for (unsigned int i = 0; i <= ngpios; i++) { + int temp = arg[i] - '0'; + if (ngpios == 0 || (ngpios != i && (temp < 0 || temp > 3))) { + msg_perr("Error: Invalid GPIOLs specified: \"%s\".\n" + "Valid values are numbers between 0 and 3. " + "Multiple GPIOLs can be specified.\n", arg); + free(arg); + return -2; + } else { + unsigned int pin = temp + 4; + cs_bits |= 1 << pin; + pindir |= 1 << pin; + } } - unsigned int pin = temp + 4; - cs_bits |= 1 << pin; - pindir |= 1 << pin; } free(arg); -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 3 Gerrit-Owner: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic@gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: merged
participants (3)
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Samir Ibradžić (Code Review)