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 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). > 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 :) 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. > @@ -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). Uwe. -- http://hermann-uwe.de | http://sigrok.org http://randomprojects.org | http://unmaintained-free-software.org _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
