Re: [flashrom] [PATCH] Added support to control the GPIOs in the ft222_spi driver from the command line

[Stefan Tauner]

On Fri, 20 Feb 2015 18:21:09 -0600
Reggie McMurtrey  wrote:
> This is my first time submitting a patch to a project, so let me know
> if I have done something wrong here.
>
> I commonly use the C232HM cable from FTDI to program various parts of
> boards I design.  This one cable allows me to program SPI, I2C, and
> JTAG parts.  Simply adding some muxes to the design I have a single
> header to attach this cable to.  The patch below allows control of the
> muxes from the GPIOL3-GPIOL0 signals on the cable and disable write
> protection.  I figured other people may like this capability if they
> are using a similar breakout cable of have designed the ft2232 chip
> into their design.
>
> Signed-off-by: Reggie McMurtrey
>
> @@ -262,6 +269,18 @@ int ft2232_spi_init(void)
>    }
>    free(arg);
> arg = extract_programmer_param("gpio_dir");
> if (arg) {
> gpio_dir = (strtoul(arg, 0, 16) & (0xf0 & ~adapter_mask)) | gpio_dir;
> }
> free(arg);
> arg = extract_programmer_param("gpio_value");
> if (arg) {
> gpio_value = (strtoul(arg, 0, 16) & (0xf0 & ~adapter_mask)) | gpio_value;
> }
> free(arg);
> arg = extract_programmer_param("port");
>  if (arg) {
>   switch (toupper((unsigned char)*arg)) {

Hello Reggie,

thanks for your patch! I had no time to review it in detail but we
definitely want some error/wrong input handling for the parameter
parsing above. There are various similar checks throughout the code
base that you may wanna look at... not all are perfect yet though. If
you have any doubts then please refer to the strtol(3) manpage.

A more detailed review will follow when that's fixed.

[PATCH] Added support to control the GPIOs in the ft222_spi driver from the command line

[Reggie McMurtrey]

Thanks for the feedback Stefan, Trying again :)

Signed-off-by: Reggie McMurtrey

---
 flashrom.8.tmpl | 17 ++++++++++++
 ft2232_spi.c    | 81
++++++++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
index 8b5f98c..40d8d84 100644
--- a/flashrom.8.tmpl
+++ b/flashrom.8.tmpl
@@ -681,6 +681,23 @@ parameter with the
 .B "  flashrom -p ft2232_spi:divisor=div"
 .sp
 syntax.
+.sp
+If the ft2232_spi device has been integrated into a custom board, or you
are using a breakout cable
+like the C232HM cable from FTDI you can control the upper bits of the port
( bits 7-4  ) via the
+.sp
+.B "  flashrom -p ft2232_spi:gpio_dir=dir,gpio_value=value"
+.sp
+syntax where
+.B dir
+is a mask defining the directions of the port's upper bits.
+.B value
+is the value of the bits for the port. Only bits defined as outputs with
the
+.B dir
+argument are affected. For example
+.B gpio_dir=0xf0,gpio_value=0x80
+would set the four upper bits of the port as outputs with bit 7 as high,
and bits 6 through 4 as
+low. This could be useful for controlling write protects, muxing options,
or other in
+circuit programming related features.
 .SS
 .BR "serprog " programmer
 A mandatory parameter specifies either a serial
diff --git a/ft2232_spi.c b/ft2232_spi.c
index 4e23797..75cb772 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -93,6 +93,9 @@ const struct dev_entry devs_ft2232spi[] = {
  */
 static uint8_t cs_bits = 0x08;
 static uint8_t pindir = 0x0b;
+static uint8_t gpio_dir = 0x00;
+static uint8_t gpio_value = 0x00;
+static uint8_t adapter_mask = 0x00;
 static struct ftdi_context ftdic_context;

static const char *get_ft2232_devicename(int ft2232_vid, int ft2232_type)
@@ -204,10 +207,11 @@ int ft2232_spi_init(void)
  ft2232_type = AMONTEC_JTAGKEY_PID;
  channel_count = 2;
  /* JTAGkey(2) needs to enable its output via Bit4 / GPIOL0
- *  value: 0x18  OE=high, CS=high, DI=low, DO=low, SK=low
- *    dir: 0x1b  OE=output, CS=output, DI=input, DO=output, SK=output */
- cs_bits = 0x18;
- pindir = 0x1b;
+ *  value: 0x10  OE=high, Reserved (CS), Reserved (DI), Reserved (DO),
Reserved (SK)
+ *    dir: 0x10  OE=output, Reserved (CS), Reserved (DI), Reserved (DO),
Reserved (SK) */
+ gpio_dir = 0x10;
+ gpio_value = 0x10;
+ adapter_mask = 0x10;
  } else if (!strcasecmp(arg, "picotap")) {
  ft2232_vid = GOEPEL_VID;
  ft2232_type = GOEPEL_PICOTAP_PID;
@@ -224,8 +228,9 @@ int ft2232_spi_init(void)
  /* In its default configuration it is a jtagkey clone */
  ft2232_type = FTDI_FT2232H_PID;
  channel_count = 2;
- cs_bits = 0x18;
- pindir = 0x1b;
+ gpio_dir = 0x10;
+ gpio_value = 0x10;
+ adapter_mask = 0x10;
  } else if (!strcasecmp(arg, "openmoko")) {
  ft2232_vid = FIC_VID;
  ft2232_type = OPENMOKO_DBGBOARD_PID;
@@ -235,10 +240,11 @@ int ft2232_spi_init(void)
  ft2232_type = OLIMEX_ARM_OCD_PID;
  channel_count = 2;
  /* arm-usb-ocd(-h) has an output buffer that needs to be enabled by
pulling ADBUS4 low.
- *  value: 0x08  #OE=low, CS=high, DI=low, DO=low, SK=low
- *    dir: 0x1b  #OE=output, CS=output, DI=input, DO=output, SK=output */
- cs_bits = 0x08;
- pindir = 0x1b;
+ *  value: 0x00  #OE=low, Reserved (CS), Reserved (DI), Reserved (DO),
Reserved (SK)
+ *    dir: 0x10  #OE=output, Reserved (CS), Reserved (DI), Reserved (DO),
Reserved (SK) */
+ gpio_dir = 0x10;
+ gpio_value = 0x00;
+ adapter_mask = 0x10;
  } else if (!strcasecmp(arg, "arm-usb-tiny")) {
  ft2232_vid = OLIMEX_VID;
  ft2232_type = OLIMEX_ARM_TINY_PID;
@@ -248,8 +254,9 @@ int ft2232_spi_init(void)
  ft2232_type = OLIMEX_ARM_OCD_H_PID;
  channel_count = 2;
  /* See arm-usb-ocd */
- cs_bits = 0x08;
- pindir = 0x1b;
+ gpio_dir = 0x10;
+ gpio_value = 0x00;
+ adapter_mask = 0x10;
  } else if (!strcasecmp(arg, "arm-usb-tiny-h")) {
  ft2232_vid = OLIMEX_VID;
  ft2232_type = OLIMEX_ARM_TINY_H_PID;
@@ -262,6 +269,38 @@ int ft2232_spi_init(void)
  }
  free(arg);

+ arg = extract_programmer_param("gpio_dir");
+ if (arg && strlen(arg)) {
+ unsigned int temp = 0;
+ char *endptr;
+ temp = strtoul(arg, &endptr, 16);
+ if (*endptr || temp > 0xFF) {
+ msg_perr("Error: Invalid GPIO direction mask "%s".\n"
+ "Valid values are between 0x0 and 0xff (the lower four bits are
ignored).\n", arg);
+ free(arg);
+ return -2;
+ } else {
+ gpio_dir = ((uint32_t) temp & (0xf0 & ~adapter_mask)) | gpio_dir;
+ }
+ }
+ free(arg);
+
+ arg = extract_programmer_param("gpio_value");
+ if (arg && strlen(arg)) {
+ unsigned int temp = 0;
+ char *endptr;
+ temp = strtoul(arg, &endptr, 16);
+ if (*endptr || temp > 0xFF) {
+ msg_perr("Error: Invalid GPIO value mask "%s".\n"
+ "Valid values are between 0x0 and 0xff (the lower four bits are
ignored).\n", arg);
+ free(arg);
+ return -2;
+ } else {
+ gpio_value = ((uint32_t) temp & (0xf0 & ~adapter_mask)) | gpio_value;
+ }
+ }
+ free(arg);
+
  arg = extract_programmer_param("port");
  if (arg) {
  switch (toupper((unsigned char)*arg)) {
@@ -391,9 +430,11 @@ int ft2232_spi_init(void)
  }

msg_pdbg("Set data bits\n");
+ msg_pspew("Value: 0x%02x\n", cs_bits | gpio_value);
+ msg_pspew("Dir: 0x%02x\n", pindir | gpio_dir);
  buf[0] = SET_BITS_LOW;
- buf[1] = cs_bits;
- buf[2] = pindir;
+ buf[1] = cs_bits | gpio_value;
+ buf[2] = pindir | gpio_dir;
  if (send_buf(ftdic, buf, 3)) {
  ret = -8;
  goto ftdi_err;
@@ -446,9 +487,11 @@ static int ft2232_spi_send_command(struct flashctx
*flash,
  * operations.
  */
  msg_pspew("Assert CS#\n");
+ msg_pspew("Value: 0x%02x\n", ~cs_bits & gpio_value);
+ msg_pspew("Dir: 0x%02x\n", pindir | gpio_dir);
  buf[i++] = SET_BITS_LOW;
- buf[i++] = 0 & ~cs_bits; /* assertive */
- buf[i++] = pindir;
+ buf[i++] = ~cs_bits & gpio_value; /* assertive */
+ buf[i++] = pindir | gpio_dir;

if (writecnt) {
  buf[i++] = MPSSE_DO_WRITE | MPSSE_WRITE_NEG;
@@ -487,9 +530,11 @@ static int ft2232_spi_send_command(struct flashctx
*flash,
  }

msg_pspew("De-assert CS#\n");
+ msg_pspew("Value: 0x%02x\n", cs_bits | gpio_value);
+ msg_pspew("Dir: 0x%02x\n", pindir | gpio_dir);
  buf[i++] = SET_BITS_LOW;
- buf[i++] = cs_bits;
- buf[i++] = pindir;
+ buf[i++] = cs_bits | gpio_value;
+ buf[i++] = pindir | gpio_dir;
  ret = send_buf(ftdic, buf, i);
  failed |= ret;
  if (ret)