| Message ID | 20181024024808.14485-2-xiaoning.wang@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] spi: lpspi: Add slave mode support for imx7ulp | expand |
Hi Clark,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on spi/for-next]
[also build test ERROR on next-20181019]
[cannot apply to v4.19]
[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/Clark-Wang/spi-lpspi-Add-slave-mode-support-for-imx7ulp/20181024-125200
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa
Note: the linux-review/Clark-Wang/spi-lpspi-Add-slave-mode-support-for-imx7ulp/20181024-125200 HEAD 37fc79ae346a636e850e4b90070c7d6e84b67b52 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/spi/spi-fsl-lpspi.c: In function 'fsl_lpspi_transfer_one':
>> drivers/spi/spi-fsl-lpspi.c:387:8: error: implicit declaration of function 'fsl_lpspi_txfifo_empty'; did you mean 'fsl_lpspi_set_cmd'? [-Werror=implicit-function-declaration]
ret = fsl_lpspi_txfifo_empty(fsl_lpspi);
^~~~~~~~~~~~~~~~~~~~~~
fsl_lpspi_set_cmd
cc1: some warnings being treated as errors
vim +387 drivers/spi/spi-fsl-lpspi.c
8e2c76586 Clark Wang 2018-10-24 365
8e2c76586 Clark Wang 2018-10-24 366 static int fsl_lpspi_transfer_one(struct spi_controller *controller,
5314987de Gao Pan 2016-11-22 367 struct spi_device *spi,
5314987de Gao Pan 2016-11-22 368 struct spi_transfer *t)
5314987de Gao Pan 2016-11-22 369 {
8e2c76586 Clark Wang 2018-10-24 370 struct fsl_lpspi_data *fsl_lpspi =
8e2c76586 Clark Wang 2018-10-24 371 spi_controller_get_devdata(controller);
5314987de Gao Pan 2016-11-22 372 int ret;
5314987de Gao Pan 2016-11-22 373
5314987de Gao Pan 2016-11-22 374 fsl_lpspi->tx_buf = t->tx_buf;
5314987de Gao Pan 2016-11-22 375 fsl_lpspi->rx_buf = t->rx_buf;
5314987de Gao Pan 2016-11-22 376 fsl_lpspi->remain = t->len;
5314987de Gao Pan 2016-11-22 377
5314987de Gao Pan 2016-11-22 378 reinit_completion(&fsl_lpspi->xfer_done);
8e2c76586 Clark Wang 2018-10-24 379 fsl_lpspi->slave_aborted = false;
8e2c76586 Clark Wang 2018-10-24 380
5314987de Gao Pan 2016-11-22 381 fsl_lpspi_write_tx_fifo(fsl_lpspi);
d2ad0a62d Gao Pan 2016-11-28 382
8e2c76586 Clark Wang 2018-10-24 383 ret = fsl_lpspi_wait_for_completion(controller);
8e2c76586 Clark Wang 2018-10-24 384 if (ret)
8e2c76586 Clark Wang 2018-10-24 385 return ret;
5314987de Gao Pan 2016-11-22 386
5314987de Gao Pan 2016-11-22 @387 ret = fsl_lpspi_txfifo_empty(fsl_lpspi);
d989eed20 Gao Pan 2016-12-02 388 if (ret)
d989eed20 Gao Pan 2016-12-02 389 return ret;
d989eed20 Gao Pan 2016-12-02 390
d989eed20 Gao Pan 2016-12-02 391 return 0;
5314987de Gao Pan 2016-11-22 392 }
5314987de Gao Pan 2016-11-22 393
:::::: The code at line 387 was first introduced by commit
:::::: 5314987de5e5f5e38436ef4a69328bc472bbd63e spi: imx: add lpspi bus driver
:::::: TO: Gao Pan <pandy.gao@nxp.com>
:::::: CC: Mark Brown <broonie@kernel.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c index 86cb38d98a39..58397acbce2f 100644 --- a/drivers/spi/spi-fsl-lpspi.c +++ b/drivers/spi/spi-fsl-lpspi.c @@ -49,9 +49,11 @@ #define CR_RST BIT(1) #define CR_MEN BIT(0) #define SR_TCF BIT(10) +#define SR_FCF BIT(9) #define SR_RDF BIT(1) #define SR_TDF BIT(0) #define IER_TCIE BIT(10) +#define IER_FCIE BIT(9) #define IER_RDIE BIT(1) #define IER_TDIE BIT(0) #define CFGR1_PCSCFG BIT(27) @@ -161,28 +163,10 @@ static int lpspi_unprepare_xfer_hardware(struct spi_controller *controller) return 0; } -static int fsl_lpspi_txfifo_empty(struct fsl_lpspi_data *fsl_lpspi) -{ - u32 txcnt; - unsigned long orig_jiffies = jiffies; - - do { - txcnt = readl(fsl_lpspi->base + IMX7ULP_FSR) & 0xff; - - if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) { - dev_dbg(fsl_lpspi->dev, "txfifo empty timeout\n"); - return -ETIMEDOUT; - } - cond_resched(); - - } while (txcnt); - - return 0; -} - static void fsl_lpspi_write_tx_fifo(struct fsl_lpspi_data *fsl_lpspi) { u8 txfifo_cnt; + u32 temp; txfifo_cnt = readl(fsl_lpspi->base + IMX7ULP_FSR) & 0xff; @@ -193,9 +177,15 @@ static void fsl_lpspi_write_tx_fifo(struct fsl_lpspi_data *fsl_lpspi) txfifo_cnt++; } - if (!fsl_lpspi->remain && (txfifo_cnt < fsl_lpspi->txfifosize)) - writel(0, fsl_lpspi->base + IMX7ULP_TDR); - else + if (txfifo_cnt < fsl_lpspi->txfifosize) { + if (!fsl_lpspi->is_slave) { + temp = readl(fsl_lpspi->base + IMX7ULP_TCR); + temp &= ~TCR_CONTC; + writel(temp, fsl_lpspi->base + IMX7ULP_TCR); + } + + fsl_lpspi_intctrl(fsl_lpspi, IER_FCIE); + } else fsl_lpspi_intctrl(fsl_lpspi, IER_TDIE); } @@ -398,8 +388,6 @@ static int fsl_lpspi_transfer_one(struct spi_controller *controller, if (ret) return ret; - fsl_lpspi_read_rx_fifo(fsl_lpspi); - return 0; } @@ -411,7 +399,6 @@ static int fsl_lpspi_transfer_one_msg(struct spi_controller *controller, struct spi_device *spi = msg->spi; struct spi_transfer *xfer; bool is_first_xfer = true; - u32 temp; int ret = 0; msg->status = 0; @@ -431,13 +418,6 @@ static int fsl_lpspi_transfer_one_msg(struct spi_controller *controller, } complete: - if (!fsl_lpspi->is_slave) { - /* de-assert SS, then finalize current message */ - temp = readl(fsl_lpspi->base + IMX7ULP_TCR); - temp &= ~TCR_CONTC; - writel(temp, fsl_lpspi->base + IMX7ULP_TCR); - } - msg->status = ret; spi_finalize_current_message(controller); @@ -446,20 +426,23 @@ static int fsl_lpspi_transfer_one_msg(struct spi_controller *controller, static irqreturn_t fsl_lpspi_isr(int irq, void *dev_id) { + u32 temp_SR, temp_IER; struct fsl_lpspi_data *fsl_lpspi = dev_id; - u32 temp; + temp_IER = readl(fsl_lpspi->base + IMX7ULP_IER); fsl_lpspi_intctrl(fsl_lpspi, 0); - temp = readl(fsl_lpspi->base + IMX7ULP_SR); + temp_SR = readl(fsl_lpspi->base + IMX7ULP_SR); fsl_lpspi_read_rx_fifo(fsl_lpspi); - if (temp & SR_TDF) { + if ((temp_SR & SR_TDF) && (temp_IER & IER_TDIE)) { fsl_lpspi_write_tx_fifo(fsl_lpspi); + return IRQ_HANDLED; + } - if (!fsl_lpspi->remain) - complete(&fsl_lpspi->xfer_done); - + if (temp_SR & SR_FCF && (temp_IER & IER_FCIE)) { + writel(SR_FCF, fsl_lpspi->base + IMX7ULP_SR); + complete(&fsl_lpspi->xfer_done); return IRQ_HANDLED; }
Use SR_TDF to judge if need send data, and SR_FCF to judge if transmission end to replace the waiting after transmission end. This waiting has no actual meaning, for the real end will set the FCF flag. Resolved an issue that could cause a transmission timeout when transferring large amounts of data. After making these changes, there is no need to use fsl_lpspi_txfifo_empty(), so remove it. Signed-off-by: Xiaoning Wang <xiaoning.wang@nxp.com> --- drivers/spi/spi-fsl-lpspi.c | 59 +++++++++++++------------------------ 1 file changed, 21 insertions(+), 38 deletions(-)