[PATCH linux dev-4.13 2/2] fsi: sbefifo: Return 0 on read() to indicate end of response

[Andrew Jeffery]

Previously the driver would return EAGAIN for a read() subsequent to
last byte being read. This makes it difficult to userspace to read
messages out of the pipe without knowing the size of the response ahead
of time, which is actually impossible given the design of the hardware.

The justification for moving the SBEFIFO_XFR_COMPLETE test is as
follows:

1. Assume that the transfer has completed from the hardware perspective,
   that is, we've acknowledged the EOT flag and set SBEFIFO_XFR_COMPLETE,
   but the client (caller) has not completely read out the response data

2. The client calls through sbefifo_read_common() and hits
   wait_event_interruptible(), which calls through sbefifo_read_ready()
   to judge the state of the transfer

3. sbefifo_read_ready() reports the number of words available and tests
   for SBEFIFO_XFR_COMPLETE. Given the assumed state above this will
   cause wait_event_interruptible() to return immediately

4. The appropriate copy is performed dependent on the client and the
   relative buffer sizes

5. If the destination buffer is smaller than the remaining transfer data
   then sbefifo_buf_readnb() will return false by its ring-buffer index
   calculations, the transfer will remain the current transfer, and we
   repeat from step 2.

6. If the destination buffer is equal to or larger than the remaining
   transfer data then sbefifo_buf_readnb() will return true indicating
   the transfer has been completely drained.

7. Assuming we take 6, we go around again, repeating steps 2 and 3,
   however sbefifo_read_read() will report 0 this time around, which
   triggers the removal of the transfer from the client and transfers
   list, and returns 0 to the caller.

Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
---
 drivers/fsi/fsi-sbefifo.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index afb5ff48aa3b..6f0e3f6c53d5 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -640,9 +640,6 @@ static ssize_t sbefifo_read_common(struct sbefifo_client *client,
 		if (sbefifo_xfr_test(client, SBEFIFO_XFR_WRITE_DONE))
 			return -EAGAIN;
 
-		if (sbefifo_xfr_test(client, SBEFIFO_XFR_COMPLETE))
-			return -EAGAIN;
-
 		if (sbefifo_xfr_test(client, SBEFIFO_XFR_CANCEL))
 			return -EAGAIN;
 	}
@@ -666,6 +663,23 @@ static ssize_t sbefifo_read_common(struct sbefifo_client *client,
 
 	n = min_t(size_t, n, len);
 
+	/* Check if we've performed the final read */
+	if (sbefifo_xfr_test(client, SBEFIFO_XFR_COMPLETE) && !n) {
+		xfr = list_first_entry_or_null(&client->xfrs,
+					       struct sbefifo_xfr, client);
+		if (!xfr) {
+			/* should be impossible to not have an xfr here */
+			WARN_ONCE(1, "no xfr in queue");
+			ret = -EPROTO;
+			goto out;
+		}
+
+		list_del(&xfr->client);
+		kfree(xfr);
+		wake_up_interruptible(&sbefifo->wait);
+		return 0;
+	}
+
 	if (ubuf) {
 		if (copy_to_user(ubuf, READ_ONCE(client->rbuf.rpos), n)) {
 			ret = -EFAULT;
@@ -690,10 +704,6 @@ static ssize_t sbefifo_read_common(struct sbefifo_client *client,
 			sbefifo_get(sbefifo);
 			if (!queue_work(sbefifo_wq, &sbefifo->work.work))
 				sbefifo_put(sbefifo);
-		} else {
-			list_del(&xfr->client);
-			kfree(xfr);
-			wake_up_interruptible(&sbefifo->wait);
 		}
 	}
 
-- 
2.17.0