From 3b68a708c2b039d9b091608eccb2206725742a47 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Sun, 21 Feb 2010 14:56:56 -0800 Subject: [PATCH] ADIv5: remove ATOMIC/COMPOSITE interface mode This removes context-sensitivity from the programming interface and makes it possible to know what a block of code does without needing to know the previous history (specifically, the DAP's "trans_mode" setting). The mode was only set to ATOMIC briefly after DAP initialization, making this patch be primarily cleanup; almost everything depends on COMPOSITE. The transactions which shouldn't have been queued were already properly flushing the queue. Signed-off-by: David Brownell --- src/target/arm_adi_v5.c | 111 ++++++++++------------------------------ src/target/arm_adi_v5.h | 12 +---- src/target/cortex_m3.c | 4 -- 3 files changed, 29 insertions(+), 98 deletions(-) diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c index 66b947e7..819dd290 100644 --- a/src/target/arm_adi_v5.c +++ b/src/target/arm_adi_v5.c @@ -43,11 +43,16 @@ * is used to access memory mapped resources and is called a MEM-AP. Also a * JTAG-AP is also defined, bridging to JTAG resources; those are uncommon. * - * @todo Remove modality (queued/nonqueued, via DAP trans_mode) from all - * procedure interfaces. Modal programming interfaces are very error prone. - * Procedures should be either queued, or synchronous. Otherwise input - * and output constraints are context-sensitive, and it's hard to know - * what a block of code will do just by reading it. + * This programming interface allows DAP pipelined operations through a + * transaction queue. This primarily affects AP operations (such as using + * a MEM-AP to access memory or registers). If the current transaction has + * not finished by the time the next one must begin, and the ORUNDETECT bit + * is set in the DP_CTRL_STAT register, the SSTICKYORUN status is set and + * further AP operations will fail. There are two basic methods to avoid + * such overrun errors. One involves polling for status instead of using + * transaction piplining. The other involves adding delays to ensure the + * AP has enough time to complete one operation before starting the next + * one. (For JTAG these delays are controlled by memaccess_tck.) */ /* @@ -67,17 +72,6 @@ #include "arm_adi_v5.h" #include -/* - * Transaction Mode: - * swjdp->trans_mode = TRANS_MODE_COMPOSITE; - * Uses Overrun checking mode and does not do actual JTAG send/receive or transaction - * result checking until swjdp_end_transaction() - * This must be done before using or deallocating any return variables. - * swjdp->trans_mode == TRANS_MODE_ATOMIC - * All reads and writes to the AHB bus are checked for valid completion, and return values - * are immediatley available. -*/ - /* ARM ADI Specification requires at least 10 bits used for TAR autoincrement */ @@ -191,47 +185,32 @@ static int adi_jtag_dp_scan_u32(struct swjdp_common *swjdp, /** * Utility to write AP registers. */ -static int ap_write_check(struct swjdp_common *dap, +static inline int ap_write_check(struct swjdp_common *dap, uint8_t reg_addr, uint8_t *outvalue) { - adi_jtag_dp_scan(dap, JTAG_DP_APACC, reg_addr, DPAP_WRITE, + return adi_jtag_dp_scan(dap, JTAG_DP_APACC, reg_addr, DPAP_WRITE, outvalue, NULL, NULL); - - /* REVISIT except in dap_setup_accessport() almost all call paths - * set up COMPOSITE. Probably worth just inlining the scan... - */ - - /* In TRANS_MODE_ATOMIC all JTAG_DP_APACC transactions wait for - * ack = OK/FAULT and the check CTRL_STAT - */ - if (dap->trans_mode == TRANS_MODE_ATOMIC) - return jtagdp_transaction_endcheck(dap); - - return ERROR_OK; } static int scan_inout_check_u32(struct swjdp_common *swjdp, uint8_t instr, uint8_t reg_addr, uint8_t RnW, uint32_t outvalue, uint32_t *invalue) { + int retval; + /* Issue the read or write */ - adi_jtag_dp_scan_u32(swjdp, instr, reg_addr, RnW, outvalue, NULL, NULL); + retval = adi_jtag_dp_scan_u32(swjdp, instr, reg_addr, + RnW, outvalue, NULL, NULL); + if (retval != ERROR_OK) + return retval; /* For reads, collect posted value; RDBUFF has no other effect. * Assumes read gets acked with OK/FAULT, and CTRL_STAT says "OK". */ if ((RnW == DPAP_READ) && (invalue != NULL)) - adi_jtag_dp_scan_u32(swjdp, JTAG_DP_DPACC, + retval = adi_jtag_dp_scan_u32(swjdp, JTAG_DP_DPACC, DP_RDBUFF, DPAP_READ, 0, invalue, &swjdp->ack); - - /* In TRANS_MODE_ATOMIC all JTAG_DP_APACC transactions wait for - * ack = OK/FAULT and then check CTRL_STAT - */ - if ((instr == JTAG_DP_APACC) - && (swjdp->trans_mode == TRANS_MODE_ATOMIC)) - return jtagdp_transaction_endcheck(swjdp); - - return ERROR_OK; + return retval; } int jtagdp_transaction_endcheck(struct swjdp_common *swjdp) @@ -437,17 +416,13 @@ static int dap_ap_write_reg(struct swjdp_common *swjdp, } /** - * Write an AP register value. - * This is synchronous iff the mode is set to ATOMIC, in which - * case any queued transactions are flushed. + * Asynchronous (queued) AP register write. * * @param swjdp The DAP whose currently selected AP will be written. * @param reg_addr Eight bit AP register address. * @param value Word to be written at reg_addr * - * @return In synchronous mode: ERROR_OK for success, and the register holds - * the specified value; else a fault code. In asynchronous mode, a status - * code reflecting whether the transaction was properly queued. + * @return ERROR_OK if the transaction was properly queued, else a fault code. */ int dap_ap_write_reg_u32(struct swjdp_common *swjdp, uint32_t reg_addr, uint32_t value) @@ -460,17 +435,13 @@ int dap_ap_write_reg_u32(struct swjdp_common *swjdp, } /** - * Read an AP register value. - * This is synchronous iff the mode is set to ATOMIC, in which - * case any queued transactions are flushed. + * Asynchronous (queued) AP register eread. * * @param swjdp The DAP whose currently selected AP will be read. * @param reg_addr Eight bit AP register address. * @param value Points to where the 32-bit (little-endian) word will be stored. * - * @return In synchronous mode: ERROR_OK for success, and *value holds - * the specified value; else a fault code. In asynchronous mode, a status - * code reflecting whether the transaction was properly queued. + * @return ERROR_OK if the transaction was properly queued, else a fault code. */ int dap_ap_read_reg_u32(struct swjdp_common *swjdp, uint32_t reg_addr, uint32_t *value) @@ -486,9 +457,8 @@ int dap_ap_read_reg_u32(struct swjdp_common *swjdp, } /** - * Set up transfer parameters for the currently selected MEM-AP. - * This is synchronous iff the mode is set to ATOMIC, in which - * case any queued transactions are flushed. + * Queue transactions setting up transfer parameters for the + * currently selected MEM-AP. * * Subsequent transfers using registers like AP_REG_DRW or AP_REG_BD2 * initiate data reads or writes using memory or peripheral addresses. @@ -503,9 +473,7 @@ int dap_ap_read_reg_u32(struct swjdp_common *swjdp, * @param tar MEM-AP Transfer Address Register (TAR) to assign. If this * matches the cached address, the register is not changed. * - * @return In synchronous mode: ERROR_OK for success, and the AP is set - * up as requested else a fault code. In asynchronous mode, a status - * code reflecting whether the transaction was properly queued. + * @return ERROR_OK if the transaction was properly queued, else a fault code. */ int dap_setup_accessport(struct swjdp_common *swjdp, uint32_t csw, uint32_t tar) { @@ -550,8 +518,6 @@ int mem_ap_read_u32(struct swjdp_common *swjdp, uint32_t address, { int retval; - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - /* Use banked addressing (REG_BDx) to avoid some link traffic * (updating TAR) when reading several consecutive addresses. */ @@ -603,8 +569,6 @@ int mem_ap_write_u32(struct swjdp_common *swjdp, uint32_t address, { int retval; - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - /* Use banked addressing (REG_BDx) to avoid some link traffic * (updating TAR) when writing several consecutive addresses. */ @@ -652,8 +616,6 @@ int mem_ap_write_buf_u32(struct swjdp_common *swjdp, uint8_t *buffer, int count, uint32_t adr = address; uint8_t* pBuffer = buffer; - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - count >>= 2; wcount = count; @@ -721,8 +683,6 @@ static int mem_ap_write_buf_packed_u16(struct swjdp_common *swjdp, int retval = ERROR_OK; int wcount, blocksize, writecount, i; - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - wcount = count >> 1; while (wcount > 0) @@ -799,8 +759,6 @@ int mem_ap_write_buf_u16(struct swjdp_common *swjdp, uint8_t *buffer, int count, if (count >= 4) return mem_ap_write_buf_packed_u16(swjdp, buffer, count, address); - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - while (count > 0) { dap_setup_accessport(swjdp, CSW_16BIT | CSW_ADDRINC_SINGLE, address); @@ -823,8 +781,6 @@ static int mem_ap_write_buf_packed_u8(struct swjdp_common *swjdp, int retval = ERROR_OK; int wcount, blocksize, writecount, i; - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - wcount = count; while (wcount > 0) @@ -896,8 +852,6 @@ int mem_ap_write_buf_u8(struct swjdp_common *swjdp, uint8_t *buffer, int count, if (count >= 4) return mem_ap_write_buf_packed_u8(swjdp, buffer, count, address); - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - while (count > 0) { dap_setup_accessport(swjdp, CSW_8BIT | CSW_ADDRINC_SINGLE, address); @@ -925,8 +879,6 @@ int mem_ap_read_buf_u32(struct swjdp_common *swjdp, uint8_t *buffer, int count, uint32_t adr = address; uint8_t* pBuffer = buffer; - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - count >>= 2; wcount = count; @@ -1009,8 +961,6 @@ static int mem_ap_read_buf_packed_u16(struct swjdp_common *swjdp, int retval = ERROR_OK; int wcount, blocksize, readcount, i; - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - wcount = count >> 1; while (wcount > 0) @@ -1063,8 +1013,6 @@ int mem_ap_read_buf_u16(struct swjdp_common *swjdp, uint8_t *buffer, int count, if (count >= 4) return mem_ap_read_buf_packed_u16(swjdp, buffer, count, address); - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - while (count > 0) { dap_setup_accessport(swjdp, CSW_16BIT | CSW_ADDRINC_SINGLE, address); @@ -1105,8 +1053,6 @@ static int mem_ap_read_buf_packed_u8(struct swjdp_common *swjdp, int retval = ERROR_OK; int wcount, blocksize, readcount, i; - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - wcount = count; while (wcount > 0) @@ -1156,8 +1102,6 @@ int mem_ap_read_buf_u8(struct swjdp_common *swjdp, uint8_t *buffer, int count, u if (count >= 4) return mem_ap_read_buf_packed_u8(swjdp, buffer, count, address); - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - while (count > 0) { dap_setup_accessport(swjdp, CSW_8BIT | CSW_ADDRINC_SINGLE, address); @@ -1203,7 +1147,6 @@ int ahbap_debugport_init(struct swjdp_common *swjdp) dap_ap_select(swjdp, 0); /* DP initialization */ - swjdp->trans_mode = TRANS_MODE_ATOMIC; dap_dp_read_reg(swjdp, &dummy, DP_CTRL_STAT); dap_dp_write_reg(swjdp, SSTICKYERR, DP_CTRL_STAT); dap_dp_read_reg(swjdp, &dummy, DP_CTRL_STAT); diff --git a/src/target/arm_adi_v5.h b/src/target/arm_adi_v5.h index 746f1cb6..316701e5 100644 --- a/src/target/arm_adi_v5.h +++ b/src/target/arm_adi_v5.h @@ -118,13 +118,6 @@ #define CSW_MASTER_DEBUG (1 << 29) /* ? */ #define CSW_DBGSWENABLE (1 << 31) -/* transaction mode */ -#define TRANS_MODE_NONE 0 -/* Transaction waits for previous to complete */ -#define TRANS_MODE_ATOMIC 1 -/* Freerunning transactions with delays and overrun checking */ -#define TRANS_MODE_COMPOSITE 2 - /** * This represents an ARM Debug Interface (v5) Debug Access Port (DAP). * A DAP has two types of component: one Debug Port (DP), which is a @@ -170,9 +163,8 @@ struct swjdp_common uint32_t ap_tar_value; /* information about current pending SWjDP-AHBAP transaction */ - uint8_t trans_mode; - uint8_t trans_rw; uint8_t ack; + /** * Configures how many extra tck clocks are added after starting a * MEM-AP access before we try to read its status (and/or result). @@ -192,7 +184,7 @@ static inline uint8_t dap_ap_get_select(struct swjdp_common *swjdp) /* AP selection applies to future AP transactions */ void dap_ap_select(struct swjdp_common *dap,uint8_t apsel); -/* AP transactions ... synchronous given TRANS_MODE_ATOMIC */ +/* Queued AP transactions */ int dap_setup_accessport(struct swjdp_common *swjdp, uint32_t csw, uint32_t tar); int dap_ap_write_reg_u32(struct swjdp_common *swjdp, diff --git a/src/target/cortex_m3.c b/src/target/cortex_m3.c index 3dd94685..3ebc34ad 100644 --- a/src/target/cortex_m3.c +++ b/src/target/cortex_m3.c @@ -70,8 +70,6 @@ static int cortexm3_dap_read_coreregister_u32(struct swjdp_common *swjdp, mem_ap_read_u32(swjdp, DCB_DCRDR, &dcrdr); - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - /* mem_ap_write_u32(swjdp, DCB_DCRSR, regnum); */ dap_setup_accessport(swjdp, CSW_32BIT | CSW_ADDRINC_OFF, DCB_DCRSR & 0xFFFFFFF0); dap_ap_write_reg_u32(swjdp, AP_REG_BD0 | (DCB_DCRSR & 0xC), regnum); @@ -101,8 +99,6 @@ static int cortexm3_dap_write_coreregister_u32(struct swjdp_common *swjdp, mem_ap_read_u32(swjdp, DCB_DCRDR, &dcrdr); - swjdp->trans_mode = TRANS_MODE_COMPOSITE; - /* mem_ap_write_u32(swjdp, DCB_DCRDR, core_regs[i]); */ dap_setup_accessport(swjdp, CSW_32BIT | CSW_ADDRINC_OFF, DCB_DCRDR & 0xFFFFFFF0); dap_ap_write_reg_u32(swjdp, AP_REG_BD0 | (DCB_DCRDR & 0xC), value); -- 2.39.2