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

Heinrich Schuchardt xypron.glpk at gmx.de
Tue Sep 10 04:43:29 CEST 2024


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 at 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));



More information about the U-Boot mailing list