Hi Andreas, Thanks again for the explanations. I'm confident I understand the current role of the CMSIS-DAP swd_driver now, and I think I follow the concerns about future refactoring & functionality.
I'd still like to try and bite off a performance improvement for CMSIS-DAP operations if I can. Unfortunately I don't have either the time or the breadth of knowledge to try and bite off any major refactors. Do you think there is a way I can implement CMSIS-DAP transfer queueing in a way that won't make future refactors more difficult? The two options I can think of is either to stick with the original suggestion and just add it to the swd_driver functions in cmsis_dap_usb.c. This is still reasonably encapsulated, and will need refactoring anyhow. I'd understand if you don't want that implementation growing, though. The second thought I have is to refactor out the CMSIS-DAP swd_driver commands to a specific interface, something like cmsis_dap_driver, to replace the current swd_driver based implementation. This can present an adapter-specific interface for CMSIS-DAP based transport(s). That way when an eventual refactor comes, that interface can hopefully stay as-is facing different/varied upstream transports (with possible extensions to support new commands for JTAG low level ops, etc.) What do you (and the others) think? - Angus On Mon, Jul 14, 2014 at 12:57:58PM +0200, Andreas Fritiofson wrote: > On Mon, Jul 14, 2014 at 4:47 AM, Angus Gratton <[email protected]> wrote: > > > Hi Andreas, > > > > Thanks for clarifying a great many things. > > > > I did a bit more reading of the source. I hadn't realised that both > > JTAG and SWD DAP ops go via dap_ops -> cmsis_dap_transport -> > > jtag_interface.swd -> cmsis_dap_swd_driver to generate the USBHID > > commands. I actually thought JTAG/CMSIS-DAP hadn't been implemented in > > openocd yet, my mistake. > > > > I'm sorry if I confused you but what I meant was that DAP operation go > through *either* adi_v5_jtag.c, adi_v5_swd.c or adi_v5_cmsis_dap.c. The > JTAG and SWD implementations are only used when another type of adapter > driver is selected, such a ftdi, jlink, vsllink etc. When a CMSIS-DAP > adapter is used, DAP ops from the target *always* go through > adi_v5_cmsis_dap.c (because of "transport select cmsis-dap") and from there > into the jtag_interface->swd which is initialized by the interface > infrastructure (because of "interface cmsis-dap") to point to the > jtag/drivers/cmsis_dap_usb.c swd_driver. The jtag_interface in the same > file is necessary as a container for the swd_driver and for providing an > initialization point and functions to change clock speed. This is the most > problematic part of the OpenOCD internal architecture today, as I see it. > > The CMSIS-DAP implementation in OpenOCD still does not support DAP > operations over JTAG, but that's a matter of initialization. The adapter > needs to be told to use JTAG and the structure of the JTAG chain (which is > going to be the messy part). The DAP ops shall not take another route > inside OpenOCD just because the adapter has been told to use JTAG instead > of SWD. > > > > Now I see cmsis_dap_swd_driver is used here not as a way of > > implementing SWD operations, but just a way of implementating DAP > > operations in a way that the DAP transport can access them. > > > > Does that sound right? > > > > I think so, yes. > > > > Exploring possibilities, I think we could remove the > > cmsis_dap_swd_driver instance and move the read_red/write_reg stuff up > > into the transport. However there's got to be a convenient interface > > between them: > > > > - If we do the actual USB operations as part of the jtag_interface, we > > need a way for the transport to queue generic CMSIS-DAP commands on > > the jtag_interface. Maybe a custom command that's just raw CMSIS-DAP > > bytes? Or a dap_driver to the jtag_interface, similar to swd_driver? > > > > - Maybe it would be possible to do the USB operations on the transport > > side? This doesn't gel with the description of a transport as > > relating only to the on-wire signalling not the messaging > > protcol. To work the jtag_interface would need a way to queue > > CMSIS-DAP commands on the transport for custom commands, > > adapter_khz, etc. > > > > - Or alternatively we leave it as it is, but maybe label the > > cmsis_dap_swd_driver instance more clearly to make it clearer that > > it's not an SWD-only interface, but just a convenient interface for > > any DAP register operations? > > > > > > I'd be happy to put time towards exploring any of these though (or > > some fourth choice) if it's helpful? > > > > > Just to provide further food for thought, here are some of the issues that > need to be considered at some point. > > The CMSIS-DAP protocol can handle raw JTAG operations (clock this many bits > in and/or out while holding TMS at that level) and we'll at some point want > to use that to enable debugging non-Cortex parts, maybe even on the same > JTAG chain and at the same time as a DAP based part. > > It may be necessary to clock out special sequences to switch between JTAG > and SWD, especially to support Dormant operation and multidrop SWD. API for > that was added at the swd_driver layer, not at the dap_ops layer. How do we > handle that if we ditch the swd_driver for CMSIS-DAP? > > Struct jtag_interface is currently overloaded with non-JTAG related > functions, such as speed setting functions, reset signals, as a holder of > struct swd_driver and moreover it can only exist in one global instance. > I'd like to minimize struct jtag_interface and keep it (for now) as a pure > JTAG API. The auxiliary functionality should IMO be transferred to a new > "struct adapter" with a separate lifetime. Using operations on that adapter > instance, you could open sessions to access a struct jtag_interface for > JTAG operations, or a struct swd_driver for low-level SWD operations, a > struct dap_ops for high-level DAP operations and so on. New APIs could be > added for new debug protocols or special adapter features. A pure SWD-only > adapter should not need to implement a struct jtag_interface. > > In JTAG mode, the CMSIS-DAP adapter needs to be told the structure of the > JTAG chain. In that use case, the TAPs should probably be declared/auto > detected as normal, which means a full JTAG API needs to be implemented > over the DAP_JTAG_Sequence command. TAPs that do not connect to a DAP, such > as old ARMs, CPLDs or whatever that may be in the chain, should be accessed > over the JTAG API but DAP parts should be able to be accessed through the > dap_ops API. Note that there may be several DAP targets on a chain so we > must be able to request several struct dap_ops from the adapter. > > /Andreas ------------------------------------------------------------------------------ Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds _______________________________________________ OpenOCD-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openocd-devel
