On Mon, Nov 5, 2012 at 4:40 AM, Evan Hunter <[email protected]> wrote:
> I would like to modify the adapter driver interfaces to allow per-instance
> driver variable structures.
>
> This would:
> · Get rid of loads of static and global variables.
> · Allow multiple interfaces
>
> The proposed prototypes below mostly add this per-instance variable as an
> instance handle.
> Drivers could keep using the current system and be modified when suitable to
> use the per –instance parameters.
>
> Is this something that is wanted?
While this is a worthwhile goal, I don't think the global state of
*the current* adapter interface is a big deal, because there are far
worse flaws in it. I would rather see a fresh new adapter API designed
and implemented in parallel. Such an API should of course be designed
with these features in mind.
>
> Do you have any thoughts on the proposed organisation below?
>
Yes, I don't like it. Because it looks a lot like what we have today,
and what we have today kind of sucks. It could work as a first step,
but unfortunately I don't buy the idea that this change is better done
incrementally.
Below is an outline of how we could redesign the API completely it in
one go without having to break or rewrite lots of stuff.
1. Design a fresh set of APIs, one for each "transport" or function of
an adapter. The APIs should correspond exactly to the features of the
function, no more, no less. For example, the jtag API should have
functions to perform statemoves, ir and dr scans, setting a desired
frequency and so on. It should NOT include any reset signal handling,
since such features are outside the JTAG spec. Reset signals should be
exposed via a separate gpio API, along with any other signal an
adapter may support.
The APIs should, where relevant, include session handling. For
example, and adapter driver may provide both a jtag and an swd
interface, but opening a jtag session should fail if there's already
an swd session open, if they're mutually exclusive.
An important aspect of the APIs is that no assumption can be made as
to WHEN the adapter driver actually performs the operation. Different
adapters have different requirements. For a USB type of adapter,
operations must be batched for efficiency reasons, but if OpenOCD is
running on the adapter itself and have direct access to I/Os it makes
most sense to perform the operations synchronously. Therefore, the
APIs should be designed such that operations are not guaranteed to
have executed until a flush/execute-function is called. It could be
taken a step further than the current jtag queue API by not even
returning errors from operations but instead discarding further
operations if there is an error in executing a previous one. Much like
the sticky error in ADIv5. The error message is returned and the error
cleared by the flush function.
2. Create a minidriver that implements the legacy minidriver API in
terms of the new jtag API and the reset parts of the gpio API.
3. Create a new style "adapter" that implements the jtag and gpio API
using the legacy jtag queue API (in effect calling into a replica of
driver.c but with its functions renamed to not clash with the
minidriver API).
4. With this in place, all legacy targets and adapter drivers should
be usable as before, but with the new APIs sitting in the loop. This
way the (JTAG parts of the) API can be tested so it's possible to
implement all the needed features over it. (But no adding awkward
features to the new APIs because of odd target requirements that can
be worked around in the target.) Life cycle management, Jim hooks and
configuration commands can be tested at this point. And the conversion
of target and adapter drivers is decoupled because old style and new
style drivers can be used interchangeably.
5. Convert some target to call the new APIs directly. The Cortex-M
(ADIv5) target is a good candidate because it's a multi-transport
target which is widely used and well working.
6. Convert (or create) a real adapter driver to implement the new APIs
directly. The ftdi driver is a good candidate because it provides
low-level access, enabling virtually any transport to be implemented.
A new CMSIS-DAP driver could be another suitable option.
7. Implement another transport for the selected target and adapter,
such as swd or high level dap (as used in CMSIS-DAP) and the necessary
infrastructure to select which to use (the existing transport
framework *may* help here).
The first point should be where most of the effort should be put and
the APIs probably need to evolve over several iterations of the above.
I just added some comments below, not necessarily directed at your
proposal as most of the stuff we have already. Just highlighting some
thoughts I have about the API.
> Proposed interface driver prototypes:
> {
> char *name;
> const char **transports;
> const struct swd_driver *swd;
I haven't got a clear picture of how the transport framework was
supposed to be used or what it's trying to solve and I'm afraid we
won't get any help from it's author. On the bright side, since nothing
is really using it, we're free to include it in the API or not, or
tweak it if it could be made to fit.
> const struct command_registration *commands;
Why can't the adapter constructor register the commands it wants to
provide and unreg them when it's destroyed? I haven't looked much at
the command framework, but I feel it's backwards.
> int (*create)(Jim_GetOptInfo *options, int interface_num,
> adapter_handle_t *new_handle);
> int (*destroy)(adapter_handle_t handle);
I think these two hint at the dynamic nature you want to introduce.
That's good. Adapters should be able to dynamically added or removed.
The adapter driver life cycle is probably one of the most important
design points.
> int (*init)(adapter_handle_t handle);
> int (*quit)(adapter_handle_t handle);
What did you have in mind for these? I'm not sure we need another pair
of constructors/destructors.
> int (*speed)(adapter_handle_t handle, int speed);
Speed is a transport specific concept.
> int (*power_dropout)(adapter_handle_t handle, int
> *power_dropout);
> int (*srst_asserted)(adapter_handle_t handle, int
> *srst_asserted);
These are very specific functions that not even a jtag adapter needs
to have. Reset signals, power detection/enabling and other I/O or
event related activity should be handled with separate API(s).
> /* JTAG functions - to be split out into separate structure
> soon */
> int (*execute_queue)(adapter_handle_t handle, struct
> jtag_command *cmd_queue);
Queuing doesn't belong in the adapter API.
> int (*khz)(adapter_handle_t handle, int khz, int
> *jtag_speed);
> int (*speed_div)(adapter_handle_t handle, int speed, int
> *khz);
We really don't need three separate functions to set the operating frequency.
> };
I'm thinking something along the lines of:
/* the "vtable" which each driver populates with the transports it
* supports, leaving the rest null */
struct adapter_ops {
void (*destroy)(void);
const struct jtag_ops *jtag;
const struct swd_ops *swd;
const struct dap_ops *dap;
const struct gpio_ops *gpio;
/* ... add more as required */
};
/* the instance struct which is allocated, initialized and returned by
* an adapter driver's constructor */
struct adapter {
const char *name;
const struct adapter_ops *ops;
/* any other per-instance data that may be needed, if any */
};
/* an example swd api */
struct swd_ops {
struct swd_session *(*open_session)(struct adapter *, ...);
void (*close_session)(struct swd_session *);
void (*read_reg)(struct swd_session *, uint8_t cmd, uint32_t *value);
void (*write_reg)(struct swd_session *,uint8_t cmd, uint32_t value);
int (*flush)(struct swd_session *);
/* ... */
};
/Andreas
------------------------------------------------------------------------------
LogMeIn Central: Instant, anywhere, Remote PC access and management.
Stay in control, update software, and manage PCs from one command center
Diagnose problems and improve visibility into emerging IT issues
Automate, monitor and manage. Do more in less time with Central
http://p.sf.net/sfu/logmein12331_d2d
_______________________________________________
OpenOCD-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openocd-devel
