Change in flashrom[master]: raiden_debug_spi.c: Adds context states and helper functions

[Brian Nemec (Code Review)]

Hello Brian Nemec,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to review the following change.

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 241 insertions(+), 94 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/1

diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c
index b18d580..4de8038 100644
--- a/raiden_debug_spi.c
+++ b/raiden_debug_spi.c
@@ -141,7 +141,21 @@

enum {
    GOOGLE_RAIDEN_SPI_PROTOCOL_V1 = 0x01,
-	GOOGLE_RAIDEN_SPI_PROTOCOL_V2 = 0x02,
+};
+
+enum {
+	/* The host failed to transfer the data with no libusb error. */
+	USB_SPI_HOST_TX_BAD_TRANSFER      = 0x10001,
+	/* We did not receive the expected USB packet. */
+	USB_SPI_HOST_RX_UNEXPECTED_PACKET = 0x18002,
+	/* We received a continue packet with an invalid data index.*/
+	USB_SPI_HOST_RX_BAD_DATA_INDEX    = 0x18003,
+	/* We too much data. */
+	USB_SPI_HOST_RX_DATA_OVERFLOW     = 0x18004,
+	/* We too little data. */
+	USB_SPI_HOST_RX_READ_FAILURE      = 0x18005,
+	/* We were unable to configure the device. */
+	USB_SPI_HOST_INIT_FAILURE         = 0x18006,
 };

enum usb_spi_error {
@@ -162,8 +176,8 @@
 };

#define PACKET_HEADER_SIZE      (2)
-#define MAX_PACKET_SIZE         (64)
-#define PAYLOAD_SIZE_V1         (MAX_PACKET_SIZE - PACKET_HEADER_SIZE)
+#define MAX_USB_PACKET_SIZE     (64)
+#define PAYLOAD_SIZE_V1         (MAX_USB_PACKET_SIZE - PACKET_HEADER_SIZE)

/*
  * Servo Micro has an error where it is capable of acknowledging USB packets
@@ -180,11 +194,10 @@
  */
 #define TRANSFER_TIMEOUT_MS     (200 + 800)

-struct raiden_debug_spi_data {
-	struct usb_device *dev;
-	uint8_t in_ep;
-	uint8_t out_ep;
-};
+
+/*
+ * Version 1 protocol specific attributes
+ */

typedef struct {
    int8_t write_count;
@@ -198,6 +211,51 @@
    uint8_t data[PAYLOAD_SIZE_V1];
 } __attribute__((packed)) usb_spi_response_v1_t;

+typedef union {
+	usb_spi_command_v1_t command;
+	usb_spi_response_v1_t response;
+} __attribute__((packed)) usb_spi_packet_v1_t;
+
+typedef struct {
+	union {
+		uint8_t bytes[MAX_USB_PACKET_SIZE];
+		usb_spi_packet_v1_t packet_v1;
+	};
+	/*
+	 * By storing the number of bytes in the header and knowing that the
+	 * USB data packets are all 64B long, we are able to use the header
+	 * size to store the offset of the buffer and it's size without
+	 * duplicating variables that can go out of sync.
+	 */
+	int header_size;
+	/* Number of bytes in the packet */
+	int packet_size;
+} usb_spi_packet_ctx_t;
+
+typedef struct {
+	/* Buffer we are reading data from. */
+	const uint8_t *buffer;
+	/* Total size of the transmit transfer. */
+	int transmit_size;
+	/* Number of bytes transmitted. */
+	int transmit_index;
+} usb_spi_transmit_ctx_t;
+
+typedef struct {
+	/* Buffer we are writing data into. */
+	uint8_t *buffer;
+	/* Total size of the receive transfer. */
+	int receive_size;
+	/* Number of bytes received. */
+	int receive_index;
+} usb_spi_receive_ctx_t;
+
+struct raiden_debug_spi_data {
+	struct usb_device *dev;
+	uint8_t in_ep;
+	uint8_t out_ep;
+};
+
 /*
  * This function will return true when an error code can potentially recover
  * if we attempt to write SPI data to the device or read from it. We know
@@ -213,7 +271,7 @@
    	 * during transfer errors to the device and can be recovered.
    	 */
    	if (USB_SPI_READ_COUNT_INVALID <= error_code &&
-		    error_code <= USB_SPI_DISABLED) {
+				error_code <= USB_SPI_DISABLED) {
    		return false;
    	}
    } else if (usb_device_is_libusb_error(error_code)) {
@@ -231,90 +289,139 @@
    return (const struct raiden_debug_spi_data *)flash->mst->spi.data;
 }

-static int write_command(const struct flashctx *flash,
-		unsigned int write_count,
-		unsigned int read_count,
-		const unsigned char *write_buffer,
-		unsigned char *read_buffer)
+/*
+ * Fill the data section in the USB packets.
+ */
+static int fill_usb_packet(usb_spi_packet_ctx_t* dst,
+		usb_spi_transmit_ctx_t* src)
 {
+	int transfer_size = src->transmit_size - src->transmit_index;
+	int max_buffer_size = MAX_USB_PACKET_SIZE - dst->header_size;
+	uint8_t *packet_buffer = dst->bytes + dst->header_size;

-	int transferred;
-	int ret;
-	usb_spi_command_v1_t command_packet;
-	const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash);
-
-	if (write_count > PAYLOAD_SIZE_V1) {
-		msg_perr("Raiden: Invalid write_count of %d\n", write_count);
-		return SPI_INVALID_LENGTH;
+	if (transfer_size > max_buffer_size) {
+		transfer_size = max_buffer_size;
    }
+	memcpy(packet_buffer, src->buffer + src->transmit_index, transfer_size);

-	if (read_count > PAYLOAD_SIZE_V1) {
-		msg_perr("Raiden: Invalid read_count of %d\n", read_count);
-		return SPI_INVALID_LENGTH;
-	}
-
-	command_packet.write_count = write_count;
-	command_packet.read_count = read_count;
-
-	memcpy(command_packet.data, write_buffer, write_count);
-
-	ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle,
-				ctx_data->out_ep,
-				(void*)&command_packet,
-				write_count + PACKET_HEADER_SIZE,
-				&transferred,
-				TRANSFER_TIMEOUT_MS));
-	if (ret != 0) {
-		msg_perr("Raiden: OUT transfer failed\n"
-		         "    write_count = %d\n"
-		         "    read_count  = %d\n",
-		         write_count, read_count);
-		return ret;
-	}
-
-	if ((unsigned) transferred != write_count + PACKET_HEADER_SIZE) {
-		msg_perr("Raiden: Write failure (wrote %d, expected %d)\n",
-			 transferred, write_count + PACKET_HEADER_SIZE);
-		return 0x10001;
-	}
-
+	dst->packet_size = dst->header_size + transfer_size;
+	src->transmit_index += transfer_size;
    return 0;
 }

-static int read_response(const struct flashctx *flash,
-		unsigned int write_count,
-		unsigned int read_count,
-		const unsigned char *write_buffer,
-		unsigned char *read_buffer)
+static int read_usb_packet(usb_spi_receive_ctx_t* dst,
+		const usb_spi_packet_ctx_t* src)
+{
+	int max_read_length = dst->receive_size - dst->receive_index;
+	int bytes_in_buffer = src->packet_size - src->header_size;
+	const uint8_t *packet_buffer = src->bytes + src->header_size;
+
+	if (bytes_in_buffer > max_read_length) {
+		/*
+		 * An error occurred, we should not receive more data than
+		 * the buffer can support.
+		 */
+		msg_perr("Raiden: Receive packet overflowed\n"
+				"    bytes_in_buffer = %d\n"
+				"    max_read_length = %d\n"
+				"    receive_index   = %d\n"
+				"    receive_size    = %d\n",
+				bytes_in_buffer, max_read_length,
+				dst->receive_size, dst->receive_index);
+		return USB_SPI_HOST_RX_DATA_OVERFLOW;
+	}
+	memcpy(dst->buffer + dst->receive_index, packet_buffer, bytes_in_buffer);
+
+	dst->receive_index += bytes_in_buffer;
+	return 0;
+}
+
+static int transmit_packet(const struct raiden_debug_spi_data * ctx_data,
+		usb_spi_packet_ctx_t* packet)
 {
    int transferred;
-	int ret;
-	usb_spi_response_v1_t response_packet;
-	const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash);
-
-	ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle,
-				ctx_data->in_ep,
-				(void*)&response_packet,
-				read_count + PACKET_HEADER_SIZE,
+	int status = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle,
+				ctx_data->out_ep,
+				packet->bytes,
+				packet->packet_size,
    			&transferred,
    			TRANSFER_TIMEOUT_MS));
-	if (ret != 0) {
+	if (status || transferred != packet->packet_size) {
+		if (!status) {
+			/* No error was reported, but we didn't transmit the data expected. */
+			status = USB_SPI_HOST_TX_BAD_TRANSFER;
+		}
+		msg_perr("Raiden: OUT transfer failed\n"
+				"    transferred = %d\n"
+				"    packet_size = %d\n"
+				"    status      = %d\n",
+				transferred, packet->packet_size, status);
+
+	}
+	return status;
+}
+
+static int receive_packet(const struct raiden_debug_spi_data * ctx_data,
+		usb_spi_packet_ctx_t* packet)
+{
+	int received;
+	int status = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle,
+				ctx_data->in_ep,
+				packet->bytes,
+				MAX_USB_PACKET_SIZE,
+				&received,
+				TRANSFER_TIMEOUT_MS));
+	packet->packet_size = received;
+	if (status) {
    	msg_perr("Raiden: IN transfer failed\n"
-		         "    write_count = %d\n"
-		         "    read_count  = %d\n",
-		         write_count, read_count);
-		return ret;
+				"    received = %d\n"
+				"    status   = %d\n",
+				received, status);
    }
+	return status;
+}

-	if ((unsigned) transferred != read_count + PACKET_HEADER_SIZE) {
-		msg_perr("Raiden: Read failure (read %d, expected %d)\n",
-				transferred, read_count + PACKET_HEADER_SIZE);
-		return 0x10002;
+/*
+ * Version 1 Protocol
+ */
+
+static int write_command_v1(const struct raiden_debug_spi_data * ctx_data,
+		usb_spi_transmit_ctx_t* write, usb_spi_receive_ctx_t* read)
+{
+	usb_spi_packet_ctx_t command = {
+		.header_size = offsetof(usb_spi_command_v1_t, data),
+		.packet_v1.command.write_count = write->transmit_size,
+		.packet_v1.command.read_count = read->receive_size
+	};
+
+	/* Reset the write context to the start. */
+	write->transmit_index = 0;
+
+	fill_usb_packet(&command, write);
+	return transmit_packet(ctx_data, &command);
+}
+
+static int read_response_v1(const struct raiden_debug_spi_data * ctx_data,
+		usb_spi_transmit_ctx_t* write, usb_spi_receive_ctx_t* read)
+{
+	int status;
+	usb_spi_packet_ctx_t response;
+
+	/* Reset the read context to the start. */
+	read->receive_index = 0;
+
+	status = receive_packet(ctx_data, &response);
+	if (status) {
+		/* Return the transfer error since the status_code is unreliable */
+		return status;
    }
+	if (response.packet_v1.response.status_code) {
+		return response.packet_v1.response.status_code;
+	}
+	response.header_size = offsetof(usb_spi_response_v1_t, data);

-	memcpy(read_buffer, response_packet.data, read_count);
-
-	return response_packet.status_code;
+	status = read_usb_packet(read, &response);
+	return status;
 }

static int send_command(const struct flashctx *flash,
@@ -323,20 +430,50 @@
    	const unsigned char *write_buffer,
    	unsigned char *read_buffer)
 {
+	const struct raiden_debug_spi_data * ctx_data =
+		get_raiden_data_from_context(flash);
    int status = -1;

-	for (int write_attempt = 0; write_attempt < WRITE_RETRY_ATTEMPTS;
-	         write_attempt++) {
+	usb_spi_transmit_ctx_t write_ctx = {
+		.buffer = write_buffer,
+		.transmit_size = write_count
+	};
+	usb_spi_receive_ctx_t read_ctx = {
+		.buffer = read_buffer,
+		.receive_size = read_count
+	};

-		status = write_command(flash, write_count, read_count,
-		                       write_buffer, read_buffer);
+	if (write_count > PAYLOAD_SIZE_V1) {
+		msg_perr("Raiden: Invalid read count\n"
+				"    write count = %d\n"
+				"    max write   = %d\n",
+				read_count, PAYLOAD_SIZE_V1);
+		return SPI_INVALID_LENGTH;
+	}
+
+	if (read_count > PAYLOAD_SIZE_V1) {
+		msg_perr("Raiden: Invalid read count\n"
+				"    read count = %d\n"
+				"    max read   = %d\n",
+				read_count, PAYLOAD_SIZE_V1);
+		return SPI_INVALID_LENGTH;
+	}
+
+	for (int write_attempt = 0; write_attempt < WRITE_RETRY_ATTEMPTS;
+			write_attempt++) {
+
+		status = write_command_v1(ctx_data, &write_ctx, &read_ctx);

if (status) {
    		/* Write operation failed. */
    		msg_perr("Raiden: Write command failed\n"
-				"Write attempt = %d\n"
-				"status = %d\n",
-				write_attempt + 1, status);
+					"    write count       = %d\n"
+					"    read count        = %d\n"
+					"    transmitted bytes = %d\n"
+					"    write attempt     = %d\n"
+					"    status            = %d\n",
+					write_count, read_count, write_ctx.transmit_index,
+					write_attempt + 1, status);
    		if (!retry_recovery(status)) {
    			/* Reattempting will not result in a recovery. */
    			return status;
@@ -344,26 +481,36 @@
    		programmer_delay(RETRY_INTERVAL_US);
    		continue;
    	}
-		for (int read_attempt = 0; read_attempt < READ_RETRY_ATTEMPTS; read_attempt++) {
+		for (int read_attempt = 0; read_attempt < READ_RETRY_ATTEMPTS;
+				read_attempt++) {

-			status = read_response(flash, write_count, read_count,
-					write_buffer, read_buffer);
+			status = read_response_v1(ctx_data, &write_ctx, &read_ctx);
+
+			if (!status) {
+				if (read_ctx.receive_size == read_ctx.receive_index) {
+					/* No errors and read is complete. */
+					return status;
+				} else {
+					status = USB_SPI_HOST_RX_READ_FAILURE;
+				}
+			}

if (status) {
    			/* Read operation failed. */
    			msg_perr("Raiden: Read response failed\n"
-					"Write attempt = %d\n"
-					"Read attempt = %d\n"
-					"status = %d\n",
-					write_attempt + 1, read_attempt + 1, status);
+						"    write count    = %d\n"
+						"    read count     = %d\n"
+						"    received bytes = %d\n"
+						"    write attempt  = %d\n"
+						"    read attempt   = %d\n"
+						"    status         = %d\n",
+						write_count, read_count, read_ctx.receive_index,
+						write_attempt + 1, read_attempt + 1, status);
    			if (!retry_recovery(status)) {
    				/* Reattempting will not result in a recovery. */
    				return status;
    			}
    			programmer_delay(RETRY_INTERVAL_US);
-			} else {
-				/* We were successful at performing the SPI transfer. */
-				return status;
    		}
    	}
    }

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#2).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 250 insertions(+), 94 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/2

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#3).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 250 insertions(+), 94 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/3

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#4).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 250 insertions(+), 93 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/4

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#5).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 253 insertions(+), 93 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/5

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#6).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 249 insertions(+), 88 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/6

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#7).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 249 insertions(+), 88 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/7

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#8).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 264 insertions(+), 99 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/8

[Brian Nemec (Code Review)]

Brian Nemec has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

Patch Set 8:

Bump

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#9).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 322 insertions(+), 104 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/9

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#10).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 326 insertions(+), 110 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/10

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#11).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 326 insertions(+), 110 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/11

[Brian Nemec (Code Review)]

Brian Nemec has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

Patch Set 11:

The changes from the code reviews on the EC side have been moved to this side when relevant to keep the 2 code bases closer in sync.

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#13).

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

raiden_debug_spi.c: Adds context states and helper functions

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol.

Adds context states to handle the USB packets, these allow us to
simplify the process of loading data from the transmit buffer
into a USB packets' data section and from a USB packet to it's
receive buffers. These will also keep track of the size of the USB
packet allowing a simpler interface to transmit them.

Helper functions have been added to help with copying data between
the transmit and receive context states to and from the USB packets.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 324 insertions(+), 108 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/13

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

Patch Set 13:

(5 comments)

Uh, this is quite large to review in one go. Could you please split this into steps?

The commit message has two rather long paragraphs. I'd put them (and the corresponding changes) in separate commits

https://review.coreboot.org/c/flashrom/+/41608/13//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/41608/13//COMMIT_MSG@7 
PS13, Line 7: Adds
Add

https://review.coreboot.org/c/flashrom/+/41608/13//COMMIT_MSG@9 
PS13, Line 9: Adds
Add

https://review.coreboot.org/c/flashrom/+/41608/13//COMMIT_MSG@14 
PS13, Line 14: Adds
Add

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c 
File raiden_debug_spi.c:

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c@149 
PS13, Line 149:       
using tabs here instead of spaces would make it easier to keep the alignment. it's hard to spot a single-space difference unless the lines are consecutive

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c@158 
PS13, Line 158: We too much data.
Missing a verb?

[Brian Nemec (Code Review)]

Brian Nemec has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

Patch Set 14:

(2 comments)
> Patch Set 13:
>
> (5 comments)
>
> Uh, this is quite large to review in one go. Could you please split this into steps?
>
> The commit message has two rather long paragraphs. I'd put them (and the corresponding changes) in separate commits

Yes, I will break it up into separate commits.

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c 
File raiden_debug_spi.c:

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c@149 
PS13, Line 149:
> using tabs here instead of spaces would make it easier to keep the alignment. […]

There's pros and cons to using either spaces or tabs for alignment. Tabs allow consistent alignment if everyone uses the same width, but only about half of developers use any particular style. This convention takes slightly more effort when writing, but allows developers to optimize tab size to what they regard as comfortable to read which is done far more.

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c@158 
PS13, Line 158: We too much data.
> Missing a verb?

Yes, it should be 'The host received more data than can fit in read buffer'

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Adds context states and helper functions
......................................................................

Patch Set 14:

(4 comments)

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c 
File raiden_debug_spi.c:

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c@489 
PS14, Line 489:  send_comman
Please keep the moving stuff around and rename patches separated from the one with the detail semantic changes. I think that should makes it easier for Angel and I to review.

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c@538 
PS14, Line 538: write count       = %u\n"
some of this hunk can go into the previous clean up patch.

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c@570 
PS14, Line 570: 	"    write count    = %u\n"
ditto

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c@657 
PS14, Line 657: *ctx_data =
              : 		(struct raiden_debug_spi_d
this change can go into the previous clean up patch.

Change in flashrom[master]: raiden_debug_spi.c: Adds transfer context states

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#15).

Change subject: raiden_debug_spi.c: Adds transfer context states
......................................................................

raiden_debug_spi.c: Adds transfer context states

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol and easier integration with a unified USB
packet context.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 72 insertions(+), 32 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/15

Change in flashrom[master]: raiden_debug_spi.c: Adds transfer context states

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#16).

Change subject: raiden_debug_spi.c: Adds transfer context states
......................................................................

raiden_debug_spi.c: Adds transfer context states

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol and easier integration with a unified USB
packet context.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 72 insertions(+), 32 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/16

Change in flashrom[master]: raiden_debug_spi.c: Adds transfer context states

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#19).

Change subject: raiden_debug_spi.c: Adds transfer context states
......................................................................

raiden_debug_spi.c: Adds transfer context states

Adds context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol and easier integration with a unified USB
packet context.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 67 insertions(+), 32 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/19

Change in flashrom[master]: raiden_debug_spi.c: Adds transfer context states

[Brian Nemec (Code Review)]

Brian Nemec has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Adds transfer context states
......................................................................

Patch Set 19:

(4 comments)

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c 
File raiden_debug_spi.c:

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c@489 
PS14, Line 489:  send_comman
> Please keep the moving stuff around and rename patches separated from the one with the detail semant […]

Done

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c@538 
PS14, Line 538: write count       = %u\n"
> some of this hunk can go into the previous clean up patch.

Done

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c@570 
PS14, Line 570: 	"    write count    = %u\n"
> ditto

Done

https://review.coreboot.org/c/flashrom/+/41608/14/raiden_debug_spi.c@657 
PS14, Line 657: *ctx_data =
              : 		(struct raiden_debug_spi_d
> this change can go into the previous clean up patch.

Done

Change in flashrom[master]: raiden_debug_spi.c: Adds transfer context states

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Adds transfer context states
......................................................................

Patch Set 20: Code-Review+2

Change in flashrom[master]: raiden_debug_spi.c: Add transfer context states

[Brian Nemec (Code Review)]

Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/41608

to look at the new patch set (#21).

Change subject: raiden_debug_spi.c: Add transfer context states
......................................................................

raiden_debug_spi.c: Add transfer context states

Add context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol and easier integration with a unified USB
packet context.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
---
M raiden_debug_spi.c
1 file changed, 67 insertions(+), 32 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/21

Change in flashrom[master]: raiden_debug_spi.c: Add transfer context states

[Brian Nemec (Code Review)]

Brian Nemec has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Add transfer context states
......................................................................

Patch Set 21:

(5 comments)

https://review.coreboot.org/c/flashrom/+/41608/13//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/41608/13//COMMIT_MSG@7 
PS13, Line 7: Adds
> Add

Done

https://review.coreboot.org/c/flashrom/+/41608/13//COMMIT_MSG@9 
PS13, Line 9: Adds
> Add

Done

https://review.coreboot.org/c/flashrom/+/41608/13//COMMIT_MSG@14 
PS13, Line 14: Adds
> Add

Done

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c 
File raiden_debug_spi.c:

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c@149 
PS13, Line 149:
> There's pros and cons to using either spaces or tabs for alignment. […]

.

https://review.coreboot.org/c/flashrom/+/41608/13/raiden_debug_spi.c@158 
PS13, Line 158: We too much data.
> Yes, it should be 'The host received more data than can fit in read buffer'

Fixed in next CL where this was moved to.

Change in flashrom[master]: raiden_debug_spi.c: Add transfer context states

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/41608 )

Change subject: raiden_debug_spi.c: Add transfer context states
......................................................................

raiden_debug_spi.c: Add transfer context states

Add context states to handle the read and write buffers as transmit
and receive states. These are used to keep track of the number of
bytes transmitted and received allowing future support of multi-packet
messages in the v2 protocol and easier integration with a unified USB
packet context.

BUG=b:139058552
BRANCH=none
TEST=Manual testing of ServoMicro and Flashrom when performing
    reads, writes, and verification of the EC firmware on Nami.
TEST=Builds

Signed-off-by: Brian J. Nemec 
Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c
Reviewed-on: https://review.coreboot.org/c/flashrom/+/41608
Reviewed-by: Edward O'Callaghan 
Tested-by: build bot (Jenkins) 
---
M raiden_debug_spi.c
1 file changed, 67 insertions(+), 32 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Edward O'Callaghan: Looks good to me, approved

diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c
index b43fc91..d6474ec 100644
--- a/raiden_debug_spi.c
+++ b/raiden_debug_spi.c
@@ -203,6 +203,24 @@
    uint8_t data[PAYLOAD_SIZE_V1];
 } __attribute__((packed));

+struct usb_spi_transmit_ctx {
+	/* Buffer we are reading data from. */
+	const uint8_t *buffer;
+	/* Number of bytes in the transfer. */
+	size_t transmit_size;
+	/* Number of bytes transferred. */
+	size_t transmit_index;
+};
+
+struct usb_spi_receive_ctx {
+	/* Buffer we are writing data into. */
+	uint8_t *buffer;
+	/* Number of bytes in the transfer. */
+	size_t receive_size;
+	/* Number of bytes transferred. */
+	size_t receive_index;
+};
+
 /*
  * This function will return true when an error code can potentially recover
  * if we attempt to write SPI data to the device or read from it. We know
@@ -241,13 +259,16 @@
  * a USB SPI transfer. Write and read counts and payloads to write from
  * the write_buffer are transmitted to the device.
  *
+ * @param flash         Flash context storing SPI capabilities and USB device
+ *                      information.
+ * @param write         Write context of data to transmit and write payload.
+ * @param read          Read context of data to receive and read buffer.
+ *
  * @returns             Returns status code with 0 on success.
  */
 static int write_command_v1(const struct flashctx *flash,
-		unsigned int write_count,
-		unsigned int read_count,
-		const unsigned char *write_buffer,
-		unsigned char *read_buffer)
+		struct usb_spi_transmit_ctx *write,
+		struct usb_spi_receive_ctx *read)
 {

int transferred;
@@ -255,28 +276,28 @@
    struct usb_spi_command_v1 command_packet;
    const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash);

-	command_packet.write_count = write_count;
-	command_packet.read_count = read_count;
+	command_packet.write_count = write->transmit_size;
+	command_packet.read_count = read->receive_size;

-	memcpy(command_packet.data, write_buffer, write_count);
+	memcpy(command_packet.data, write->buffer, write->transmit_size);

ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle,
    			ctx_data->out_ep,
    			(void*)&command_packet,
-				write_count + PACKET_HEADER_SIZE,
+				write->transmit_size + PACKET_HEADER_SIZE,
    			&transferred,
    			TRANSFER_TIMEOUT_MS));
    if (ret != 0) {
    	msg_perr("Raiden: OUT transfer failed\n"
-		         "    write_count = %d\n"
-		         "    read_count  = %d\n",
-		         write_count, read_count);
+		         "    write_count = %zu\n"
+		         "    read_count  = %zu\n",
+		         write->transmit_size, read->receive_size);
    	return ret;
    }

-	if ((unsigned) transferred != write_count + PACKET_HEADER_SIZE) {
-		msg_perr("Raiden: Write failure (wrote %d, expected %d)\n",
-			 transferred, write_count + PACKET_HEADER_SIZE);
+	if ((unsigned) transferred != write->transmit_size + PACKET_HEADER_SIZE) {
+		msg_perr("Raiden: Write failure (wrote %d, expected %zu)\n",
+			 transferred, write->transmit_size + PACKET_HEADER_SIZE);
    	return 0x10001;
    }

@@ -288,13 +309,16 @@
  * transfer. Status codes from the transfer and any read payload are copied
  * to the read_buffer.
  *
+ * @param flash         Flash context storing SPI capabilities and USB device
+ *                      information.
+ * @param write         Write context of data to transmit and write payload.
+ * @param read          Read context of data to receive and read buffer.
+ *
  * @returns             Returns status code with 0 on success.
  */
 static int read_response_v1(const struct flashctx *flash,
-		unsigned int write_count,
-		unsigned int read_count,
-		const unsigned char *write_buffer,
-		unsigned char *read_buffer)
+		struct usb_spi_transmit_ctx *write,
+		struct usb_spi_receive_ctx *read)
 {
    int transferred;
    int ret;
@@ -304,24 +328,24 @@
    ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle,
    			ctx_data->in_ep,
    			(void*)&response_packet,
-				read_count + PACKET_HEADER_SIZE,
+				read->receive_size + PACKET_HEADER_SIZE,
    			&transferred,
    			TRANSFER_TIMEOUT_MS));
    if (ret != 0) {
    	msg_perr("Raiden: IN transfer failed\n"
-		         "    write_count = %d\n"
-		         "    read_count  = %d\n",
-		         write_count, read_count);
+		         "    write_count = %zu\n"
+		         "    read_count  = %zu\n",
+		         write->transmit_size, read->receive_size);
    	return ret;
    }

-	if ((unsigned) transferred != read_count + PACKET_HEADER_SIZE) {
-		msg_perr("Raiden: Read failure (read %d, expected %d)\n",
-				transferred, read_count + PACKET_HEADER_SIZE);
+	if ((unsigned) transferred != read->receive_size + PACKET_HEADER_SIZE) {
+		msg_perr("Raiden: Read failure (read %d, expected %zu)\n",
+				transferred, read->receive_size + PACKET_HEADER_SIZE);
    	return 0x10002;
    }

-	memcpy(read_buffer, response_packet.data, read_count);
+	memcpy(read->buffer, response_packet.data, read->receive_size);

return response_packet.status_code;
 }
@@ -348,6 +372,15 @@
 {
    int status = -1;

+	struct usb_spi_transmit_ctx write_ctx = {
+		.buffer = write_buffer,
+		.transmit_size = write_count
+	};
+	struct usb_spi_receive_ctx read_ctx = {
+		.buffer = read_buffer,
+		.receive_size = read_count
+	};
+
    if (write_count > PAYLOAD_SIZE_V1) {
    	msg_perr("Raiden: Invalid write count\n"
    		 "    write count = %u\n"
@@ -367,17 +400,19 @@
    for (unsigned int write_attempt = 0; write_attempt < WRITE_RETRY_ATTEMPTS;
             write_attempt++) {

-		status = write_command_v1(flash, write_count, read_count,
-		                       write_buffer, read_buffer);
+
+		status = write_command_v1(flash, &write_ctx, &read_ctx);

if (status) {
    		/* Write operation failed. */
    		msg_perr("Raiden: Write command failed\n"
    			 "    write count       = %u\n"
    			 "    read count        = %u\n"
+				 "    transmitted bytes = %zu\n"
    			 "    write attempt     = %u\n"
    			 "    status            = 0x%05x\n",
-				 write_count, read_count,
+
+				 write_count, read_count, write_ctx.transmit_index,
    			 write_attempt + 1, status);
    		if (!retry_recovery(status)) {
    			/* Reattempting will not result in a recovery. */
@@ -389,18 +424,18 @@
    	for (unsigned int read_attempt = 0; read_attempt < READ_RETRY_ATTEMPTS;
    			read_attempt++) {

-			status = read_response_v1(flash, write_count, read_count,
-					write_buffer, read_buffer);
+			status = read_response_v1(flash, &write_ctx, &read_ctx);

if (status) {
    			/* Read operation failed. */
    			msg_perr("Raiden: Read response failed\n"
    				 "    write count    = %u\n"
    				 "    read count     = %u\n"
+					 "    received bytes = %zu\n"
    				 "    write attempt  = %u\n"
    				 "    read attempt   = %u\n"
    				 "    status         = 0x%05x\n",
-					 write_count, read_count,
+					 write_count, read_count, read_ctx.receive_index,
    				 write_attempt + 1, read_attempt + 1, status);
    			if (!retry_recovery(status)) {
    				/* Reattempting will not result in a recovery. */