Code Review / openocd.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | review | tree
raw | patch | inline | side by side (from parent 1: 61ee632)
ADIv5: use new DAP ops for AP read/write
authorDavid Brownell <dbrownell@users.sourceforge.net>
Wed, 3 Mar 2010 06:49:36 +0000 (22:49 -0800)
committerDavid Brownell <dbrownell@users.sourceforge.net>
Wed, 3 Mar 2010 06:49:36 +0000 (22:49 -0800)
Make ADIv5 internals use the two new transport-neutral calls for reading
and writing DP registers; and do the same for external callers.  Also,
bugfix some of their call sites to handle the fault returns, instead of
ignoring them.

Remove most of the JTAG-specific calls, using their code as the bodies
of the JTAG-specific implementation for the new methods.

NOTE that there's a remaining issue:  mem_ap_read_buf_u32() makes calls
which are JTAG-specific.  A later patch will need to remove those, so
JTAG-specific operations can be removed from this file, and so that SWD
support will be able to properly drop in as just a transport layer to the
ADIv5 infrastructure.  (The way read results are posted may need some more
attention in the transport-neutrality interface.)

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
src/target/arm_adi_v5.c patch | blob | history
src/target/arm_adi_v5.h patch | blob | history
src/target/cortex_m3.c patch | blob | history

diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c
index 2d9ea0daedd491368e034c888ce7b1b1c63aefba..c5e0dd38a097b3500ad0f2f55f55e47906ed78af 100644 (file)
--- a/src/target/arm_adi_v5.c
+++ b/src/target/arm_adi_v5.c
@@ -328,8 +328,16 @@ static int jtagdp_transaction_endcheck(struct swjdp_common *swjdp)
 
                        LOG_DEBUG("jtag-dp: CTRL/STAT 0x%" PRIx32, ctrlstat);
 
-                       dap_ap_read_reg_u32(swjdp, AP_REG_CSW, &mem_ap_csw);
-                       dap_ap_read_reg_u32(swjdp, AP_REG_TAR, &mem_ap_tar);
+                       retval = dap_queue_ap_read(swjdp,
+                                       AP_REG_CSW, &mem_ap_csw);
+                       if (retval != ERROR_OK)
+                               return retval;
+
+                       retval = dap_queue_ap_read(swjdp,
+                                       AP_REG_TAR, &mem_ap_tar);
+                       if (retval != ERROR_OK)
+                               return retval;
+
                        if ((retval = dap_run(swjdp)) != ERROR_OK)
                                return retval;
                        LOG_ERROR("MEM_AP_CSW 0x%" PRIx32 ", MEM_AP_TAR 0x%"
@@ -375,79 +383,6 @@ void dap_ap_select(struct swjdp_common *swjdp,uint8_t apsel)
        }
 }
 
-/** Select the AP register bank matching bits 7:4 of ap_reg. */
-static int dap_ap_bankselect(struct swjdp_common *swjdp, uint32_t ap_reg)
-{
-       uint32_t select = (ap_reg & 0x000000F0);
-
-       if (select != swjdp->ap_bank_value)
-       {
-               swjdp->ap_bank_value = select;
-               select |= swjdp->apsel;
-               return dap_queue_dp_write(swjdp, DP_SELECT, select);
-       } else
-               return ERROR_OK;
-}
-
-/* FIXME remove dap_ap_{read,write}_reg() and dap_ap_write_reg_u32()
- * ... these should become the bodies of the JTAG implementations of
- * dap_queue_ap_{read,write}(), then all their current callers should
- * switch over to the transport-neutral calls.
- */
-
-static int dap_ap_write_reg(struct swjdp_common *swjdp,
-               uint32_t reg_addr, uint8_t *out_value_buf)
-{
-       int retval;
-
-       retval = dap_ap_bankselect(swjdp, reg_addr);
-       if (retval != ERROR_OK)
-               return retval;
-
-       return adi_jtag_ap_write_check(swjdp, reg_addr, out_value_buf);
-}
-
-/**
- * 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 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)
-{
-       uint8_t out_value_buf[4];
-
-       buf_set_u32(out_value_buf, 0, 32, value);
-       return dap_ap_write_reg(swjdp,
-                       reg_addr, out_value_buf);
-}
-
-/**
- * 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 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)
-{
-       int retval;
-
-       retval = dap_ap_bankselect(swjdp, reg_addr);
-       if (retval != ERROR_OK)
-               return retval;
-
-       return adi_jtag_scan_inout_check_u32(swjdp, JTAG_DP_APACC, reg_addr,
-                       DPAP_READ, 0, value);
-}
-
 /**
  * Queue transactions setting up transfer parameters for the
  * currently selected MEM-AP.
@@ -475,7 +410,7 @@ int dap_setup_accessport(struct swjdp_common *swjdp, uint32_t csw, uint32_t tar)
        if (csw != swjdp->ap_csw_value)
        {
                /* LOG_DEBUG("DAP: Set CSW %x",csw); */
-               retval = dap_ap_write_reg_u32(swjdp, AP_REG_CSW, csw);
+               retval = dap_queue_ap_write(swjdp, AP_REG_CSW, csw);
                if (retval != ERROR_OK)
                        return retval;
                swjdp->ap_csw_value = csw;
@@ -483,7 +418,7 @@ int dap_setup_accessport(struct swjdp_common *swjdp, uint32_t csw, uint32_t tar)
        if (tar != swjdp->ap_tar_value)
        {
                /* LOG_DEBUG("DAP: Set TAR %x",tar); */
-               retval = dap_ap_write_reg_u32(swjdp, AP_REG_TAR, tar);
+               retval = dap_queue_ap_write(swjdp, AP_REG_TAR, tar);
                if (retval != ERROR_OK)
                        return retval;
                swjdp->ap_tar_value = tar;
@@ -518,7 +453,7 @@ int mem_ap_read_u32(struct swjdp_common *swjdp, uint32_t address,
        if (retval != ERROR_OK)
                return retval;
 
-       return dap_ap_read_reg_u32(swjdp, AP_REG_BD0 | (address & 0xC), value);
+       return dap_queue_ap_read(swjdp, AP_REG_BD0 | (address & 0xC), value);
 }
 
 /**
@@ -569,7 +504,7 @@ int mem_ap_write_u32(struct swjdp_common *swjdp, uint32_t address,
        if (retval != ERROR_OK)
                return retval;
 
-       return dap_ap_write_reg_u32(swjdp, AP_REG_BD0 | (address & 0xC),
+       return dap_queue_ap_write(swjdp, AP_REG_BD0 | (address & 0xC),
                        value);
 }
 
@@ -645,7 +580,10 @@ int mem_ap_write_buf_u32(struct swjdp_common *swjdp, uint8_t *buffer, int count,
 
                for (writecount = 0; writecount < blocksize; writecount++)
                {
-                       dap_ap_write_reg(swjdp, AP_REG_DRW, buffer + 4 * writecount);
+                       retval = dap_queue_ap_write(swjdp, AP_REG_DRW,
+                               *(uint32_t *) (buffer + 4 * writecount));
+                       if (retval != ERROR_OK)
+                               break;
                }
 
                if (dap_run(swjdp) == ERROR_OK)
@@ -725,7 +663,11 @@ static int mem_ap_write_buf_packed_u16(struct swjdp_common *swjdp,
                                }
 
                                memcpy(&outvalue, buffer, sizeof(uint32_t));
-                               dap_ap_write_reg_u32(swjdp, AP_REG_DRW, outvalue);
+                               retval = dap_queue_ap_write(swjdp,
+                                               AP_REG_DRW, outvalue);
+                               if (retval != ERROR_OK)
+                                       break;
+
                                if (dap_run(swjdp) != ERROR_OK)
                                {
                                        LOG_WARNING("Block write error address "
@@ -759,7 +701,10 @@ int mem_ap_write_buf_u16(struct swjdp_common *swjdp, uint8_t *buffer, int count,
                uint16_t svalue;
                memcpy(&svalue, buffer, sizeof(uint16_t));
                uint32_t outvalue = (uint32_t)svalue << 8 * (address & 0x3);
-               dap_ap_write_reg_u32(swjdp, AP_REG_DRW, outvalue);
+               retval = dap_queue_ap_write(swjdp, AP_REG_DRW, outvalue);
+               if (retval != ERROR_OK)
+                       break;
+
                retval = dap_run(swjdp);
                if (retval != ERROR_OK)
                        break;
@@ -822,7 +767,11 @@ static int mem_ap_write_buf_packed_u8(struct swjdp_common *swjdp,
                                }
 
                                memcpy(&outvalue, buffer, sizeof(uint32_t));
-                               dap_ap_write_reg_u32(swjdp, AP_REG_DRW, outvalue);
+                               retval = dap_queue_ap_write(swjdp,
+                                               AP_REG_DRW, outvalue);
+                               if (retval != ERROR_OK)
+                                       break;
+
                                if (dap_run(swjdp) != ERROR_OK)
                                {
                                        LOG_WARNING("Block write error address "
@@ -854,7 +803,10 @@ int mem_ap_write_buf_u8(struct swjdp_common *swjdp, uint8_t *buffer, int count,
        {
                dap_setup_accessport(swjdp, CSW_8BIT | CSW_ADDRINC_SINGLE, address);
                uint32_t outvalue = (uint32_t)*buffer << 8 * (address & 0x3);
-               dap_ap_write_reg_u32(swjdp, AP_REG_DRW, outvalue);
+               retval = dap_queue_ap_write(swjdp, AP_REG_DRW, outvalue);
+               if (retval != ERROR_OK)
+                       break;
+
                retval = dap_run(swjdp);
                if (retval != ERROR_OK)
                        break;
@@ -903,6 +855,13 @@ int mem_ap_read_buf_u32(struct swjdp_common *swjdp, uint8_t *buffer,
                dap_setup_accessport(swjdp, CSW_32BIT | CSW_ADDRINC_SINGLE,
                                address);
 
+               /* FIXME remove these three calls to adi_jtag_dp_scan(),
+                * so this routine becomes transport-neutral.  Be careful
+                * not to cause performance problems with JTAG; would it
+                * suffice to loop over dap_queue_ap_read(), or would that
+                * be slower when JTAG is the chosen transport?
+                */
+
                /* Scan out first read */
                adi_jtag_dp_scan(swjdp, JTAG_DP_APACC, AP_REG_DRW,
                                DPAP_READ, 0, NULL, NULL);
@@ -992,7 +951,7 @@ static int mem_ap_read_buf_packed_u16(struct swjdp_common *swjdp,
 
                do
                {
-                       dap_ap_read_reg_u32(swjdp, AP_REG_DRW, &invalue);
+                       retval = dap_queue_ap_read(swjdp, AP_REG_DRW, &invalue);
                        if (dap_run(swjdp) != ERROR_OK)
                        {
                                LOG_WARNING("Block read error address 0x%" PRIx32 ", count 0x%x", address, count);
@@ -1037,7 +996,10 @@ int mem_ap_read_buf_u16(struct swjdp_common *swjdp, uint8_t *buffer,
        while (count > 0)
        {
                dap_setup_accessport(swjdp, CSW_16BIT | CSW_ADDRINC_SINGLE, address);
-               dap_ap_read_reg_u32(swjdp, AP_REG_DRW, &invalue);
+               retval = dap_queue_ap_read(swjdp, AP_REG_DRW, &invalue);
+               if (retval != ERROR_OK)
+                       break;
+
                retval = dap_run(swjdp);
                if (retval != ERROR_OK)
                        break;
@@ -1094,7 +1056,7 @@ static int mem_ap_read_buf_packed_u8(struct swjdp_common *swjdp,
 
                do
                {
-                       dap_ap_read_reg_u32(swjdp, AP_REG_DRW, &invalue);
+                       retval = dap_queue_ap_read(swjdp, AP_REG_DRW, &invalue);
                        if (dap_run(swjdp) != ERROR_OK)
                        {
                                LOG_WARNING("Block read error address 0x%" PRIx32 ", count 0x%x", address, count);
@@ -1139,7 +1101,7 @@ int mem_ap_read_buf_u8(struct swjdp_common *swjdp, uint8_t *buffer,
        while (count > 0)
        {
                dap_setup_accessport(swjdp, CSW_8BIT | CSW_ADDRINC_SINGLE, address);
-               dap_ap_read_reg_u32(swjdp, AP_REG_DRW, &invalue);
+               retval = dap_queue_ap_read(swjdp, AP_REG_DRW, &invalue);
                retval = dap_run(swjdp);
                if (retval != ERROR_OK)
                        break;
@@ -1199,6 +1161,7 @@ static int jtag_dp_q_write(struct swjdp_common *dap, unsigned reg,
                        reg, DPAP_WRITE, data, NULL);
 }
 
+/** Select the AP register bank matching bits 7:4 of reg. */
 static int jtag_ap_q_bankselect(struct swjdp_common *dap, unsigned reg)
 {
        uint32_t select = reg & 0x000000F0;
@@ -1219,17 +1182,23 @@ static int jtag_ap_q_read(struct swjdp_common *dap, unsigned reg,
 
        if (retval != ERROR_OK)
                return retval;
-       return dap_ap_read_reg_u32(dap, reg, data);
+
+       return adi_jtag_scan_inout_check_u32(dap, JTAG_DP_APACC, reg,
+                       DPAP_READ, 0, data);
 }
 
 static int jtag_ap_q_write(struct swjdp_common *dap, unsigned reg,
                uint32_t data)
 {
-       int retval = jtag_ap_q_bankselect(dap, reg);
+       uint8_t out_value_buf[4];
 
+       int retval = jtag_ap_q_bankselect(dap, reg);
        if (retval != ERROR_OK)
                return retval;
-       return dap_ap_write_reg_u32(dap, reg, data);
+
+       buf_set_u32(out_value_buf, 0, 32, data);
+
+       return adi_jtag_ap_write_check(dap, reg, out_value_buf);
 }
 
 static int jtag_ap_q_abort(struct swjdp_common *dap, uint8_t *ack)
@@ -1355,8 +1324,8 @@ int ahbap_debugport_init(struct swjdp_common *swjdp)
         * Should it?  If the ROM address is valid, is this the right
         * place to scan the table and do any topology detection?
         */
-       dap_ap_read_reg_u32(swjdp, AP_REG_IDR, &idreg);
-       dap_ap_read_reg_u32(swjdp, AP_REG_BASE, &romaddr);
+       retval = dap_queue_ap_read(swjdp, AP_REG_IDR, &idreg);
+       retval = dap_queue_ap_read(swjdp, AP_REG_BASE, &romaddr);
 
        LOG_DEBUG("MEM-AP #%d ID Register 0x%" PRIx32
                ", Debug ROM Address 0x%" PRIx32,
@@ -1398,8 +1367,8 @@ int dap_info_command(struct command_context *cmd_ctx,
 
        apselold = swjdp->apsel;
        dap_ap_select(swjdp, apsel);
-       dap_ap_read_reg_u32(swjdp, AP_REG_BASE, &dbgbase);
-       dap_ap_read_reg_u32(swjdp, AP_REG_IDR, &apid);
+       retval = dap_queue_ap_read(swjdp, AP_REG_BASE, &dbgbase);
+       retval = dap_queue_ap_read(swjdp, AP_REG_IDR, &apid);
        retval = dap_run(swjdp);
        if (retval != ERROR_OK)
                return retval;
@@ -1770,7 +1739,7 @@ DAP_COMMAND_HANDLER(dap_baseaddr_command)
         * though they're not common for now.  This should
         * use the ID register to verify it's a MEM-AP.
         */
-       dap_ap_read_reg_u32(swjdp, AP_REG_BASE, &baseaddr);
+       retval = dap_queue_ap_read(swjdp, AP_REG_BASE, &baseaddr);
        retval = dap_run(swjdp);
        if (retval != ERROR_OK)
                return retval;
@@ -1825,7 +1794,7 @@ DAP_COMMAND_HANDLER(dap_apsel_command)
        }
 
        dap_ap_select(swjdp, apsel);
-       dap_ap_read_reg_u32(swjdp, AP_REG_IDR, &apid);
+       retval = dap_queue_ap_read(swjdp, AP_REG_IDR, &apid);
        retval = dap_run(swjdp);
        if (retval != ERROR_OK)
                return retval;
@@ -1859,7 +1828,7 @@ DAP_COMMAND_HANDLER(dap_apid_command)
        if (apselsave != apsel)
                dap_ap_select(swjdp, apsel);
 
-       dap_ap_read_reg_u32(swjdp, AP_REG_IDR, &apid);
+       retval = dap_queue_ap_read(swjdp, AP_REG_IDR, &apid);
        retval = dap_run(swjdp);
        if (retval != ERROR_OK)
                return retval;
diff --git a/src/target/arm_adi_v5.h b/src/target/arm_adi_v5.h
index e71feb96a368c87cfa431863c09636c663cf18bc..e867b85504bb209a9fe1a9af550b1e1857917f68 100644 (file)
--- a/src/target/arm_adi_v5.h
+++ b/src/target/arm_adi_v5.h
@@ -350,10 +350,6 @@ void dap_ap_select(struct swjdp_common *dap,uint8_t apsel);
 /* 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,
-               uint32_t addr, uint32_t value);
-int dap_ap_read_reg_u32(struct swjdp_common *swjdp,
-               uint32_t addr, uint32_t *value);
 
 /* Queued MEM-AP memory mapped single word transfers */
 int mem_ap_read_u32(struct swjdp_common *swjdp, uint32_t address, uint32_t *value);
diff --git a/src/target/cortex_m3.c b/src/target/cortex_m3.c
index 7aec01513637bfe1653b9ae218d3a6577b48e764..3178ce3b40953c7da8ea8611b9b97d3ce539163b 100644 (file)
--- a/src/target/cortex_m3.c
+++ b/src/target/cortex_m3.c
@@ -72,13 +72,19 @@ static int cortexm3_dap_read_coreregister_u32(struct swjdp_common *swjdp,
 
        /* 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);
+       retval = dap_queue_ap_write(swjdp, AP_REG_BD0 | (DCB_DCRSR & 0xC), regnum);
+       if (retval != ERROR_OK)
+               return retval;
 
        /* mem_ap_read_u32(swjdp, DCB_DCRDR, value); */
        dap_setup_accessport(swjdp, CSW_32BIT | CSW_ADDRINC_OFF, DCB_DCRDR & 0xFFFFFFF0);
-       dap_ap_read_reg_u32(swjdp, AP_REG_BD0 | (DCB_DCRDR & 0xC), value);
+       retval = dap_queue_ap_read(swjdp, AP_REG_BD0 | (DCB_DCRDR & 0xC), value);
+       if (retval != ERROR_OK)
+               return retval;
 
        retval = dap_run(swjdp);
+       if (retval != ERROR_OK)
+               return retval;
 
        /* restore DCB_DCRDR - this needs to be in a seperate
         * transaction otherwise the emulated DCC channel breaks */
@@ -101,11 +107,13 @@ static int cortexm3_dap_write_coreregister_u32(struct swjdp_common *swjdp,
 
        /* 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);
+       retval = dap_queue_ap_write(swjdp, AP_REG_BD0 | (DCB_DCRDR & 0xC), value);
+       // XXX check retval
 
        /* mem_ap_write_u32(swjdp, DCB_DCRSR, i | DCRSR_WnR); */
        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 | DCRSR_WnR);
+       retval = dap_queue_ap_write(swjdp, AP_REG_BD0 | (DCB_DCRSR & 0xC), regnum | DCRSR_WnR);
+       // XXX check retval
 
        retval = dap_run(swjdp);
 
Open On-Chip Debugger

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)