[flashrom] [PATCH] Add support for PicoTAP (by GOEPEL) FT2232 SPI programmer

[Samir Ibradžić]

On 2011年10月15日 05:45, Uwe Hermann wrote:
> On Sat, Oct 15, 2011 at 03:00:45AM +0900, Samir Ibradžić wrote:
>> Hello,
>>
>> I would like to submit a patch, that enables support for another
>> FT2232 device, PicoTAP by GOEPEL electronic GmbH.
>
> Great, thanks a lot!
>
> You forgot the "Signed-off-by" line though, please check
>
> http://flashrom.org/Development_Guidelines
> and thus also
> http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure
>

Oops, sorry, here it is.

> and resend the (updated) patch. Thanks!
>
>
>> Btw, I managed to run PicoTAP in 10MHz, 15MHz and 30Mhz modes (by
>> forcing DIVIDE_BY), against SST25VF016B SPI flash, read/write/erase
>> all worked fine (write seems somewhat slow). For the sake of more
>> testing, is there any way 20Mhz can be set in FT2232?
>
> Hm, dunno, need to check datasheets.
>
>
>> I am also thinking about implementing passing frequency divider as
>> an option, for example:
>>
>> # flashrom -p ft2232_spi:type=picotap,divider=2
>>
>> DIVIDE_BY constant would be used as default. I think this parameter
>> would be useful, but would like to hear more opinions.
>
> Sounds useful, yes. I don't know if there are reasons not to make this a
> user-visible option, I'd wait for more feedback here (this would be
> another patch anyway).
>

Well, ill submit this in separate patch for evaluation anyway.

>
>> Index: ft2232_spi.c
>> ===================================================================
>> --- ft2232_spi.c	(revision 1450)
>> +++ ft2232_spi.c	(working copy)
>> @@ -43,6 +43,9 @@
>>   #define OLIMEX_ARM_OCD_H_PID	0x002B
>>   #define OLIMEX_ARM_TINY_H_PID	0x002A
>>
>> +#define GOEPEL_VID		0x096C
>> +#define PICOTAP_PID		0x1449
>> +
>>   const struct usbdev_status devs_ft2232spi[] = {
>>   	{FTDI_VID, FTDI_FT2232H_PID, OK, "FTDI", "FT2232H"},
>>   	{FTDI_VID, FTDI_FT4232H_PID, OK, "FTDI", "FT4232H"},
>> @@ -53,6 +56,7 @@
>>   	{OLIMEX_VID, OLIMEX_ARM_TINY_PID, OK, "Olimex", "ARM-USB-TINY"},
>>   	{OLIMEX_VID, OLIMEX_ARM_OCD_H_PID, NT, "Olimex", "ARM-USB-OCD-H"},
>>   	{OLIMEX_VID, OLIMEX_ARM_TINY_H_PID, NT, "Olimex", "ARM-USB-TINY-H"},
>> +	{GOEPEL_VID, PICOTAP_PID, OK, "GOEPEL electronic GmbH", "PicoTAP"},
>
> Please sort the entries by vendor ID, then by device ID (we added
> comment for that sorting a few minutes ago in the repository :)
>

Corrected.

> Also use just "GOEPEL" instead of "GOEPEL electronic GmbH" here, please.
> We don't include legalese such as "GmbH", "Inc.", etc. in the vendor names.
>

Corrected.

>
>> @@ -66,7 +70,7 @@
>>    * In either case, the divisor is a simple integer clock divider.
>>    * 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. */
>> +#define DIVIDE_BY 1  /* e.g. '3' will give either 10MHz or 2MHz SPI
>> clock. */
>
> This should probably not be in the initial patch, but rather in another
> DIVIDE_BY related patch (see above).

Yes, this was some testing left-over. Here's cleaned-up patch.

>
>
> Uwe.

Signed-off-by: Samir Ibradžić <sibradzic at gmail.com>
---
$ svn diff
Index: ft2232_spi.c
===================================================================
--- ft2232_spi.c	(revision 1451)
+++ ft2232_spi.c	(working copy)
@@ -37,6 +37,9 @@
  #define TIAO_TUMPA_PID		0x8a98
  #define AMONTEC_JTAGKEY_PID	0xCFF8

+#define GOEPEL_VID		0x096C
+#define PICOTAP_PID		0x1449
+
  #define FIC_VID			0x1457
  #define OPENMOKO_DBGBOARD_PID	0x5118

@@ -51,6 +54,7 @@
  	{FTDI_VID, FTDI_FT4232H_PID, OK, "FTDI", "FT4232H"},
  	{FTDI_VID, TIAO_TUMPA_PID, OK, "TIAO", "USB Multi-Protocol Adapter"},
  	{FTDI_VID, AMONTEC_JTAGKEY_PID, OK, "Amontec", "JTAGkey"},
+	{GOEPEL_VID, PICOTAP_PID, OK, "GOEPEL", "PicoTAP"},
  	{FIC_VID, OPENMOKO_DBGBOARD_PID, OK, "FIC",
  		"OpenMoko Neo1973 Debug board (V2+)"},
  	{OLIMEX_VID, OLIMEX_ARM_OCD_PID, NT, "Olimex", "ARM-USB-OCD"},
@@ -176,6 +180,10 @@
  			ft2232_interface = INTERFACE_A;
  			cs_bits = 0x18;
  			pindir = 0x1b;
+		} else if (!strcasecmp(arg, "picotap")) {
+			ft2232_vid = GOEPEL_VID;
+			ft2232_type = PICOTAP_PID;
+			ft2232_interface = INTERFACE_A;
  		} else if (!strcasecmp(arg, "tumpa")) {
  			/* Interface A is SPI1, B is SPI2. */
  			ft2232_type = TIAO_TUMPA_PID
--

R,
S