From 438c0e4a4e9a494072a26f222727b91b0ab9ba07 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 19 Mar 2022 00:23:14 -0400 Subject: [PATCH] avr: Cleaned up the breakpoint configuration code, making it more robust --- src/target/atxmega.c | 2 +- src/target/avr.h | 4 ++-- src/target/avr_jtagdp.c | 2 +- src/target/avr_pdi.c | 44 ++++++++++++++++++++++++++--------------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/target/atxmega.c b/src/target/atxmega.c index fe4224288f1..cfc258b80fc 100644 --- a/src/target/atxmega.c +++ b/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; diff --git a/src/target/avr.h b/src/target/avr.h index b43dd51bac1..c23675c41f8 100644 --- a/src/target/avr.h +++ b/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); diff --git a/src/target/avr_jtagdp.c b/src/target/avr_jtagdp.c index 1fc093285f0..a762522cebc 100644 --- a/src/target/avr_jtagdp.c +++ b/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))) { // } diff --git a/src/target/avr_pdi.c b/src/target/avr_pdi.c index f13302253b6..091ba6f06f6 100644 --- a/src/target/avr_pdi.c +++ b/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: