Instead of magic numbers, use their AP_REG_* constants.  Rename
the ROM address symbol as BASE to match ARM's documentation.

Comment various other symbols in the header; add some missing ones.
Remove an unused struct.  Add some doxygen for the DAP structure
and DAP initialization.
---
 src/target/arm_adi_v5.c |   35 ++++++++++++++++++++++++++++-------
 src/target/arm_adi_v5.h |   41 ++++++++++++++++++++++++++++++-----------
 2 files changed, 58 insertions(+), 18 deletions(-)

--- a/src/target/arm_adi_v5.c
+++ b/src/target/arm_adi_v5.c
@@ -950,6 +950,13 @@ int mem_ap_read_buf_u8(struct swjdp_comm
        return retval;
 }
 
+/**
+ * Initialize a DAP.
+ *
+ * @todo Rename this.  We also need an initialization scheme which account
+ * for SWD transports not just JTAG; that will need to address differences
+ * in layering.  (JTAG is useful without any debug target; but not SWD.)
+ */
 int ahbap_debugport_init(struct swjdp_common *swjdp)
 {
        uint32_t idreg, romaddr, dummy;
@@ -959,9 +966,17 @@ int ahbap_debugport_init(struct swjdp_co
 
        LOG_DEBUG(" ");
 
+       /* Default MEM-AP setup.
+        *
+        * REVISIT AP #0 may be an inappropriate default for this.
+        * Should we probe, or receve a hint from the caller?
+        * Presumably we can ignore the possibility of multiple APs.
+        */
        swjdp->apsel = 0;
        swjdp->ap_csw_value = -1;
        swjdp->ap_tar_value = -1;
+
+       /* 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);
@@ -999,8 +1014,14 @@ int ahbap_debugport_init(struct swjdp_co
        dap_dp_write_reg(swjdp, swjdp->dp_ctrl_stat, DP_CTRL_STAT);
        dap_dp_read_reg(swjdp, &dummy, DP_CTRL_STAT);
 
-       dap_ap_read_reg_u32(swjdp, 0xFC, &idreg);
-       dap_ap_read_reg_u32(swjdp, 0xF8, &romaddr);
+       /*
+        * REVISIT this isn't actually *initializing* anything in an AP,
+        * and doesn't care if it's a MEM-AP at all (much less AHB-AP).
+        * 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);
 
        LOG_DEBUG("AHB-AP ID Register 0x%" PRIx32 ", Debug ROM Address 0x%" 
PRIx32 "", idreg, romaddr);
 
@@ -1035,8 +1056,8 @@ int dap_info_command(struct command_cont
 
        apselold = swjdp->apsel;
        dap_ap_select(swjdp, apsel);
-       dap_ap_read_reg_u32(swjdp, 0xF8, &dbgbase);
-       dap_ap_read_reg_u32(swjdp, 0xFC, &apid);
+       dap_ap_read_reg_u32(swjdp, AP_REG_BASE, &dbgbase);
+       dap_ap_read_reg_u32(swjdp, AP_REG_IDR, &apid);
        swjdp_transaction_endcheck(swjdp);
        /* Now we read ROM table ID registers, ref. ARM IHI 0029B sec  */
        mem_ap = ((apid&0x10000) && ((apid&0x0F) != 0));
@@ -1387,7 +1408,7 @@ DAP_COMMAND_HANDLER(dap_baseaddr_command
        if (apselsave != apsel)
                dap_ap_select(swjdp, apsel);
 
-       dap_ap_read_reg_u32(swjdp, 0xF8, &baseaddr);
+       dap_ap_read_reg_u32(swjdp, AP_REG_BASE, &baseaddr);
        retval = swjdp_transaction_endcheck(swjdp);
        command_print(CMD_CTX, "0x%8.8" PRIx32, baseaddr);
 
@@ -1436,7 +1457,7 @@ DAP_COMMAND_HANDLER(dap_apsel_command)
        }
 
        dap_ap_select(swjdp, apsel);
-       dap_ap_read_reg_u32(swjdp, 0xFC, &apid);
+       dap_ap_read_reg_u32(swjdp, AP_REG_IDR, &apid);
        retval = swjdp_transaction_endcheck(swjdp);
        command_print(CMD_CTX, "ap %" PRIi32 " selected, identification 
register 0x%8.8" PRIx32,
                        apsel, apid);
@@ -1464,7 +1485,7 @@ DAP_COMMAND_HANDLER(dap_apid_command)
        if (apselsave != apsel)
                dap_ap_select(swjdp, apsel);
 
-       dap_ap_read_reg_u32(swjdp, 0xFC, &apid);
+       dap_ap_read_reg_u32(swjdp, AP_REG_IDR, &apid);
        retval = swjdp_transaction_endcheck(swjdp);
        command_print(CMD_CTX, "0x%8.8" PRIx32, apid);
        if (apselsave != apsel)
--- a/src/target/arm_adi_v5.h
+++ b/src/target/arm_adi_v5.h
@@ -30,14 +30,22 @@
 
 #define DPAP_WRITE             0
 #define DPAP_READ              1
+
+/* JTAG-DP addresses in DPACC A[3:2] for DP registers  */
 #define DP_ZERO                        0
 #define DP_CTRL_STAT   0x4
 #define DP_SELECT              0x8
 #define DP_RDBUFF              0xC
 
+/* Fields of the DP's CTRL/STAT register */
 #define CORUNDETECT            (1 << 0)
 #define SSTICKYORUN            (1 << 1)
+/* 3:2 - transaction mode (e.g. pushed compare) */
 #define SSTICKYERR             (1 << 5)
+#define READOK                 (1 << 6)
+#define WDATAERR               (1 << 7)
+/* 11:8 - mask lanes for pushed compare or verify ops */
+/* 21:12 - transaction counter */
 #define CDBGRSTREQ             (1 << 26)
 #define CDBGRSTACK             (1 << 27)
 #define CDBGPWRUPREQ   (1 << 28)
@@ -45,26 +53,35 @@
 #define CSYSPWRUPREQ   (1 << 30)
 #define CSYSPWRUPACK   (1 << 31)
 
-#define        AP_REG_CSW              0x00
+/* MEM-AP register addresses */
+/* TODO: rename as MEM_AP_REG_* */
+#define AP_REG_CSW             0x00
 #define AP_REG_TAR             0x04
 #define AP_REG_DRW             0x0C
 #define AP_REG_BD0             0x10
 #define AP_REG_BD1             0x14
 #define AP_REG_BD2             0x18
 #define AP_REG_BD3             0x1C
-#define AP_REG_DBGROMA 0xF8
+#define AP_REG_CFG             0xF4            /* big endian? */
+#define AP_REG_BASE            0xF8
+
+/* Generic AP register address */
 #define AP_REG_IDR             0xFC
 
+/* Fields of the MEM-AP's CSW register */
 #define CSW_8BIT               0
 #define CSW_16BIT              1
 #define CSW_32BIT              2
-
 #define CSW_ADDRINC_MASK       (3 << 4)
 #define CSW_ADDRINC_OFF                0
 #define CSW_ADDRINC_SINGLE     (1 << 4)
 #define CSW_ADDRINC_PACKED     (2 << 4)
-#define CSW_HPROT                      (1 << 25)
-#define CSW_MASTER_DEBUG       (1 << 29)
+#define CSW_DEVICE_EN          (1 << 6)
+#define CSW_TRIN_PROG          (1 << 7)
+#define CSW_SPIDEN                     (1 << 23)
+/* 30:24 - implementation-defined! */
+#define CSW_HPROT                      (1 << 25)               /* ? */
+#define CSW_MASTER_DEBUG       (1 << 29)               /* ? */
 #define CSW_DBGSWENABLE                (1 << 31)
 
 /* transaction mode */
@@ -74,12 +91,14 @@
 /* Freerunning transactions with delays and overrun checking */
 #define TRANS_MODE_COMPOSITE   2
 
-struct swjdp_reg
-{
-       int addr;
-       struct arm_jtag *jtag_info;
-};
-
+/**
+ * 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
+ * transport agent; and at least one Access Port (AP), controlling
+ * resource access.  Most common is a MEM-AP, for memory access.
+ *
+ * @todo Rename "swjdp_common" as "dap".  Use of SWJ-DP is optional!
+ */
 struct swjdp_common
 {
        struct arm_jtag *jtag_info;
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to