forked from mirrors/linux
		
	spi: spi-cadence: Avoid read of RX FIFO before its ready
Recent changes to cdns_spi_irq introduced some issues.
Firstly, when writing the end of a longer transaction, the code in
cdns_spi_irq will write data into the TX FIFO, then immediately
fall into the if (!xspi->tx_bytes) path and attempt to read data
from the RX FIFO. However this required waiting for the TX FIFO to
empty before the RX data was ready.
Secondly, the variable trans_cnt is now rather inaccurately named
since in cases, where the watermark is set to 1, trans_cnt will be
1 but the count of bytes transferred would be much longer.
Finally, when setting up the transaction we set the watermark to 50%
of the FIFO if the transaction is great than 50% of the FIFO. However,
there is no need to split a tranaction that is smaller than the
whole FIFO, so anything up to the FIFO size can be done in a single
transaction.
Tidy up the code a little, to avoid repeatedly calling
cdns_spi_read_rx_fifo with a count of 1, and correct the three issues
noted above.
Fixes: b1b90514ea ("spi: spi-cadence: Add support for Slave mode")
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com
Link: https://lore.kernel.org/r/20230509164153.3907694-1-ckeepax@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org
			
			
This commit is contained in:
		
							parent
							
								
									4c329f5da7
								
							
						
					
					
						commit
						a84c11e16d
					
				
					 1 changed files with 14 additions and 26 deletions
				
			
		|  | @ -304,13 +304,11 @@ static int cdns_spi_setup_transfer(struct spi_device *spi, | ||||||
|  * cdns_spi_fill_tx_fifo - Fills the TX FIFO with as many bytes as possible |  * cdns_spi_fill_tx_fifo - Fills the TX FIFO with as many bytes as possible | ||||||
|  * @xspi:	Pointer to the cdns_spi structure |  * @xspi:	Pointer to the cdns_spi structure | ||||||
|  */ |  */ | ||||||
| static void cdns_spi_fill_tx_fifo(struct cdns_spi *xspi) | static void cdns_spi_fill_tx_fifo(struct cdns_spi *xspi, unsigned int avail) | ||||||
| { | { | ||||||
| 	unsigned long trans_cnt = 0; | 	unsigned long trans_cnt = 0; | ||||||
| 
 | 
 | ||||||
| 	while ((trans_cnt < xspi->tx_fifo_depth) && | 	while ((trans_cnt < avail) && (xspi->tx_bytes > 0)) { | ||||||
| 	       (xspi->tx_bytes > 0)) { |  | ||||||
| 
 |  | ||||||
| 		/* When xspi in busy condition, bytes may send failed,
 | 		/* When xspi in busy condition, bytes may send failed,
 | ||||||
| 		 * then spi control did't work thoroughly, add one byte delay | 		 * then spi control did't work thoroughly, add one byte delay | ||||||
| 		 */ | 		 */ | ||||||
|  | @ -381,33 +379,23 @@ static irqreturn_t cdns_spi_irq(int irq, void *dev_id) | ||||||
| 		spi_finalize_current_transfer(ctlr); | 		spi_finalize_current_transfer(ctlr); | ||||||
| 		status = IRQ_HANDLED; | 		status = IRQ_HANDLED; | ||||||
| 	} else if (intr_status & CDNS_SPI_IXR_TXOW) { | 	} else if (intr_status & CDNS_SPI_IXR_TXOW) { | ||||||
| 		int trans_cnt = cdns_spi_read(xspi, CDNS_SPI_THLD); | 		int threshold = cdns_spi_read(xspi, CDNS_SPI_THLD); | ||||||
|  | 		int trans_cnt = xspi->rx_bytes - xspi->tx_bytes; | ||||||
|  | 
 | ||||||
|  | 		if (threshold > 1) | ||||||
|  | 			trans_cnt -= threshold; | ||||||
|  | 
 | ||||||
| 		/* Set threshold to one if number of pending are
 | 		/* Set threshold to one if number of pending are
 | ||||||
| 		 * less than half fifo | 		 * less than half fifo | ||||||
| 		 */ | 		 */ | ||||||
| 		if (xspi->tx_bytes < xspi->tx_fifo_depth >> 1) | 		if (xspi->tx_bytes < xspi->tx_fifo_depth >> 1) | ||||||
| 			cdns_spi_write(xspi, CDNS_SPI_THLD, 1); | 			cdns_spi_write(xspi, CDNS_SPI_THLD, 1); | ||||||
| 
 | 
 | ||||||
| 		while (trans_cnt) { | 		cdns_spi_read_rx_fifo(xspi, trans_cnt); | ||||||
| 			cdns_spi_read_rx_fifo(xspi, 1); |  | ||||||
| 
 | 
 | ||||||
| 			if (xspi->tx_bytes) { | 		if (xspi->tx_bytes) { | ||||||
| 				if (xspi->txbuf) | 			cdns_spi_fill_tx_fifo(xspi, trans_cnt); | ||||||
| 					cdns_spi_write(xspi, CDNS_SPI_TXD, | 		} else { | ||||||
| 						       *xspi->txbuf++); |  | ||||||
| 				else |  | ||||||
| 					cdns_spi_write(xspi, CDNS_SPI_TXD, 0); |  | ||||||
| 				xspi->tx_bytes--; |  | ||||||
| 			} |  | ||||||
| 			trans_cnt--; |  | ||||||
| 		} |  | ||||||
| 		if (!xspi->tx_bytes) { |  | ||||||
| 			/* Fixed delay due to controller limitation with
 |  | ||||||
| 			 * RX_NEMPTY incorrect status |  | ||||||
| 			 * Xilinx AR:65885 contains more details |  | ||||||
| 			 */ |  | ||||||
| 			udelay(10); |  | ||||||
| 			cdns_spi_read_rx_fifo(xspi, xspi->rx_bytes); |  | ||||||
| 			cdns_spi_write(xspi, CDNS_SPI_IDR, | 			cdns_spi_write(xspi, CDNS_SPI_IDR, | ||||||
| 				       CDNS_SPI_IXR_DEFAULT); | 				       CDNS_SPI_IXR_DEFAULT); | ||||||
| 			spi_finalize_current_transfer(ctlr); | 			spi_finalize_current_transfer(ctlr); | ||||||
|  | @ -456,10 +444,10 @@ static int cdns_transfer_one(struct spi_controller *ctlr, | ||||||
| 	/* Set TX empty threshold to half of FIFO depth
 | 	/* Set TX empty threshold to half of FIFO depth
 | ||||||
| 	 * only if TX bytes are more than half FIFO depth. | 	 * only if TX bytes are more than half FIFO depth. | ||||||
| 	 */ | 	 */ | ||||||
| 	if (xspi->tx_bytes > (xspi->tx_fifo_depth >> 1)) | 	if (xspi->tx_bytes > xspi->tx_fifo_depth) | ||||||
| 		cdns_spi_write(xspi, CDNS_SPI_THLD, xspi->tx_fifo_depth >> 1); | 		cdns_spi_write(xspi, CDNS_SPI_THLD, xspi->tx_fifo_depth >> 1); | ||||||
| 
 | 
 | ||||||
| 	cdns_spi_fill_tx_fifo(xspi); | 	cdns_spi_fill_tx_fifo(xspi, xspi->tx_fifo_depth); | ||||||
| 	spi_transfer_delay_exec(transfer); | 	spi_transfer_delay_exec(transfer); | ||||||
| 
 | 
 | ||||||
| 	cdns_spi_write(xspi, CDNS_SPI_IER, CDNS_SPI_IXR_DEFAULT); | 	cdns_spi_write(xspi, CDNS_SPI_IER, CDNS_SPI_IXR_DEFAULT); | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Charles Keepax
						Charles Keepax