Fix in write_memory_bus_v1: Read sbcs properly (#578)

* Fix in write_memory_bus_v1: Read sbcs properly

Fixed an issue with system bus write: sberrors were not properly
detected because of an incomplete read of "sbcs". The read was initiated
but not completed (value not acquired by a second DMI scan).

Added debug prints in case sberror != 0.
Added few comments to explain the algorithm.

Change-Id: Id5eb07f2f1bf8e9afee2dec04b9ff5c5a57f606b
Signed-off-by: Jan Matyas <matyas@codasip.com>

* Updated per review discussion at #577.

Change-Id: I65c07edcd4e86eaa5327280a81f74db0b9c84f9c
Signed-off-by: Jan Matyas <jmatyas@codasip.com>

* Empty commit to re-trigger Travis build.

Change-Id: I95deeb28584a891203c8904be621e48003f069dc

Co-authored-by: Jan Matyas <jmatyas@codasip.com>

1 files changed, 38 insertions, 16 deletions

diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c
index b8b513d..87aff06 100644
--- a/src/target/riscv/riscv-013.c
+++ b/src/target/riscv/riscv-013.c
@@ -3713,54 +3713,76 @@ static int write_memory_bus_v1(struct target *target, target_addr_t address,
 			next_address += size;
 		}
 
+		/* Execute the batch of writes */
 		result = batch_run(target, batch);
 		riscv_batch_free(batch);
 		if (result != ERROR_OK)
 			return result;
 
+		/* Read sbcs value.
+		 * At the same time, detect if DMI busy has occurred during the batch write. */
 		bool dmi_busy_encountered;
 		if (dmi_op(target, &sbcs, &dmi_busy_encountered, DMI_OP_READ,
-				DM_SBCS, 0, false, false) != ERROR_OK)
+				DM_SBCS, 0, false, true) != ERROR_OK)
 			return ERROR_FAIL;
+		if (dmi_busy_encountered)
+			LOG_DEBUG("DMI busy encountered during system bus write.");
 
+		/* Wait until sbbusy goes low */
 		time_t start = time(NULL);
-		bool dmi_busy = dmi_busy_encountered;
-		while (get_field(sbcs, DM_SBCS_SBBUSY) || dmi_busy) {
+		while (get_field(sbcs, DM_SBCS_SBBUSY)) {
 			if (time(NULL) - start > riscv_command_timeout_sec) {
 				LOG_ERROR("Timed out after %ds waiting for sbbusy to go low (sbcs=0x%x). "
-					  "Increase the timeout with riscv set_command_timeout_sec.",
-					  riscv_command_timeout_sec, sbcs);
+						  "Increase the timeout with riscv set_command_timeout_sec.",
+						  riscv_command_timeout_sec, sbcs);
 				return ERROR_FAIL;
 			}
-
-			if (dmi_op(target, &sbcs, &dmi_busy, DMI_OP_READ,
-						DM_SBCS, 0, false, true) != ERROR_OK)
+			if (dmi_read(target, &sbcs, DM_SBCS) != ERROR_OK)
 				return ERROR_FAIL;
 		}
 
 		if (get_field(sbcs, DM_SBCS_SBBUSYERROR)) {
-			/* We wrote while the target was busy. Slow down and try again. */
+			/* We wrote while the target was busy. */
+			LOG_DEBUG("Sbbusyerror encountered during system bus write.");
+			/* Clear the sticky error flag. */
 			dmi_write(target, DM_SBCS, sbcs | DM_SBCS_SBBUSYERROR);
+			/* Slow down before trying again. */
 			info->bus_master_write_delay += info->bus_master_write_delay / 10 + 1;
 		}
 
 		if (get_field(sbcs, DM_SBCS_SBBUSYERROR) || dmi_busy_encountered) {
+			/* Recover from the case when the write commands were issued too fast.
+			 * Determine the address from which to resume writing. */
 			next_address = sb_read_address(target);
 			if (next_address < address) {
 				/* This should never happen, probably buggy hardware. */
-				LOG_DEBUG("unexpected system bus address 0x%" TARGET_PRIxADDR,
-					  next_address);
+				LOG_DEBUG("unexpected sbaddress=0x%" TARGET_PRIxADDR
+						  " - buggy sbautoincrement in hw?", next_address);
+				/* Fail the whole operation. */
 				return ERROR_FAIL;
 			}
-
+			/* Try again - resume writing. */
 			continue;
 		}
 
-		unsigned error = get_field(sbcs, DM_SBCS_SBERROR);
-		if (error != 0) {
-			/* Some error indicating the bus access failed, but not because of
-			 * something we did wrong. */
+		unsigned sberror = get_field(sbcs, DM_SBCS_SBERROR);
+		if (sberror != 0) {
+			/* Sberror indicates the bus access failed, but not because we issued the writes
+			 * too fast. Cannot recover. Sbaddress holds the address where the error occurred
+			 * (unless sbautoincrement in the HW is buggy).
+			 */
+			target_addr_t sbaddress = sb_read_address(target);
+			LOG_DEBUG("System bus access failed with sberror=%u (sbaddress=0x%" TARGET_PRIxADDR ")",
+					  sberror, sbaddress);
+			if (sbaddress < address) {
+				/* This should never happen, probably buggy hardware.
+				 * Make a note to the user not to trust the sbaddress value. */
+				LOG_DEBUG("unexpected sbaddress=0x%" TARGET_PRIxADDR
+						  " - buggy sbautoincrement in hw?", next_address);
+			}
+			/* Clear the sticky error flag */
 			dmi_write(target, DM_SBCS, DM_SBCS_SBERROR);
+			/* Fail the whole operation */
 			return ERROR_FAIL;
 		}
 	}