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

Antony Pavlov antonynpavlov at gmail.com
Sat Feb 21 19:53:46 CET 2015


On Sat, 21 Feb 2015 09:34:23 -0600
Reggie McMurtrey <reggie.mcmurtrey at 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 at 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 at alumni.tuwien.ac.at> wrote:
> 
> > On Fri, 20 Feb 2015 18:21:09 -0600
> > Reggie McMurtrey <reggie.mcmurtrey at 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 at 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
> >


-- 
-- 
Best regards,
  Antony Pavlov




More information about the flashrom mailing list