[PATCH v2 1/2] spi: qcom: geni: set the error code for gpi transfer

[PATCH v2 1/2] spi: qcom: geni: set the error code for gpi transfer

[Vinod Koul]

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

Fixes: b59c122484ec ("spi: spi-geni-qcom: Add support for GPI dma")
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---

Changes in v2:
 - add missing spi_finalize_current_transfer() for dma error case

 drivers/spi/spi-geni-qcom.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 413fa1a7a936..b82f3ddff0f4 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -346,17 +346,21 @@ 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);
+		spi_finalize_current_transfer(spi);
 		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,
-- 
2.31.1

[PATCH v2 2/2] spi: qcom: geni: handle timeout for gpi mode

[Vinod Koul]

We missed adding handle_err for gpi mode, so add a new function
spi_geni_handle_err() which would call handle_fifo_timeout() or newly
added handle_gpi_timeout() based on mode

Fixes: b59c122484ec ("spi: spi-geni-qcom: Add support for GPI dma")
Reported-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
changes in v2:
 - add tags from Doug

 drivers/spi/spi-geni-qcom.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index b82f3ddff0f4..f7d905d2a90f 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -164,6 +164,30 @@ static void handle_fifo_timeout(struct spi_master *spi,
 	}
 }
 
+static void handle_gpi_timeout(struct spi_master *spi, struct spi_message *msg)
+{
+	struct spi_geni_master *mas = spi_master_get_devdata(spi);
+
+	dmaengine_terminate_sync(mas->tx);
+	dmaengine_terminate_sync(mas->rx);
+}
+
+static void spi_geni_handle_err(struct spi_master *spi, struct spi_message *msg)
+{
+	struct spi_geni_master *mas = spi_master_get_devdata(spi);
+
+	switch (mas->cur_xfer_mode) {
+	case GENI_SE_FIFO:
+		handle_fifo_timeout(spi, msg);
+		break;
+	case GENI_GPI_DMA:
+		handle_gpi_timeout(spi, msg);
+		break;
+	default:
+		dev_err(mas->dev, "Abort on Mode:%d not supported", mas->cur_xfer_mode);
+	}
+}
+
 static bool spi_geni_is_abort_still_pending(struct spi_geni_master *mas)
 {
 	struct geni_se *se = &mas->se;
@@ -922,7 +946,7 @@ static int spi_geni_probe(struct platform_device *pdev)
 	spi->can_dma = geni_can_dma;
 	spi->dma_map_dev = dev->parent;
 	spi->auto_runtime_pm = true;
-	spi->handle_err = handle_fifo_timeout;
+	spi->handle_err = spi_geni_handle_err;
 	spi->use_gpio_descriptors = true;
 
 	init_completion(&mas->cs_done);
-- 
2.31.1

Re: [PATCH v2 1/2] spi: qcom: geni: set the error code for gpi transfer

[Doug Anderson]

Hi,

On Sun, Jan 2, 2022 at 11:11 PM Vinod Koul <vkoul@kernel.org> 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).
>
> The helps to return error on transfer and not wait till it timesout on
> error
>
> Fixes: b59c122484ec ("spi: spi-geni-qcom: Add support for GPI dma")
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>
> Changes in v2:
>  - add missing spi_finalize_current_transfer() for dma error case
>
>  drivers/spi/spi-geni-qcom.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 413fa1a7a936..b82f3ddff0f4 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -346,17 +346,21 @@ 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);
> +               spi_finalize_current_transfer(spi);
>                 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);
>  }

What you have here should work and seems fine, though it's a bit
awkward. Every exit path now calls spi_finalize_current_transfer().
IMO this would be slightly cleaner like this (also moving the error
cases to both be first)

if (result->result != DMA_TRANS_NOERROR) {
  dev_err(...);
} else if (result->residue)
  dev_err(...);
} else {
  spi->cur_msg->status = 0;
  dev_dbg(...);
}

spi_finalize_current_transfer(spi);

I'll let Mark decide if he wants it to be respun with the above, wants
a follow-on patch, or doesn't care either way. In any case:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v2 1/2] spi: qcom: geni: set the error code for gpi transfer

[Mark Brown]

On Mon, 3 Jan 2022 12:41:17 +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).
> 
> The helps to return error on transfer and not wait till it timesout on
> error
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/2] spi: qcom: geni: set the error code for gpi transfer
      commit: 74b86d6af81be73bb74995ebeba74417e84b6b6f
[2/2] spi: qcom: geni: handle timeout for gpi mode
      commit: f8039ea55d4ccac2238a247a574f0acb3bc1dc4b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark