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

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(-)

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>

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