[PATCH 0/5] ft2232_spi: OpenMoko DebugBoard support

[Alex Badea]

Hi,

This patch series adds support for the OpenMoko DebugBoard V2+, which is
based on the FT2232D.  A few fixes were required to make it work.

Any feedback is greatly appreciated, this is my first contribution to
flashrom.

Thanks.

Alex Badea (5):
  ft2232_spi: don't hardcode USB VID
  ft2232_spi: add support for the OpenMoko DebugBoard V2+
  ft2232_spi: allow CLOCK_X5 to be set at runtime
  ft2232_spi: retry short reads
  ft2232_spi: flag OpenMoko DebugBoard as tested

ft2232_spi.c |   67 ++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 44 insertions(+), 23 deletions(-)

[PATCH 1/5] ft2232_spi: don't hardcode USB VID

[Alex Badea]

ftdi_usb_open() is called with the constant FTDI_VID vendor ID.
Fix it by using the programmer-type-dependent ft2232_vid variable,
to allow programmers with other vendor IDs.

Signed-off-by: Alex Badea 
---
 ft2232_spi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/ft2232_spi.c b/ft2232_spi.c
index a2ea2a8..37fa231 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -173,7 +173,7 @@ int ft2232_spi_init(void)
    	return EXIT_FAILURE; // TODO
    }

-	f = ftdi_usb_open(ftdic, FTDI_VID, ft2232_type);
+	f = ftdi_usb_open(ftdic, ft2232_vid, ft2232_type);

if (f < 0 && f != -5) {
    	msg_perr("Unable to open FTDI device: %d (%s)\n", f,

[PATCH 1/5] ft2232_spi: don't hardcode USB VID

[Carl-Daniel Hailfinger]

On 16.10.2010 14:20, Alex Badea wrote:
> ftdi_usb_open() is called with the constant FTDI_VID vendor ID.
> Fix it by using the programmer-type-dependent ft2232_vid variable,
> to allow programmers with other vendor IDs.
>
> Signed-off-by: Alex Badea

Thanks for your patch.
Acked-by: Carl-Daniel Hailfinger 
and committed in r1230.

Regards,
Carl-Daniel

[PATCH 2/5] ft2232_spi: add support for the OpenMoko DebugBoard V2+

[Alex Badea]

Add support for the OpenMoko Neo1973/Neo FreeRunner debug board
version 2 or 3 (vid:pid 1457:5118).  The new type is called "openmoko".

Information about the debug board can be found at
http://wiki.openmoko.org/wiki/Debug_Board_v3

Signed-off-by: Alex Badea 
---
 ft2232_spi.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/ft2232_spi.c b/ft2232_spi.c
index 37fa231..f8ba518 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -35,10 +35,16 @@
 #define FTDI_FT4232H_PID	0x6011
 #define AMONTEC_JTAGKEY_PID	0xCFF8

+#define FIC_VID			0x1457
+#define OPENMOKO_DBGBOARD_PID	0x5118
+
 const struct usbdev_status devs_ft2232spi[] = {
    {FTDI_VID, FTDI_FT2232H_PID, OK, "FTDI", "FT2232H"},
    {FTDI_VID, FTDI_FT4232H_PID, OK, "FTDI", "FT4232H"},
    {FTDI_VID, AMONTEC_JTAGKEY_PID, OK, "Amontec", "JTAGkey"},
+	{FIC_VID, OPENMOKO_DBGBOARD_PID, NT,
+		"First International Computer, Inc.",
+		"OpenMoko Neo1973 Debug board (V2+)"},
    {},
 };

@@ -138,8 +144,11 @@ int ft2232_spi_init(void)
    		ft2232_interface = INTERFACE_A;
    		cs_bits = 0x18;
    		pindir = 0x1b;
-		}
-		else {
+		} else if (!strcasecmp(arg, "openmoko")) {
+			ft2232_vid = FIC_VID;
+			ft2232_type = OPENMOKO_DBGBOARD_PID;
+			ft2232_interface = INTERFACE_A;
+		} else {
    		msg_perr("Error: Invalid device type specified.\n");
    		free(arg);
    		return 1;

[PATCH 2/5] ft2232_spi: add support for the OpenMoko DebugBoard V2+

[Carl-Daniel Hailfinger]

On 16.10.2010 14:20, Alex Badea wrote:
> Add support for the OpenMoko Neo1973/Neo FreeRunner debug board
> version 2 or 3 (vid:pid 1457:5118).  The new type is called "openmoko".
>
> Information about the debug board can be found at
> http://wiki.openmoko.org/wiki/Debug_Board_v3
>
> Signed-off-by: Alex Badea

Acked-by: Carl-Daniel Hailfinger 
and combined with patch 5/5, committed in r1231.

Regards,
Carl-Daniel

[PATCH 3/5] ft2232_spi: allow CLOCK_X5 to be set at runtime

[Alex Badea]

Check at init-time whether the chip is a type 'H' (FT2232H or FT4232H).
If not, omit the disable-divide-by-5 (0x8a) command which can confuse
older chips.

Signed-off-by: Alex Badea 
---
 ft2232_spi.c |   37 ++++++++++++++++++++++---------------
 1 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/ft2232_spi.c b/ft2232_spi.c
index f8ba518..84a70de 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -52,11 +52,11 @@ const struct usbdev_status devs_ft2232spi[] = {
  * The 'H' chips can run internally at either 12MHz or 60MHz.
  * The non-H chips can only run at 12MHz.
  */
-#define CLOCK_5X 1
+static uint8_t clock_5x = 1;

/*
  * In either case, the divisor is a simple integer clock divider.
- * If CLOCK_5X is set, this divisor divides 30MHz, else it divides 6MHz.
+ * If clock_5x is set, this divisor divides 30MHz, else it divides 6MHz.
  */
 #define DIVIDE_BY 3  /* e.g. '3' will give either 10MHz or 2MHz SPI clock. */

@@ -132,6 +132,7 @@ int ft2232_spi_init(void)
    int ft2232_type = FTDI_FT4232H_PID;
    enum ftdi_interface ft2232_interface = INTERFACE_B;
    char *arg;
+	double mpsse_clk;

arg = extract_programmer_param("type");
    if (arg) {
@@ -190,6 +191,12 @@ int ft2232_spi_init(void)
    	exit(-1); // TODO
    }

+	if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H) {
+		msg_pdbg("FTDI chip type %d is not high-speed\n",
+			ftdic->type);
+		clock_5x = 0;
+	}
+
    if (ftdi_set_interface(ftdic, ft2232_interface) < 0) {
    	msg_perr("Unable to select interface: %s\n",
    			ftdic->error_str);
@@ -211,18 +218,16 @@ int ft2232_spi_init(void)
    	msg_perr("Unable to set bitmode to SPI\n");
    }

-#if CLOCK_5X
-	msg_pdbg("Disable divide-by-5 front stage\n");
-	buf[0] = 0x8a;		/* Disable divide-by-5. */
-	if (send_buf(ftdic, buf, 1))
-		return -1;
-#define MPSSE_CLK 60.0
-
-#else
-
-#define MPSSE_CLK 12.0
+	if (clock_5x) {
+		msg_pdbg("Disable divide-by-5 front stage\n");
+		buf[0] = 0x8a;		/* Disable divide-by-5. */
+		if (send_buf(ftdic, buf, 1))
+			return -1;
+		mpsse_clk = 60.0;
+	} else {
+		mpsse_clk = 12.0;
+	}

-#endif
    msg_pdbg("Set clock divisor\n");
    buf[0] = 0x86;		/* command "set divisor" */
    /* valueL/valueH are (desired_divisor - 1) */
@@ -231,8 +236,10 @@ int ft2232_spi_init(void)
    if (send_buf(ftdic, buf, 3))
    	return -1;

-	msg_pdbg("SPI clock is %fMHz\n",
-		 (double)(MPSSE_CLK / (((DIVIDE_BY - 1) + 1) * 2)));
+	msg_pdbg("MPSSE clock: %f MHz divisor: %d "
+		 "SPI clock: %f MHz\n",
+		 mpsse_clk, DIVIDE_BY,
+		 (double)(mpsse_clk / (((DIVIDE_BY - 1) + 1) * 2)));

/* Disconnect TDI/DO to TDO/DI for loopback. */
    msg_pdbg("No loopback of TDI/DO TDO/DI\n");

[PATCH 3/5] ft2232_spi: allow CLOCK_X5 to be set at runtime

[Carl-Daniel Hailfinger]

On 16.10.2010 14:20, Alex Badea wrote:
> Check at init-time whether the chip is a type 'H' (FT2232H or FT4232H).
> If not, omit the disable-divide-by-5 (0x8a) command which can confuse
> older chips.
>
> Signed-off-by: Alex Badea

Thanks for your patch.
Acked-by: Carl-Daniel Hailfinger 
and committed in r1229.

Regards,
Carl-Daniel

[PATCH 3/5] ft2232_spi: allow CLOCK_X5 to be set at runtime

[Carl-Daniel Hailfinger]

On 10.11.2010 04:20, Carl-Daniel Hailfinger wrote:
> On 16.10.2010 14:20, Alex Badea wrote:
> > Check at init-time whether the chip is a type 'H' (FT2232H or FT4232H).
> > If not, omit the disable-divide-by-5 (0x8a) command which can confuse
> > older chips.
> >
> > Signed-off-by: Alex Badea
>
> Thanks for your patch.
> Acked-by: Carl-Daniel Hailfinger 
> and committed in r1229.

The following hunk in flashrom caused compilation problems on libftdi
<0.16, e.g. the libftdi in Debian stable:
> @@ -186,6 +187,12 @@
>    	exit(-1); // TODO
>    }
> if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H) {
> msg_pdbg("FTDI chip type %d is not high-speed\n",
> 	ftdic->type);
> clock_5x = 0;
> }
> if (ftdi_set_interface(ftdic, ft2232_interface) < 0) {
>   msg_perr("Unable to select interface: %s\n",
>   		ftdic->error_str);

TYPE_2232H and TYPE_4232H are not defined before libftdi 0.16.

Do you have any idea how to work around this except requiring a newer
libftdi?

Regards,
Carl-Daniel

[PATCH 3/5] ft2232_spi: allow CLOCK_X5 to be set at runtime

[Joerg Mayer]

On Fri, Nov 12, 2010 at 12:52:14PM +0100, Carl-Daniel Hailfinger wrote:

#if (defined TYPE_2232H and defined TYPE_4232H)
> > if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H) {
> > msg_pdbg("FTDI chip type %d is not high-speed\n",
> > 	ftdic->type);
> > clock_5x = 0;
> > }

#endif
> TYPE_2232H and TYPE_4232H are not defined before libftdi 0.16.
>
> Do you have any idea how to work around this except requiring a newer
> libftdi?

You'll just need to figure out the correct syntax :-)

Ciao
 Joerg

[PATCH 3/5] ft2232_spi: allow CLOCK_X5 to be set at runtime

[Carl-Daniel Hailfinger]

On 12.11.2010 13:44, Joerg Mayer wrote:
> On Fri, Nov 12, 2010 at 12:52:14PM +0100, Carl-Daniel Hailfinger wrote:
>
> #if (defined TYPE_2232H and defined TYPE_4232H)
> > > if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H) {
> > > msg_pdbg("FTDI chip type %d is not high-speed\n",
> > > 	ftdic->type);
> > > clock_5x = 0;
> > > }
>
> #endif

That doesn't work because TYPE_2232H is an enum.
> > TYPE_2232H and TYPE_4232H are not defined before libftdi 0.16.
> >
> > Do you have any idea how to work around this except requiring a newer
> > libftdi?
>
> You'll just need to figure out the correct syntax :-)

Any other ideas?

Regards,
Carl-Daniel

[PATCH 3/5] ft2232_spi: allow CLOCK_X5 to be set at runtime

[Joerg Mayer]

On Sat, Nov 13, 2010 at 05:33:56AM +0100, Carl-Daniel Hailfinger wrote:
> On 12.11.2010 13:44, Joerg Mayer wrote:
> > On Fri, Nov 12, 2010 at 12:52:14PM +0100, Carl-Daniel Hailfinger wrote:
> >
> > #if (defined TYPE_2232H and defined TYPE_4232H)
> > > > if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H) {
> > > > msg_pdbg("FTDI chip type %d is not high-speed\n",
> > > > 	ftdic->type);
> > > > clock_5x = 0;
> > > > }
> >
> > #endif
>
> That doesn't work because TYPE_2232H is an enum.

...
> Any other ideas?

Just the normal stuff that cmake and configure do: Add a check outside
the file, then #define a preprocessor variable, then guard the code
with that variable.
If there is real interest, I'd be willing to create the necessary cmake
stuff for review - it may take up to two weeks because of travel and work
for me to provide a first patch.

Ciao
   Joerg

[PATCH 3/5] ft2232_spi: allow CLOCK_X5 to be set at runtime

[Alex Badea]

On Sat, Nov 13, 2010 at 9:21 AM, Joerg Mayer  wrote:
> Just the normal stuff that cmake and configure do: Add a check outside
> the file, then #define a preprocessor variable, then guard the code
> with that variable.
> If there is real interest, I'd be willing to create the necessary cmake
> stuff for review - it may take up to two weeks because of travel and work
> for me to provide a first patch.

I proposed a patch with your suggestion:

http://patchwork.coreboot.org/patch/2320/

Thanks,
Alex

[PATCH 4/5] ft2232_spi: retry short reads

[Alex Badea]

It's possible that ftdi_read_data() returns less data
than requested.  Catch this case and retry reading the rest
of the buffer.

Signed-off-by: Alex Badea 
---
 ft2232_spi.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/ft2232_spi.c b/ft2232_spi.c
index 84a70de..0f9bc79 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -114,11 +114,16 @@ static int get_buf(struct ftdi_context *ftdic, const unsigned char *buf,
    	   int size)
 {
    int r;
-	r = ftdi_read_data(ftdic, (unsigned char *) buf, size);
-	if (r < 0) {
-		msg_perr("ftdi_read_data: %d, %s\n", r,
-				ftdi_get_error_string(ftdic));
-		return 1;
+
+	while (size > 0) {
+		r = ftdi_read_data(ftdic, (unsigned char *) buf, size);
+		if (r < 0) {
+			msg_perr("ftdi_read_data: %d, %s\n", r,
+					ftdi_get_error_string(ftdic));
+			return 1;
+		}
+		buf += r;
+		size -= r;
    }
    return 0;
 }

[PATCH 4/5] ft2232_spi: retry short reads

[Carl-Daniel Hailfinger]

On 16.10.2010 14:20, Alex Badea wrote:
> It's possible that ftdi_read_data() returns less data
> than requested.  Catch this case and retry reading the rest
> of the buffer.
>
> Signed-off-by: Alex Badea

Thanks for spotting and fixing this bug!
Acked-by: Carl-Daniel Hailfinger 
and committed in r1228.

Regards,
Carl-Daniel

[PATCH 5/5] ft2232_spi: flag OpenMoko DebugBoard as tested

[Alex Badea]

With the clock_5x and short read fixes, the board can successfully
interact with an M25P16.  Change status from NT to OK.

Signed-off-by: Alex Badea 
---
 ft2232_spi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/ft2232_spi.c b/ft2232_spi.c
index 0f9bc79..7373624 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -42,7 +42,7 @@ const struct usbdev_status devs_ft2232spi[] = {
    {FTDI_VID, FTDI_FT2232H_PID, OK, "FTDI", "FT2232H"},
    {FTDI_VID, FTDI_FT4232H_PID, OK, "FTDI", "FT4232H"},
    {FTDI_VID, AMONTEC_JTAGKEY_PID, OK, "Amontec", "JTAGkey"},
-	{FIC_VID, OPENMOKO_DBGBOARD_PID, NT,
+	{FIC_VID, OPENMOKO_DBGBOARD_PID, OK,
    	"First International Computer, Inc.",
    	"OpenMoko Neo1973 Debug board (V2+)"},
    {},

[PATCH 5/5] ft2232_spi: flag OpenMoko DebugBoard as tested

[Carl-Daniel Hailfinger]

On 16.10.2010 14:20, Alex Badea wrote:
> With the clock_5x and short read fixes, the board can successfully
> interact with an M25P16.  Change status from NT to OK.
>
> Signed-off-by: Alex Badea

Acked-by: Carl-Daniel Hailfinger 
and combined with patch 2/5, committed in r1231.

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

Hi Alex,

On 16.10.2010 14:20, Alex Badea wrote:
> This patch series adds support for the OpenMoko DebugBoard V2+, which is
> based on the FT2232D.  A few fixes were required to make it work.

The patches look good, I plan to merge them once the transition to
partial writes is over.

Regards,
Carl-Daniel