diff mbox series

[1/2] spi: fsi: Fix spurious timeout

Message ID 20220525165852.33167-2-eajames@linux.ibm.com
State Accepted
Commit 61bf40ef51aa73f6216b33563271b6acf7ea8d70
Headers show
Series spi: fsi: Fix spurious timeout | expand

Commit Message

Eddie James May 25, 2022, 4:58 p.m. UTC 
The driver may return a timeout error even if the status register
indicates that the transfer may proceed. Fix this by restructuring
the polling loop.

Fixes: 89b35e3f2851 ("spi: fsi: Implement a timeout for polling status")
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 I have one concern still, that if the kernel is very busy, it may
 schedule other work for the entire timeout period between assigning
 "end" and checking if timed out in the do/while loop... Is it worth
 worrying about this case?

 drivers/spi/spi-fsi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Mark Brown May 25, 2022, 5:55 p.m. UTC | #1 
On Wed, May 25, 2022 at 11:58:51AM -0500, Eddie James wrote:

>  I have one concern still, that if the kernel is very busy, it may
>  schedule other work for the entire timeout period between assigning
>  "end" and checking if timed out in the do/while loop... Is it worth
>  worrying about this case?

I'm not sure we can entirely avoid there being a gap, but with it being
a busy loop you'd have to be very unlucky to hit that case.  If you come
up with a fix that'd be nice.
diff mbox series

Patch

diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c
index d403a7a3021d..72ab066ce552 100644
--- a/drivers/spi/spi-fsi.c
+++ b/drivers/spi/spi-fsi.c
@@ -319,12 +319,12 @@  static int fsi_spi_transfer_data(struct fsi_spi *ctx,
 
 			end = jiffies + msecs_to_jiffies(SPI_FSI_STATUS_TIMEOUT_MS);
 			do {
+				if (time_after(jiffies, end))
+					return -ETIMEDOUT;
+
 				rc = fsi_spi_status(ctx, &status, "TX");
 				if (rc)
 					return rc;
-
-				if (time_after(jiffies, end))
-					return -ETIMEDOUT;
 			} while (status & SPI_FSI_STATUS_TDR_FULL);
 
 			sent += nb;
@@ -337,12 +337,12 @@  static int fsi_spi_transfer_data(struct fsi_spi *ctx,
 		while (transfer->len > recv) {
 			end = jiffies + msecs_to_jiffies(SPI_FSI_STATUS_TIMEOUT_MS);
 			do {
+				if (time_after(jiffies, end))
+					return -ETIMEDOUT;
+
 				rc = fsi_spi_status(ctx, &status, "RX");
 				if (rc)
 					return rc;
-
-				if (time_after(jiffies, end))
-					return -ETIMEDOUT;
 			} while (!(status & SPI_FSI_STATUS_RDR_FULL));
 
 			rc = fsi_spi_read_reg(ctx, SPI_FSI_DATA_RX, &in);