firehose: Unbreak db410c/msm8916 support

Somewhere along the road, we broke support for the programmer for
msm8916 (and presumably other similar socs). Attempts to operate
firehose against these boards would exclusively result in USB timeouts
on write.

The reason for this is that, in contrast to all other commonly used
targets, the programmer for MSM8916 generates <log/> messages after the
<response/> message and writes are not services until all the <log/>
entries are consumed.

Rework the read loop, to continue reading messages until a timeout or an
error occurs, and then return the response status, if one was previously
found. This cost us 100ms per attempt to read a response, but I've not
found any other way to determine that we've consumed all responses.

Naively doing this does however break <read/>, as immediately following
the response with "rawmode=true" the device starts sending the data.
Adding support for parsing out "rawmode" and immediately terminating the
loop solves this, and reduces the impact of the 100ms penalty (as it no
longer applies to either <program/> or <read/>.

Signed-off-by: Bjorn Andersson <[email protected]>

Author: quic-bjorande

firehose.c

@@ -83,7 +83,7 @@ static xmlNode *firehose_response_parse(const void *buf, size_t len, int *error)
 	return node;
 }
 
-static int firehose_generic_parser(xmlNode *node, void *data)
+static int firehose_generic_parser(xmlNode *node, void *data, bool *rawmode)
 {
 	xmlChar *value;
 	int ret = -EINVAL;
@@ -103,18 +103,27 @@ static int firehose_generic_parser(xmlNode *node, void *data)
 
 	xmlFree(value);
 
+	value = xmlGetProp(node, (xmlChar *)"rawmode");
+	if (value) {
+		if (xmlStrcmp(value, (xmlChar *)"true") == 0)
+			*rawmode = true;
+		xmlFree(value);
+	}
+
 	return ret;
 }
 
 static int firehose_read(struct qdl_device *qdl, int timeout_ms,
-			 int (*response_parser)(xmlNode *node, void *data),
+			 int (*response_parser)(xmlNode *node, void *data, bool *rawmode),
 			 void *data)
 {
 	char buf[4096];
 	xmlNode *node;
 	int error;
+	int resp = -EIO;
 	int ret = -EAGAIN;
 	int n;
+	bool rawmode = false;
 	struct timeval timeout;
 	struct timeval now;
 	struct timeval delta = { .tv_sec = timeout_ms / 1000,
@@ -127,9 +136,40 @@ static int firehose_read(struct qdl_device *qdl, int timeout_ms,
 	if (qdl->dev_type == QDL_DEVICE_SIM)
 		return 0;
 
-	do {
+	/*
+	 * The goal of firehose_read() is to find a response to a request among
+	 * one or more incoming messages AND to consume all incoming messages
+	 * (otherwise subsequent writes will time out).
+	 * The messages can be one of:
+	 * - <log/>
+	 * - <response value=""/>
+	 * - <response value="" rawmode="true"/>
+	 *
+	 * Generally <log/> messages are coming prior to the <response/>, but
+	 * on MSM8916 (at least) it's been observed that <log/> messages can
+	 * arrive after the <response/>.
+	 *
+	 * We therefor need to consume messages until there are no more
+	 * (timeout) and we have been able to parse out a response (using
+	 * @response_parser).
+	 *
+	 * In the special case that the <response/> contain an attribute
+	 * "rawmode=true", the device signals that it has entered a mode where
+	 * it will not send/receive XML-formatted commands. So, (at least for
+	 * reads) we need to shortcircuit the logic and directly terminate the
+	 * consumption of incoming data.
+	 */
+	for (;;) {
 		n = qdl_read(qdl, buf, sizeof(buf), 100);
-		if (n <= 0) {
+
+		/* Timeout after seeing a response, we're done waiting for logs */
+		if (n == -ETIMEDOUT && resp >= 0)
+			break;
+		/* We want to return resp on error, to not loose the reset resposne */
+		else if (n == -EIO)
+			break;
+
+		if (n == -ETIMEDOUT || n == 0) {
 			gettimeofday(&now, NULL);
 			if (timercmp(&now, &timeout, <))
 				continue;
@@ -144,11 +184,17 @@ static int firehose_read(struct qdl_device *qdl, int timeout_ms,
 		if (!node)
 			return error;
 
-		ret = response_parser(node, data);
+		ret = response_parser(node, data, &rawmode);
 		xmlFreeDoc(node->doc);
-	} while (ret == -EAGAIN);
 
-	return ret;
+		if (ret >= 0)
+			resp = ret;
+
+		if (rawmode)
+			break;
+	}
+
+	return resp;
 }
 
 static int firehose_write(struct qdl_device *qdl, xmlDoc *doc)
@@ -208,7 +254,7 @@ static int firehose_write(struct qdl_device *qdl, xmlDoc *doc)
  *
  * Return: max size supported by the remote, or negative errno on failure
  */
-static int firehose_configure_response_parser(xmlNode *node, void *data)
+static int firehose_configure_response_parser(xmlNode *node, void *data, bool *rawmode)
 {
 	xmlChar *payload;
 	xmlChar *value;

io.c

@@ -38,6 +38,16 @@ void qdl_close(struct qdl_device *qdl)
 	qdl->close(qdl);
 }
 
+/**
+ * qdl_read() - Read a message from the device
+ * @qdl: device handle
+ * @buf: buffer to write the data into
+ * @len: maximum length of data to be read
+ * @timeout: timeout for the read, in milliseconds
+ *
+ * Returns: number of bytes read, might be zero for a ZLP
+ *	    negative errno on failure (notably -ETIMEDOUT)
+ */
 int qdl_read(struct qdl_device *qdl, void *buf, size_t len, unsigned int timeout)
 {
 	return qdl->read(qdl, buf, len, timeout);

usb.c

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: BSD-3-Clause
 #include <sys/types.h>
+#include <errno.h>
 #include <fcntl.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -237,9 +238,11 @@ static int usb_read(struct qdl_device *qdl, void *buf, size_t len, unsigned int
 	int ret;
 
 	ret = libusb_bulk_transfer(qdl_usb->usb_handle, qdl_usb->in_ep, buf, len, &actual, timeout);
-	if ((ret != 0 && ret != LIBUSB_ERROR_TIMEOUT) ||
-	    (ret == LIBUSB_ERROR_TIMEOUT && actual == 0))
-		return -1;
+	if (ret != 0 && ret != LIBUSB_ERROR_TIMEOUT)
+		return -EIO;
+
+	if (ret == LIBUSB_ERROR_TIMEOUT && actual == 0)
+		return -ETIMEDOUT;
 
 	return actual;
 }