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>

4 files changed, 60 insertions, 34 deletions

diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c
index 94c8ed8f..2e3dafb9 100644
--- 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 759f2333..746f1cb6 100644
--- 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 050238ce..f4818f8d 100644
--- 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 a3b3d425..3dd94685 100644
--- 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;