Re: [mtd:spi-mem-ecc 14/29] drivers/mtd/nand/ecc-mxic.c:548 mxic_ecc_prepare_io_req_external() error: uninitialized symbol 'ret'.
From: Miquel Raynal
Date: Wed Feb 09 2022 - 03:47:40 EST
Hi Dan,
dan.carpenter@xxxxxxxxxx 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@xxxxxxxxx/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@xxxxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxx
>
Thanks,
Miquèl