[OpenOCD-devel] [PATCH]: 9192319 target: ls1_sap: Add -sync-delay configuration parameter

[<ge...@op...>]

This is an automated email from Gerrit.

Esben Haabendal (esb...@gm...) just uploaded a new patch set to Gerrit, which you can find at http://openocd.zylin.com/4234

-- gerrit

commit 91923192aa7ff3a66f2d3e8ee14a32f4a9e17bce
Author: Esben Haabendal <es...@ha...>
Date:   Mon Sep 25 13:02:46 2017 +0200

    target: ls1_sap: Add -sync-delay configuration parameter
    
    When doing read_memory() using an ls1_sap target, there are 3 JTAG commands
    involved. The first two setup the memory (read) transaction, while the
    last command reads the value.  Internally, a bus transaction needs to be
    executed in the CPU, and if the last JTAG command is done too fast, the bus
    transaction will not have been completed, and the value from the last
    memory_read() operation will be shifted out instead.
    
    This is most likely when the CPU is busy (lots of AHB activity).  I expect
    that SAP has lowest priority in bus arbitration, and will therefore have
    to wait for all other masters to be idle.
    
    It would be nice if there was a way to poll on the completion of the
    completion of the bus transaction, but I haven't figured out how to do
    that.  It might not be possible, but without any kind of specification,
    we don't really know.
    
    The required delay value will be highly dependant on the exact setup.
    If you suspect that you sometimes get wrong values from
    ls1_sap read_memory(), you should try increasing the sync_delay parameter.
    
    Signed-off-by: Esben Haabendal <es...@ha...>
    Change-Id: I84f567816c97077c2f33e197fbd5ca29d5b88555

diff --git a/doc/openocd.texi b/doc/openocd.texi
index 89ee5eb..d72910a 100644
--- a/doc/openocd.texi
+++ b/doc/openocd.texi
@@ -4257,6 +4257,13 @@ to separate access ports of the same DAP.
 @item @code{-ctibase} @var{address} -- set base address of Cross-Trigger interface (CTI) connected
 to the target. Currently, only the @code{aarch64} target makes use of this option, where it is
 a mandatory configuration for the target run control.
+
+@item @code{-sync-delay} @var{sync_delay_us} -- set delay (in microseconds)
+between read request and read of value. Currently, only the @code{ls1_sap}
+target makes use of this option, where it might be needed to avoid problems
+with reads returning data from previous read requests.  The @var{sync_delay_us}
+value needed will depend on the various parameters such as the host speed
+and the target system bus load.
 @end itemize
 @end deffn
 
diff --git a/src/target/ls1_sap.c b/src/target/ls1_sap.c
index bc46ed4..e943cba 100644
--- a/src/target/ls1_sap.c
+++ b/src/target/ls1_sap.c
@@ -26,6 +26,10 @@ struct ls1_sap {
 	struct jtag_tap *tap;
 };
 
+struct ls1_sap_config {
+	unsigned sync_delay;
+};
+
 static int ls1_sap_target_create(struct target *target, Jim_Interp *interp)
 {
 	struct ls1_sap *ls1_sap = calloc(1, sizeof(struct ls1_sap));
@@ -36,6 +40,45 @@ static int ls1_sap_target_create(struct target *target, Jim_Interp *interp)
 	return ERROR_OK;
 }
 
+static int ls1_sap_target_configure(struct target *target, Jim_GetOptInfo *goi)
+{
+	const char *arg;
+	int err;
+	jim_wide sync_delay;
+	struct ls1_sap_config *pc;
+
+	/* check if argv[0] is for us */
+	arg = Jim_GetString(goi->argv[0], NULL);
+	if (strcmp(arg, "-sync-delay"))
+		return JIM_CONTINUE;
+
+	err = Jim_GetOpt_String(goi, &arg, NULL);
+	if (err != JIM_OK)
+		return err;
+
+	if (goi->argc == 0) {
+		Jim_WrongNumArgs(goi->interp, goi->argc, goi->argv,
+				 "-sync-delay ?sync_delay_us? ...");
+		return JIM_ERR;
+	}
+
+	err = Jim_GetOpt_Wide(goi, &sync_delay);
+	if (err != JIM_OK)
+		return err;
+
+	if (target->private_config == NULL) {
+		pc = calloc(1, sizeof(struct ls1_sap_config));
+		if (pc == NULL) {
+			LOG_ERROR("Out of memory");
+			return ERROR_FAIL;
+		}
+		pc->sync_delay = sync_delay;
+		target->private_config = pc;
+	}
+
+	return JIM_OK;
+}
+
 static int ls1_sap_init_target(struct command_context *cmd_ctx, struct target *target)
 {
 	LOG_DEBUG("%s", __func__);
@@ -181,6 +224,8 @@ static void ls1_sap_memory_write(struct jtag_tap *tap, uint32_t size,
 static int ls1_sap_read_memory(struct target *target, target_addr_t address,
 			       uint32_t size, uint32_t count, uint8_t *buffer)
 {
+	struct ls1_sap_config *pc = target->private_config;
+
 	LOG_DEBUG("Reading memory at physical address 0x%" TARGET_PRIxADDR
 		  "; size %" PRId32 "; count %" PRId32, address, size, count);
 
@@ -191,6 +236,14 @@ static int ls1_sap_read_memory(struct target *target, target_addr_t address,
 
 	while (count--) {
 		ls1_sap_memory_cmd(target->tap, address, size, true);
+		/* In lack of a proper (known) method for waiting for the
+		 * memory bus transaction to be completed, we need to wait
+		 * "some" time.  Unfortunately, the time needed seems to
+		 * depend on a number of different parameters, such as speed
+		 * of the host OpenOCD host, tje JTAG clock speed, the speed
+		 * and load of the target system. */
+		if (pc && pc->sync_delay)
+			jtag_add_sleep(pc->sync_delay);
 		ls1_sap_memory_read(target->tap, size, buffer);
 		address += size;
 		buffer += size;
@@ -226,6 +279,7 @@ struct target_type ls1_sap_target = {
 	.name = "ls1_sap",
 
 	.target_create = ls1_sap_target_create,
+	.target_jim_configure = ls1_sap_target_configure,
 	.init_target = ls1_sap_init_target,
 
 	.poll = ls1_sap_poll,

--