Oleg, Thanks for being responsive to David's requests. While this patch looks great for reviewing your new device's support, I think that we should avoid pushing it to the repository. We should wait for the libftdi patch to be committed and scheduled for some future release.
Users should never be asked to patch a library to enable a configure option, so -- until those changes are in its mainline -- you should repost (with status updates) when this patch no longer applies cleanly. So long as users need to wrangle the libftdi patch, they can manage to apply this one too. So, this is not rejection, but rather a request for temporary deferral. If the libftdi maintainers repeatedly ignore your patch, then we can revisit this topic at that point; however, that seems unlikely to occur. If it's similarly well-written, it should be easy to get it accepted, at which time I would fully endorse including a patch to OpenOCD (that also addresses the configuration issues). Clean working copies (or future releases) for both projects would be sufficient to build this feature. I also suggest starting a new thread to address the other new features that you mention will be required to support this device fully (e.g. current and voltage measurement commands), so that topic might receive the device-neutral attention that it deserves. Cheers, Zach On Sat, 2009-10-24 at 13:47 -0700, Signalyzer ( www.signalyzer.com ) wrote: > Hi Dave, > > Thank you for your feedback! > > Unfortunately new Signalyzer H relies on FTDI's > FT_WriteEE()/FT_ReadEE() and similar functions from libftdi to > implement some of the functionality. The only reason for adding new > #define and --enable-signalyzer_h is because current libftdi > implementation does not have function calls that correspond to FTDI's > WriteEE and ReadEE, I'm proposing a patch to libftdi to include these > into libftdi, but until this functionality is added to libftdi I > thought #define and --enable-signalyzer_h would be a workaround > allowing OpenOCD to compile with libftdi-0.16 but without Signalyzer-H > support. Once libftdi has support for individual EEPROM locations > reads and writes Signalyzer H support can be made transparent as any > other FT2232 based devices. > > Said that, I'm hoping to extend the functionality a bit further and > add control for adjustable power supply Signalyzer H has when used > with corresponding adapter modules (these details have not been yet > published on the web site). The adapter will offer adjustable (1.0V to > ~5.00V) power supply with current measurement abilities. In addition > to all of this, Signalyzer H is capable to measure Vtarget voltages. > None of these features have yet been added to the OpenOCD and I was > hoping to discuss these with you and other members of the project in > regards to the best approach of adding these features to the OpenOCD. > I'm guessing one route would be to completely separate Signalyzer H > from ft2232.c/h as Presto, but I would love to be able to utilize the > ft2232.c/h code as much as possible. > > Anyways, I would kindly ask you to accept the current implementation > as interim approach for supporting Signalyzer H2 and H4 in OpenOCD, > and as a next step, to discuss what would the best approach for the > extended functionality I think many users will find useful. > > I cleaned-up code and reviewed all indents. Please let me if you still > see them being misaligned? My code should be following the coding > style chosen for openocd. > > Regards, > Oleg > www.signalyzer.com > > On Fri, Oct 23, 2009 at 9:27 PM, David Brownell <davi...@pacbell.net> wrote: > > On Friday 23 October 2009, Signalyzer ( www.signalyzer.com ) wrote: > >> > >> This patch adds support for Signalyzer H2 (FT2232H) and Signalyzer H4 > >> (FT4232H) to OpenOCD using D2XX and/or libftdi drivers. > > > > Great ... but could you spin a version that doesn't need > > all that configuration? (And tidy up the code a bit too; > > indents aren't consistent, etc) > > > > Basically, this should be included just like the other > > FT2232 drivers if the FT2232 support is enabled. So no > > special "configure ... --enable=signalyzer-h" needed, > > and no #ifdefs in the code. > > > > The current Signalyzer doesn't need explicit config, > > neither do other FT2232 based adapters. Let's keep > > things that simple going forward, too. > > > > - Dave > > > > > > > > > > > _______________________________________________ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development _______________________________________________ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
