Nico Huber submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
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@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55695
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
---
M ft2232_spi.c
1 file changed, 21 insertions(+), 9 deletions(-)

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 change 55695. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2b84ede01759c80f69d5ad17e43783d09ecd1107
Gerrit-Change-Number: 55695
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Alan Green <avg@google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus@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: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@mailbox.org>
Gerrit-MessageType: merged