diff mbox series

[3/5] spi: zynqmp-gqspi: Abort operations on timeout

Message ID 20250116225521.2688224-4-sean.anderson@linux.dev (mailing list archive)
State New
Headers show
Series spi: zynqmp-gqspi: Improve error recovery by resetting | expand

Commit Message

Sean Anderson Jan. 16, 2025, 10:55 p.m. UTC 
When an operation times out, we leave the device (and driver) in an
inconsistent state. This generally results in all subsequent operations
timing out. Attempt to address this by resetting/reinitializing the
device when we have a timeout. This tends to be fairly robust.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/spi/spi-zynqmp-gqspi.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

Comments

Michal Simek Jan. 17, 2025, 7:15 a.m. UTC | #1 
On 1/16/25 23:55, Sean Anderson wrote:
> When an operation times out, we leave the device (and driver) in an
> inconsistent state. This generally results in all subsequent operations
> timing out. Attempt to address this by resetting/reinitializing the
> device when we have a timeout. This tends to be fairly robust.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
>   drivers/spi/spi-zynqmp-gqspi.c | 30 ++++++++++++++++++++----------
>   1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
> index 7d138f45b692..cf47466ec982 100644
> --- a/drivers/spi/spi-zynqmp-gqspi.c
> +++ b/drivers/spi/spi-zynqmp-gqspi.c
> @@ -1057,6 +1057,21 @@ static unsigned long zynqmp_qspi_timeout(struct zynqmp_qspi *xqspi, u8 bits,
>   	return msecs_to_jiffies(timeout + 100);
>   }
>   
> +

unnecessary newline here.

> +static int zynqmp_qspi_wait(struct zynqmp_qspi *xqspi, unsigned long timeout)
> +{
> +	int ret;
> +
> +	ret = wait_for_completion_timeout(&xqspi->data_completion, timeout);
> +	if (ret)
> +		return 0;

newline here please

> +	dev_err(xqspi->dev, "Operation timed out\n");
> +
> +	/* Attempt to recover as best we can */
> +	zynqmp_qspi_init_hw(xqspi);
> +	return -ETIMEDOUT;
> +}
> +
>   /**
>    * zynqmp_qspi_exec_op() - Initiates the QSPI transfer
>    * @mem: The SPI memory
> @@ -1104,11 +1119,9 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem,
>   				   GQSPI_IER_TXNOT_FULL_MASK);
>   		timeout = zynqmp_qspi_timeout(xqspi, op->cmd.buswidth,
>   					      op->cmd.nbytes);
> -		if (!wait_for_completion_timeout(&xqspi->data_completion,
> -						 timeout)) {
> -			err = -ETIMEDOUT;
> +		err = zynqmp_qspi_wait(xqspi, timeout);
> +		if (err)
>   			goto return_err;
> -		}
>   	}
>   
>   	if (op->addr.nbytes) {
> @@ -1133,11 +1146,9 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem,
>   				   GQSPI_IER_TXNOT_FULL_MASK);
>   		timeout = zynqmp_qspi_timeout(xqspi, op->addr.buswidth,
>   					      op->addr.nbytes);
> -		if (!wait_for_completion_timeout(&xqspi->data_completion,
> -						 timeout)) {
> -			err = -ETIMEDOUT;
> +		err = zynqmp_qspi_wait(xqspi, timeout);
> +		if (err)
>   			goto return_err;
> -		}
>   	}
>   
>   	if (op->dummy.nbytes) {
> @@ -1204,8 +1215,7 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem,
>   		}
>   		timeout = zynqmp_qspi_timeout(xqspi, op->data.buswidth,
>   					      op->data.nbytes);
> -		if (!wait_for_completion_timeout(&xqspi->data_completion, timeout))
> -			err = -ETIMEDOUT;
> +		err = zynqmp_qspi_wait(xqspi, timeout);
>   	}
>   
>   return_err:

M
diff mbox series

Patch

diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index 7d138f45b692..cf47466ec982 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -1057,6 +1057,21 @@  static unsigned long zynqmp_qspi_timeout(struct zynqmp_qspi *xqspi, u8 bits,
 	return msecs_to_jiffies(timeout + 100);
 }
 
+
+static int zynqmp_qspi_wait(struct zynqmp_qspi *xqspi, unsigned long timeout)
+{
+	int ret;
+
+	ret = wait_for_completion_timeout(&xqspi->data_completion, timeout);
+	if (ret)
+		return 0;
+	dev_err(xqspi->dev, "Operation timed out\n");
+
+	/* Attempt to recover as best we can */
+	zynqmp_qspi_init_hw(xqspi);
+	return -ETIMEDOUT;
+}
+
 /**
  * zynqmp_qspi_exec_op() - Initiates the QSPI transfer
  * @mem: The SPI memory
@@ -1104,11 +1119,9 @@  static int zynqmp_qspi_exec_op(struct spi_mem *mem,
 				   GQSPI_IER_TXNOT_FULL_MASK);
 		timeout = zynqmp_qspi_timeout(xqspi, op->cmd.buswidth,
 					      op->cmd.nbytes);
-		if (!wait_for_completion_timeout(&xqspi->data_completion,
-						 timeout)) {
-			err = -ETIMEDOUT;
+		err = zynqmp_qspi_wait(xqspi, timeout);
+		if (err)
 			goto return_err;
-		}
 	}
 
 	if (op->addr.nbytes) {
@@ -1133,11 +1146,9 @@  static int zynqmp_qspi_exec_op(struct spi_mem *mem,
 				   GQSPI_IER_TXNOT_FULL_MASK);
 		timeout = zynqmp_qspi_timeout(xqspi, op->addr.buswidth,
 					      op->addr.nbytes);
-		if (!wait_for_completion_timeout(&xqspi->data_completion,
-						 timeout)) {
-			err = -ETIMEDOUT;
+		err = zynqmp_qspi_wait(xqspi, timeout);
+		if (err)
 			goto return_err;
-		}
 	}
 
 	if (op->dummy.nbytes) {
@@ -1204,8 +1215,7 @@  static int zynqmp_qspi_exec_op(struct spi_mem *mem,
 		}
 		timeout = zynqmp_qspi_timeout(xqspi, op->data.buswidth,
 					      op->data.nbytes);
-		if (!wait_for_completion_timeout(&xqspi->data_completion, timeout))
-			err = -ETIMEDOUT;
+		err = zynqmp_qspi_wait(xqspi, timeout);
 	}
 
 return_err: