[2/5] sbefifo: Rework the memory interfaces
diff mbox series

Message ID 20190820071906.25950-2-alistair@popple.id.au
State Accepted
Headers show
Series
  • [1/5] libpdbg: Rename adu class to mem
Related show

Checks

Context Check Description
snowpatch_ozlabs/apply_patch fail Failed to apply to any branch

Commit Message

Alistair Popple Aug. 20, 2019, 7:19 a.m. UTC
The SBEFIFO unit introduced it's own implementation specific interface
for accessing memory. Instead it should conform to the existing memory
access interfaces. This patch splits the SBEFIFO memory access into
it's own hardware unit so that it can conform with the existing
interfaces.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 libpdbg/hwunit.h  |  2 --
 libpdbg/sbefifo.c | 38 ++++++++++++++++++++++++++++++--------
 libpdbg/target.c  | 38 ++++++++------------------------------
 p9-kernel.dts.m4  |  8 ++++++++
 p9-pib.dts.m4     |  5 -----
 src/mem.c         | 46 ++++++----------------------------------------
 6 files changed, 52 insertions(+), 85 deletions(-)

Comments

Amitay Isaacs Aug. 20, 2019, 7:52 a.m. UTC | #1
On Tue, 2019-08-20 at 17:19 +1000, Alistair Popple wrote:
> The SBEFIFO unit introduced it's own implementation specific
> interface
> for accessing memory. Instead it should conform to the existing
> memory
> access interfaces. This patch splits the SBEFIFO memory access into
> it's own hardware unit so that it can conform with the existing
> interfaces.
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  libpdbg/hwunit.h  |  2 --
>  libpdbg/sbefifo.c | 38 ++++++++++++++++++++++++++++++--------
>  libpdbg/target.c  | 38 ++++++++------------------------------
>  p9-kernel.dts.m4  |  8 ++++++++
>  p9-pib.dts.m4     |  5 -----
>  src/mem.c         | 46 ++++++---------------------------------------
> -
>  6 files changed, 52 insertions(+), 85 deletions(-)
> 
> diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h
> index 2618941..a359af0 100644
> --- a/libpdbg/hwunit.h
> +++ b/libpdbg/hwunit.h
> @@ -68,8 +68,6 @@ struct mem {
>  struct sbefifo {
>  	struct pdbg_target target;
>  	int (*istep)(struct sbefifo *, uint32_t major, uint32_t minor);
> -	int (*mem_read)(struct sbefifo *, uint64_t, uint8_t *,
> uint64_t, bool);
> -	int (*mem_write)(struct sbefifo *, uint64_t, uint8_t *,
> uint64_t, bool);
>  	int (*thread_start)(struct sbefifo *, uint32_t core_id,
> uint32_t thread_id);
>  	int (*thread_stop)(struct sbefifo *, uint32_t core_id, uint32_t
> thread_id);
>  	int (*thread_step)(struct sbefifo *, uint32_t core_id, uint32_t
> thread_id);
> diff --git a/libpdbg/sbefifo.c b/libpdbg/sbefifo.c
> index 784db32..5c63512 100644
> --- a/libpdbg/sbefifo.c
> +++ b/libpdbg/sbefifo.c
> @@ -244,9 +244,9 @@ static int sbefifo_op_istep(struct sbefifo
> *sbefifo,
>  	return 0;
>  }
>  
> -static int sbefifo_op_getmem(struct sbefifo *sbefifo,
> +static int sbefifo_op_getmem(struct mem *sbefifo,
>  			     uint64_t addr, uint8_t *data, uint64_t
> size,
> -			     bool ci)
> +			     uint8_t block_size, bool ci)
>  {
>  	uint8_t *out;
>  	uint64_t start_addr, end_addr;
> @@ -257,6 +257,11 @@ static int sbefifo_op_getmem(struct sbefifo
> *sbefifo,
>  
>  	align = ci ? 8 : 128;
>  
> +	if (block_size && block_size != 8) {
> +		PR_ERROR("sbefifo: Only 8 byte block sizes are
> supported\n");
> +		return -1;
> +	};
> +

Well, I am not sure if block_size warning is useful.  There is no block
size specification in sbefifo api, so it's ignored anyway.

>  	start_addr = addr & (~(uint64_t)(align-1));
>  	end_addr = (addr + size + (align-1)) & (~(uint64_t)(align-1));
>  
> @@ -285,7 +290,8 @@ static int sbefifo_op_getmem(struct sbefifo
> *sbefifo,
>  	msg[5] = htobe32(len);
>  
>  	out_len = len + 4;
> -	rc = sbefifo_op(sbefifo, msg, sizeof(msg), cmd, &out, &out_len,
> &status);
> +	rc = sbefifo_op(target_to_sbefifo(sbefifo->target.parent), msg,
> sizeof(msg), cmd,
> +			&out, &out_len, &status);
>  	if (rc)
>  		return rc;
>  
> @@ -304,9 +310,9 @@ static int sbefifo_op_getmem(struct sbefifo
> *sbefifo,
>  	return 0;
>  }
>  
> -static int sbefifo_op_putmem(struct sbefifo *sbefifo,
> +static int sbefifo_op_putmem(struct mem *sbefifo,
>  			     uint64_t addr, uint8_t *data, uint64_t
> size,
> -			     bool ci)
> +			     uint8_t block_size, bool ci)
>  {
>  	uint8_t *out;
>  	uint32_t *msg;
> @@ -316,6 +322,11 @@ static int sbefifo_op_putmem(struct sbefifo
> *sbefifo,
>  
>  	align = ci ? 8 : 128;
>  
> +	if (block_size && block_size != 8) {
> +		PR_ERROR("sbefifo: Only 8 byte block sizes are
> supported\n");
> +		return -1;
> +	};
> +


And same here....


>  	if (addr & (align-1)) {
>  		PR_ERROR("sbefifo: Address must be aligned to %d
> bytes\n", align);
>  		return -1;
> @@ -353,7 +364,8 @@ static int sbefifo_op_putmem(struct sbefifo
> *sbefifo,
>  	memcpy(&msg[6], data, len);
>  
>  	out_len = 4;
> -	rc = sbefifo_op(sbefifo, msg, msg_len, cmd, &out, &out_len,
> &status);
> +	rc = sbefifo_op(target_to_sbefifo(sbefifo->target.parent), msg,
> msg_len, cmd,
> +			&out, &out_len, &status);
>  	if (rc)
>  		return rc;
>  
> @@ -484,6 +496,17 @@ static int sbefifo_probe(struct pdbg_target
> *target)
>  	return 0;
>  }
>  
> +struct mem sbefifo_mem = {
> +	.target = {
> +		.name = "SBE FIFO Chip-op based memory access",
> +		.compatible = "ibm,sbefifo-mem",
> +		.class = "mem",
> +	},
> +	.read = sbefifo_op_getmem,
> +	.write = sbefifo_op_putmem,
> +};
> +DECLARE_HW_UNIT(sbefifo_mem);
> +
>  struct sbefifo kernel_sbefifo = {
>  	.target = {
>  		.name =	"Kernel based FSI SBE FIFO",
> @@ -492,8 +515,6 @@ struct sbefifo kernel_sbefifo = {
>  		.probe = sbefifo_probe,
>  	},
>  	.istep = sbefifo_op_istep,
> -	.mem_read = sbefifo_op_getmem,
> -	.mem_write = sbefifo_op_putmem,
>  	.thread_start = sbefifo_op_thread_start,
>  	.thread_stop = sbefifo_op_thread_stop,
>  	.thread_step = sbefifo_op_thread_step,
> @@ -508,4 +529,5 @@ __attribute__((constructor))
>  static void register_sbefifo(void)
>  {
>  	pdbg_hwunit_register(&kernel_sbefifo_hw_unit);
> +	pdbg_hwunit_register(&sbefifo_mem_hw_unit);
>  }
> diff --git a/libpdbg/target.c b/libpdbg/target.c
> index 2396804..67191b5 100644
> --- a/libpdbg/target.c
> +++ b/libpdbg/target.c
> @@ -247,48 +247,26 @@ int fsi_write_mask(struct pdbg_target *fsi_dt,
> uint32_t addr, uint32_t data, uin
>  
>  int mem_read(struct pdbg_target *target, uint64_t addr, uint8_t
> *output, uint64_t size, uint8_t block_size, bool ci)
>  {
> +	struct mem *mem;
>  	int rc = -1;
>  
> -	assert(pdbg_target_is_class(target, "sbefifo") ||
> -	       pdbg_target_is_class(target, "mem"));
> +	assert(pdbg_target_is_class(target, "mem"));
>  
> -	if (pdbg_target_is_class(target, "sbefifo")) {
> -		struct sbefifo *sbefifo;
> -
> -		sbefifo = target_to_sbefifo(target);
> -		rc = sbefifo->mem_read(sbefifo, addr, output, size,
> ci);
> -	}
> -
> -	if (pdbg_target_is_class(target, "mem")) {
> -		struct mem *mem;
> -
> -		mem = target_to_mem(target);
> -		rc = mem->read(mem, addr, output, size, block_size,
> ci);
> -	}
> +	mem = target_to_mem(target);
> +	rc = mem->read(mem, addr, output, size, block_size, ci);
>  
>  	return rc;
>  }
>  
>  int mem_write(struct pdbg_target *target, uint64_t addr, uint8_t
> *input, uint64_t size, uint8_t block_size, bool ci)
>  {
> +	struct mem *mem;
>  	int rc = -1;
>  
> -	assert(pdbg_target_is_class(target, "sbefifo") ||
> -	       pdbg_target_is_class(target, "mem"));
> +	assert(pdbg_target_is_class(target, "mem"));
>  
> -	if (pdbg_target_is_class(target, "sbefifo")) {
> -		struct sbefifo *sbefifo;
> -
> -		sbefifo = target_to_sbefifo(target);
> -		rc = sbefifo->mem_write(sbefifo, addr, input, size,
> ci);
> -	}
> -
> -	if (pdbg_target_is_class(target, "mem")) {
> -		struct mem *mem;
> -
> -		mem = target_to_mem(target);
> -		rc = mem->write(mem, addr, input, size, block_size,
> ci);
> -	}
> +	mem = target_to_mem(target);
> +	rc = mem->write(mem, addr, input, size, block_size, ci);
>  
>  	return rc;
>  }
> diff --git a/p9-kernel.dts.m4 b/p9-kernel.dts.m4
> index 9ab46b8..aa4bcac 100644
> --- a/p9-kernel.dts.m4
> +++ b/p9-kernel.dts.m4
> @@ -28,6 +28,10 @@
>  			index = <0x0>;
>  			compatible = "ibm,kernel-sbefifo";
>  			device-path = "/dev/sbefifo1";
> +
> +			sbefifo-mem@0 {
> +				      compatible = "ibm,sbefifo-mem";
> +			};
>  		};
>  
>  		hmfsi@100000 {
> @@ -53,6 +57,10 @@
>  				index = <0x1>;
>  				compatible = "ibm,kernel-sbefifo";
>  				device-path = "/dev/sbefifo2";
> +
> +				sbefifo-mem@0 {
> +				      compatible = "ibm,sbefifo-mem";
> +				};
>  			};
>  		};
>  	};
> diff --git a/p9-pib.dts.m4 b/p9-pib.dts.m4
> index 3e312e5..3a99157 100644
> --- a/p9-pib.dts.m4
> +++ b/p9-pib.dts.m4
> @@ -36,11 +36,6 @@ reg = <0x0 HEX(CHIPLET_BASE($1)) 0xfffff>;
>  }')dnl
>  
>  
> -adu@90000 {
> -	  compatible = "ibm,power9-adu";
> -	  reg = <0x0 0x90000 0x5>;
> -};
> -


Do we want to stick adu in any backend?


>  htm@5012880 {
>  	compatible = "ibm,power9-nhtm";
>  	reg = <0x0 0x5012880 0x40>;
> diff --git a/src/mem.c b/src/mem.c
> index cacd394..53d33b8 100644
> --- a/src/mem.c
> +++ b/src/mem.c
> @@ -95,23 +95,6 @@ static int _getmem(uint64_t addr, uint64_t size,
> uint8_t block_size, bool ci, bo
>  	buf = malloc(size);
>  	assert(buf);
>  
> -	pdbg_for_each_class_target("sbefifo", target) {
> -		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> -			continue;
> -
> -		pdbg_set_progress_tick(progress_tick);
> -		progress_init();
> -		rc = mem_read(target, addr, buf, size, block_size, ci);
> -		progress_end();
> -		if (rc) {
> -			PR_ERROR("Unable to read memory using
> sbefifo\n");
> -			continue;
> -		}
> -
> -		count++;
> -		goto done;
> -	}
> -
>  	pdbg_for_each_class_target("mem", target) {
>  		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
>  			continue;
> @@ -121,15 +104,15 @@ static int _getmem(uint64_t addr, uint64_t
> size, uint8_t block_size, bool ci, bo
>  		rc = mem_read(target, addr, buf, size, block_size, ci);
>  		progress_end();
>  		if (rc) {
> -			PR_ERROR("Unable to read memory using adu\n");
> +			PR_ERROR("Unable to read memory from %s\n",
> +				 pdbg_target_path(target));
>  			continue;
>  		}
>  
>  		count++;
> -		goto done;
> +		break;
>  	}
>  
> -done:
>  	if (count > 0) {
>  		uint64_t i;
>  		bool printable = true;
> @@ -188,23 +171,6 @@ static int _putmem(uint64_t addr, uint8_t
> block_size, bool ci)
>  	buf = read_stdin(&buflen);
>  	assert(buf);
>  
> -	pdbg_for_each_class_target("sbefifo", target) {
> -		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> -			continue;
> -
> -		pdbg_set_progress_tick(progress_tick);
> -		progress_init();
> -		rc = mem_write(target, addr, buf, buflen, block_size,
> ci);
> -		progress_end();
> -		if (rc) {
> -			printf("Unable to write memory using
> sbefifo\n");
> -			continue;
> -		}
> -
> -		count++;
> -		goto done;
> -	}
> -
>  	pdbg_for_each_class_target("mem", target) {
>  		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
>  			continue;
> @@ -214,15 +180,15 @@ static int _putmem(uint64_t addr, uint8_t
> block_size, bool ci)
>  		rc = mem_write(target, addr, buf, buflen, block_size,
> ci);
>  		progress_end();
>  		if (rc) {
> -			printf("Unable to write memory using adu\n");
> +			printf("Unable to write memory using %s\n",
> +			       pdbg_target_path(target));
>  			continue;
>  		}
>  
>  		count++;
> -		goto done;
> +		break;
>  	}
>  
> -done:
>  	if (count > 0)
>  		printf("Wrote %zu bytes starting at 0x%016" PRIx64
> "\n", buflen, addr);
>  
> -- 
> 2.20.1
> 

Amitay.

Patch
diff mbox series

diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h
index 2618941..a359af0 100644
--- a/libpdbg/hwunit.h
+++ b/libpdbg/hwunit.h
@@ -68,8 +68,6 @@  struct mem {
 struct sbefifo {
 	struct pdbg_target target;
 	int (*istep)(struct sbefifo *, uint32_t major, uint32_t minor);
-	int (*mem_read)(struct sbefifo *, uint64_t, uint8_t *, uint64_t, bool);
-	int (*mem_write)(struct sbefifo *, uint64_t, uint8_t *, uint64_t, bool);
 	int (*thread_start)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
 	int (*thread_stop)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
 	int (*thread_step)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
diff --git a/libpdbg/sbefifo.c b/libpdbg/sbefifo.c
index 784db32..5c63512 100644
--- a/libpdbg/sbefifo.c
+++ b/libpdbg/sbefifo.c
@@ -244,9 +244,9 @@  static int sbefifo_op_istep(struct sbefifo *sbefifo,
 	return 0;
 }
 
-static int sbefifo_op_getmem(struct sbefifo *sbefifo,
+static int sbefifo_op_getmem(struct mem *sbefifo,
 			     uint64_t addr, uint8_t *data, uint64_t size,
-			     bool ci)
+			     uint8_t block_size, bool ci)
 {
 	uint8_t *out;
 	uint64_t start_addr, end_addr;
@@ -257,6 +257,11 @@  static int sbefifo_op_getmem(struct sbefifo *sbefifo,
 
 	align = ci ? 8 : 128;
 
+	if (block_size && block_size != 8) {
+		PR_ERROR("sbefifo: Only 8 byte block sizes are supported\n");
+		return -1;
+	};
+
 	start_addr = addr & (~(uint64_t)(align-1));
 	end_addr = (addr + size + (align-1)) & (~(uint64_t)(align-1));
 
@@ -285,7 +290,8 @@  static int sbefifo_op_getmem(struct sbefifo *sbefifo,
 	msg[5] = htobe32(len);
 
 	out_len = len + 4;
-	rc = sbefifo_op(sbefifo, msg, sizeof(msg), cmd, &out, &out_len, &status);
+	rc = sbefifo_op(target_to_sbefifo(sbefifo->target.parent), msg, sizeof(msg), cmd,
+			&out, &out_len, &status);
 	if (rc)
 		return rc;
 
@@ -304,9 +310,9 @@  static int sbefifo_op_getmem(struct sbefifo *sbefifo,
 	return 0;
 }
 
-static int sbefifo_op_putmem(struct sbefifo *sbefifo,
+static int sbefifo_op_putmem(struct mem *sbefifo,
 			     uint64_t addr, uint8_t *data, uint64_t size,
-			     bool ci)
+			     uint8_t block_size, bool ci)
 {
 	uint8_t *out;
 	uint32_t *msg;
@@ -316,6 +322,11 @@  static int sbefifo_op_putmem(struct sbefifo *sbefifo,
 
 	align = ci ? 8 : 128;
 
+	if (block_size && block_size != 8) {
+		PR_ERROR("sbefifo: Only 8 byte block sizes are supported\n");
+		return -1;
+	};
+
 	if (addr & (align-1)) {
 		PR_ERROR("sbefifo: Address must be aligned to %d bytes\n", align);
 		return -1;
@@ -353,7 +364,8 @@  static int sbefifo_op_putmem(struct sbefifo *sbefifo,
 	memcpy(&msg[6], data, len);
 
 	out_len = 4;
-	rc = sbefifo_op(sbefifo, msg, msg_len, cmd, &out, &out_len, &status);
+	rc = sbefifo_op(target_to_sbefifo(sbefifo->target.parent), msg, msg_len, cmd,
+			&out, &out_len, &status);
 	if (rc)
 		return rc;
 
@@ -484,6 +496,17 @@  static int sbefifo_probe(struct pdbg_target *target)
 	return 0;
 }
 
+struct mem sbefifo_mem = {
+	.target = {
+		.name = "SBE FIFO Chip-op based memory access",
+		.compatible = "ibm,sbefifo-mem",
+		.class = "mem",
+	},
+	.read = sbefifo_op_getmem,
+	.write = sbefifo_op_putmem,
+};
+DECLARE_HW_UNIT(sbefifo_mem);
+
 struct sbefifo kernel_sbefifo = {
 	.target = {
 		.name =	"Kernel based FSI SBE FIFO",
@@ -492,8 +515,6 @@  struct sbefifo kernel_sbefifo = {
 		.probe = sbefifo_probe,
 	},
 	.istep = sbefifo_op_istep,
-	.mem_read = sbefifo_op_getmem,
-	.mem_write = sbefifo_op_putmem,
 	.thread_start = sbefifo_op_thread_start,
 	.thread_stop = sbefifo_op_thread_stop,
 	.thread_step = sbefifo_op_thread_step,
@@ -508,4 +529,5 @@  __attribute__((constructor))
 static void register_sbefifo(void)
 {
 	pdbg_hwunit_register(&kernel_sbefifo_hw_unit);
+	pdbg_hwunit_register(&sbefifo_mem_hw_unit);
 }
diff --git a/libpdbg/target.c b/libpdbg/target.c
index 2396804..67191b5 100644
--- a/libpdbg/target.c
+++ b/libpdbg/target.c
@@ -247,48 +247,26 @@  int fsi_write_mask(struct pdbg_target *fsi_dt, uint32_t addr, uint32_t data, uin
 
 int mem_read(struct pdbg_target *target, uint64_t addr, uint8_t *output, uint64_t size, uint8_t block_size, bool ci)
 {
+	struct mem *mem;
 	int rc = -1;
 
-	assert(pdbg_target_is_class(target, "sbefifo") ||
-	       pdbg_target_is_class(target, "mem"));
+	assert(pdbg_target_is_class(target, "mem"));
 
-	if (pdbg_target_is_class(target, "sbefifo")) {
-		struct sbefifo *sbefifo;
-
-		sbefifo = target_to_sbefifo(target);
-		rc = sbefifo->mem_read(sbefifo, addr, output, size, ci);
-	}
-
-	if (pdbg_target_is_class(target, "mem")) {
-		struct mem *mem;
-
-		mem = target_to_mem(target);
-		rc = mem->read(mem, addr, output, size, block_size, ci);
-	}
+	mem = target_to_mem(target);
+	rc = mem->read(mem, addr, output, size, block_size, ci);
 
 	return rc;
 }
 
 int mem_write(struct pdbg_target *target, uint64_t addr, uint8_t *input, uint64_t size, uint8_t block_size, bool ci)
 {
+	struct mem *mem;
 	int rc = -1;
 
-	assert(pdbg_target_is_class(target, "sbefifo") ||
-	       pdbg_target_is_class(target, "mem"));
+	assert(pdbg_target_is_class(target, "mem"));
 
-	if (pdbg_target_is_class(target, "sbefifo")) {
-		struct sbefifo *sbefifo;
-
-		sbefifo = target_to_sbefifo(target);
-		rc = sbefifo->mem_write(sbefifo, addr, input, size, ci);
-	}
-
-	if (pdbg_target_is_class(target, "mem")) {
-		struct mem *mem;
-
-		mem = target_to_mem(target);
-		rc = mem->write(mem, addr, input, size, block_size, ci);
-	}
+	mem = target_to_mem(target);
+	rc = mem->write(mem, addr, input, size, block_size, ci);
 
 	return rc;
 }
diff --git a/p9-kernel.dts.m4 b/p9-kernel.dts.m4
index 9ab46b8..aa4bcac 100644
--- a/p9-kernel.dts.m4
+++ b/p9-kernel.dts.m4
@@ -28,6 +28,10 @@ 
 			index = <0x0>;
 			compatible = "ibm,kernel-sbefifo";
 			device-path = "/dev/sbefifo1";
+
+			sbefifo-mem@0 {
+				      compatible = "ibm,sbefifo-mem";
+			};
 		};
 
 		hmfsi@100000 {
@@ -53,6 +57,10 @@ 
 				index = <0x1>;
 				compatible = "ibm,kernel-sbefifo";
 				device-path = "/dev/sbefifo2";
+
+				sbefifo-mem@0 {
+				      compatible = "ibm,sbefifo-mem";
+				};
 			};
 		};
 	};
diff --git a/p9-pib.dts.m4 b/p9-pib.dts.m4
index 3e312e5..3a99157 100644
--- a/p9-pib.dts.m4
+++ b/p9-pib.dts.m4
@@ -36,11 +36,6 @@  reg = <0x0 HEX(CHIPLET_BASE($1)) 0xfffff>;
 }')dnl
 
 
-adu@90000 {
-	  compatible = "ibm,power9-adu";
-	  reg = <0x0 0x90000 0x5>;
-};
-
 htm@5012880 {
 	compatible = "ibm,power9-nhtm";
 	reg = <0x0 0x5012880 0x40>;
diff --git a/src/mem.c b/src/mem.c
index cacd394..53d33b8 100644
--- a/src/mem.c
+++ b/src/mem.c
@@ -95,23 +95,6 @@  static int _getmem(uint64_t addr, uint64_t size, uint8_t block_size, bool ci, bo
 	buf = malloc(size);
 	assert(buf);
 
-	pdbg_for_each_class_target("sbefifo", target) {
-		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
-			continue;
-
-		pdbg_set_progress_tick(progress_tick);
-		progress_init();
-		rc = mem_read(target, addr, buf, size, block_size, ci);
-		progress_end();
-		if (rc) {
-			PR_ERROR("Unable to read memory using sbefifo\n");
-			continue;
-		}
-
-		count++;
-		goto done;
-	}
-
 	pdbg_for_each_class_target("mem", target) {
 		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
 			continue;
@@ -121,15 +104,15 @@  static int _getmem(uint64_t addr, uint64_t size, uint8_t block_size, bool ci, bo
 		rc = mem_read(target, addr, buf, size, block_size, ci);
 		progress_end();
 		if (rc) {
-			PR_ERROR("Unable to read memory using adu\n");
+			PR_ERROR("Unable to read memory from %s\n",
+				 pdbg_target_path(target));
 			continue;
 		}
 
 		count++;
-		goto done;
+		break;
 	}
 
-done:
 	if (count > 0) {
 		uint64_t i;
 		bool printable = true;
@@ -188,23 +171,6 @@  static int _putmem(uint64_t addr, uint8_t block_size, bool ci)
 	buf = read_stdin(&buflen);
 	assert(buf);
 
-	pdbg_for_each_class_target("sbefifo", target) {
-		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
-			continue;
-
-		pdbg_set_progress_tick(progress_tick);
-		progress_init();
-		rc = mem_write(target, addr, buf, buflen, block_size, ci);
-		progress_end();
-		if (rc) {
-			printf("Unable to write memory using sbefifo\n");
-			continue;
-		}
-
-		count++;
-		goto done;
-	}
-
 	pdbg_for_each_class_target("mem", target) {
 		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
 			continue;
@@ -214,15 +180,15 @@  static int _putmem(uint64_t addr, uint8_t block_size, bool ci)
 		rc = mem_write(target, addr, buf, buflen, block_size, ci);
 		progress_end();
 		if (rc) {
-			printf("Unable to write memory using adu\n");
+			printf("Unable to write memory using %s\n",
+			       pdbg_target_path(target));
 			continue;
 		}
 
 		count++;
-		goto done;
+		break;
 	}
 
-done:
 	if (count > 0)
 		printf("Wrote %zu bytes starting at 0x%016" PRIx64 "\n", buflen, addr);