diff mbox series

[2/3] spi: qcom: geni: set the error code for gpi transfer

Message ID 20211117133110.2682631-2-vkoul@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/3] spi: qcom: geni: remove unused defines | expand

Commit Message

Vinod Koul Nov. 17, 2021, 1:31 p.m. UTC
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(-)

Comments

Doug Anderson Nov. 17, 2021, 10:20 p.m. UTC | #1
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
Mark Brown Nov. 18, 2021, 12:09 p.m. UTC | #2
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.
Vinod Koul Jan. 3, 2022, 7:10 a.m. UTC | #3
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 mbox series

Patch

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,