David Brownell <david-b@pacbell.net>:

Make it so the magic "reset_config" keywords can be provided in any
order.  This eliminates needless error paths, and makes it easier
to define things at the right level (adapter, board, target).
It also includes two other behavioral changes:

  (1)	When "handle_reset_config" sees a parameter error, it
  	exits without changing anything.   This is best viewed
	as a bugfix.  (Old behavior:  restore 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 now specify SRST
	drive requirements without saying anything about the
	three unrelated topics you previously had to specify.

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.  (There were no such configurations in
the current OpenOCD source tree.)

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.


git-svn-id: svn://svn.berlios.de/openocd/trunk@1944 b42882b7-edfa-0310-969c-e2dbd0fdcd60

2 files changed, 145 insertions, 72 deletions

diff --git a/doc/openocd.texi b/doc/openocd.texi
index 2e4f9921..b5c325d2 100644
--- a/doc/openocd.texi
+++ b/doc/openocd.texi
@@ -1574,9 +1574,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
@@ -1587,6 +1591,7 @@ which will be true for most (or all) boards using that chip.
 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
 
@@ -1671,11 +1676,18 @@ Use the @command{reset_config} @var{trst_type} and
 
 There can also be other issues.
 Some devices don't fully conform to the JTAG specifications.
-Others have chip-specific extensions like extra steps needed
-during TAP reset, or a requirement to use the normally-optional TRST
-signal.
 Trivial system-specific differences are common, such as
 SRST and TRST using slightly different names.
+There are also vendors who distribute key JTAG documentation for
+their chips only to developers who have signed a Non-Disclosure
+Agreement (NDA).
+
+Sometimes there are chip-specific extensions like a requirement to use
+the normally-optional TRST signal (precluding use of JTAG adapters which
+don't pass TRST through), or needing extra steps to complete a TAP reset.
+
+In short, SRST and especially TRST handling may be very finicky,
+needing to cope with both architecture and board specific constraints.
 
 @section Commands for Handling Resets
 
@@ -1691,31 +1703,58 @@ How long (in milliseconds) OpenOCD should wait after deasserting
 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 @var{mode_flag} options 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 the default @option{srst_open_drain}, 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
 
 
diff --git a/src/jtag/jtag.c b/src/jtag/jtag.c
index 56a8075b..ecd4931a 100644
--- a/src/jtag/jtag.c
+++ b/src/jtag/jtag.c
@@ -2651,77 +2651,111 @@ static int handle_scan_chain_command(struct command_context_s *cmd_ctx, char *cm
 
 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--, args++) {
+		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 (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;
-			}
+		if (m)
+			goto next;
+
+		/* 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;
 }