| Message ID | 20211117133110.2682631-3-vkoul@kernel.org |
|---|---|
| State | Superseded |
| 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: > > 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 > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > --- > drivers/spi/spi-geni-qcom.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) 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> > diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c > index b9769de1f5f0..5b6e9a6643d8 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); Slight nit that maybe you should call it handle_gpi_err() instead of handle_gpi_timeout(). As I understand it this function will get called for _all_ errors, including errors reported by spi_gsi_callback_result(). So basically if you have residue then you'll immediately finalize the transfer with -EIO in the status and then spi_geni_handle_err() will get called. It seems a little strange that it then goes and calls a function whose name makes it sound as if it's only called for "timeout". (For the FIFO case we actually only hit this for timeouts since we don't currently terminate transfers early for errors, so the FIFO name is actually OK). -Doug
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index b9769de1f5f0..5b6e9a6643d8 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; @@ -921,7 +945,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);
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 Signed-off-by: Vinod Koul <vkoul@kernel.org> --- drivers/spi/spi-geni-qcom.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)