Discussion:
[OpenOCD-devel] [PATCH]: 96e5444 arm_adi_v5: fix wrong addressing after change of CSW_ADDRINC
gerrit
2017-06-15 09:09:46 UTC
Permalink
This is an automated email from Gerrit.

Tomas Vanek (***@fbl.cz) just uploaded a new patch set to Gerrit, which you can find at http://openocd.zylin.com/4162

-- gerrit

commit 96e5444b2321358da0cfb5ed06a944563e86a2c2
Author: Tomas Vanek <***@fbl.cz>
Date: Thu Jun 15 08:59:01 2017 +0200

arm_adi_v5: fix wrong addressing after change of CSW_ADDRINC

Problem: If the same memory location is accessed alternatively
by MEM-AP banked data registers without autoincrement and by standard
autoincremented read/write, TAR register is not updated correctly.

How to replicate: On a Cortex-M issue
mdw 0xe000edf0
multiple times. When poll is on (poll reads the same memory location)
only the first read is correct.
0xe000edf0: 01000000
0xe000edf0: 00000000
0xe000edf0: 20002640
0xe000edf0: 01000000
0xe000edf0: 00000000
0xe000edf0: 00000000

No problems with poll off.
0xe000edf0: 01000000
0xe000edf0: 01000000
0xe000edf0: 01000000

mem_ap_setup_tar() writes to MEM_AP_REG_TAR if requested TAR value
changed or CSW_ADDRINC_... is currently active.
However if an autoincremented access has been issued and autoinc
switched off in CSW afterwards, TAR does not get updated.

The change introduces mem_ap_increment_tar() which is called
after queuing of any access to MEM_AP_REG_DRW. It invalidates
cached tar_value if an autoincrement mode is active.
Note that access to a banked data registers MEM_AP_REG_BD0..3
does not increment TAR regardless to the current autoincrement mode.

Change-Id: I815c2283d2989cffd6ea9a4100ce2f29dc3fb7b4
Signed-off-by: Tomas Vanek <***@fbl.cz>

diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c
index eafc2dd..5b9536b 100644
--- a/src/target/arm_adi_v5.c
+++ b/src/target/arm_adi_v5.c
@@ -111,8 +111,7 @@ static int mem_ap_setup_csw(struct adiv5_ap *ap, uint32_t csw)

static int mem_ap_setup_tar(struct adiv5_ap *ap, uint32_t tar)
{
- if (tar != ap->tar_value ||
- (ap->csw_value & CSW_ADDRINC_MASK)) {
+ if (tar != ap->tar_value) {
/* LOG_DEBUG("DAP: Set TAR %x",tar); */
int retval = dap_queue_ap_write(ap, MEM_AP_REG_TAR, tar);
if (retval != ERROR_OK)
@@ -122,6 +121,16 @@ static int mem_ap_setup_tar(struct adiv5_ap *ap, uint32_t tar)
return ERROR_OK;
}

+/* mem_ap_increment_tar is called after an access to MEM_AP_REG_DRW
+ * Currently there is no real increment implementation,
+ * just cached tar_value is invalidated
+ */
+static void mem_ap_increment_tar(struct adiv5_ap *ap)
+{
+ if (ap->csw_value & CSW_ADDRINC_MASK)
+ ap->tar_value = -1;
+}
+
/**
* Queue transactions setting up transfer parameters for the
* currently selected MEM-AP.
@@ -142,10 +151,13 @@ static int mem_ap_setup_tar(struct adiv5_ap *ap, uint32_t tar)
static int mem_ap_setup_transfer(struct adiv5_ap *ap, uint32_t csw, uint32_t tar)
{
int retval;
- retval = mem_ap_setup_csw(ap, csw);
+ /* mem_ap_setup_tar() should be called with the old csw state
+ * to prevent useless TAR update when autoinc is just switched on
+ */
+ retval = mem_ap_setup_tar(ap, tar);
if (retval != ERROR_OK)
return retval;
- retval = mem_ap_setup_tar(ap, tar);
+ retval = mem_ap_setup_csw(ap, csw);
if (retval != ERROR_OK)
return retval;
return ERROR_OK;
@@ -364,7 +376,8 @@ static int mem_ap_write(struct adiv5_ap *ap, const uint8_t *buffer, uint32_t siz
retval = mem_ap_setup_tar(ap, address ^ addr_xor);
if (retval != ERROR_OK)
break;
- }
+ } else
+ mem_ap_increment_tar(ap);
}

/* REVISIT: Might want to have a queued version of this function that does not run. */
@@ -469,7 +482,8 @@ static int mem_ap_read(struct adiv5_ap *ap, uint8_t *buffer, uint32_t size, uint
retval = mem_ap_setup_tar(ap, address);
if (retval != ERROR_OK)
break;
- }
+ } else
+ mem_ap_increment_tar(ap);
}

if (retval == ERROR_OK)

--

Loading...