| Message ID | 20220103071118.27220-1-vkoul@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2,1/2] spi: qcom: geni: set the error code for gpi transfer | expand |
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
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,
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(-)