linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [mtd:spi-mem-ecc 14/29] drivers/mtd/nand/ecc-mxic.c:548 mxic_ecc_prepare_io_req_external() error: uninitialized symbol 'ret'.
@ 2022-02-09  7:22 Dan Carpenter
  2022-02-09  8:45 ` Miquel Raynal
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-02-09  7:22 UTC (permalink / raw)
  To: kbuild, Miquel Raynal; +Cc: lkp, kbuild-all, linux-kernel

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-mem-ecc
head:   d6986e74ec6ee6a48ce9ee1d8051b2988d747558
commit: cfe5cf69e97267e9d1de65cc894b7a2310644a15 [14/29] mtd: nand: mxic-ecc: Add Macronix external ECC engine support
config: x86_64-randconfig-m001-20220207 (https://download.01.org/0day-ci/archive/20220209/202202090415.SS8TwwHj-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 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/mtd/nand/ecc-mxic.c:548 mxic_ecc_prepare_io_req_external() error: uninitialized symbol 'ret'.

vim +/ret +548 drivers/mtd/nand/ecc-mxic.c

cfe5cf69e97267 Miquel Raynal 2021-12-16  494  static int mxic_ecc_prepare_io_req_external(struct nand_device *nand,
cfe5cf69e97267 Miquel Raynal 2021-12-16  495  					    struct nand_page_io_req *req)
cfe5cf69e97267 Miquel Raynal 2021-12-16  496  {
cfe5cf69e97267 Miquel Raynal 2021-12-16  497  	struct mxic_ecc_engine *mxic = nand_to_mxic(nand);
cfe5cf69e97267 Miquel Raynal 2021-12-16  498  	struct mxic_ecc_ctx *ctx = nand_to_ecc_ctx(nand);
cfe5cf69e97267 Miquel Raynal 2021-12-16  499  	struct mtd_info *mtd = nanddev_to_mtd(nand);
cfe5cf69e97267 Miquel Raynal 2021-12-16  500  	int offset, nents, step, ret;
cfe5cf69e97267 Miquel Raynal 2021-12-16  501  
cfe5cf69e97267 Miquel Raynal 2021-12-16  502  	if (req->mode == MTD_OPS_RAW)
cfe5cf69e97267 Miquel Raynal 2021-12-16  503  		return 0;
cfe5cf69e97267 Miquel Raynal 2021-12-16  504  
cfe5cf69e97267 Miquel Raynal 2021-12-16  505  	nand_ecc_tweak_req(&ctx->req_ctx, req);
cfe5cf69e97267 Miquel Raynal 2021-12-16  506  	ctx->req = req;
cfe5cf69e97267 Miquel Raynal 2021-12-16  507  
cfe5cf69e97267 Miquel Raynal 2021-12-16  508  	if (req->type == NAND_PAGE_READ)
cfe5cf69e97267 Miquel Raynal 2021-12-16  509  		return 0;
cfe5cf69e97267 Miquel Raynal 2021-12-16  510  
cfe5cf69e97267 Miquel Raynal 2021-12-16  511  	mxic_ecc_add_room_in_oobbuf(ctx, ctx->oobwithstat,
cfe5cf69e97267 Miquel Raynal 2021-12-16  512  				    ctx->req->oobbuf.out);
cfe5cf69e97267 Miquel Raynal 2021-12-16  513  
cfe5cf69e97267 Miquel Raynal 2021-12-16  514  	sg_set_buf(&ctx->sg[0], req->databuf.out, req->datalen);
cfe5cf69e97267 Miquel Raynal 2021-12-16  515  	sg_set_buf(&ctx->sg[1], ctx->oobwithstat,
cfe5cf69e97267 Miquel Raynal 2021-12-16  516  		   req->ooblen + (ctx->steps * STAT_BYTES));
cfe5cf69e97267 Miquel Raynal 2021-12-16  517  
cfe5cf69e97267 Miquel Raynal 2021-12-16  518  	nents = dma_map_sg(mxic->dev, ctx->sg, 2, DMA_BIDIRECTIONAL);
cfe5cf69e97267 Miquel Raynal 2021-12-16  519  	if (!nents)
cfe5cf69e97267 Miquel Raynal 2021-12-16  520  		return -EINVAL;
cfe5cf69e97267 Miquel Raynal 2021-12-16  521  
cfe5cf69e97267 Miquel Raynal 2021-12-16  522  	mutex_lock(&mxic->lock);
cfe5cf69e97267 Miquel Raynal 2021-12-16  523  
cfe5cf69e97267 Miquel Raynal 2021-12-16  524  	for (step = 0; step < ctx->steps; step++) {
cfe5cf69e97267 Miquel Raynal 2021-12-16  525  		writel(sg_dma_address(&ctx->sg[0]) + (step * ctx->data_step_sz),
cfe5cf69e97267 Miquel Raynal 2021-12-16  526  		       mxic->regs + SDMA_MAIN_ADDR);
cfe5cf69e97267 Miquel Raynal 2021-12-16  527  		writel(sg_dma_address(&ctx->sg[1]) + (step * (ctx->oob_step_sz + STAT_BYTES)),
cfe5cf69e97267 Miquel Raynal 2021-12-16  528  		       mxic->regs + SDMA_SPARE_ADDR);
cfe5cf69e97267 Miquel Raynal 2021-12-16  529  		ret = mxic_ecc_process_data(mxic, ctx->req->type);
cfe5cf69e97267 Miquel Raynal 2021-12-16  530  		if (ret)
cfe5cf69e97267 Miquel Raynal 2021-12-16  531  			break;
cfe5cf69e97267 Miquel Raynal 2021-12-16  532  	}
cfe5cf69e97267 Miquel Raynal 2021-12-16  533  
cfe5cf69e97267 Miquel Raynal 2021-12-16  534  	mutex_unlock(&mxic->lock);
cfe5cf69e97267 Miquel Raynal 2021-12-16  535  
cfe5cf69e97267 Miquel Raynal 2021-12-16  536  	dma_unmap_sg(mxic->dev, ctx->sg, 2, DMA_BIDIRECTIONAL);
cfe5cf69e97267 Miquel Raynal 2021-12-16  537  

Smatch is complaining that ctx->steps might be zero.  I should probably
try to silence that kind of warning if the cross function DB has not
been built.  It tends to have false positives.

But shouldn't we have an if (ret) return ret; after this dma_unmap_sg()?
Can we really retrieve the calculated ECC bytes when processing the data
failed?

cfe5cf69e97267 Miquel Raynal 2021-12-16  538  	/* Retrieve the calculated ECC bytes */
cfe5cf69e97267 Miquel Raynal 2021-12-16  539  	for (step = 0; step < ctx->steps; step++) {
cfe5cf69e97267 Miquel Raynal 2021-12-16  540  		offset = ctx->meta_sz + (step * ctx->oob_step_sz);
cfe5cf69e97267 Miquel Raynal 2021-12-16  541  		mtd_ooblayout_get_eccbytes(mtd,
cfe5cf69e97267 Miquel Raynal 2021-12-16  542  					   (u8 *)ctx->req->oobbuf.out + offset,
cfe5cf69e97267 Miquel Raynal 2021-12-16  543  					   ctx->oobwithstat + (step * STAT_BYTES),
cfe5cf69e97267 Miquel Raynal 2021-12-16  544  					   step * ctx->parity_sz,
cfe5cf69e97267 Miquel Raynal 2021-12-16  545  					   ctx->parity_sz);
cfe5cf69e97267 Miquel Raynal 2021-12-16  546  	}
cfe5cf69e97267 Miquel Raynal 2021-12-16  547  
cfe5cf69e97267 Miquel Raynal 2021-12-16 @548  	return ret;
cfe5cf69e97267 Miquel Raynal 2021-12-16  549  }

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


^ permalink raw reply	[flat|nested] 2+ messages in thread
* Re: [mtd:spi-mem-ecc 14/29] drivers/mtd/nand/ecc-mxic.c:548 mxic_ecc_prepare_io_req_external() error: uninitialized symbol 'ret'.
  2022-02-09  7:22 [mtd:spi-mem-ecc 14/29] drivers/mtd/nand/ecc-mxic.c:548 mxic_ecc_prepare_io_req_external() error: uninitialized symbol 'ret' Dan Carpenter
@ 2022-02-09  8:45 ` Miquel Raynal
  0 siblings, 0 replies; 2+ messages in thread
From: Miquel Raynal @ 2022-02-09  8:45 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kbuild, lkp, kbuild-all, linux-kernel

Hi Dan,

dan.carpenter@oracle.com wrote on Wed, 9 Feb 2022 10:22:04 +0300:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-mem-ecc
> head:   d6986e74ec6ee6a48ce9ee1d8051b2988d747558
> commit: cfe5cf69e97267e9d1de65cc894b7a2310644a15 [14/29] mtd: nand: mxic-ecc: Add Macronix external ECC engine support
> config: x86_64-randconfig-m001-20220207 (https://download.01.org/0day-ci/archive/20220209/202202090415.SS8TwwHj-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 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/mtd/nand/ecc-mxic.c:548 mxic_ecc_prepare_io_req_external() error: uninitialized symbol 'ret'.
> 
> vim +/ret +548 drivers/mtd/nand/ecc-mxic.c
> 
> cfe5cf69e97267 Miquel Raynal 2021-12-16  494  static int mxic_ecc_prepare_io_req_external(struct nand_device *nand,
> cfe5cf69e97267 Miquel Raynal 2021-12-16  495  					    struct nand_page_io_req *req)
> cfe5cf69e97267 Miquel Raynal 2021-12-16  496  {
> cfe5cf69e97267 Miquel Raynal 2021-12-16  497  	struct mxic_ecc_engine *mxic = nand_to_mxic(nand);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  498  	struct mxic_ecc_ctx *ctx = nand_to_ecc_ctx(nand);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  499  	struct mtd_info *mtd = nanddev_to_mtd(nand);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  500  	int offset, nents, step, ret;
> cfe5cf69e97267 Miquel Raynal 2021-12-16  501  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  502  	if (req->mode == MTD_OPS_RAW)
> cfe5cf69e97267 Miquel Raynal 2021-12-16  503  		return 0;
> cfe5cf69e97267 Miquel Raynal 2021-12-16  504  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  505  	nand_ecc_tweak_req(&ctx->req_ctx, req);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  506  	ctx->req = req;
> cfe5cf69e97267 Miquel Raynal 2021-12-16  507  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  508  	if (req->type == NAND_PAGE_READ)
> cfe5cf69e97267 Miquel Raynal 2021-12-16  509  		return 0;
> cfe5cf69e97267 Miquel Raynal 2021-12-16  510  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  511  	mxic_ecc_add_room_in_oobbuf(ctx, ctx->oobwithstat,
> cfe5cf69e97267 Miquel Raynal 2021-12-16  512  				    ctx->req->oobbuf.out);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  513  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  514  	sg_set_buf(&ctx->sg[0], req->databuf.out, req->datalen);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  515  	sg_set_buf(&ctx->sg[1], ctx->oobwithstat,
> cfe5cf69e97267 Miquel Raynal 2021-12-16  516  		   req->ooblen + (ctx->steps * STAT_BYTES));
> cfe5cf69e97267 Miquel Raynal 2021-12-16  517  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  518  	nents = dma_map_sg(mxic->dev, ctx->sg, 2, DMA_BIDIRECTIONAL);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  519  	if (!nents)
> cfe5cf69e97267 Miquel Raynal 2021-12-16  520  		return -EINVAL;
> cfe5cf69e97267 Miquel Raynal 2021-12-16  521  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  522  	mutex_lock(&mxic->lock);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  523  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  524  	for (step = 0; step < ctx->steps; step++) {
> cfe5cf69e97267 Miquel Raynal 2021-12-16  525  		writel(sg_dma_address(&ctx->sg[0]) + (step * ctx->data_step_sz),
> cfe5cf69e97267 Miquel Raynal 2021-12-16  526  		       mxic->regs + SDMA_MAIN_ADDR);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  527  		writel(sg_dma_address(&ctx->sg[1]) + (step * (ctx->oob_step_sz + STAT_BYTES)),
> cfe5cf69e97267 Miquel Raynal 2021-12-16  528  		       mxic->regs + SDMA_SPARE_ADDR);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  529  		ret = mxic_ecc_process_data(mxic, ctx->req->type);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  530  		if (ret)
> cfe5cf69e97267 Miquel Raynal 2021-12-16  531  			break;
> cfe5cf69e97267 Miquel Raynal 2021-12-16  532  	}
> cfe5cf69e97267 Miquel Raynal 2021-12-16  533  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  534  	mutex_unlock(&mxic->lock);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  535  
> cfe5cf69e97267 Miquel Raynal 2021-12-16  536  	dma_unmap_sg(mxic->dev, ctx->sg, 2, DMA_BIDIRECTIONAL);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  537  
> 
> Smatch is complaining that ctx->steps might be zero.  I should probably
> try to silence that kind of warning if the cross function DB has not
> been built.  It tends to have false positives.

I'm sorry I don't know what you mean by "if the cross function DB has
not been built"?

Both ->prepare() and ->finish() hooks cannot be called if ->init_ctx()
failed (the core enforces that). In the init implementation, there is
this:

conf->step_size = SZ_1K;
steps = mtd->writesize / conf->step_size;
ctx->steps = steps;

Also, mtd->writesize == 0 is impossible here because:
in drivers/mtd/nand/core.c:nanddev_init():
we check that memorg->pagesize != 0 and then we assign mtd->writesize
to memorg->pagesize.

nanddev_init() is called by the raw NAND core and SPI NAND core, which
are the only two possible users of this driver.

So I would say this is a false positive that you can silence.

> But shouldn't we have an if (ret) return ret; after this dma_unmap_sg()?
> Can we really retrieve the calculated ECC bytes when processing the data
> failed?

Well yeah, that's right, I'll fix it inline, thanks for catching that.

> cfe5cf69e97267 Miquel Raynal 2021-12-16  538  	/* Retrieve the calculated ECC bytes */
> cfe5cf69e97267 Miquel Raynal 2021-12-16  539  	for (step = 0; step < ctx->steps; step++) {
> cfe5cf69e97267 Miquel Raynal 2021-12-16  540  		offset = ctx->meta_sz + (step * ctx->oob_step_sz);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  541  		mtd_ooblayout_get_eccbytes(mtd,
> cfe5cf69e97267 Miquel Raynal 2021-12-16  542  					   (u8 *)ctx->req->oobbuf.out + offset,
> cfe5cf69e97267 Miquel Raynal 2021-12-16  543  					   ctx->oobwithstat + (step * STAT_BYTES),
> cfe5cf69e97267 Miquel Raynal 2021-12-16  544  					   step * ctx->parity_sz,
> cfe5cf69e97267 Miquel Raynal 2021-12-16  545  					   ctx->parity_sz);
> cfe5cf69e97267 Miquel Raynal 2021-12-16  546  	}
> cfe5cf69e97267 Miquel Raynal 2021-12-16  547  
> cfe5cf69e97267 Miquel Raynal 2021-12-16 @548  	return ret;
> cfe5cf69e97267 Miquel Raynal 2021-12-16  549  }
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 


Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-02-09  8:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09  7:22 [mtd:spi-mem-ecc 14/29] drivers/mtd/nand/ecc-mxic.c:548 mxic_ecc_prepare_io_req_external() error: uninitialized symbol 'ret' Dan Carpenter
2022-02-09  8:45 ` Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).