avr: Cleaned up the breakpoint configuration code, making it more robust

src/target/atxmega.c

@@ -67,7 +67,7 @@ bool atxmega_probe(target *t)
 			avr_add_flash(t, 0x00000000, 0x40000);
 			avr_add_flash(t, 0x00040000, 0x2000);
 			t->tdesc = tdesc_xmega6;
-			pdi->hw_breakpoint_max = 2;
+			pdi->hw_breakpoints_max = 2;
 			return true;
 	}
 	return false;

src/target/avr.h

@@ -21,8 +21,8 @@ typedef struct avr_pdi_s {
 	target_addr programCounter;
 
 	target_addr hw_breakpoint[AVR_MAX_BREAKPOINTS];
-	uint8_t hw_breakpoint_enabled;
-	uint8_t hw_breakpoint_max;
+	uint32_t hw_breakpoints_enabled;
+	uint32_t hw_breakpoints_max;
 } avr_pdi_t;
 
 bool avr_pdi_init(avr_pdi_t *pdi);

src/target/avr_jtagdp.c

@@ -16,7 +16,7 @@ void avr_jtag_pdi_handler(uint8_t jd_index)
 	pdi->dp_jd_index = jd_index;
 	pdi->idcode = jtag_devs[jd_index].jd_idcode;
 	pdi->error_state = pdi_ok;
-	pdi->hw_breakpoint_enabled = 0;
+	pdi->hw_breakpoints_enabled = 0;
 	if ((PC_HOSTED == 0) || (!platform_avr_jtag_pdi_init(pdi))) {
 		//
 	}

src/target/avr_pdi.c

@@ -370,15 +370,26 @@ static bool avr_ensure_nvm_idle(avr_pdi_t *pdi)
 		avr_pdi_write(pdi, PDI_DATA_8, AVR_ADDR_NVM_DATA, 0xFFU);
 }
 
-static bool avr_config_breakpoints(avr_pdi_t *pdi)
+static bool avr_config_breakpoints(avr_pdi_t *pdi, const bool step)
 {
-	const uint32_t addr_breakpoint_counter = AVR_ADDR_DBG_BREAK_BASE + (pdi->hw_breakpoint_max * 4);
-	const uint16_t breakpoint_count = pdi->hw_breakpoint_enabled << 8U;
-	for (uint8_t i = 0; i < pdi->hw_breakpoint_max; ++i)
+	const uint32_t addr_breakpoint_counter = AVR_ADDR_DBG_BREAK_BASE + (pdi->hw_breakpoints_max * 4);
+	const uint16_t breakpoint_count = step ? 0U : (pdi->hw_breakpoints_enabled << 8U);
+	if (step)
 	{
-		if (!avr_pdi_write(pdi, PDI_DATA_32, AVR_ADDR_DBG_BREAK_BASE + (i * 4),
-				pdi->hw_breakpoint[i] & AVR_DBG_BREAK_MASK))
-			return false;
+		for (uint8_t i = 0; i < pdi->hw_breakpoints_max; ++i)
+		{
+			if (!avr_pdi_write(pdi, PDI_DATA_32, AVR_ADDR_DBG_BREAK_BASE + (i * 4), 0U))
+				return false;
+		}
+	}
+	else
+	{
+		for (uint8_t i = 0; i < pdi->hw_breakpoints_max; ++i)
+		{
+			if (!avr_pdi_write(pdi, PDI_DATA_32, AVR_ADDR_DBG_BREAK_BASE + (i * 4),
+					pdi->hw_breakpoint[i] & AVR_DBG_BREAK_MASK))
+				return false;
+		}
 	}
 	if (!avr_pdi_write(pdi, PDI_DATA_8, AVR_ADDR_DBG_BREAK_UNK1, 0) ||
 		!avr_pdi_write(pdi, PDI_DATA_8, AVR_ADDR_DBG_BREAK_UNK2, 0) ||
@@ -465,13 +476,14 @@ static void avr_halt_resume(target *t, const bool step)
 		 * Write the program counter with the address to resume execution on
 		 */
 		if (avr_pdi_reg_read(pdi, PDI_REG_R3) != 0x04U ||
+			!avr_config_breakpoints(pdi, step) ||
 			!avr_pdi_write(pdi, PDI_DATA_8, AVR_ADDR_DBG_CTRL, 4) ||
 			!avr_pdi_write(pdi, PDI_DATA_32, AVR_ADDR_DBG_CTR, nextPC) ||
 			!avr_pdi_write(pdi, PDI_DATA_32, AVR_ADDR_DBG_PC, currentPC) ||
 			!avr_pdi_reg_write(pdi, PDI_REG_R4, 1) ||
 			avr_pdi_reg_read(pdi, PDI_REG_R3) != 0x04U ||
 			avr_pdi_reg_read(pdi, PDI_REG_STATUS) != 0x06U)
-			raise_exception(EXCEPTION_ERROR, "Error resuming device, device in incorrect state");
+			raise_exception(EXCEPTION_ERROR, "Error stepping device, device in incorrect state");
 		pdi->halt_reason = TARGET_HALT_STEPPING;
 	}
 	else
@@ -484,7 +496,7 @@ static void avr_halt_resume(target *t, const bool step)
 		 * Read r3 to see that the processor is resuming
 		 */
 		if (avr_pdi_reg_read(pdi, PDI_REG_R3) != 0x04U ||
-			!avr_config_breakpoints(pdi) ||
+			!avr_config_breakpoints(pdi, step) ||
 			!avr_pdi_write(pdi, PDI_DATA_32, AVR_ADDR_DBG_PC, pdi->programCounter) ||
 			!avr_pdi_reg_write(pdi, PDI_REG_RESET, 0) ||
 			!avr_pdi_write(pdi, PDI_DATA_8, AVR_ADDR_DBG_CTRL, 0) ||
@@ -608,12 +620,12 @@ static int avr_breakwatch_set(target *t, struct breakwatch *bw)
 	{
 		case TARGET_BREAK_HARD:
 		{
-			const uint8_t bp = pdi->hw_breakpoint_enabled;
-			if (bp == pdi->hw_breakpoint_max)
+			const uint8_t bp = pdi->hw_breakpoints_enabled;
+			if (bp == pdi->hw_breakpoints_max)
 				return -1;
 			pdi->hw_breakpoint[bp] = AVR_DBG_BREAK_ENABLED | bw->addr;
 			bw->reserved[0] = pdi->hw_breakpoint[bp];
-			++pdi->hw_breakpoint_enabled;
+			++pdi->hw_breakpoints_enabled;
 			return 0;
 		}
 		default:
@@ -630,21 +642,21 @@ static int avr_breakwatch_clear(target *t, struct breakwatch *bw)
 		{
 			uint8_t bp = 0;
 			// Locate the breakpoint
-			for (; bp < pdi->hw_breakpoint_max; ++bp)
+			for (; bp < pdi->hw_breakpoints_max; ++bp)
 			{
 				if (pdi->hw_breakpoint[bp] == bw->reserved[0])
 					break;
 			}
 			// Fail if we cannot find it
-			if (bp == pdi->hw_breakpoint_max)
+			if (bp == pdi->hw_breakpoints_max)
 				return -1;
 			// Shuffle the remaining breakpoints.
-			for (; bp < (pdi->hw_breakpoint_enabled - 1U); ++bp)
+			for (; bp < (pdi->hw_breakpoints_enabled - 1U); ++bp)
 				pdi->hw_breakpoint[bp] = pdi->hw_breakpoint[bp + 1];
 			// Cleanup by disabling the breakpoint and fixing the count
 			pdi->hw_breakpoint[bp] = 0;
 			bw->reserved[0] = 0;
-			--pdi->hw_breakpoint_enabled;
+			--pdi->hw_breakpoints_enabled;
 			return 0;
 		}
 		default: