ADIv5 clean up AP selection and register caching

Handling of AP (and AP register bank) selection, and cached AP
registers, is pretty loose ... start tightening it:

 - It's "AP bank" select support ... there are no DP banks.  Rename.
   + dap_dp_bankselect() becomes dap_ap_bankselect()
   + "dp_select_value" struct field becomes "ap_bank_value"

 - Remove duplicate AP cache init paths ... only use dap_ap_select(),
 and don't make Cortex (A8 or M3) cores roll their own code.

 - For dap_ap_bankselect(), pass up any fault code from writing
 the SELECT register.  (Nothing yet checks those codes.)

 - Add various bits of Doxygen

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c
index 94c8ed8f8f01a8dff35cfb1b53df61a237f9e3ee..2e3dafb93d008b1554e5cac1a068644f431383b2 100644 (file)
--- a/src/target/arm_adi_v5.c
+++ b/src/target/arm_adi_v5.c
@@ -349,7 +349,7 @@ int jtagdp_transaction_endcheck(struct swjdp_common *swjdp)
                                        "ap_bank 0x%" PRIx32
                                        ", ap_csw 0x%" PRIx32
                                        ", ap_tar 0x%" PRIx32,
-                                       swjdp->dp_select_value,
+                                       swjdp->ap_bank_value,
                                        swjdp->ap_csw_value,
                                        swjdp->ap_tar_value);
 
@@ -419,38 +419,38 @@ static int dap_dp_read_reg(struct swjdp_common *swjdp,
  */
 void dap_ap_select(struct swjdp_common *swjdp,uint8_t apsel)
 {
-       uint32_t select;
-       select = (apsel << 24) & 0xFF000000;
+       uint32_t select = (apsel << 24) & 0xFF000000;
 
        if (select != swjdp->apsel)
        {
                swjdp->apsel = select;
-               /* Switching AP invalidates cached values */
-               swjdp->dp_select_value = -1;
+               /* Switching AP invalidates cached values.
+                * Values MUST BE UPDATED BEFORE AP ACCESS.
+                */
+               swjdp->ap_bank_value = -1;
                swjdp->ap_csw_value = -1;
                swjdp->ap_tar_value = -1;
        }
 }
 
-static int dap_dp_bankselect(struct swjdp_common *swjdp, uint32_t ap_reg)
+/** 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;
-       select = (ap_reg & 0x000000F0);
+       uint32_t select = (ap_reg & 0x000000F0);
 
-       if (select != swjdp->dp_select_value)
+       if (select != swjdp->ap_bank_value)
        {
-               dap_dp_write_reg(swjdp, select | swjdp->apsel, DP_SELECT);
-               swjdp->dp_select_value = select;
-       }
-
-       /* FIXME return any fault code from write() call */
-       return ERROR_OK;
+               swjdp->ap_bank_value = select;
+               select |= swjdp->apsel;
+               return dap_dp_write_reg(swjdp, select, DP_SELECT);
+       } else
+               return ERROR_OK;
 }
 
 static int dap_ap_write_reg(struct swjdp_common *swjdp,
                uint32_t reg_addr, uint8_t *out_value_buf)
 {
-       dap_dp_bankselect(swjdp, reg_addr);
+       dap_ap_bankselect(swjdp, reg_addr);
        scan_inout_check(swjdp, JTAG_DP_APACC, reg_addr,
                        DPAP_WRITE, out_value_buf, NULL);
 
@@ -477,7 +477,7 @@ int dap_ap_write_reg_u32(struct swjdp_common *swjdp,
        uint8_t out_value_buf[4];
 
        buf_set_u32(out_value_buf, 0, 32, value);
-       dap_dp_bankselect(swjdp, reg_addr);
+       dap_ap_bankselect(swjdp, reg_addr);
        scan_inout_check(swjdp, JTAG_DP_APACC, reg_addr,
                        DPAP_WRITE, out_value_buf, NULL);
 
@@ -501,7 +501,7 @@ int dap_ap_write_reg_u32(struct swjdp_common *swjdp,
 int dap_ap_read_reg_u32(struct swjdp_common *swjdp,
                uint32_t reg_addr, uint32_t *value)
 {
-       dap_dp_bankselect(swjdp, reg_addr);
+       dap_ap_bankselect(swjdp, reg_addr);
        scan_inout_check_u32(swjdp, JTAG_DP_APACC, reg_addr,
                        DPAP_READ, 0, value);
 
@@ -1206,12 +1206,11 @@ int ahbap_debugport_init(struct swjdp_common *swjdp)
        /* Default MEM-AP setup.
         *
         * REVISIT AP #0 may be an inappropriate default for this.
-        * Should we probe, or receve a hint from the caller?
+        * Should we probe, or take 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;
+       swjdp->apsel = !0;
+       dap_ap_select(swjdp, 0);
 
        /* DP initialization */
        swjdp->trans_mode = TRANS_MODE_ATOMIC;

diff --git a/src/target/arm_adi_v5.h b/src/target/arm_adi_v5.h
index 759f2333a6fde8110824d6e0ebfce238ee0e32c5..746f1cb6541b25d6b89bae9912dc810e2ba08ab7 100644 (file)
--- a/src/target/arm_adi_v5.h
+++ b/src/target/arm_adi_v5.h
@@ -138,17 +138,45 @@ struct swjdp_common
        struct arm_jtag *jtag_info;
        /* Control config */
        uint32_t dp_ctrl_stat;
-       /* Support for several AP's in one DAP */
+
+       /**
+        * Cache for DP_SELECT bits identifying the current AP.  A DAP may
+        * connect to multiple APs, such as one MEM-AP for general access,
+        * another reserved for accessing debug modules, and a JTAG-DP.
+        * "-1" indicates no cached value.
+        */
        uint32_t apsel;
-       /* Register select cache */
-       uint32_t dp_select_value;
+
+       /**
+        * Cache for DP_SELECT bits identifying the current four-word AP
+        * register bank.  This caches AP register addresss bits 7:4; JTAG
+        * and SWD access primitves pass address bits 3:2; bits 1:0 are zero.
+        * "-1" indicates no cached value.
+        */
+       uint32_t ap_bank_value;
+
+       /**
+        * Cache for (MEM-AP) AP_REG_CSW register value.  This is written to
+        * configure an access mode, such as autoincrementing AP_REG_TAR during
+        * word access.  "-1" indicates no cached value.
+        */
        uint32_t ap_csw_value;
+
+       /**
+        * Cache for (MEM-AP) AP_REG_TAR register value This is written to
+        * configure the address being read or written
+        * "-1" indicates no cached value.
+        */
        uint32_t ap_tar_value;
+
        /* information about current pending SWjDP-AHBAP transaction */
        uint8_t  trans_mode;
        uint8_t  trans_rw;
        uint8_t  ack;
-       /* extra tck clocks for memory bus access */
+       /**
+        * Configures how many extra tck clocks are added after starting a
+        * MEM-AP access before we try to read its status (and/or result).
+        */
        uint32_t        memaccess_tck;
        /* Size of TAR autoincrement block, ARM ADI Specification requires at least 10 bits */
        uint32_t tar_autoincr_block;

diff --git a/src/target/cortex_a8.c b/src/target/cortex_a8.c
index 050238ce2df962f7f2ef530a933843d1061e44d8..f4818f8d0f2146a81a5869bb4df5890bd76f1ad1 100644 (file)
--- a/src/target/cortex_a8.c
+++ b/src/target/cortex_a8.c
@@ -53,7 +53,9 @@ static int cortex_a8_dap_write_coreregister_u32(struct target *target,
                uint32_t value, int regnum);
 /*
  * FIXME do topology discovery using the ROM; don't
- * assume this is an OMAP3.
+ * assume this is an OMAP3.   Also, allow for multiple ARMv7-A
+ * cores, with different AP numbering ... don't use a #define
+ * for these numbers, use per-core armv7a state.
  */
 #define swjdp_memoryap 0
 #define swjdp_debugap 1
@@ -1570,9 +1572,7 @@ static int cortex_a8_init_arch_info(struct target *target,
        cortex_a8->jtag_info.tap = tap;
        cortex_a8->jtag_info.scann_size = 4;
 
-       swjdp->dp_select_value = -1;
-       swjdp->ap_csw_value = -1;
-       swjdp->ap_tar_value = -1;
+       /* Leave (only) generic DAP stuff for debugport_init() */
        swjdp->jtag_info = &cortex_a8->jtag_info;
        swjdp->memaccess_tck = 80;
 

diff --git a/src/target/cortex_m3.c b/src/target/cortex_m3.c
index a3b3d425a9b2dabb2e7ea85f7aee7908a2db47a2..3dd9468594acf17844b69761a00a56f5aefd41be 100644 (file)
--- a/src/target/cortex_m3.c
+++ b/src/target/cortex_m3.c
@@ -1848,12 +1848,11 @@ static int cortex_m3_init_arch_info(struct target *target,
        cortex_m3->jtag_info.tap = tap;
        cortex_m3->jtag_info.scann_size = 4;
 
-       armv7m->swjdp_info.dp_select_value = -1;
-       armv7m->swjdp_info.ap_csw_value = -1;
-       armv7m->swjdp_info.ap_tar_value = -1;
+       /* Leave (only) generic DAP stuff for debugport_init(); */
        armv7m->swjdp_info.jtag_info = &cortex_m3->jtag_info;
        armv7m->swjdp_info.memaccess_tck = 8;
-       armv7m->swjdp_info.tar_autoincr_block = (1 << 12);      /* Cortex-M3 has 4096 bytes autoincrement range */
+       /* Cortex-M3 has 4096 bytes autoincrement range */
+       armv7m->swjdp_info.tar_autoincr_block = (1 << 12);
 
        /* register arch-specific functions */
        armv7m->examine_debug_reason = cortex_m3_examine_debug_reason;