Make it so the magic "reset_config" keywords can be provided in any
order. Example, "trst_and_srst" after "srst_open_drain", omitting
the "separate" combination (which should be the default in any case).
Why? (a) Makes it easier to define things at the right
level (adapter, board, target). You can just specify the
thing you *know* instead of having to somehow "know"
extraneous things, just so you can provide a long enough
keyword list. (b) Removing needless error paths is always
good; such sequencing shouldn't matter.
This includes two behavioral changes:
(1) When "handle_reset_config" sees a parameter error, it
immediately exits without changing anything. This is
best viewed as a bugfix. (Old behavior: restore all
the defaults, even if they weren't previously active.)
(2) Only the behaviors that were explicitly specified get
changed. (Old behavior: everything else gets reset to
the "default".) So for example you can specify SRST
drive requuirements without saying anything about the
three unrelated topics you previously had to specify,
and thus with no risk of providing incorrect values.
That second one might cause confusion for any configs that end
up calling "reset_config" twice, so it will deserve to be called
out in the release notes.
Update docs accordingly. Note that at least some versions of
the texi-to-html tools can't handle "@xref{with spaces}", but
those work properly in PDF and in the info files.
---
doc/openocd.texi | 62 +++++++++++++++++------
src/jtag/jtag.c | 140 ++++++++++++++++++++++++++++++++---------------------
2 files changed, 134 insertions(+), 68 deletions(-)
... this is "PROPOSAL 1" implemented. Didn't yet sanity test.
Make it so the magic "reset_config" keywords can be provided in any
order. Example, "trst_and_srst" after "srst_open_drain", omitting
the "separate" combination (which should be the default in any case).
Why? (a) Makes it easier to define things at the right
level (adapter, board, target). You can just specify the
thing you *know* instead of having to somehow "know"
extraneous things, just so you can provide a long enough
keyword list. (b) Removing needless error paths is always
good; such sequencing shouldn't matter.
This includes two behavioral changes:
(1) When "handle_reset_config" sees a parameter error, it
immediately exits without changing anything. This is
best viewed as a bugfix. (Old behavior: restore all
the defaults, even if they weren't previously active.)
(2) Only the behaviors that were explicitly specified get
changed. (Old behavior: everything else gets reset to
the "default".) So for example you can specify SRST
drive requuirements without saying anything about the
three unrelated topics you previously had to specify,
and thus with no risk of providing incorrect values.
That second one might cause confusion for any configs that end
up calling "reset_config" twice, so it will deserve to be called
out in the release notes.
Update docs accordingly. Note that at least some versions of
the texi-to-html tools can't handle "@xref{with spaces}", but
those work properly in PDF and in the info files.
---
doc/openocd.texi | 62 +++++++++++++++++------
src/jtag/jtag.c | 140 ++++++++++++++++++++++++++++++++---------------------
2 files changed, 134 insertions(+), 68 deletions(-)
--- a/doc/openocd.texi
+++ b/doc/openocd.texi
@@ -1552,9 +1552,13 @@ jtag_rclk 3000
Every system configuration may require a different reset
configuration. This can also be quite confusing.
+Resets also interact with @var{reset-init} event handlers,
+which do things like setting up clocks and DRAM, and
+JTAG clock rates. (@xref{JTAG Speed}.)
Please see the various board files for examples.
-...@b{note} to maintainers and integrators:
+...@quotation Note
+To maintainers and integrators:
Reset configuration touches several things at once.
Normally the board configuration file
should define it and assume that the JTAG adapter supports
@@ -1565,6 +1569,7 @@ which will be true for most (or all) boa
And when the JTAG adapter doesn't support everything, the
system configuration file will need to override parts of
the reset configuration provided by other files.
+...@end quotation
@section Types of Reset
@@ -1669,31 +1674,58 @@ How long (in milliseconds) OpenOCD shoul
nTRST (active-low JTAG TAP reset) before starting new JTAG operations.
@end deffn
-...@deffn {Command} reset_config signals [combination [trst_type [srst_type]]]
+...@deffn {Command} reset_config mode_flag ...
This command tells OpenOCD the reset configuration
of your combination of JTAG interface, board, and target.
-If the JTAG interface provides SRST, but the board doesn't connect
-that signal properly, then OpenOCD can't use it. @var{signals} can
-be @option{none}, @option{trst_only}, @option{srst_only} or
-...@option{trst_and_srst}.
+
+The mode flags can be specified in any order, but only one
+of each type -- @var{signals}, @var{combination}, @var{trst_type},
+and @var{srst_type} -- may be specified at a time.
+If you don't provide a new value for a given type, its previous
+value (perhaps the default) is unchanged.
+For example, this means that you don't need to say anything at all about
+TRST just to declare that if the JTAG adapter should want to drive SRST,
+it must explicitly be driven high (@option{srst_push_pull}).
+
+...@var{signals} can specify which of the reset signals are connected.
+For example, If the JTAG interface provides SRST, but the board doesn't
+connect that signal properly, then OpenOCD can't use it.
+Possible values are @option{none} (the default), @option{trst_only},
+...@option{srst_only} and @option{trst_and_srst}.
+
+...@quotation Tip
+If your board provides SRST or TRST through the JTAG connector,
+you must declare that or else those signals will not be used.
+...@end quotation
The @var{combination} is an optional value specifying broken reset
-signal implementations. @option{srst_pulls_trst} states that the
+signal implementations.
+The default behaviour if no option given is @option{separate},
+indicating everything behaves normally.
+...@option{srst_pulls_trst} states that the
test logic is reset together with the reset of the system (e.g. Philips
LPC2000, "broken" board layout), @option{trst_pulls_srst} says that
the system is reset together with the test logic (only hypothetical, I
haven't seen hardware with such a bug, and can be worked around).
@option{combined} implies both @option{srst_pulls_trst} and
-...@option{trst_pulls_srst}. The default behaviour if no option given is
-...@option{separate}.
+...@option{trst_pulls_srst}.
The optional @var{trst_type} and @var{srst_type} parameters allow the
-driver type of the reset lines to be specified. Possible values are
-...@option{trst_push_pull} (default) and @option{trst_open_drain} for the
-test reset signal, and @option{srst_open_drain} (default) and
-...@option{srst_push_pull} for the system reset. These values only affect
-JTAG interfaces with support for different drivers, like the Amontec
-JTAGkey and JTAGAccelerator.
+driver mode of each reset line to be specified. These values only affect
+JTAG interfaces with support for different driver modes, like the Amontec
+JTAGkey and JTAGAccelerator. Also, they are necessarily ignored if the
+relevant signal (TRST or SRST) is not connected.
+
+Possible @var{trst_type} driver modes for the test reset signal (TRST)
+are @option{trst_push_pull} (default) and @option{trst_open_drain}.
+Most boards connect this signal to a pulldown, so the JTAG TAPs
+never leave reset unless they are hooked up to a JTAG adapter.
+
+Possible @var{srst_type} driver modes for the system reset signal (SRST)
+are @option{srst_open_drain} (default) and @option{srst_push_pull}.
+Most boards connect this signal to a pullup, and allow the
+signal to be pulled low by various events including system
+powerup and pressing a reset button.
@end deffn
--- a/src/jtag/jtag.c
+++ b/src/jtag/jtag.c
@@ -2651,77 +2651,111 @@ static int handle_scan_chain_command(str
static int handle_reset_config_command(struct command_context_s *cmd_ctx, char *cmd, char **args, int argc)
{
+ int new_cfg = 0;
+ int mask = 0;
+
if (argc < 1)
return ERROR_COMMAND_SYNTAX_ERROR;
- if (argc >= 1)
- {
- if (strcmp(args[0], "none") == 0)
- jtag_reset_config = RESET_NONE;
- else if (strcmp(args[0], "trst_only") == 0)
- jtag_reset_config = RESET_HAS_TRST;
- else if (strcmp(args[0], "srst_only") == 0)
- jtag_reset_config = RESET_HAS_SRST;
- else if (strcmp(args[0], "trst_and_srst") == 0)
- jtag_reset_config = RESET_TRST_AND_SRST;
+ /* Original versions cared about the order of these tokens:
+ * reset_config signals [combination [trst_type [srst_type]]]
+ * They also clobbered the previous configuration even on error.
+ *
+ * Here we don't care about the order, and only change values
+ * which have been explicitly specified.
+ */
+ for (; argc; argc++) {
+ int tmp = 0;
+ int m;
+
+ /* signals */
+ m = RESET_HAS_TRST | RESET_HAS_SRST;
+ if (strcmp(*args, "none") == 0)
+ tmp = RESET_NONE;
+ else if (strcmp(*args, "trst_only") == 0)
+ tmp = RESET_HAS_TRST;
+ else if (strcmp(*args, "srst_only") == 0)
+ tmp = RESET_HAS_SRST;
+ else if (strcmp(*args, "trst_and_srst") == 0)
+ tmp = RESET_HAS_TRST | RESET_HAS_SRST;
else
- {
- LOG_ERROR("(1) invalid reset_config argument (%s), defaulting to none", args[0]);
- jtag_reset_config = RESET_NONE;
+ m = 0;
+ if (mask & m) {
+ LOG_ERROR("extra reset_config %s spec (%s)",
+ "signal", *args);
return ERROR_INVALID_ARGUMENTS;
}
- }
+ if (m)
+ goto next;
- if (argc >= 2)
- {
- if (strcmp(args[1], "separate") == 0)
- {
- /* seperate reset lines - default */
- } else
- {
- if (strcmp(args[1], "srst_pulls_trst") == 0)
- jtag_reset_config |= RESET_SRST_PULLS_TRST;
- else if (strcmp(args[1], "trst_pulls_srst") == 0)
- jtag_reset_config |= RESET_TRST_PULLS_SRST;
- else if (strcmp(args[1], "combined") == 0)
- jtag_reset_config |= RESET_SRST_PULLS_TRST | RESET_TRST_PULLS_SRST;
- else
- {
- LOG_ERROR("(2) invalid reset_config argument (%s), defaulting to none", args[1]);
- jtag_reset_config = RESET_NONE;
- return ERROR_INVALID_ARGUMENTS;
- }
+ /* combination (options for broken wiring) */
+ m = RESET_SRST_PULLS_TRST | RESET_TRST_PULLS_SRST;
+ if (strcmp(*args, "separate") == 0)
+ /* separate reset lines - default */;
+ else if (strcmp(*args, "srst_pulls_trst") == 0)
+ tmp |= RESET_SRST_PULLS_TRST;
+ else if (strcmp(*args, "trst_pulls_srst") == 0)
+ tmp |= RESET_TRST_PULLS_SRST;
+ else if (strcmp(*args, "combined") == 0)
+ tmp |= RESET_SRST_PULLS_TRST | RESET_TRST_PULLS_SRST;
+ else
+ m = 0;
+ if (mask & m) {
+ LOG_ERROR("extra reset_config %s spec (%s)",
+ "combination", *args);
+ return ERROR_INVALID_ARGUMENTS;
}
- }
+ if (m)
+ goto next;
- if (argc >= 3)
- {
- if (strcmp(args[2], "trst_open_drain") == 0)
- jtag_reset_config |= RESET_TRST_OPEN_DRAIN;
- else if (strcmp(args[2], "trst_push_pull") == 0)
- jtag_reset_config &= ~RESET_TRST_OPEN_DRAIN;
+ /* trst_type (NOP without HAS_TRST) */
+ m = RESET_TRST_OPEN_DRAIN;
+ if (strcmp(*args, "trst_open_drain") == 0)
+ tmp |= RESET_TRST_OPEN_DRAIN;
+ else if (strcmp(*args, "trst_push_pull") == 0)
+ /* push/pull from adapter - default */;
else
- {
- LOG_ERROR("(3) invalid reset_config argument (%s) defaulting to none", args[2] );
- jtag_reset_config = RESET_NONE;
+ m = 0;
+ if (mask & m) {
+ LOG_ERROR("extra reset_config %s spec (%s)",
+ "trst_type", *args);
return ERROR_INVALID_ARGUMENTS;
}
- }
+ if (m)
+ goto next;
- if (argc >= 4)
- {
- if (strcmp(args[3], "srst_push_pull") == 0)
- jtag_reset_config |= RESET_SRST_PUSH_PULL;
- else if (strcmp(args[3], "srst_open_drain") == 0)
- jtag_reset_config &= ~RESET_SRST_PUSH_PULL;
+ /* srst_type (NOP without HAS_SRST) */
+ m |= RESET_SRST_PUSH_PULL;
+ if (strcmp(*args, "srst_push_pull") == 0)
+ tmp |= RESET_SRST_PUSH_PULL;
+ else if (strcmp(*args, "srst_open_drain") == 0)
+ /* open drain from adapter - default */;
else
- {
- LOG_ERROR("(4) invalid reset_config argument (%s), defaulting to none", args[3]);
- jtag_reset_config = RESET_NONE;
+ m = 0;
+ if (mask & m) {
+ LOG_ERROR("extra reset_config %s spec (%s)",
+ "srst_type", *args);
return ERROR_INVALID_ARGUMENTS;
}
+ if (m)
+ goto next;
+
+ /* caller provided nonsense; fail */
+ LOG_ERROR("unknown reset_config flag (%s)", *args);
+ return ERROR_INVALID_ARGUMENTS;
+
+next:
+ /* Remember the bits which were specified (mask)
+ * and their new values (new_cfg).
+ */
+ mask |= m;
+ new_cfg |= tmp;
}
+ /* clear previous values of those bits, save new values */
+ jtag_reset_config &= ~mask;
+ jtag_reset_config |= new_cfg;
+
return ERROR_OK;
}
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
