Re: [PATCH 10/18] spi: qup: Fix DMA mode interrupt handling

Andy Gross
2017-06-14T20:06:14Z

On Wed, Jun 14, 2017 at 12:51:11PM +0530, Sricharan R wrote:
> Hi Varada,
>
> On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote:
> > This is needed for v1, where the i/o completion is not
> > handled in the dma driver.
> >
> > Signed-off-by: Andy Gross <andy.gross@linaro.org>
> > Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> > ---
> > drivers/spi/spi-qup.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
> > index 872de28..bd53e82 100644
> > --- a/drivers/spi/spi-qup.c
> > +++ b/drivers/spi/spi-qup.c
> > @@ -510,9 +510,9 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
> >
> > writel_relaxed(qup_err, controller->base + QUP_ERROR_FLAGS);
> > writel_relaxed(spi_err, controller->base + SPI_ERROR_FLAGS);
> > - writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
> >
> > if (!xfer) {
> > + writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
>
> This does look correct to remove acknowledging the QUP in normal case and
> do it conditionally only when xfer = NULL.

This is to probably mask the issue of getting erroneous/spurious IRQs.

>
> > dev_err_ratelimited(controller->dev, "unexpected irq %08x %08x %08x\n",
> > qup_err, spi_err, opflags);
> > return IRQ_HANDLED;
> > @@ -540,7 +540,15 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
> > error = -EIO;
> > }
> >
> > - if (!spi_qup_is_dma_xfer(controller->mode)) {
> > + if (spi_qup_is_dma_xfer(controller->mode)) {
> > + writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
> > + if (opflags & QUP_OP_IN_SERVICE_FLAG &&
> > + opflags & QUP_OP_MAX_INPUT_DONE_FLAG)
> > + complete(&controller->rxc);
> > + if (opflags & QUP_OP_OUT_SERVICE_FLAG &&
> > + opflags & QUP_OP_MAX_OUTPUT_DONE_FLAG)
> > + complete(&controller->txc);
> > + } else {
>
> Is this because in patch #8 that we do not populate the dma callback
> for v1. If that is done, this should not be required at all, as the
> complete would be signalled from the dma callback.

I believe that is true. There shouldn't be any IRQs for DMA enabled
transactions (at least BAM-dma).