target: improve robustness of reset command

Before this change jim_target_reset() checked examined state of a target
and failed without calling .assert_reset in particular target layer
(and without comprehensible warning to user).
Cortex-M target (which refuses access to DP under active SRST):
If connection is lost then reset process fails before asserting SRST
and connection with MCU is not restored.
This resulted in:
1) A lot of Cortex-M MCUs required use of reset button or cycling power
after firmware blocked SWD access somehow (sleep, misconfigured clock etc).
If firmware blocks SWD access early during initialization, a MCU could
become completely inaccessible by SWD.
2) If OpenOCD is (re)started and a MCU is in a broken state unresponsive
to SWD, reset command does not work even if it could help to restore communication.
Hopefully this scenario is not possible under full JTAG.

jim_target_reset() in target.c now does not check examined state
and delegates this task to a particular target. All targets have been checked
and xx_assert_reset() (or xx_deassert_reset()) procedures were changed
to check examined state if needed. Targets except arm11, cortex_a and cortex_m
just fail if target is not examined although it may be possible to use
at least hw reset. Left as TODO for developers familiar with these targets.

cortex_m_assert_reset(): memory access errors are stored
instead of immediate returning them to a higher level.
Errors from less important reads/writes are ignored.
Requested reset always leads to a configured action.

arm11_assert_reset() just asserts hw reset in case of not examined target.
cortex_a_assert_reset() works as usual in case of not examined target.

Change-Id: I84fa869f4f58e2fa83b6ea75de84440d9dc3d929
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-on: http://openocd.zylin.com/2606
Tested-by: jenkins
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>
Reviewed-by: Paul Fertser <fercerpav@gmail.com>

1 files changed, 26 insertions, 31 deletions

diff --git a/src/target/cortex_m.c b/src/target/cortex_m.c
index 6786c46..831d01a 100644
--- a/src/target/cortex_m.c
+++ b/src/target/cortex_m.c
@@ -992,34 +992,24 @@ static int cortex_m_assert_reset(struct target *target)
 	/* Enable debug requests */
 	int retval;
 	retval = mem_ap_read_atomic_u32(armv7m->debug_ap, DCB_DHCSR, &cortex_m->dcb_dhcsr);
-	if (retval != ERROR_OK)
-		return retval;
-	if (!(cortex_m->dcb_dhcsr & C_DEBUGEN)) {
+	/* Store important errors instead of failing and proceed to reset assert */
+
+	if (retval != ERROR_OK || !(cortex_m->dcb_dhcsr & C_DEBUGEN))
 		retval = mem_ap_write_u32(armv7m->debug_ap, DCB_DHCSR, DBGKEY | C_DEBUGEN);
-		if (retval != ERROR_OK)
-			return retval;
-	}
 
 	/* If the processor is sleeping in a WFI or WFE instruction, the
 	 * C_HALT bit must be asserted to regain control */
-	if (cortex_m->dcb_dhcsr & S_SLEEP) {
+	if (retval == ERROR_OK && (cortex_m->dcb_dhcsr & S_SLEEP))
 		retval = mem_ap_write_u32(armv7m->debug_ap, DCB_DHCSR, DBGKEY | C_HALT | C_DEBUGEN);
-		if (retval != ERROR_OK)
-			return retval;
-	}
 
-	retval = mem_ap_write_u32(armv7m->debug_ap, DCB_DCRDR, 0);
-	if (retval != ERROR_OK)
-		return retval;
+	mem_ap_write_u32(armv7m->debug_ap, DCB_DCRDR, 0);
+	/* Ignore less important errors */
 
 	if (!target->reset_halt) {
 		/* Set/Clear C_MASKINTS in a separate operation */
-		if (cortex_m->dcb_dhcsr & C_MASKINTS) {
-			retval = mem_ap_write_atomic_u32(armv7m->debug_ap, DCB_DHCSR,
+		if (cortex_m->dcb_dhcsr & C_MASKINTS)
+			mem_ap_write_atomic_u32(armv7m->debug_ap, DCB_DHCSR,
 					DBGKEY | C_DEBUGEN | C_HALT);
-			if (retval != ERROR_OK)
-				return retval;
-		}
 
 		/* clear any debug flags before resuming */
 		cortex_m_clear_halt(target);
@@ -1033,16 +1023,20 @@ static int cortex_m_assert_reset(struct target *target)
 		 * bad vector table entries.  Should this include MMERR or
 		 * other flags too?
 		 */
-		retval = mem_ap_write_atomic_u32(armv7m->debug_ap, DCB_DEMCR,
+		int retval2;
+		retval2 = mem_ap_write_atomic_u32(armv7m->debug_ap, DCB_DEMCR,
 				TRCENA | VC_HARDERR | VC_BUSERR | VC_CORERESET);
-		if (retval != ERROR_OK)
-			return retval;
+		if (retval != ERROR_OK || retval2 != ERROR_OK)
+			LOG_INFO("AP write error, reset will not halt");
 	}
 
 	if (jtag_reset_config & RESET_HAS_SRST) {
 		/* default to asserting srst */
 		if (!srst_asserted)
 			adapter_assert_reset();
+
+		/* srst is asserted, ignore AP access errors */
+		retval = ERROR_OK;
 	} else {
 		/* Use a standard Cortex-M3 software reset mechanism.
 		 * We default to using VECRESET as it is supported on all current cores.
@@ -1057,27 +1051,24 @@ static int cortex_m_assert_reset(struct target *target)
 				"handler to reset any peripherals or configure hardware srst support.");
 		}
 
-		retval = mem_ap_write_atomic_u32(armv7m->debug_ap, NVIC_AIRCR,
+		int retval3;
+		retval3 = mem_ap_write_atomic_u32(armv7m->debug_ap, NVIC_AIRCR,
 				AIRCR_VECTKEY | ((reset_config == CORTEX_M_RESET_SYSRESETREQ)
 				? AIRCR_SYSRESETREQ : AIRCR_VECTRESET));
-		if (retval != ERROR_OK)
+		if (retval3 != ERROR_OK)
 			LOG_DEBUG("Ignoring AP write error right after reset");
 
-		retval = dap_dp_init(armv7m->debug_ap->dap);
-		if (retval != ERROR_OK) {
+		retval3 = dap_dp_init(armv7m->debug_ap->dap);
+		if (retval3 != ERROR_OK)
 			LOG_ERROR("DP initialisation failed");
-			return retval;
-		}
 
-		{
+		else {
 			/* I do not know why this is necessary, but it
 			 * fixes strange effects (step/resume cause NMI
 			 * after reset) on LM3S6918 -- Michael Schwingen
 			 */
 			uint32_t tmp;
-			retval = mem_ap_read_atomic_u32(armv7m->debug_ap, NVIC_AIRCR, &tmp);
-			if (retval != ERROR_OK)
-				return retval;
+			mem_ap_read_atomic_u32(armv7m->debug_ap, NVIC_AIRCR, &tmp);
 		}
 	}
 
@@ -1086,6 +1077,10 @@ static int cortex_m_assert_reset(struct target *target)
 
 	register_cache_invalidate(cortex_m->armv7m.arm.core_cache);
 
+	/* now return stored error code if any */
+	if (retval != ERROR_OK)
+		return retval;
+
 	if (target->reset_halt) {
 		retval = target_halt(target);
 		if (retval != ERROR_OK)