diff mbox

[07/15] spi: qup: Fix transaction done signaling

Message ID 1497603538-12750-8-git-send-email-varada@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Varadarajan Narayanan June 16, 2017, 8:58 a.m. UTC 
Wait to signal done until we get all of the interrupts we are expecting
to get for a transaction.  If we don't wait for the input done flag, we
can be in between transactions when the done flag comes in and this can
mess up the next transaction.

While here cleaning up the code which sets controller->xfer = NULL and
restores it in the ISR. This looks to be some debug code which is not
required.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

Comments

kernel test robot June 18, 2017, 2:11 a.m. UTC | #1 
Hi Varadarajan,

[auto build test ERROR on spi/for-next]
[also build test ERROR on v4.12-rc5 next-20170616]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Varadarajan-Narayanan/spi-qup-Fixes-and-add-support-for-64k-transfers/20170618-072148
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

Note: the linux-review/Varadarajan-Narayanan/spi-qup-Fixes-and-add-support-for-64k-transfers/20170618-072148 HEAD 6d576268af40336063b1df090673a480642ce26d builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/spi/spi-qup.c: In function 'spi_qup_do_pio':
   drivers/spi/spi-qup.c:382:24: warning: format '%s' expects a matching 'char *' argument [-Wformat=]
      dev_warn(qup->dev, "%s(%d): cannot set RUN state\n");
                           ^
   drivers/spi/spi-qup.c:382:27: warning: format '%d' expects a matching 'int' argument [-Wformat=]
      dev_warn(qup->dev, "%s(%d): cannot set RUN state\n");
                              ^
   drivers/spi/spi-qup.c:388:24: warning: format '%s' expects a matching 'char *' argument [-Wformat=]
      dev_warn(qup->dev, "%s(%d): cannot set PAUSE state\n");
                           ^
   drivers/spi/spi-qup.c:388:27: warning: format '%d' expects a matching 'int' argument [-Wformat=]
      dev_warn(qup->dev, "%s(%d): cannot set PAUSE state\n");
                              ^
   drivers/spi/spi-qup.c:396:24: warning: format '%s' expects a matching 'char *' argument [-Wformat=]
      dev_warn(qup->dev, "%s(%d): cannot set RUN state\n");
                           ^
   drivers/spi/spi-qup.c:396:27: warning: format '%d' expects a matching 'int' argument [-Wformat=]
      dev_warn(qup->dev, "%s(%d): cannot set RUN state\n");
                              ^
   drivers/spi/spi-qup.c: In function 'spi_qup_qup_irq':
>> drivers/spi/spi-qup.c:445:34: error: 'xfer' undeclared (first use in this function)
       spi_qup_fifo_read(controller, xfer);
                                     ^~~~
   drivers/spi/spi-qup.c:445:34: note: each undeclared identifier is reported only once for each function it appears in

vim +/xfer +445 drivers/spi/spi-qup.c

612762e8 Andy Gross            2015-03-04  390  	}
612762e8 Andy Gross            2015-03-04  391  
612762e8 Andy Gross            2015-03-04  392  	spi_qup_fifo_write(qup, xfer);
612762e8 Andy Gross            2015-03-04  393  
4c05e25b Varadarajan Narayanan 2017-06-16  394  	ret = spi_qup_set_state(qup, QUP_STATE_RUN);
4c05e25b Varadarajan Narayanan 2017-06-16  395  	if (ret) {
4c05e25b Varadarajan Narayanan 2017-06-16 @396  		dev_warn(qup->dev, "%s(%d): cannot set RUN state\n");
4c05e25b Varadarajan Narayanan 2017-06-16  397  		return ret;
4c05e25b Varadarajan Narayanan 2017-06-16  398  	}
4c05e25b Varadarajan Narayanan 2017-06-16  399  
dd4ce14d Varadarajan Narayanan 2017-06-16  400  	if (!wait_for_completion_timeout(&qup->done, timeout))
dd4ce14d Varadarajan Narayanan 2017-06-16  401  		return -ETIMEDOUT;
dd4ce14d Varadarajan Narayanan 2017-06-16  402  
612762e8 Andy Gross            2015-03-04  403  	return 0;
612762e8 Andy Gross            2015-03-04  404  }
612762e8 Andy Gross            2015-03-04  405  
64ff247a Ivan T. Ivanov        2014-02-13  406  static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
64ff247a Ivan T. Ivanov        2014-02-13  407  {
64ff247a Ivan T. Ivanov        2014-02-13  408  	struct spi_qup *controller = dev_id;
64ff247a Ivan T. Ivanov        2014-02-13  409  	u32 opflags, qup_err, spi_err;
64ff247a Ivan T. Ivanov        2014-02-13  410  	int error = 0;
64ff247a Ivan T. Ivanov        2014-02-13  411  
64ff247a Ivan T. Ivanov        2014-02-13  412  	qup_err = readl_relaxed(controller->base + QUP_ERROR_FLAGS);
64ff247a Ivan T. Ivanov        2014-02-13  413  	spi_err = readl_relaxed(controller->base + SPI_ERROR_FLAGS);
64ff247a Ivan T. Ivanov        2014-02-13  414  	opflags = readl_relaxed(controller->base + QUP_OPERATIONAL);
64ff247a Ivan T. Ivanov        2014-02-13  415  
64ff247a Ivan T. Ivanov        2014-02-13  416  	writel_relaxed(qup_err, controller->base + QUP_ERROR_FLAGS);
64ff247a Ivan T. Ivanov        2014-02-13  417  	writel_relaxed(spi_err, controller->base + SPI_ERROR_FLAGS);
64ff247a Ivan T. Ivanov        2014-02-13  418  
64ff247a Ivan T. Ivanov        2014-02-13  419  	if (qup_err) {
64ff247a Ivan T. Ivanov        2014-02-13  420  		if (qup_err & QUP_ERROR_OUTPUT_OVER_RUN)
64ff247a Ivan T. Ivanov        2014-02-13  421  			dev_warn(controller->dev, "OUTPUT_OVER_RUN\n");
64ff247a Ivan T. Ivanov        2014-02-13  422  		if (qup_err & QUP_ERROR_INPUT_UNDER_RUN)
64ff247a Ivan T. Ivanov        2014-02-13  423  			dev_warn(controller->dev, "INPUT_UNDER_RUN\n");
64ff247a Ivan T. Ivanov        2014-02-13  424  		if (qup_err & QUP_ERROR_OUTPUT_UNDER_RUN)
64ff247a Ivan T. Ivanov        2014-02-13  425  			dev_warn(controller->dev, "OUTPUT_UNDER_RUN\n");
64ff247a Ivan T. Ivanov        2014-02-13  426  		if (qup_err & QUP_ERROR_INPUT_OVER_RUN)
64ff247a Ivan T. Ivanov        2014-02-13  427  			dev_warn(controller->dev, "INPUT_OVER_RUN\n");
64ff247a Ivan T. Ivanov        2014-02-13  428  
64ff247a Ivan T. Ivanov        2014-02-13  429  		error = -EIO;
64ff247a Ivan T. Ivanov        2014-02-13  430  	}
64ff247a Ivan T. Ivanov        2014-02-13  431  
64ff247a Ivan T. Ivanov        2014-02-13  432  	if (spi_err) {
64ff247a Ivan T. Ivanov        2014-02-13  433  		if (spi_err & SPI_ERROR_CLK_OVER_RUN)
64ff247a Ivan T. Ivanov        2014-02-13  434  			dev_warn(controller->dev, "CLK_OVER_RUN\n");
64ff247a Ivan T. Ivanov        2014-02-13  435  		if (spi_err & SPI_ERROR_CLK_UNDER_RUN)
64ff247a Ivan T. Ivanov        2014-02-13  436  			dev_warn(controller->dev, "CLK_UNDER_RUN\n");
64ff247a Ivan T. Ivanov        2014-02-13  437  
64ff247a Ivan T. Ivanov        2014-02-13  438  		error = -EIO;
64ff247a Ivan T. Ivanov        2014-02-13  439  	}
64ff247a Ivan T. Ivanov        2014-02-13  440  
c812cf10 Varadarajan Narayanan 2017-06-16  441  	if (spi_qup_is_dma_xfer(controller->mode)) {
c812cf10 Varadarajan Narayanan 2017-06-16  442  		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
c812cf10 Varadarajan Narayanan 2017-06-16  443  	} else {
64ff247a Ivan T. Ivanov        2014-02-13  444  		if (opflags & QUP_OP_IN_SERVICE_FLAG)
64ff247a Ivan T. Ivanov        2014-02-13 @445  			spi_qup_fifo_read(controller, xfer);
64ff247a Ivan T. Ivanov        2014-02-13  446  
64ff247a Ivan T. Ivanov        2014-02-13  447  		if (opflags & QUP_OP_OUT_SERVICE_FLAG)
64ff247a Ivan T. Ivanov        2014-02-13  448  			spi_qup_fifo_write(controller, xfer);

:::::: The code at line 445 was first introduced by commit
:::::: 64ff247a978facc437d40f0c9b754675846a98f0 spi: Add Qualcomm QUP SPI controller support

:::::: TO: Ivan T. Ivanov <iivanov@mm-sol.com>
:::::: CC: Mark Brown <broonie@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 2124815..9324093 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -406,29 +406,15 @@  static int spi_qup_do_pio(struct spi_master *master, struct spi_transfer *xfer,
 static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 {
 	struct spi_qup *controller = dev_id;
-	struct spi_transfer *xfer;
 	u32 opflags, qup_err, spi_err;
-	unsigned long flags;
 	int error = 0;
 
-	spin_lock_irqsave(&controller->lock, flags);
-	xfer = controller->xfer;
-	controller->xfer = NULL;
-	spin_unlock_irqrestore(&controller->lock, flags);
-
 	qup_err = readl_relaxed(controller->base + QUP_ERROR_FLAGS);
 	spi_err = readl_relaxed(controller->base + SPI_ERROR_FLAGS);
 	opflags = readl_relaxed(controller->base + QUP_OPERATIONAL);
 
 	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) {
-		dev_err_ratelimited(controller->dev, "unexpected irq %08x %08x %08x\n",
-				    qup_err, spi_err, opflags);
-		return IRQ_HANDLED;
-	}
 
 	if (qup_err) {
 		if (qup_err & QUP_ERROR_OUTPUT_OVER_RUN)
@@ -452,7 +438,9 @@  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);
+	} else {
 		if (opflags & QUP_OP_IN_SERVICE_FLAG)
 			spi_qup_fifo_read(controller, xfer);
 
@@ -460,12 +448,7 @@  static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 			spi_qup_fifo_write(controller, xfer);
 	}
 
-	spin_lock_irqsave(&controller->lock, flags);
-	controller->error = error;
-	controller->xfer = xfer;
-	spin_unlock_irqrestore(&controller->lock, flags);
-
-	if (controller->rx_bytes == xfer->len || error)
+	if ((opflags & QUP_OP_MAX_INPUT_DONE_FLAG) || error)
 		complete(&controller->done);
 
 	return IRQ_HANDLED;
@@ -665,7 +648,6 @@  static int spi_qup_transfer_one(struct spi_master *master,
 exit:
 	spi_qup_set_state(controller, QUP_STATE_RESET);
 	spin_lock_irqsave(&controller->lock, flags);
-	controller->xfer = NULL;
 	if (!ret)
 		ret = controller->error;
 	spin_unlock_irqrestore(&controller->lock, flags);