| Message ID | 20240123153421.715951-9-tudor.ambarus@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | spi: s3c64xx: winter cleanup and gs101 support | expand |
Hi Tudor, On Tue, 2024-01-23 at 15:34 +0000, Tudor Ambarus wrote: > @@ -538,13 +538,8 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, > cpu_relax(); > status = readl(regs + S3C64XX_SPI_STATUS); > } > - > } > > - /* If timed out while checking rx/tx status return error */ > - if (!val) > - return -EIO; > - This change behaviour of this function. The loop just above adjusts val and it is used to determine if there was a timeout or not: if (val && !xfer->rx_buf) { val = msecs_to_loops(10); status = readl(regs + S3C64XX_SPI_STATUS); while ((TX_FIFO_LVL(status, sdd) || !S3C64XX_SPI_ST_TX_DONE(status, sdd)) && --val) { cpu_relax(); status = readl(regs + S3C64XX_SPI_STATUS); } That doesn't work anymore now. Cheers, Andre'
On 1/24/24 09:21, André Draszik wrote: > Hi Tudor, > Hi! > On Tue, 2024-01-23 at 15:34 +0000, Tudor Ambarus wrote: >> @@ -538,13 +538,8 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, >> cpu_relax(); >> status = readl(regs + S3C64XX_SPI_STATUS); >> } >> - >> } >> >> - /* If timed out while checking rx/tx status return error */ >> - if (!val) >> - return -EIO; >> - > > This change behaviour of this function. The loop just above adjusts val and it is used to > determine if there was a timeout or not: > > if (val && !xfer->rx_buf) { > val = msecs_to_loops(10); > status = readl(regs + S3C64XX_SPI_STATUS); > while ((TX_FIFO_LVL(status, sdd) > || !S3C64XX_SPI_ST_TX_DONE(status, sdd)) > && --val) { > cpu_relax(); > status = readl(regs + S3C64XX_SPI_STATUS); > } > Oh, yes, the timeout in this block. You're right, I'll drop the patch. Thanks! ta
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 16eea56892a2..128c3b8211ce 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -519,17 +519,17 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, val = msecs_to_jiffies(ms) + 10; val = wait_for_completion_timeout(&sdd->xfer_completion, val); - + if (!val) + return -EIO; /* - * If the previous xfer was completed within timeout, then - * proceed further else return -EIO. + * If the previous xfer was completed within timeout proceed further. * DmaTx returns after simply writing data in the FIFO, * w/o waiting for real transmission on the bus to finish. * DmaRx returns only after Dma read data from FIFO which * needs bus transmission to finish, so we don't worry if * Xfer involved Rx(with or without Tx). */ - if (val && !xfer->rx_buf) { + if (!xfer->rx_buf) { val = msecs_to_loops(10); status = readl(regs + S3C64XX_SPI_STATUS); while ((TX_FIFO_LVL(status, sdd) @@ -538,13 +538,8 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, cpu_relax(); status = readl(regs + S3C64XX_SPI_STATUS); } - } - /* If timed out while checking rx/tx status return error */ - if (!val) - return -EIO; - return 0; }
Check the return value of wait_for_completion_timeout() immediately after the call so that we avoid checking the return value twice. Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- drivers/spi/spi-s3c64xx.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)