7:45 p.m.
Hi Dan,
dan.carpenter(a)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-lk...)
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(a)intel.com>
Reported-by: Dan Carpenter <dan.carpenter(a)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