Re: [RESEND PATCH v2 3/3] cmd: mtd: add nandtest command support

On 9/10/24 00:21, Mikhail Kshevetskiy wrote:

This patch implements readonly test of nand flashes.

nits

%s/readonly test of nand flashes/a read-only test for NAND flash devices/

Signed-off-by: Mikhail Kshevetskiy mikhail.kshevetskiy@iopsys.eu

cmd/Kconfig | 6 ++ cmd/mtd.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 209 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig index 698e0e697f2..db5113e016b 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1445,6 +1445,12 @@ config CMD_MTD_TORTURE help MTD torture command support.

+config CMD_MTD_NANDTEST

• bool "mtd nandtest"
• depends on CMD_MTD
• help

•  MTD nandtest command support.
  

• config CMD_MUX bool "mux" depends on MULTIPLEXER

diff --git a/cmd/mtd.c b/cmd/mtd.c index 69ddfe1d9c6..4a380dcaf83 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -931,6 +931,202 @@ out_put_mtd: } #endif

+#ifdef CONFIG_CMD_MTD_NANDTEST +enum nandtest_status {

• NANDTEST_STATUS_UNKNOWN = 0,
• NANDTEST_STATUS_NONECC_READ_FAIL,
• NANDTEST_STATUS_ECC_READ_FAIL,
• NANDTEST_STATUS_BAD_BLOCK,
• NANDTEST_STATUS_BITFLIP_ABOVE_MAX,
• NANDTEST_STATUS_BITFLIP_MISMATCH,
• NANDTEST_STATUS_BITFLIP_MAX,
• NANDTEST_STATUS_OK,

+};

+static enum nandtest_status nandtest_block_check(struct mtd_info *mtd,

• 				 loff_t off, size_t blocksize)
  

+{

• struct mtd_oob_ops ops = {};
• u_char *buf;
• int i, d, ret, len, pos, cnt, max;
• if (blocksize % mtd->writesize != 0) {

• printf("\r  block at %llx: bad block size\n", off);
  

• return NANDTEST_STATUS_UNKNOWN;
  

• }
• buf = malloc_cache_aligned(2 * blocksize);
• if (buf == NULL) {

• printf("\r  block at %llx: can't allocate memory\n", off);
  

• return NANDTEST_STATUS_UNKNOWN;
  

• }
• ops.mode = MTD_OPS_RAW;
• ops.len = blocksize;
• ops.datbuf = buf;
• ops.ooblen = 0;
• ops.oobbuf = NULL;
• if (mtd->_read_oob)

• ret = mtd->_read_oob(mtd, off, &ops);
  

• else

• ret = mtd->_read(mtd, off, ops.len, &ops.retlen, ops.datbuf);
  

• if (ret != 0) {

• free(buf);
  

• printf("\r  block at %llx: non-ecc reading error %d\n",
  

•        off, ret);
  

• return NANDTEST_STATUS_NONECC_READ_FAIL;
  

• }
• ops.mode = MTD_OPS_AUTO_OOB;
• ops.datbuf = buf + blocksize;
• if (mtd->_read_oob)

• ret = mtd->_read_oob(mtd, off, &ops);
  

• else

• ret = mtd->_read(mtd, off, ops.len, &ops.retlen, ops.datbuf);
  

• if (ret == -EBADMSG) {

• free(buf);
  

• printf("\r  block at %llx: bad block\n", off);
  

• return NANDTEST_STATUS_BAD_BLOCK;
  

• }
• if (ret < 0) {

• free(buf);
  

• printf("\r  block at %llx: ecc reading error %d\n", off, ret);
  

• return NANDTEST_STATUS_ECC_READ_FAIL;
  

• }
• if (mtd->ecc_strength == 0) {

• free(buf);
  

• return NANDTEST_STATUS_OK;
  

• }
• if (ret > mtd->ecc_strength) {

• free(buf);
  

• printf("\r  block at %llx: returned bit-flips value %d "
  

•        "is above maximum value %d\n",
  

•        off, ret, mtd->ecc_strength);
  

• return NANDTEST_STATUS_BITFLIP_ABOVE_MAX;
  

• }
• max = 0;
• pos = 0;
• len = blocksize;
• while (len > 0) {

• cnt = 0;
  

• for (i = 0; i < mtd->ecc_step_size; i++) {
  

• 	d = buf[pos + i] ^ buf[blocksize + pos + i];
  

• 	if (d == 0)
  

• 		continue;
  

• 	while (d > 0) {
  

• 		d &= (d - 1);
  

• 		cnt++;
  

• 	}
  

• }
  

• if (cnt > max)
  

• 	max = cnt;
  

• len -= mtd->ecc_step_size;
  

• pos += mtd->ecc_step_size;
  

• }
• free(buf);
• if (max > ret) {

• printf("\r  block at %llx: bitflip mismatch, "
  

•        "read %d but actual %d\n", off, ret, max);
  

• return NANDTEST_STATUS_BITFLIP_MISMATCH;
  

• }
• if (ret == mtd->ecc_strength) {

• printf("\r  block at %llx: max bitflip reached, "
  

•        "block is unreliable\n", off);
  

• return NANDTEST_STATUS_BITFLIP_MAX;
  

• }
• return NANDTEST_STATUS_OK;

+}

+static int do_mtd_nandtest(struct cmd_tbl *cmdtp, int flag, int argc,

• 	   char *const argv[])
  

+{

• struct mtd_info *mtd;
• loff_t off, len;

These variables should use the same type as the field size in struct mtd_info to avoid overflows, i.e. u64.

• int stat[NANDTEST_STATUS_OK + 1];
• enum nandtest_status ret;
• size_t block_size;
• if (argc < 2)

• return CMD_RET_USAGE;
  

• mtd = get_mtd_by_name(argv[1]);
• if (IS_ERR_OR_NULL(mtd))

• return CMD_RET_FAILURE;
  

• if (!mtd_can_have_bb(mtd)) {

• printf("Only NAND-based devices can be checked\n");
  

• goto out_put_mtd;
  

• }
• off = 0;
• len = mtd->size;
• block_size = mtd->erasesize;
• printf("ECC strength: %d\n", mtd->ecc_strength);
• printf("ECC step size: %d\n", mtd->ecc_step_size);
• printf("Erase block size: 0x%x\n", mtd->erasesize);
• printf("Total blocks: %d\n", (u32)len / mtd->erasesize);

Please, use do_div() for the division instead of truncating len. Cf. mtd_is_aligned_with_block_size().

• memset(stat, 0, sizeof(stat));
• while (len > 0) {

A for loop using the struct mtd_info fields would be easier to read:

for (u64 off = 0; off < mtd->size; off += mtd->erase_size) {

• if (mtd_is_aligned_with_block_size(mtd, off))
  

• 	printf("\r  block at %llx ", off);
  

How could you arrive at a value of `off` which is not aligned to the erase size?

If you could get to such a value, why would you continue?

Best regards

Heinrich

• ret = nandtest_block_check(mtd, off, block_size);
  

• stat[ret]++;
  

• switch (ret) {
  

• case NANDTEST_STATUS_BAD_BLOCK:
  

• case NANDTEST_STATUS_BITFLIP_MAX:
  

• 	if (!mtd_block_isbad(mtd, off))
  

• 		printf("\r  block at %llx: should be marked "
  

• 		       "as BAD\n", off);
  

• 	break;
  

• case NANDTEST_STATUS_OK:
  

• 	if (mtd_block_isbad(mtd, off))
  

• 		printf("\r  block at %llx: marked as BAD, but "
  

• 		       "probably is GOOD\n", off);
  

• 	break;
  

• default:
  

• 	break;
  

• }
  

• off += block_size;
  

• len -= block_size;
  

• }

+out_put_mtd:

• put_mtd_device(mtd);
• printf("\n");
• printf("results:\n");
• printf(" Good blocks: %d\n", stat[NANDTEST_STATUS_OK]);
• printf(" Physically bad blocks: %d\n", stat[NANDTEST_STATUS_BAD_BLOCK]);
• printf(" Unreliable blocks: %d\n", stat[NANDTEST_STATUS_BITFLIP_MAX]);
• printf(" Non checked blocks: %d\n", stat[NANDTEST_STATUS_UNKNOWN]);
• printf(" Failed to check blocks: %d\n", stat[NANDTEST_STATUS_NONECC_READ_FAIL] +

• 				 stat[NANDTEST_STATUS_ECC_READ_FAIL]);
  

• printf(" Suspictious blocks: %d\n", stat[NANDTEST_STATUS_BITFLIP_ABOVE_MAX] +

• 				 stat[NANDTEST_STATUS_BITFLIP_MISMATCH]);
  

• return CMD_RET_SUCCESS;

+} +#endif

• static int do_mtd_bad(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) {

@@ -1019,6 +1215,9 @@ U_BOOT_LONGHELP(mtd, #endif #ifdef CONFIG_CMD_MTD_TORTURE "mtd torture <name> [<off> [<size>]]\n" +#endif +#ifdef CONFIG_CMD_MTD_NANDTEST

• "mtd nandtest <name>\n" #endif "\n" "With:\n"

@@ -1060,6 +1259,10 @@ U_BOOT_CMD_WITH_SUBCMDS(mtd, "MTD utils", mtd_help_text, #ifdef CONFIG_CMD_MTD_TORTURE U_BOOT_SUBCMD_MKENT_COMPLETE(torture, 4, 0, do_mtd_torture, mtd_name_complete), +#endif +#ifdef CONFIG_CMD_MTD_NANDTEST

• U_BOOT_SUBCMD_MKENT_COMPLETE(nandtest, 2, 0, do_mtd_nandtest,
  

• 			     mtd_name_complete),
  

  #endif U_BOOT_SUBCMD_MKENT_COMPLETE(bad, 2, 1, do_mtd_bad, mtd_name_complete));