[mtd:spi-mem-ecc 14/29] drivers/mtd/nand/ecc-mxic.c:548 mxic_ecc_prepare_io_req_external() error: uninitialized symbol 'ret'.

From: Dan Carpenter
Date: Wed Feb 09 2022 - 02:22:40 EST


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.

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@xxxxxxxxxxxx