ARM11: minor cleanup, mostly ITR comments

ITR register handling seemed to be giving me problems, so I updated
the comments to better say what the code is trying to do ... and to
note the preconditions (one of which seems to be an issue) as listed
in the ARM1136 TRM.

Also removed the unused "ARM11_TAP_DEFAULT" from the ITR scan code;
all the callers already specify an exit path, since this register
isn't usable with such vague semantics.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

2 files changed, 43 insertions, 28 deletions

diff --git a/src/target/arm11.c b/src/target/arm11.c
index 7b29f53e..7c747c06 100644
--- a/src/target/arm11.c
+++ b/src/target/arm11.c
@@ -1012,27 +1012,25 @@ static int arm11_write_memory_inner(struct target *target,
 		}
 
 	case 4: {
+		/* increment:		STC p14,c5,[R0],#4 */
+		/* no increment:	STC p14,c5,[R0]*/
 		uint32_t instr = !no_increment ? 0xeca05e01 : 0xed805e00;
 
 		/** \todo TODO: buffer cast to uint32_t* causes alignment warnings */
 		uint32_t *words = (uint32_t*)buffer;
 
+		/* "burst" here just means trusting each instruction executes
+		 * fully before we run the next one:  per-word roundtrips, to
+		 * check the Ready flag, are not used.
+		 */
 		if (!burst)
-		{
-			/* STC p14,c5,[R0],#4 */
-			/* STC p14,c5,[R0]*/
-			retval = arm11_run_instr_data_to_core(arm11, instr, words, count);
-			if (retval != ERROR_OK)
-				return retval;
-		}
+			retval = arm11_run_instr_data_to_core(arm11,
+					instr, words, count);
 		else
-		{
-			/* STC p14,c5,[R0],#4 */
-			/* STC p14,c5,[R0]*/
-			retval = arm11_run_instr_data_to_core_noack(arm11, instr, words, count);
-			if (retval != ERROR_OK)
-				return retval;
-		}
+			retval = arm11_run_instr_data_to_core_noack(arm11,
+					instr, words, count);
+		if (retval != ERROR_OK)
+			return retval;
 
 		break;
 	}
@@ -1309,7 +1307,7 @@ static const struct command_registration arm11_mw_command_handlers[] = {
 		.name = "burst",
 		.handler = &arm11_handle_bool_memwrite_burst,
 		.mode = COMMAND_ANY,
-		.help = "Enable/Disable non-standard but fast burst mode"
+		.help = "Enable/Disable potentially risky fast burst mode"
 			" (default: enabled)",
 	},
 	{
diff --git a/src/target/arm11_dbgtap.c b/src/target/arm11_dbgtap.c
index 9ad56622..554d6e2b 100644
--- a/src/target/arm11_dbgtap.c
+++ b/src/target/arm11_dbgtap.c
@@ -230,22 +230,23 @@ int arm11_add_debug_SCAN_N(struct arm11_common *arm11,
 	return jtag_execute_queue();
 }
 
-/** Write an instruction into the ITR register
+/**
+ * Queue a DR scan of the ITR register.  Caller must have selected
+ * scan chain 4 (ITR), possibly using ITRSEL.
  *
  * \param arm11		Target state variable.
  * \param inst		An ARM11 processor instruction/opcode.
- * \param flag		Optional parameter to retrieve the InstCompl flag
- *					(this will be written when the JTAG chain is executed).
- * \param state		Pass the final TAP state or ARM11_TAP_DEFAULT for the default
- *					value (Run-Test/Idle).
+ * \param flag		Optional parameter to retrieve the Ready flag;
+ *	this address will be written when the JTAG chain is scanned.
+ * \param state		The TAP state to enter after the DR scan.
  *
- * \remarks			By default this ends with Run-Test/Idle state
- *					and causes the instruction to be executed. If
- *					a subsequent write to DTR is needed before
- *					executing the instruction then TAP_DRPAUSE should be
- *					passed to \p state.
+ * Going through the TAP_DRUPDATE state writes ITR only if Ready was
+ * previously set.  Only the Ready flag is readable by the scan.
  *
- * \remarks			This adds to the JTAG command queue but does \em not execute it.
+ * An instruction loaded into ITR is executed when going through the
+ * TAP_IDLE state only if Ready was previously set and the debug state
+ * is properly set up.  Depending on the instruction, you may also need
+ * to ensure that the rDTR is ready before that Run-Test/Idle state.
  */
 static void arm11_add_debug_INST(struct arm11_common * arm11,
 		uint32_t inst, uint8_t * flag, tap_state_t state)
@@ -257,7 +258,7 @@ static void arm11_add_debug_INST(struct arm11_common * arm11,
 	arm11_setup_field(arm11, 32,    &inst,	NULL, itr + 0);
 	arm11_setup_field(arm11, 1,	    NULL,	flag, itr + 1);
 
-	arm11_add_dr_scan_vc(ARRAY_SIZE(itr), itr, state == ARM11_TAP_DEFAULT ? TAP_IDLE : state);
+	arm11_add_dr_scan_vc(ARRAY_SIZE(itr), itr, state);
 }
 
 /**
@@ -374,7 +375,11 @@ int arm11_run_instr_data_finish(struct arm11_common * arm11)
 
 
 
-/** Execute one or multiple instructions via ITR
+/**
+ * Execute one or more instructions via ITR.
+ * Caller guarantees that processor is in debug state, that DSCR_ITR_EN
+ * is set, the ITR Ready flag is set (as seen on the previous entry to
+ * TAP_DRCAPTURE), and the DSCR sticky abort flag is clear.
  *
  * \pre arm11_run_instr_data_prepare() /  arm11_run_instr_data_finish() block
  *
@@ -444,6 +449,10 @@ int arm11_run_instr_no_data1(struct arm11_common * arm11, uint32_t opcode)
 /** Execute one instruction via ITR repeatedly while
  *  passing data to the core via DTR on each execution.
  *
+ * Caller guarantees that processor is in debug state, that DSCR_ITR_EN
+ * is set, the ITR Ready flag is set (as seen on the previous entry to
+ * TAP_DRCAPTURE), and the DSCR sticky abort flag is clear.
+ *
  *  The executed instruction \em must read data from DTR.
  *
  * \pre arm11_run_instr_data_prepare() /  arm11_run_instr_data_finish() block
@@ -570,6 +579,10 @@ static const tap_state_t arm11_MOVE_DRPAUSE_IDLE_DRPAUSE_with_delay[] =
 /** Execute one instruction via ITR repeatedly while
  *  passing data to the core via DTR on each execution.
  *
+ * Caller guarantees that processor is in debug state, that DSCR_ITR_EN
+ * is set, the ITR Ready flag is set (as seen on the previous entry to
+ * TAP_DRCAPTURE), and the DSCR sticky abort flag is clear.
+ *
  *  No Ready check during transmission.
  *
  *  The executed instruction \em must read data from DTR.
@@ -678,6 +691,10 @@ int arm11_run_instr_data_to_core1(struct arm11_common * arm11, uint32_t opcode,
 /** Execute one instruction via ITR repeatedly while
  *  reading data from the core via DTR on each execution.
  *
+ * Caller guarantees that processor is in debug state, that DSCR_ITR_EN
+ * is set, the ITR Ready flag is set (as seen on the previous entry to
+ * TAP_DRCAPTURE), and the DSCR sticky abort flag is clear.
+ *
  *  The executed instruction \em must write data to DTR.
  *
  * \pre arm11_run_instr_data_prepare() /  arm11_run_instr_data_finish() block