Attention is currently required from: Michał Żygowski, Angel Pons.
Michael Niewöhner 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 4:
(1 comment)
File acpi_ec.c:
https://review.coreboot.org/c/flashrom/+/55714/comment/851fde2a_a4647fe6
PS4, Line 23: ontrol_port
cmd_port
--
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: 4
Gerrit-Owner: Michał Żygowski <michal.zygowski(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: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 25 Jun 2021 13:12:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/55695 )
Change subject: ft2232_spi: Revise comments about output pin states
......................................................................
ft2232_spi: Revise comments about output pin states
The meaning of the variables is easy to misunderstand as some
states are merely implicit: All output pins that are not set
in the `cs_bits` mask will be constantly driven low. This may
be sheer coincidence as all programmers that need additional
pins driven use active-low signals to enable buffers.
While other pins stay low, *all* pins set in the `cs_bits`
mask are supposed to be toggled during SPI transactions.
Also drop some irritating dead code and try to explain things
in a comment.
Change-Id: I2b84ede01759c80f69d5ad17e43783d09ecd1107
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55695
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
---
M ft2232_spi.c
1 file changed, 21 insertions(+), 9 deletions(-)
Approvals:
build bot (Jenkins): Verified
Angel Pons: Looks good to me, approved
diff --git a/ft2232_spi.c b/ft2232_spi.c
index eb98933..0d0c32c 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -86,15 +86,27 @@
#define BITMODE_BITBANG_NORMAL 1
#define BITMODE_BITBANG_SPI 2
-/* 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.
+/*
+ * 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. `cs_bits` pins default
+ * to high and will be toggled during SPI transactions. All other
+ * output pins will be kept low all the time. On exit, all pins
+ * will be reconfigured as inputs.
*
- * The default values (set below in ft2232_spi_init) are used for most devices:
- * value: 0x08 CS=high, DI=low, DO=low, SK=low
+ * The pin offsets are as follows:
+ * 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 default values (set below in ft2232_spi_init) are used for
+ * most devices:
+ * value: 0x08 CS=high, DI=low, DO=low, SK=low
* dir: 0x0b CS=output, DI=input, DO=output, SK=output
*/
struct ft2232_data {
@@ -219,7 +231,7 @@
msg_pspew("Assert CS#\n");
buf[i++] = SET_BITS_LOW;
- buf[i++] = 0 & ~spi_data->cs_bits; /* assertive */
+ buf[i++] = 0; /* assert CS# pins, all other output pins stay low */
buf[i++] = spi_data->pindir;
/* WREN, OP(PROGRAM, ERASE), ADDR, DATA */
--
To view, visit https://review.coreboot.org/c/flashrom/+/55695
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2b84ede01759c80f69d5ad17e43783d09ecd1107
Gerrit-Change-Number: 55695
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Alan Green <avg(a)google.com>
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: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 )
Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS4:
> Forcing to run the programmer code on an unknown board would be new
to flashrom. I don't see why we would suddenly need that.
I haven't checked all flash drivers in flashrom but I don't remember any DMI checks or anything like that in other ec or superio drivers. Why do we need it now?
--
To view, visit https://review.coreboot.org/c/flashrom/+/55715
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031
Gerrit-Change-Number: 55715
Gerrit-PatchSet: 9
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 25 Jun 2021 12:52:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/55694 )
Change subject: Revert "ft2232_spi: Enhance csgpiol parameter for FT2232"
......................................................................
Revert "ft2232_spi: Enhance csgpiol parameter for FT2232"
This reverts commit ba6575de82f091b97ea0f2efcf2f79ef3739d64f.
Technically, the only thing that is wrong here is the lack of docu-
mentation (manpage update). However, as this change was succeeded by
a regressing fixup patch, it seems likely that the meaning of the
`csgpiol` parameter was just misunderstood and these changes were
not what the author intended.
Change-Id: I460237b9d275b1cd1d8a069f852d17dea393b14e
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55694
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
---
M ft2232_spi.c
1 file changed, 14 insertions(+), 26 deletions(-)
Approvals:
build bot (Jenkins): Verified
Angel Pons: Looks good to me, approved
diff --git a/ft2232_spi.c b/ft2232_spi.c
index 44975db..eb98933 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -88,17 +88,10 @@
/* 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:
- * 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.
+ * SCK is bit 0.
+ * DO is bit 1.
+ * DI is bit 2.
+ * CS is bit 3.
*
* The default values (set below in ft2232_spi_init) are used for most devices:
* value: 0x08 CS=high, DI=low, DO=low, SK=low
@@ -459,24 +452,19 @@
}
free(arg);
- /* Allows setting multiple GPIOL states, for example: csgpiol=012 */
arg = extract_programmer_param("csgpiol");
if (arg) {
- 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;
- }
+ 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 pin = temp + 4;
+ cs_bits |= 1 << pin;
+ pindir |= 1 << pin;
}
free(arg);
--
To view, visit https://review.coreboot.org/c/flashrom/+/55694
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I460237b9d275b1cd1d8a069f852d17dea393b14e
Gerrit-Change-Number: 55694
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Alan Green <avg(a)google.com>
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: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged