mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Eddie James <eajames@linux.ibm.com>
To: linux-fsi@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org, alistair@popple.id.au,
	joel@jms.id.au, jk@ozlabs.org,
	Eddie James <eajames@linux.ibm.com>
Subject: [PATCH] fsi: occ: Improve response status checking
Date: Mon, 10 Jan 2022 09:58:27 -0600	[thread overview]
Message-ID: <20220110155827.13976-1-eajames@linux.ibm.com> (raw)

If the driver sequence number coincidentally equals the previous
command response sequence number, the driver may proceed with
fetching the entire buffer before the OCC has processed the current
command. To be sure the correct response is obtained, check the
command type and also retry if any of the response parameters have
changed when the rest of the buffer is fetched.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/fsi/fsi-occ.c | 63 ++++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
index 7eaab1be0aa4..67569282dd69 100644
--- a/drivers/fsi/fsi-occ.c
+++ b/drivers/fsi/fsi-occ.c
@@ -451,6 +451,15 @@ static int occ_trigger_attn(struct occ *occ)
 	return rc;
 }
 
+static void fsi_occ_print_timeout(struct occ *occ, struct occ_response *resp,
+				  u8 seq_no, u8 cmd_type)
+{
+	dev_err(occ->dev,
+		"resp timeout status=%02x seq=%d cmd=%d, our seq=%d cmd=%d\n",
+		resp->return_status, resp->seq_no, resp->cmd_type, seq_no,
+		cmd_type);
+}
+
 int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 		   void *response, size_t *resp_len)
 {
@@ -461,12 +470,14 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 	struct occ_response *resp = response;
 	size_t user_resp_len = *resp_len;
 	u8 seq_no;
+	u8 cmd_type;
 	u16 checksum = 0;
 	u16 resp_data_length;
 	const u8 *byte_request = (const u8 *)request;
-	unsigned long start;
+	unsigned long end;
 	int rc;
 	size_t i;
+	bool retried = false;
 
 	*resp_len = 0;
 
@@ -478,6 +489,8 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 		return -EINVAL;
 	}
 
+	cmd_type = byte_request[1];
+
 	/* Checksum the request, ignoring first byte (sequence number). */
 	for (i = 1; i < req_len - 2; ++i)
 		checksum += byte_request[i];
@@ -509,30 +522,30 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 	if (rc)
 		goto done;
 
-	/* Read occ response header */
-	start = jiffies;
+retry:
+	end = jiffies + timeout;
 	do {
+		/* Read occ response header */
 		rc = occ_getsram(occ, 0, resp, 8);
 		if (rc)
 			goto done;
 
 		if (resp->return_status == OCC_RESP_CMD_IN_PRG ||
 		    resp->return_status == OCC_RESP_CRIT_INIT ||
-		    resp->seq_no != seq_no) {
-			rc = -ETIMEDOUT;
-
-			if (time_after(jiffies, start + timeout)) {
-				dev_err(occ->dev, "resp timeout status=%02x "
-					"resp seq_no=%d our seq_no=%d\n",
-					resp->return_status, resp->seq_no,
-					seq_no);
+		    resp->seq_no != seq_no || resp->cmd_type != cmd_type) {
+			if (time_after(jiffies, end)) {
+				fsi_occ_print_timeout(occ, resp, seq_no,
+						      cmd_type);
+				rc = -ETIMEDOUT;
 				goto done;
 			}
 
 			set_current_state(TASK_UNINTERRUPTIBLE);
 			schedule_timeout(wait_time);
+		} else {
+			break;
 		}
-	} while (rc);
+	} while (true);
 
 	/* Extract size of response data */
 	resp_data_length = get_unaligned_be16(&resp->data_length);
@@ -543,17 +556,29 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 		goto done;
 	}
 
-	dev_dbg(dev, "resp_status=%02x resp_data_len=%d\n",
-		resp->return_status, resp_data_length);
-
-	/* Grab the rest */
+	/* Now get the entire response; get header again in case it changed */
 	if (resp_data_length > 1) {
-		/* already got 3 bytes resp, also need 2 bytes checksum */
-		rc = occ_getsram(occ, 8, &resp->data[3], resp_data_length - 1);
+		rc = occ_getsram(occ, 0, resp, resp_data_length + 7);
 		if (rc)
 			goto done;
+
+		if (resp->return_status == OCC_RESP_CMD_IN_PRG ||
+		    resp->return_status == OCC_RESP_CRIT_INIT ||
+		    resp->seq_no != seq_no || resp->cmd_type != cmd_type) {
+			if (!retried) {
+				retried = true;
+				goto retry;
+			}
+
+			fsi_occ_print_timeout(occ, resp, seq_no, cmd_type);
+			rc = -ETIMEDOUT;
+			goto done;
+		}
 	}
 
+	dev_dbg(dev, "resp_status=%02x resp_data_len=%d\n",
+		resp->return_status, resp_data_length);
+
 	occ->client_response_size = resp_data_length + 7;
 	rc = occ_verify_checksum(occ, resp, resp_data_length);
 
@@ -598,7 +623,7 @@ static int occ_probe(struct platform_device *pdev)
 	occ->version = (uintptr_t)of_device_get_match_data(dev);
 	occ->dev = dev;
 	occ->sbefifo = dev->parent;
-	occ->sequence_number = 1;
+	occ->sequence_number = (u8)((jiffies % 0xff) + 1);
 	mutex_init(&occ->occ_lock);
 
 	if (dev->of_node) {
-- 
2.27.0


             reply	other threads:[~2022-01-10 15:58 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-10 15:58 Eddie James [this message]
2022-01-31  5:56 ` Joel Stanley
2022-01-31 15:29   ` Eddie James
2022-02-07  9:56     ` Joel Stanley
2022-02-08 15:27       ` Eddie James

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220110155827.13976-1-eajames@linux.ibm.com \
    --to=eajames@linux.ibm.com \
    --cc=alistair@popple.id.au \
    --cc=jk@ozlabs.org \
    --cc=joel@jms.id.au \
    --cc=linux-fsi@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome