All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Stanley <joel@jms.id.au>
To: Jeremy Kerr <jk@ozlabs.org>,
	Alistar Popple <alistair@popple.id.au>,
	Eddie James <eajames@linux.ibm.com>
Cc: Andrew Jeffery <andrew@aj.id.au>,
	linux-fsi@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: [PATCH] fsi: scom: Remove retries
Date: Thu, 27 May 2021 16:31:09 +0930	[thread overview]
Message-ID: <20210527070109.225198-1-joel@jms.id.au> (raw)

On a functioning FSI link there is not need to retry a write when doing
a scom in the driver.

Allow the higher layers (eg. userspace) to attempt a retry if they want,
or to accept that the address they are talking to is not accessible.

By removing the retries we can separate the error handling from retry
logic. In particular -EBUSY was used to force the get/put scom logic to
retry.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
This will go in after Eddie's patch:

 https://lore.kernel.org/r/20210329151344.14246-1-eajames@linux.ibm.com

 drivers/fsi/fsi-scom.c | 88 ++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 60 deletions(-)

diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
index b45bfab7b7f5..1d04ffa542c1 100644
--- a/drivers/fsi/fsi-scom.c
+++ b/drivers/fsi/fsi-scom.c
@@ -60,7 +60,6 @@
 #define XSCOM_ADDR_FORM1_HI_SHIFT	20
 
 /* Retries */
-#define SCOM_MAX_RETRIES		100	/* Retries on busy */
 #define SCOM_MAX_IND_RETRIES		10	/* Retries indirect not ready */
 
 struct scom_device {
@@ -247,7 +246,6 @@ static int handle_fsi2pib_status(struct scom_device *scom, uint32_t status)
 				 sizeof(uint32_t));
 		return -EIO;
 	}
-	/* Return -EBUSY on PIB abort to force a retry */
 	if (status & SCOM_STATUS_PIB_ABORT)
 		return -EBUSY;
 	return 0;
@@ -284,69 +282,39 @@ static int handle_pib_status(struct scom_device *scom, uint8_t status)
 static int put_scom(struct scom_device *scom, uint64_t value,
 		    uint64_t addr)
 {
-	uint32_t status, dummy = -1;
-	int rc, retries;
-
-	for (retries = 0; retries < SCOM_MAX_RETRIES; retries++) {
-		rc = raw_put_scom(scom, value, addr, &status);
-		if (rc) {
-			/* Try resetting the bridge if FSI fails */
-			if (rc != -ENODEV && retries == 0) {
-				fsi_device_write(scom->fsi_dev, SCOM_FSI2PIB_RESET_REG,
-						 &dummy, sizeof(uint32_t));
-				rc = -EBUSY;
-			} else
-				return rc;
-		} else
-			rc = handle_fsi2pib_status(scom, status);
-		if (rc && rc != -EBUSY)
-			break;
-		if (rc == 0) {
-			rc = handle_pib_status(scom,
-					       (status & SCOM_STATUS_PIB_RESP_MASK)
-					       >> SCOM_STATUS_PIB_RESP_SHIFT);
-			if (rc && rc != -EBUSY)
-				break;
-		}
-		if (rc == 0)
-			break;
-		msleep(1);
-	}
-	return rc;
+	uint32_t status;
+	int rc;
+
+	rc = raw_put_scom(scom, value, addr, &status);
+	if (rc == -ENODEV)
+		return rc;
+
+	rc = handle_fsi2pib_status(scom, status);
+	if (rc)
+		return rc;
+
+	return handle_pib_status(scom,
+				 (status & SCOM_STATUS_PIB_RESP_MASK)
+				 >> SCOM_STATUS_PIB_RESP_SHIFT);
 }
 
 static int get_scom(struct scom_device *scom, uint64_t *value,
 		    uint64_t addr)
 {
-	uint32_t status, dummy = -1;
-	int rc, retries;
-
-	for (retries = 0; retries < SCOM_MAX_RETRIES; retries++) {
-		rc = raw_get_scom(scom, value, addr, &status);
-		if (rc) {
-			/* Try resetting the bridge if FSI fails */
-			if (rc != -ENODEV && retries == 0) {
-				fsi_device_write(scom->fsi_dev, SCOM_FSI2PIB_RESET_REG,
-						 &dummy, sizeof(uint32_t));
-				rc = -EBUSY;
-			} else
-				return rc;
-		} else
-			rc = handle_fsi2pib_status(scom, status);
-		if (rc && rc != -EBUSY)
-			break;
-		if (rc == 0) {
-			rc = handle_pib_status(scom,
-					       (status & SCOM_STATUS_PIB_RESP_MASK)
-					       >> SCOM_STATUS_PIB_RESP_SHIFT);
-			if (rc && rc != -EBUSY)
-				break;
-		}
-		if (rc == 0)
-			break;
-		msleep(1);
-	}
-	return rc;
+	uint32_t status;
+	int rc;
+
+	rc = raw_get_scom(scom, value, addr, &status);
+	if (rc == -ENODEV)
+		return rc;
+
+	rc = handle_fsi2pib_status(scom, status);
+	if (rc)
+		return rc;
+
+	return handle_pib_status(scom,
+				 (status & SCOM_STATUS_PIB_RESP_MASK)
+				 >> SCOM_STATUS_PIB_RESP_SHIFT);
 }
 
 static ssize_t scom_read(struct file *filep, char __user *buf, size_t len,
-- 
2.30.2


                 reply	other threads:[~2021-05-27  7:01 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20210527070109.225198-1-joel@jms.id.au \
    --to=joel@jms.id.au \
    --cc=alistair@popple.id.au \
    --cc=andrew@aj.id.au \
    --cc=eajames@linux.ibm.com \
    --cc=jk@ozlabs.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.