On Sat, 21 Feb 2015 09:34:23 -0600 Reggie McMurtrey reggie.mcmurtrey@gmail.com wrote:
Thanks for the feedback Stefan, Trying again :)
You have submitted version 2 of your patch. So please note this fact in the Subject. E.g. use "[PATCH v2]" instead of "[PATCH]" in the message's subject. You can use git-format-patch '--subject-prefix' option or just edit subject by hand.
Signed-off-by: Reggie McMurtrey reggie.mcmurtrey@gmail.com
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)
-- 1.9.1
On Sat, Feb 21, 2015 at 4:20 AM, Stefan Tauner < stefan.tauner@alumni.tuwien.ac.at> wrote:
On Fri, 20 Feb 2015 18:21:09 -0600 Reggie McMurtrey reggie.mcmurtrey@gmail.com 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 reggie.mcmurtrey@gmail.com
@@ -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.
Kind regards/Mit freundlichen Grüßen, Stefan Tauner