xscale: fix sw breakpoints for thumb; set bp immediately

Hi everyone,

Version 2 of this patch.  Code added to breakpoints.c was removed from previous
patch, and item 3 added, per discussion with Øyvind regarding error reporting.
Item 4 added, which I just noticed.

I tried to use a software breakpoint in thumb code on the xscale for the first
time recently, and was surprised to find that it didn't work.  The result was
this patch, which does four things:

1): fix trivial cut-n-paste error that caused thumb breakpoints to not work
2): call xscale_set_breakpoint() from xscale_add_breakpoint()
3): log error on data abort in xscale_write_memory()
4): fixed incorrect error code returned by xscale_set_breakpoint() when no
    breakpoint register is available; added comment

Item 2 not only makes the xscale breakpoint code consistent with other targets,
but also alerts the user immediately if an error occurs when writing the
breakpoint instruction to target memory (previously, xscale_set_breakpoint() was
not called until execution resumed).  Also, calling xscale_breakpoint_set() as
part of the call chain starting with handle_bp_command() and propagating the
return status back up the chain avoids the situation where OpenOCD "thinks" the
breakpoint is set when in reality an error ocurred.

Item 3 provides a helpful message for a common reason for failure to set sw
breakpoint.

This was thoroughly tested, mindful of the fact that breakpoint management is
somewhat dicey during single-stepping.

Comments and criticisms of course gratefully received.

Mike

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

1 files changed, 5 insertions, 4 deletions

diff --git a/src/target/xscale.c b/src/target/xscale.c
index 2b8bff16..82b0f342 100644
--- a/src/target/xscale.c
+++ b/src/target/xscale.c
@@ -1979,6 +1979,7 @@ static int xscale_write_memory(struct target *target, uint32_t address,
 		if ((retval = xscale_send_u32(target, 0x60)) != ERROR_OK)
 			return retval;
 
+		LOG_ERROR("data abort writing memory");
 		return ERROR_TARGET_DATA_ABORT;
 	}
 
@@ -2141,9 +2142,9 @@ static int xscale_set_breakpoint(struct target *target,
 			breakpoint->set = 2;	/* breakpoint set on second breakpoint register */
 		}
 		else
-		{
+		{	/* bug: availability previously verified in xscale_add_breakpoint() */
 			LOG_ERROR("BUG: no hardware comparator available");
-			return ERROR_OK;
+			return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
 		}
 	}
 	else if (breakpoint->type == BKPT_SOFT)
@@ -2169,7 +2170,7 @@ static int xscale_set_breakpoint(struct target *target,
 				return retval;
 			}
 			/* write the bkpt instruction in target endianness (arm7_9->arm_bkpt is host endian) */
-			if ((retval = target_write_u32(target, breakpoint->address, xscale->thumb_bkpt)) != ERROR_OK)
+			if ((retval = target_write_u16(target, breakpoint->address, xscale->thumb_bkpt)) != ERROR_OK)
 			{
 				return retval;
 			}
@@ -2207,7 +2208,7 @@ static int xscale_add_breakpoint(struct target *target,
 		xscale->ibcr_available--;
 	}
 
-	return ERROR_OK;
+	return xscale_set_breakpoint(target, breakpoint);
 }
 
 static int xscale_unset_breakpoint(struct target *target,