| Message ID | 20211117133110.2682631-2-vkoul@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/3] spi: qcom: geni: remove unused defines | expand |
Hi, On Wed, Nov 17, 2021 at 5:31 AM Vinod Koul <vkoul@kernel.org> wrote: > > @@ -346,17 +346,20 @@ spi_gsi_callback_result(void *cb, const struct dmaengine_result *result) > { > struct spi_master *spi = cb; > > + spi->cur_msg->status = -EIO; > if (result->result != DMA_TRANS_NOERROR) { > dev_err(&spi->dev, "DMA txn failed: %d\n", result->result); > return; > } Don't you want to call spi_finalize_current_transfer() in the case of a DMA error? Otherwise I think you're still going to wait for a timeout? ...and then when you get the timeout then spi_transfer_wait() will return -ETIMEDOUT and that will overwrite your -EIO, won't it? -Doug
On Wed, Nov 17, 2021 at 07:01:09PM +0530, Vinod Koul wrote: > Before we invoke spi_finalize_current_transfer() in > spi_gsi_callback_result() we should set the spi->cur_msg->status as > appropriate (0 for success, error otherwise). Fixes should come at the start of the patch series to make sure they can be applied as fixes without pulling in anything else.
On 17-11-21, 14:20, Doug Anderson wrote: > Hi, > > On Wed, Nov 17, 2021 at 5:31 AM Vinod Koul <vkoul@kernel.org> wrote: > > > > @@ -346,17 +346,20 @@ spi_gsi_callback_result(void *cb, const struct dmaengine_result *result) > > { > > struct spi_master *spi = cb; > > > > + spi->cur_msg->status = -EIO; > > if (result->result != DMA_TRANS_NOERROR) { > > dev_err(&spi->dev, "DMA txn failed: %d\n", result->result); > > return; > > } > > Don't you want to call spi_finalize_current_transfer() in the case of > a DMA error? Otherwise I think you're still going to wait for a > timeout? ...and then when you get the timeout then spi_transfer_wait() > will return -ETIMEDOUT and that will overwrite your -EIO, won't it? Yes missed that and this reply :( Fixed now and posting v2
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 413fa1a7a936..b9769de1f5f0 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -346,17 +346,20 @@ spi_gsi_callback_result(void *cb, const struct dmaengine_result *result) { struct spi_master *spi = cb; + spi->cur_msg->status = -EIO; if (result->result != DMA_TRANS_NOERROR) { dev_err(&spi->dev, "DMA txn failed: %d\n", result->result); return; } if (!result->residue) { + spi->cur_msg->status = 0; dev_dbg(&spi->dev, "DMA txn completed\n"); - spi_finalize_current_transfer(spi); } else { dev_err(&spi->dev, "DMA xfer has pending: %d\n", result->residue); } + + spi_finalize_current_transfer(spi); } static int setup_gsi_xfer(struct spi_transfer *xfer, struct spi_geni_master *mas,
Before we invoke spi_finalize_current_transfer() in spi_gsi_callback_result() we should set the spi->cur_msg->status as appropriate (0 for success, error otherwise). The helps to return error on transfer and not wait till it timesout on error Signed-off-by: Vinod Koul <vkoul@kernel.org> --- drivers/spi/spi-geni-qcom.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)