All of lore.kernel.org
 help / color / mirror / Atom feed
* [ragnatech:upstream/mmc/next 61/71] drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922)
@ 2020-09-12 21:39 kernel test robot
  0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2020-09-12 21:39 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 13651 bytes --]

CC: kbuild-all(a)lists.01.org
TO: Krzysztof Kozlowski <krzk@kernel.org>
CC: Ulf Hansson <ulf.hansson@linaro.org>

tree:   git://git.ragnatech.se/linux upstream/mmc/next
head:   91ca244bdcb6a0b6ad12c8325e5e9228b9472374
commit: 54d8454436a205682bd89d66d8d9eedbc8452d15 [61/71] mmc: host: Enable compile testing of multiple drivers
:::::: branch date: 2 days ago
:::::: commit date: 5 days ago
config: parisc-randconfig-m031-20200913 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922)
drivers/mmc/host/sdhci-esdhc-imx.c:1380 sdhci_esdhc_imx_hwinit() warn: inconsistent indenting
drivers/mmc/host/sdhci-sprd.c:390 sdhci_sprd_request_done() warn: inconsistent indenting
drivers/mmc/host/moxart-mmc.c:692 moxart_remove() warn: variable dereferenced before check 'mmc' (see line 688)

git remote add ragnatech git://git.ragnatech.se/linux
git fetch --no-tags ragnatech upstream/mmc/next
git checkout 54d8454436a205682bd89d66d8d9eedbc8452d15
vim +/data +939 drivers/mmc/host/davinci_mmc.c

f9db92cb8084c7 Alagu Sankar        2011-01-03   863  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   864  static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
b4cff4549b7a8c Vipin Bhandari      2009-12-14   865  {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   866  	struct mmc_davinci_host *host = (struct mmc_davinci_host *)dev_id;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   867  	unsigned int status, qstatus;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   868  	int end_command = 0;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   869  	int end_transfer = 0;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   870  	struct mmc_data *data = host->data;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   871  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   872  	if (host->cmd == NULL && host->data == NULL) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   873  		status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   874  		dev_dbg(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   875  			"Spurious interrupt 0x%04x\n", status);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   876  		/* Disable the interrupt from mmcsd */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   877  		writel(0, host->base + DAVINCI_MMCIM);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   878  		return IRQ_NONE;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   879  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   880  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   881  	status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   882  	qstatus = status;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   883  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   884  	/* handle FIFO first when using PIO for data.
b4cff4549b7a8c Vipin Bhandari      2009-12-14   885  	 * bytes_left will decrease to zero as I/O progress and status will
b4cff4549b7a8c Vipin Bhandari      2009-12-14   886  	 * read zero over iteration because this controller status
b4cff4549b7a8c Vipin Bhandari      2009-12-14   887  	 * register(MMCST0) reports any status only once and it is cleared
b4cff4549b7a8c Vipin Bhandari      2009-12-14   888  	 * by read. So, it is not unbouned loop even in the case of
b4cff4549b7a8c Vipin Bhandari      2009-12-14   889  	 * non-dma.
b4cff4549b7a8c Vipin Bhandari      2009-12-14   890  	 */
be7b5622e60818 Ido Yariv           2012-03-11   891  	if (host->bytes_left && (status & (MMCST0_DXRDY | MMCST0_DRRDY))) {
be7b5622e60818 Ido Yariv           2012-03-11   892  		unsigned long im_val;
be7b5622e60818 Ido Yariv           2012-03-11   893  
be7b5622e60818 Ido Yariv           2012-03-11   894  		/*
be7b5622e60818 Ido Yariv           2012-03-11   895  		 * If interrupts fire during the following loop, they will be
be7b5622e60818 Ido Yariv           2012-03-11   896  		 * handled by the handler, but the PIC will still buffer these.
be7b5622e60818 Ido Yariv           2012-03-11   897  		 * As a result, the handler will be called again to serve these
be7b5622e60818 Ido Yariv           2012-03-11   898  		 * needlessly. In order to avoid these spurious interrupts,
be7b5622e60818 Ido Yariv           2012-03-11   899  		 * keep interrupts masked during the loop.
be7b5622e60818 Ido Yariv           2012-03-11   900  		 */
be7b5622e60818 Ido Yariv           2012-03-11   901  		im_val = readl(host->base + DAVINCI_MMCIM);
be7b5622e60818 Ido Yariv           2012-03-11   902  		writel(0, host->base + DAVINCI_MMCIM);
be7b5622e60818 Ido Yariv           2012-03-11   903  
be7b5622e60818 Ido Yariv           2012-03-11   904  		do {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   905  			davinci_fifo_data_trans(host, rw_threshold);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   906  			status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   907  			qstatus |= status;
be7b5622e60818 Ido Yariv           2012-03-11   908  		} while (host->bytes_left &&
be7b5622e60818 Ido Yariv           2012-03-11   909  			 (status & (MMCST0_DXRDY | MMCST0_DRRDY)));
be7b5622e60818 Ido Yariv           2012-03-11   910  
be7b5622e60818 Ido Yariv           2012-03-11   911  		/*
be7b5622e60818 Ido Yariv           2012-03-11   912  		 * If an interrupt is pending, it is assumed it will fire when
be7b5622e60818 Ido Yariv           2012-03-11   913  		 * it is unmasked. This assumption is also taken when the MMCIM
be7b5622e60818 Ido Yariv           2012-03-11   914  		 * is first set. Otherwise, writing to MMCIM after reading the
be7b5622e60818 Ido Yariv           2012-03-11   915  		 * status is race-prone.
be7b5622e60818 Ido Yariv           2012-03-11   916  		 */
be7b5622e60818 Ido Yariv           2012-03-11   917  		writel(im_val, host->base + DAVINCI_MMCIM);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   918  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   919  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   920  	if (qstatus & MMCST0_DATDNE) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   921  		/* All blocks sent/received, and CRC checks passed */
b4cff4549b7a8c Vipin Bhandari      2009-12-14  @922  		if (data != NULL) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   923  			if ((host->do_dma == 0) && (host->bytes_left > 0)) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   924  				/* if datasize < rw_threshold
b4cff4549b7a8c Vipin Bhandari      2009-12-14   925  				 * no RX ints are generated
b4cff4549b7a8c Vipin Bhandari      2009-12-14   926  				 */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   927  				davinci_fifo_data_trans(host, host->bytes_left);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   928  			}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   929  			end_transfer = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   930  			data->bytes_xfered = data->blocks * data->blksz;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   931  		} else {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   932  			dev_err(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   933  					"DATDNE with no host->data\n");
b4cff4549b7a8c Vipin Bhandari      2009-12-14   934  		}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   935  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   936  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   937  	if (qstatus & MMCST0_TOUTRD) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   938  		/* Read data timeout */
b4cff4549b7a8c Vipin Bhandari      2009-12-14  @939  		data->error = -ETIMEDOUT;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   940  		end_transfer = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   941  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   942  		dev_dbg(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   943  			"read data timeout, status %x\n",
b4cff4549b7a8c Vipin Bhandari      2009-12-14   944  			qstatus);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   945  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   946  		davinci_abort_data(host, data);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   947  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   948  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   949  	if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   950  		/* Data CRC error */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   951  		data->error = -EILSEQ;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   952  		end_transfer = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   953  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   954  		/* NOTE:  this controller uses CRCWR to report both CRC
b4cff4549b7a8c Vipin Bhandari      2009-12-14   955  		 * errors and timeouts (on writes).  MMCDRSP values are
b4cff4549b7a8c Vipin Bhandari      2009-12-14   956  		 * only weakly documented, but 0x9f was clearly a timeout
b4cff4549b7a8c Vipin Bhandari      2009-12-14   957  		 * case and the two three-bit patterns in various SD specs
b4cff4549b7a8c Vipin Bhandari      2009-12-14   958  		 * (101, 010) aren't part of it ...
b4cff4549b7a8c Vipin Bhandari      2009-12-14   959  		 */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   960  		if (qstatus & MMCST0_CRCWR) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   961  			u32 temp = readb(host->base + DAVINCI_MMCDRSP);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   962  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   963  			if (temp == 0x9f)
b4cff4549b7a8c Vipin Bhandari      2009-12-14   964  				data->error = -ETIMEDOUT;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   965  		}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   966  		dev_dbg(mmc_dev(host->mmc), "data %s %s error\n",
b4cff4549b7a8c Vipin Bhandari      2009-12-14   967  			(qstatus & MMCST0_CRCWR) ? "write" : "read",
b4cff4549b7a8c Vipin Bhandari      2009-12-14   968  			(data->error == -ETIMEDOUT) ? "timeout" : "CRC");
b4cff4549b7a8c Vipin Bhandari      2009-12-14   969  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   970  		davinci_abort_data(host, data);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   971  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   972  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   973  	if (qstatus & MMCST0_TOUTRS) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   974  		/* Command timeout */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   975  		if (host->cmd) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   976  			dev_dbg(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   977  				"CMD%d timeout, status %x\n",
b4cff4549b7a8c Vipin Bhandari      2009-12-14   978  				host->cmd->opcode, qstatus);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   979  			host->cmd->error = -ETIMEDOUT;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   980  			if (data) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   981  				end_transfer = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   982  				davinci_abort_data(host, data);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   983  			} else
b4cff4549b7a8c Vipin Bhandari      2009-12-14   984  				end_command = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   985  		}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   986  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   987  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   988  	if (qstatus & MMCST0_CRCRS) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   989  		/* Command CRC error */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   990  		dev_dbg(mmc_dev(host->mmc), "Command CRC error\n");
b4cff4549b7a8c Vipin Bhandari      2009-12-14   991  		if (host->cmd) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   992  			host->cmd->error = -EILSEQ;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   993  			end_command = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   994  		}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   995  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   996  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   997  	if (qstatus & MMCST0_RSPDNE) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   998  		/* End of command phase */
8c7f51effd7387 Krzysztof Kozlowski 2020-09-02   999  		end_command = host->cmd ? 1 : 0;
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1000  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1001  
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1002  	if (end_command)
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1003  		mmc_davinci_cmd_done(host, host->cmd);
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1004  	if (end_transfer)
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1005  		mmc_davinci_xfer_done(host, data);
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1006  	return IRQ_HANDLED;
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1007  }
b4cff4549b7a8c Vipin Bhandari      2009-12-14  1008  

:::::: The code at line 939 was first introduced by commit
:::::: b4cff4549b7a8c5fc8b88e3493b6287555f0512c DaVinci: MMC: MMC/SD controller driver for DaVinci family

:::::: TO: Vipin Bhandari <vipin.bhandari@ti.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28159 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [ragnatech:upstream/mmc/next 61/71] drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922)
  2020-09-14 14:46 ` Dan Carpenter
  (?)
@ 2020-09-23 15:39 ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2020-09-23 15:39 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]

On Mon, 14 Sep 2020 at 16:46, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> [ Enabling compile test turns on ancient static checker warnings...  -dan ]
>
> tree:   git://git.ragnatech.se/linux upstream/mmc/next
> head:   91ca244bdcb6a0b6ad12c8325e5e9228b9472374
> commit: 54d8454436a205682bd89d66d8d9eedbc8452d15 [61/71] mmc: host: Enable compile testing of multiple drivers
> config: parisc-randconfig-m031-20200913 (attached as .config)
> compiler: hppa-linux-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922)

Thanks for the report.

This is most likely false positive.

> drivers/mmc/host/sdhci-esdhc-imx.c:1380 sdhci_esdhc_imx_hwinit() warn: inconsistent indenting
> drivers/mmc/host/sdhci-sprd.c:390 sdhci_sprd_request_done() warn: inconsistent indenting
> drivers/mmc/host/moxart-mmc.c:692 moxart_remove() warn: variable dereferenced before check 'mmc' (see line 688)

These I fixed now with your reported-by.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ragnatech:upstream/mmc/next 61/71] drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922)
@ 2020-09-14 14:46 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2020-09-14 14:46 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 8833 bytes --]

[ Enabling compile test turns on ancient static checker warnings...  -dan ]

tree:   git://git.ragnatech.se/linux upstream/mmc/next
head:   91ca244bdcb6a0b6ad12c8325e5e9228b9472374
commit: 54d8454436a205682bd89d66d8d9eedbc8452d15 [61/71] mmc: host: Enable compile testing of multiple drivers
config: parisc-randconfig-m031-20200913 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922)
drivers/mmc/host/sdhci-esdhc-imx.c:1380 sdhci_esdhc_imx_hwinit() warn: inconsistent indenting
drivers/mmc/host/sdhci-sprd.c:390 sdhci_sprd_request_done() warn: inconsistent indenting
drivers/mmc/host/moxart-mmc.c:692 moxart_remove() warn: variable dereferenced before check 'mmc' (see line 688)

git remote add ragnatech git://git.ragnatech.se/linux
git fetch --no-tags ragnatech upstream/mmc/next
git checkout 54d8454436a205682bd89d66d8d9eedbc8452d15
vim +/data +939 drivers/mmc/host/davinci_mmc.c

b4cff4549b7a8c Vipin Bhandari      2009-12-14   864  static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
b4cff4549b7a8c Vipin Bhandari      2009-12-14   865  {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   866  	struct mmc_davinci_host *host = (struct mmc_davinci_host *)dev_id;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   867  	unsigned int status, qstatus;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   868  	int end_command = 0;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   869  	int end_transfer = 0;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   870  	struct mmc_data *data = host->data;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   871  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   872  	if (host->cmd == NULL && host->data == NULL) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   873  		status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   874  		dev_dbg(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   875  			"Spurious interrupt 0x%04x\n", status);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   876  		/* Disable the interrupt from mmcsd */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   877  		writel(0, host->base + DAVINCI_MMCIM);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   878  		return IRQ_NONE;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   879  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   880  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   881  	status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   882  	qstatus = status;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   883  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   884  	/* handle FIFO first when using PIO for data.
b4cff4549b7a8c Vipin Bhandari      2009-12-14   885  	 * bytes_left will decrease to zero as I/O progress and status will
b4cff4549b7a8c Vipin Bhandari      2009-12-14   886  	 * read zero over iteration because this controller status
b4cff4549b7a8c Vipin Bhandari      2009-12-14   887  	 * register(MMCST0) reports any status only once and it is cleared
b4cff4549b7a8c Vipin Bhandari      2009-12-14   888  	 * by read. So, it is not unbouned loop even in the case of
b4cff4549b7a8c Vipin Bhandari      2009-12-14   889  	 * non-dma.
b4cff4549b7a8c Vipin Bhandari      2009-12-14   890  	 */
be7b5622e60818 Ido Yariv           2012-03-11   891  	if (host->bytes_left && (status & (MMCST0_DXRDY | MMCST0_DRRDY))) {
be7b5622e60818 Ido Yariv           2012-03-11   892  		unsigned long im_val;
be7b5622e60818 Ido Yariv           2012-03-11   893  
be7b5622e60818 Ido Yariv           2012-03-11   894  		/*
be7b5622e60818 Ido Yariv           2012-03-11   895  		 * If interrupts fire during the following loop, they will be
be7b5622e60818 Ido Yariv           2012-03-11   896  		 * handled by the handler, but the PIC will still buffer these.
be7b5622e60818 Ido Yariv           2012-03-11   897  		 * As a result, the handler will be called again to serve these
be7b5622e60818 Ido Yariv           2012-03-11   898  		 * needlessly. In order to avoid these spurious interrupts,
be7b5622e60818 Ido Yariv           2012-03-11   899  		 * keep interrupts masked during the loop.
be7b5622e60818 Ido Yariv           2012-03-11   900  		 */
be7b5622e60818 Ido Yariv           2012-03-11   901  		im_val = readl(host->base + DAVINCI_MMCIM);
be7b5622e60818 Ido Yariv           2012-03-11   902  		writel(0, host->base + DAVINCI_MMCIM);
be7b5622e60818 Ido Yariv           2012-03-11   903  
be7b5622e60818 Ido Yariv           2012-03-11   904  		do {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   905  			davinci_fifo_data_trans(host, rw_threshold);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   906  			status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   907  			qstatus |= status;
be7b5622e60818 Ido Yariv           2012-03-11   908  		} while (host->bytes_left &&
be7b5622e60818 Ido Yariv           2012-03-11   909  			 (status & (MMCST0_DXRDY | MMCST0_DRRDY)));
be7b5622e60818 Ido Yariv           2012-03-11   910  
be7b5622e60818 Ido Yariv           2012-03-11   911  		/*
be7b5622e60818 Ido Yariv           2012-03-11   912  		 * If an interrupt is pending, it is assumed it will fire when
be7b5622e60818 Ido Yariv           2012-03-11   913  		 * it is unmasked. This assumption is also taken when the MMCIM
be7b5622e60818 Ido Yariv           2012-03-11   914  		 * is first set. Otherwise, writing to MMCIM after reading the
be7b5622e60818 Ido Yariv           2012-03-11   915  		 * status is race-prone.
be7b5622e60818 Ido Yariv           2012-03-11   916  		 */
be7b5622e60818 Ido Yariv           2012-03-11   917  		writel(im_val, host->base + DAVINCI_MMCIM);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   918  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   919  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   920  	if (qstatus & MMCST0_DATDNE) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   921  		/* All blocks sent/received, and CRC checks passed */
b4cff4549b7a8c Vipin Bhandari      2009-12-14  @922  		if (data != NULL) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   923  			if ((host->do_dma == 0) && (host->bytes_left > 0)) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   924  				/* if datasize < rw_threshold
b4cff4549b7a8c Vipin Bhandari      2009-12-14   925  				 * no RX ints are generated
b4cff4549b7a8c Vipin Bhandari      2009-12-14   926  				 */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   927  				davinci_fifo_data_trans(host, host->bytes_left);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   928  			}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   929  			end_transfer = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   930  			data->bytes_xfered = data->blocks * data->blksz;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   931  		} else {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   932  			dev_err(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   933  					"DATDNE with no host->data\n");

This code assumes host->data can be NULL

b4cff4549b7a8c Vipin Bhandari      2009-12-14   934  		}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   935  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   936  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   937  	if (qstatus & MMCST0_TOUTRD) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   938  		/* Read data timeout */
b4cff4549b7a8c Vipin Bhandari      2009-12-14  @939  		data->error = -ETIMEDOUT;

But this code doesn't check

b4cff4549b7a8c Vipin Bhandari      2009-12-14   940  		end_transfer = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   941  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   942  		dev_dbg(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   943  			"read data timeout, status %x\n",
b4cff4549b7a8c Vipin Bhandari      2009-12-14   944  			qstatus);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   945  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   946  		davinci_abort_data(host, data);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   947  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   948  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   949  	if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   950  		/* Data CRC error */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   951  		data->error = -EILSEQ;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28159 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ragnatech:upstream/mmc/next 61/71] drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922)
@ 2020-09-14 14:46 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2020-09-14 14:46 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 8833 bytes --]

[ Enabling compile test turns on ancient static checker warnings...  -dan ]

tree:   git://git.ragnatech.se/linux upstream/mmc/next
head:   91ca244bdcb6a0b6ad12c8325e5e9228b9472374
commit: 54d8454436a205682bd89d66d8d9eedbc8452d15 [61/71] mmc: host: Enable compile testing of multiple drivers
config: parisc-randconfig-m031-20200913 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922)
drivers/mmc/host/sdhci-esdhc-imx.c:1380 sdhci_esdhc_imx_hwinit() warn: inconsistent indenting
drivers/mmc/host/sdhci-sprd.c:390 sdhci_sprd_request_done() warn: inconsistent indenting
drivers/mmc/host/moxart-mmc.c:692 moxart_remove() warn: variable dereferenced before check 'mmc' (see line 688)

git remote add ragnatech git://git.ragnatech.se/linux
git fetch --no-tags ragnatech upstream/mmc/next
git checkout 54d8454436a205682bd89d66d8d9eedbc8452d15
vim +/data +939 drivers/mmc/host/davinci_mmc.c

b4cff4549b7a8c Vipin Bhandari      2009-12-14   864  static irqreturn_t mmc_davinci_irq(int irq, void *dev_id)
b4cff4549b7a8c Vipin Bhandari      2009-12-14   865  {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   866  	struct mmc_davinci_host *host = (struct mmc_davinci_host *)dev_id;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   867  	unsigned int status, qstatus;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   868  	int end_command = 0;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   869  	int end_transfer = 0;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   870  	struct mmc_data *data = host->data;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   871  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   872  	if (host->cmd == NULL && host->data == NULL) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   873  		status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   874  		dev_dbg(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   875  			"Spurious interrupt 0x%04x\n", status);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   876  		/* Disable the interrupt from mmcsd */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   877  		writel(0, host->base + DAVINCI_MMCIM);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   878  		return IRQ_NONE;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   879  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   880  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   881  	status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   882  	qstatus = status;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   883  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   884  	/* handle FIFO first when using PIO for data.
b4cff4549b7a8c Vipin Bhandari      2009-12-14   885  	 * bytes_left will decrease to zero as I/O progress and status will
b4cff4549b7a8c Vipin Bhandari      2009-12-14   886  	 * read zero over iteration because this controller status
b4cff4549b7a8c Vipin Bhandari      2009-12-14   887  	 * register(MMCST0) reports any status only once and it is cleared
b4cff4549b7a8c Vipin Bhandari      2009-12-14   888  	 * by read. So, it is not unbouned loop even in the case of
b4cff4549b7a8c Vipin Bhandari      2009-12-14   889  	 * non-dma.
b4cff4549b7a8c Vipin Bhandari      2009-12-14   890  	 */
be7b5622e60818 Ido Yariv           2012-03-11   891  	if (host->bytes_left && (status & (MMCST0_DXRDY | MMCST0_DRRDY))) {
be7b5622e60818 Ido Yariv           2012-03-11   892  		unsigned long im_val;
be7b5622e60818 Ido Yariv           2012-03-11   893  
be7b5622e60818 Ido Yariv           2012-03-11   894  		/*
be7b5622e60818 Ido Yariv           2012-03-11   895  		 * If interrupts fire during the following loop, they will be
be7b5622e60818 Ido Yariv           2012-03-11   896  		 * handled by the handler, but the PIC will still buffer these.
be7b5622e60818 Ido Yariv           2012-03-11   897  		 * As a result, the handler will be called again to serve these
be7b5622e60818 Ido Yariv           2012-03-11   898  		 * needlessly. In order to avoid these spurious interrupts,
be7b5622e60818 Ido Yariv           2012-03-11   899  		 * keep interrupts masked during the loop.
be7b5622e60818 Ido Yariv           2012-03-11   900  		 */
be7b5622e60818 Ido Yariv           2012-03-11   901  		im_val = readl(host->base + DAVINCI_MMCIM);
be7b5622e60818 Ido Yariv           2012-03-11   902  		writel(0, host->base + DAVINCI_MMCIM);
be7b5622e60818 Ido Yariv           2012-03-11   903  
be7b5622e60818 Ido Yariv           2012-03-11   904  		do {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   905  			davinci_fifo_data_trans(host, rw_threshold);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   906  			status = readl(host->base + DAVINCI_MMCST0);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   907  			qstatus |= status;
be7b5622e60818 Ido Yariv           2012-03-11   908  		} while (host->bytes_left &&
be7b5622e60818 Ido Yariv           2012-03-11   909  			 (status & (MMCST0_DXRDY | MMCST0_DRRDY)));
be7b5622e60818 Ido Yariv           2012-03-11   910  
be7b5622e60818 Ido Yariv           2012-03-11   911  		/*
be7b5622e60818 Ido Yariv           2012-03-11   912  		 * If an interrupt is pending, it is assumed it will fire when
be7b5622e60818 Ido Yariv           2012-03-11   913  		 * it is unmasked. This assumption is also taken when the MMCIM
be7b5622e60818 Ido Yariv           2012-03-11   914  		 * is first set. Otherwise, writing to MMCIM after reading the
be7b5622e60818 Ido Yariv           2012-03-11   915  		 * status is race-prone.
be7b5622e60818 Ido Yariv           2012-03-11   916  		 */
be7b5622e60818 Ido Yariv           2012-03-11   917  		writel(im_val, host->base + DAVINCI_MMCIM);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   918  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   919  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   920  	if (qstatus & MMCST0_DATDNE) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   921  		/* All blocks sent/received, and CRC checks passed */
b4cff4549b7a8c Vipin Bhandari      2009-12-14  @922  		if (data != NULL) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   923  			if ((host->do_dma == 0) && (host->bytes_left > 0)) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   924  				/* if datasize < rw_threshold
b4cff4549b7a8c Vipin Bhandari      2009-12-14   925  				 * no RX ints are generated
b4cff4549b7a8c Vipin Bhandari      2009-12-14   926  				 */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   927  				davinci_fifo_data_trans(host, host->bytes_left);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   928  			}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   929  			end_transfer = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   930  			data->bytes_xfered = data->blocks * data->blksz;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   931  		} else {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   932  			dev_err(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   933  					"DATDNE with no host->data\n");

This code assumes host->data can be NULL

b4cff4549b7a8c Vipin Bhandari      2009-12-14   934  		}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   935  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   936  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   937  	if (qstatus & MMCST0_TOUTRD) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   938  		/* Read data timeout */
b4cff4549b7a8c Vipin Bhandari      2009-12-14  @939  		data->error = -ETIMEDOUT;

But this code doesn't check

b4cff4549b7a8c Vipin Bhandari      2009-12-14   940  		end_transfer = 1;
b4cff4549b7a8c Vipin Bhandari      2009-12-14   941  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   942  		dev_dbg(mmc_dev(host->mmc),
b4cff4549b7a8c Vipin Bhandari      2009-12-14   943  			"read data timeout, status %x\n",
b4cff4549b7a8c Vipin Bhandari      2009-12-14   944  			qstatus);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   945  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   946  		davinci_abort_data(host, data);
b4cff4549b7a8c Vipin Bhandari      2009-12-14   947  	}
b4cff4549b7a8c Vipin Bhandari      2009-12-14   948  
b4cff4549b7a8c Vipin Bhandari      2009-12-14   949  	if (qstatus & (MMCST0_CRCWR | MMCST0_CRCRD)) {
b4cff4549b7a8c Vipin Bhandari      2009-12-14   950  		/* Data CRC error */
b4cff4549b7a8c Vipin Bhandari      2009-12-14   951  		data->error = -EILSEQ;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28159 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-09-23 15:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-12 21:39 [ragnatech:upstream/mmc/next 61/71] drivers/mmc/host/davinci_mmc.c:939 mmc_davinci_irq() error: we previously assumed 'data' could be null (see line 922) kernel test robot
2020-09-14 14:46 Dan Carpenter
2020-09-14 14:46 ` Dan Carpenter
2020-09-23 15:39 ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.