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

login
register
about
Submitter Alex Badea
Date 2010-10-16 12:20:55
Message ID <1287231657-16981-4-git-send-email-vamposdecampos@gmail.com>
Download mbox | patch
Permalink /patch/2121/
State Accepted
Commit r1229
Headers show

Comments

Alex Badea - 2010-10-16 12:20:55
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 <vamposdecampos@gmail.com>
---
 ft2232_spi.c |   37 ++++++++++++++++++++++---------------
 1 files changed, 22 insertions(+), 15 deletions(-)
Carl-Daniel Hailfinger - 2010-11-10 03:20:02
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 <vamposdecampos@gmail.com>
>   

Thanks for your patch.
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
and committed in r1229.

Regards,
Carl-Daniel
Carl-Daniel Hailfinger - 2010-11-12 11:52:14
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 <vamposdecampos@gmail.com>
>>   
>>     
>
> Thanks for your patch.
> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
> 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
Joerg Mayer - 2010-11-12 12:44:48
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
Carl-Daniel Hailfinger - 2010-11-13 04:33:56
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
Joerg Mayer - 2010-11-13 07:21:45
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
Alex Badea - 2010-11-18 08:03:03
On Sat, Nov 13, 2010 at 9:21 AM, Joerg Mayer <jmayer@loplof.de> 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

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");