[PATCH] spi: spi-cadence: Move TX FIFO full busy-wait into FIFO

Srikanth Boyapally posted 1 patch 4 days, 13 hours ago
drivers/spi/spi-cadence.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
[PATCH] spi: spi-cadence: Move TX FIFO full busy-wait into FIFO
Posted by Srikanth Boyapally 4 days, 13 hours ago
SPI host transfers could intermittently stall with spi_transfer timeouts.
The TXFULL condition was checked only once in cdns_transfer_one() before
cdns_spi_process_fifo(), so if the FIFO became full again during refill,
writes could be dropped and the transfer would never complete.

Move the TXFULL busy-wait into the TX path of cdns_spi_process_fifo() so
the 10µs back-off is applied per FIFO entry during filling, ensuring
forward progress and eliminating spurious timeouts.

Restrict the delay to host mode using spi_controller_is_target(), the
controller is passed into cdns_spi_process_fifo() so the check is made at
the point of use. In target mode this delay must not run as it causes the
target to miss its transfer window and corrupt data.

Fixes: 49530e641178 ("spi: cadence: Add usleep_range() for cdns_spi_fill_tx_fifo()")
Signed-off-by: Srikanth Boyapally <srikanth.boyapally@amd.com>
---
Note: checkpatch suggests usleep_range() over udelay(), but this code
      runs in interrupt context where sleeping is not allowed, so udelay() is
      intentional.
---
 drivers/spi/spi-cadence.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
index 9b4e5b7013ae..af1a05e78492 100644
--- a/drivers/spi/spi-cadence.c
+++ b/drivers/spi/spi-cadence.c
@@ -388,11 +388,13 @@ static inline void cdns_spi_writer(struct cdns_spi *xspi)
 
 /**
  * cdns_spi_process_fifo - Fills the TX FIFO, and drain the RX FIFO
+ * @ctlr:	Pointer to the spi_controller structure
  * @xspi:	Pointer to the cdns_spi structure
  * @ntx:	Number of bytes to pack into the TX FIFO
  * @nrx:	Number of bytes to drain from the RX FIFO
  */
-static void cdns_spi_process_fifo(struct cdns_spi *xspi, int ntx, int nrx)
+static void cdns_spi_process_fifo(struct spi_controller *ctlr,
+				  struct cdns_spi *xspi, int ntx, int nrx)
 {
 	ntx = clamp(ntx, 0, xspi->tx_bytes);
 	nrx = clamp(nrx, 0, xspi->rx_bytes);
@@ -407,6 +409,16 @@ static void cdns_spi_process_fifo(struct cdns_spi *xspi, int ntx, int nrx)
 		}
 
 		if (ntx) {
+			/* When xspi in busy condition, bytes may send failed,
+			 * then spi control didn't work thoroughly, add one byte
+			 * delay. Only in host mode; in target mode this delay
+			 * causes data corruption as the target fails to prepare
+			 * data in time.
+			 */
+			if (!spi_controller_is_target(ctlr) &&
+			    (cdns_spi_read(xspi, CDNS_SPI_ISR) & CDNS_SPI_IXR_TXFULL))
+				udelay(10);
+
 			cdns_spi_writer(xspi);
 			ntx--;
 		}
@@ -460,14 +472,14 @@ static irqreturn_t cdns_spi_irq(int irq, void *dev_id)
 			cdns_spi_write(xspi, CDNS_SPI_THLD, 1);
 
 		if (xspi->tx_bytes) {
-			cdns_spi_process_fifo(xspi, trans_cnt, trans_cnt);
+			cdns_spi_process_fifo(ctlr, xspi, trans_cnt, trans_cnt);
 		} else {
 			/* Fixed delay due to controller limitation with
 			 * RX_NEMPTY incorrect status
 			 * Xilinx AR:65885 contains more details
 			 */
 			udelay(10);
-			cdns_spi_process_fifo(xspi, 0, trans_cnt);
+			cdns_spi_process_fifo(ctlr, xspi, 0, trans_cnt);
 			cdns_spi_write(xspi, CDNS_SPI_IDR,
 				       CDNS_SPI_IXR_DEFAULT);
 			spi_finalize_current_transfer(ctlr);
@@ -520,17 +532,11 @@ static int cdns_transfer_one(struct spi_controller *ctlr,
 			cdns_spi_write(xspi, CDNS_SPI_THLD, xspi->tx_fifo_depth >> 1);
 	}
 
-	/* When xspi in busy condition, bytes may send failed,
-	 * then spi control didn't work thoroughly, add one byte delay
-	 */
-	if (cdns_spi_read(xspi, CDNS_SPI_ISR) & CDNS_SPI_IXR_TXFULL)
-		udelay(10);
-
 	xspi->n_bytes = cdns_spi_n_bytes(transfer);
 	xspi->tx_bytes = DIV_ROUND_UP(xspi->tx_bytes, xspi->n_bytes);
 	xspi->rx_bytes = DIV_ROUND_UP(xspi->rx_bytes, xspi->n_bytes);
 
-	cdns_spi_process_fifo(xspi, xspi->tx_fifo_depth, 0);
+	cdns_spi_process_fifo(ctlr, xspi, xspi->tx_fifo_depth, 0);
 
 	cdns_spi_write(xspi, CDNS_SPI_IER, CDNS_SPI_IXR_DEFAULT);
 	return transfer->len;
-- 
2.34.1

Re: [PATCH] spi: spi-cadence: Move TX FIFO full busy-wait into FIFO
Posted by Pandey, Radhey Shyam 4 days, 10 hours ago
On 7/20/2026 6:25 PM, Srikanth Boyapally wrote:
> SPI host transfers could intermittently stall with spi_transfer timeouts.
> The TXFULL condition was checked only once in cdns_transfer_one() before
> cdns_spi_process_fifo(), so if the FIFO became full again during refill,
> writes could be dropped and the transfer would never complete.
> 
> Move the TXFULL busy-wait into the TX path of cdns_spi_process_fifo() so
> the 10µs back-off is applied per FIFO entry during filling, ensuring
> forward progress and eliminating spurious timeouts.
> 
> Restrict the delay to host mode using spi_controller_is_target(), the
> controller is passed into cdns_spi_process_fifo() so the check is made at
> the point of use. In target mode this delay must not run as it causes the
> target to miss its transfer window and corrupt data.
> 
> Fixes: 49530e641178 ("spi: cadence: Add usleep_range() for cdns_spi_fill_tx_fifo()")
> Signed-off-by: Srikanth Boyapally <srikanth.boyapally@amd.com>
> ---

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!

> Note: checkpatch suggests usleep_range() over udelay(), but this code
>        runs in interrupt context where sleeping is not allowed, so udelay() is
>        intentional.
> ---
>   drivers/spi/spi-cadence.c | 26 ++++++++++++++++----------
>   1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
> index 9b4e5b7013ae..af1a05e78492 100644
> --- a/drivers/spi/spi-cadence.c
> +++ b/drivers/spi/spi-cadence.c
> @@ -388,11 +388,13 @@ static inline void cdns_spi_writer(struct cdns_spi *xspi)
>   
>   /**
>    * cdns_spi_process_fifo - Fills the TX FIFO, and drain the RX FIFO
> + * @ctlr:	Pointer to the spi_controller structure
>    * @xspi:	Pointer to the cdns_spi structure
>    * @ntx:	Number of bytes to pack into the TX FIFO
>    * @nrx:	Number of bytes to drain from the RX FIFO
>    */
> -static void cdns_spi_process_fifo(struct cdns_spi *xspi, int ntx, int nrx)
> +static void cdns_spi_process_fifo(struct spi_controller *ctlr,
> +				  struct cdns_spi *xspi, int ntx, int nrx)
>   {
>   	ntx = clamp(ntx, 0, xspi->tx_bytes);
>   	nrx = clamp(nrx, 0, xspi->rx_bytes);
> @@ -407,6 +409,16 @@ static void cdns_spi_process_fifo(struct cdns_spi *xspi, int ntx, int nrx)
>   		}
>   
>   		if (ntx) {
> +			/* When xspi in busy condition, bytes may send failed,
> +			 * then spi control didn't work thoroughly, add one byte
> +			 * delay. Only in host mode; in target mode this delay
> +			 * causes data corruption as the target fails to prepare
> +			 * data in time.
> +			 */
> +			if (!spi_controller_is_target(ctlr) &&
> +			    (cdns_spi_read(xspi, CDNS_SPI_ISR) & CDNS_SPI_IXR_TXFULL))
> +				udelay(10);
> +
>   			cdns_spi_writer(xspi);
>   			ntx--;
>   		}
> @@ -460,14 +472,14 @@ static irqreturn_t cdns_spi_irq(int irq, void *dev_id)
>   			cdns_spi_write(xspi, CDNS_SPI_THLD, 1);
>   
>   		if (xspi->tx_bytes) {
> -			cdns_spi_process_fifo(xspi, trans_cnt, trans_cnt);
> +			cdns_spi_process_fifo(ctlr, xspi, trans_cnt, trans_cnt);
>   		} else {
>   			/* Fixed delay due to controller limitation with
>   			 * RX_NEMPTY incorrect status
>   			 * Xilinx AR:65885 contains more details
>   			 */
>   			udelay(10);
> -			cdns_spi_process_fifo(xspi, 0, trans_cnt);
> +			cdns_spi_process_fifo(ctlr, xspi, 0, trans_cnt);
>   			cdns_spi_write(xspi, CDNS_SPI_IDR,
>   				       CDNS_SPI_IXR_DEFAULT);
>   			spi_finalize_current_transfer(ctlr);
> @@ -520,17 +532,11 @@ static int cdns_transfer_one(struct spi_controller *ctlr,
>   			cdns_spi_write(xspi, CDNS_SPI_THLD, xspi->tx_fifo_depth >> 1);
>   	}
>   
> -	/* When xspi in busy condition, bytes may send failed,
> -	 * then spi control didn't work thoroughly, add one byte delay
> -	 */
> -	if (cdns_spi_read(xspi, CDNS_SPI_ISR) & CDNS_SPI_IXR_TXFULL)
> -		udelay(10);
> -
>   	xspi->n_bytes = cdns_spi_n_bytes(transfer);
>   	xspi->tx_bytes = DIV_ROUND_UP(xspi->tx_bytes, xspi->n_bytes);
>   	xspi->rx_bytes = DIV_ROUND_UP(xspi->rx_bytes, xspi->n_bytes);
>   
> -	cdns_spi_process_fifo(xspi, xspi->tx_fifo_depth, 0);
> +	cdns_spi_process_fifo(ctlr, xspi, xspi->tx_fifo_depth, 0);
>   
>   	cdns_spi_write(xspi, CDNS_SPI_IER, CDNS_SPI_IXR_DEFAULT);
>   	return transfer->len;