On Sunday 25 October 2009, simon qian wrote:
> I want to add a interface_mode command with only one parameter, default is
> JTAG mode.

As Øyvind noted, $SUBJECT is a Big Deal.  It'd be nice if
we could get a start on that in the 0.4.x series... we
should start from a more complete proposal.

It seems to me there are several components involved in
having another debug protocol work.  And that most of them
need to be in place before a clean SWD patch can merge:

 - Something like what you sketched, in terms configuring a
   protocol to use for the debug session.  (Or perhaps, for
   the next chunk of this session...)  Let's continue to use
   JTAG as the default.

 - Interface support.  Today we have "JTAG adapters"; but
   with this we may also have "JTAG -or- SWD adapters", and
   "SWD adapters" (two-wire only), "SPI adapters", etc.  So
   changes might be required like:

      (a) src/interface/* holding a lot of what's now in
          src/jtag/* ... but not JTAG state machines, etc,
          or JTAG-only drivers
      (b) "struct debug_interface" not "struct jtag_interface"
      (c) new debug interface methods to (c1) retrieve the
          current mode, and (c2) change it.
      (d) enumeration models.  SWD, and its yet-to-finalize
          IEEE 1149.7 cousin, are sort of close to JTAG.
          But there are more point-to-point models, like how
          SPI tends to be used at this level.
      (e) specifically for SWD ... doesn't the SWO trace
          output imply a new primitive to just collect a
          stream of trace data?

 - Target support.  Target code may need to know it's using
   SWD instead of JTAG, e.g. since the state transitions are
   different, or because they've got to switch out of JTAG
   mode to get TPIU output.

 - Flash support.  Consider AVR8 connections via SPI; they
   aren't good for a lot more than flash access.  The same is
   true for direct read/write access to most SPI flash chips.

 - Reset integration.  Not that this is very clean today; but
   if one uses JTAG it's at least clear that debug and system
   reset ought to be different things, if the vendor didn't
   integrate their ARM core strangely.  I don't think that's
   quite as clear with the non-JTAG access.  (Plus there's
   the issue of JTAG reset vs SWD reset issued on the same
   two wires...)

There are likely a few other points, but I think that hits
a lot of the major ones.  


> In configuration file, it will be as follow to use SWJ interface:
>         interface_mode swj

Correct me if I'm wrong, but doesn't ARM call this "SWD" mode?

There's a bit of an acronym soup there; "SWJ-DP" is a debug
port supporting JTAG (the J) and SWD, and at least on Cortex-M3
it can support trace output using SWO, with a pin labeled SWV,
so long as SWD is active...


> in jtag_register_commands in tcl.c, add:
> register_command(cmd_ctx, NULL, "interface_mode",
> handle_interface_mode_command,
>   COMMAND_ANY, "set debug interface mode [jtag|swj]");
> 
> And handle_interface_mode_command will be:
> static int handle_interface_mode_command(struct command_context_s *cmd_ctx,
> char *cmd, char **args, int argc)
> {
>  if (argc != 1)
>   return ERROR_COMMAND_SYNTAX_ERROR;
>  int retval = ERROR_OK;
>  if (strcmp(*args, "jtag") == 0)

My rule of thumb with respect to generalizing interfaces is
the "rule of three":  it's almost certainly NOT general enough
unless there are at least three peer options.

So I'll suggest you plan a smidgeon more aggressively.  You
did the AVR8 target stub, yes?  So perhaps you are aware enough
of AVR8 to include its ISP programming interface (which I called
SPI above) in your thought and planning.  Not necessarily in
implementation.

Here, that could map to having the command look up its parameter
in a table, and invoke the method found by that lookup.


>  {
>   // jtag mode
>   set_jtag_intarface();

A nitpick:  comments like that are pointless.  :)

More substantively:  clearly that command should be
able to fail, for e.g. a SWD-only debug adapter.


>  }
>  else if (strcmp(*args, "swj") == 0)
>  {
>   // swj mode
>   set_swj_intarface();

Likewise.


>  }
>  else
>  {
>   // mode not supported
>   LOG_ERROR("%s is not supported.", *args);
>   return ERROR_FAIL;
>  }
>  return ERROR_OK;
> }
> 
> set_jtag_intarface and set_swj_interface will be implemented in core.c.
> Is that OK?

I'd think most of their work would be interface-specific; so
maybe that lookup table should use the current interface?
At any rate, interface-specific code would get called.

There are a lot of ways to factor that stuff.  I'm not keen
on adding layers needlessly; I've seen it done too many times,
only adding complication.  So if you're adding a layer, I'd
want to know why.

- Dave



_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to