[U-Boot] [PATCH 1/2] nand: fix nand torture to use changed mtd api

The mtd subsystem deprecated and renamed the direct use of the mtd_info struct's functionpointers. Instead the corresponding mtd_xxx function should be used.
See also: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3...
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com ---
drivers/mtd/nand/nand_util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 71285b6..c957c69 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -847,7 +847,7 @@ int nand_torture(nand_info_t *nand, loff_t offset) }
for (i = 0; i < patt_count; i++) { - err = nand->erase(nand, &instr); + err = mtd_erase(nand, &instr); if (err) { printf("%s: erase() failed for block at 0x%llx: %d\n", nand->name, instr.addr, err); @@ -855,7 +855,7 @@ int nand_torture(nand_info_t *nand, loff_t offset) }
/* Make sure the block contains only 0xff bytes */ - err = nand->read(nand, offset, nand->erasesize, &retlen, buf); + err = mtd_read(nand, offset, nand->erasesize, &retlen, buf); if ((err && err != -EUCLEAN) || retlen != nand->erasesize) { printf("%s: read() failed for block at 0x%llx: %d\n", nand->name, instr.addr, err); @@ -872,14 +872,14 @@ int nand_torture(nand_info_t *nand, loff_t offset)
/* Write a pattern and check it */ memset(buf, patterns[i], nand->erasesize); - err = nand->write(nand, offset, nand->erasesize, &retlen, buf); + err = mtd_write(nand, offset, nand->erasesize, &retlen, buf); if (err || retlen != nand->erasesize) { printf("%s: write() failed for block at 0x%llx: %d\n", nand->name, instr.addr, err); goto out; }
- err = nand->read(nand, offset, nand->erasesize, &retlen, buf); + err = mtd_read(nand, offset, nand->erasesize, &retlen, buf); if ((err && err != -EUCLEAN) || retlen != nand->erasesize) { printf("%s: read() failed for block at 0x%llx: %d\n", nand->name, instr.addr, err);

nand torture currently works on exactly one nand block which is specified by giving the byteoffset to the beginning of the block.
Extend this by allowing for a second parameter specifying the byte offset to the last block to be tested.
e.g. ==> nand torture 1000000
NAND torture: device 0 offset 0x1000000 size 0x20000 passed 1, failed 0
==> nand torture 1000000 1040000
NAND torture: device 0 offset 0x1000000 size 0x20000 passed 2, failed 0
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com ---
cmd/nand.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/cmd/nand.c b/cmd/nand.c index a6b67e2..615dbd5 100644 --- a/cmd/nand.c +++ b/cmd/nand.c @@ -646,6 +646,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
#ifdef CONFIG_CMD_NAND_TORTURE if (strcmp(cmd, "torture") == 0) { + loff_t endoff; + unsigned failed = 0, passed = 0; if (argc < 3) goto usage;
@@ -653,13 +655,27 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) puts("Offset is not a valid number\n"); return 1; } - + endoff = off + nand->erasesize; + if (argc > 3) + if (!str2off(argv[3], &endoff)) { + puts("End is not a valid number\n"); + return 1; + } printf("\nNAND torture: device %d offset 0x%llx size 0x%x\n", dev, off, nand->erasesize); - ret = nand_torture(nand, off); - printf(" %s\n", ret ? "Failed" : "Passed"); + while (off < endoff) {
- return ret == 0 ? 0 : 1; + ret = nand_torture(nand, off); + if (ret) { + failed++; + printf(" off 0x%llx %s\n", off, "Failed"); + } else { + passed++; + } + off += nand->erasesize; + } + printf("passed %u, failed %u\n", passed, failed); + return failed == 0 ? 0 : 1; } #endif
@@ -775,6 +791,7 @@ static char nand_help_text[] = "nand dump[.oob] off - dump page\n" #ifdef CONFIG_CMD_NAND_TORTURE "nand torture off - torture block at offset\n" + "nand torture start end - torture blocks from start to end offset\n" #endif "nand scrub [-y] off size | scrub.part partition | scrub.chip\n" " really clean NAND erasing bad blocks (UNSAFE)\n"

Dear Max Krummenacher,
On Mon, May 30, 2016 at 4:28 PM, Max Krummenacher max.oss.09@gmail.com wrote:
nand torture currently works on exactly one nand block which is specified by giving the byteoffset to the beginning of the block.
Extend this by allowing for a second parameter specifying the byte offset to the last block to be tested.
End offsets are always ambiguous because users can hesitate between the offset of the first byte of the last block, the offset of the last byte of the last block, and the offset of the first byte of the block following the last one (if any). A byte size would probably be better here, and it would also be more consistent with the other nand commands.
e.g. ==> nand torture 1000000
NAND torture: device 0 offset 0x1000000 size 0x20000 passed 1, failed 0
==> nand torture 1000000 1040000
NAND torture: device 0 offset 0x1000000 size 0x20000 passed 2, failed 0
With more than one block to test, the printed size becomes ambiguous here. It would be better to indicate that it is the erase size of the block. The total test size could also be printed, either instead of the erase size, or besides it.
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com
cmd/nand.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/cmd/nand.c b/cmd/nand.c index a6b67e2..615dbd5 100644 --- a/cmd/nand.c +++ b/cmd/nand.c @@ -646,6 +646,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
#ifdef CONFIG_CMD_NAND_TORTURE if (strcmp(cmd, "torture") == 0) {
loff_t endoff;
unsigned failed = 0, passed = 0; if (argc < 3) goto usage;
@@ -653,13 +655,27 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) puts("Offset is not a valid number\n"); return 1; }
endoff = off + nand->erasesize;
if (argc > 3)
if (!str2off(argv[3], &endoff)) {
puts("End is not a valid number\n");
return 1;
} printf("\nNAND torture: device %d offset 0x%llx size 0x%x\n", dev, off, nand->erasesize);
ret = nand_torture(nand, off);
printf(" %s\n", ret ? "Failed" : "Passed");
while (off < endoff) {
return ret == 0 ? 0 : 1;
ret = nand_torture(nand, off);
if (ret) {
failed++;
printf(" off 0x%llx %s\n", off, "Failed");
} else {
passed++;
}
off += nand->erasesize;
}
printf("passed %u, failed %u\n", passed, failed);
return failed == 0 ? 0 : 1; }
#endif
A size parameter could probably be added to nand_torture() instead of handling the range in the command, so that the direct usages of nand_torture() (in or out of tree) can also benefit from this enhancement.
@@ -775,6 +791,7 @@ static char nand_help_text[] = "nand dump[.oob] off - dump page\n" #ifdef CONFIG_CMD_NAND_TORTURE "nand torture off - torture block at offset\n"
"nand torture start end - torture blocks from start to end offset\n"
#endif "nand scrub [-y] off size | scrub.part partition | scrub.chip\n" " really clean NAND erasing bad blocks (UNSAFE)\n"
doc/README.nand should also be updated accordingly.
-- 2.5.5
Best regards, Benoît

Hi Benoît,
Thank you for your review.
I wanted to wait for Scott's patchseries to make it into master to allow for potential needed changes.
2016-05-31 22:21 GMT+02:00 Benoît Thébaudeau benoit.thebaudeau.dev@gmail.com: ...
Extend this by allowing for a second parameter specifying the byte offset to the last block to be tested.
End offsets are always ambiguous because users can hesitate between the offset of the first byte of the last block, the offset of the last byte of the last block, and the offset of the first byte of the block following the last one (if any). A byte size would probably be better here, and it would also be more consistent with the other nand commands.
Ok. I will change the interface to use offset/size.
...
NAND torture: device 0 offset 0x1000000 size 0x20000 passed 2, failed 0
With more than one block to test, the printed size becomes ambiguous here. It would be better to indicate that it is the erase size of the block. The total test size could also be printed, either instead of the erase size, or besides it.
Ok. I will change this to print test size and block size, e.g. like: NAND torture: device 0 offset 0x1000000 size 0x40000 (nand block size 0x20000)
...
return ret == 0 ? 0 : 1;
ret = nand_torture(nand, off);
if (ret) {
...
A size parameter could probably be added to nand_torture() instead of handling the range in the command, so that the direct usages of nand_torture() (in or out of tree) can also benefit from this enhancement.
I disagree here. Likely one uses the extended functionality only during HW bringup and only interactively. If one would want to test multiple blocks from code one would also want to know the testresult of each individual block rather than only having a return parameter indicating a 'all good' or 'at least one block failed'. Even here in the interactive 'nand torture' cmd the printf of a failed block in the loop is a usecase of this.
Best regards Max

Hi Max,
On Tue, Jun 7, 2016 at 12:57 PM, Max Krummenacher max.oss.09@gmail.com wrote:
Hi Benoît,
Thank you for your review.
You're welcome.
I wanted to wait for Scott's patchseries to make it into master to allow for potential needed changes.
No problem.
2016-05-31 22:21 GMT+02:00 Benoît Thébaudeau benoit.thebaudeau.dev@gmail.com: ...
Extend this by allowing for a second parameter specifying the byte offset to the last block to be tested.
End offsets are always ambiguous because users can hesitate between the offset of the first byte of the last block, the offset of the last byte of the last block, and the offset of the first byte of the block following the last one (if any). A byte size would probably be better here, and it would also be more consistent with the other nand commands.
Ok. I will change the interface to use offset/size.
Good.
...
NAND torture: device 0 offset 0x1000000 size 0x20000 passed 2, failed 0
With more than one block to test, the printed size becomes ambiguous here. It would be better to indicate that it is the erase size of the block. The total test size could also be printed, either instead of the erase size, or besides it.
Ok. I will change this to print test size and block size, e.g. like: NAND torture: device 0 offset 0x1000000 size 0x40000 (nand block size 0x20000)
Good.
...
return ret == 0 ? 0 : 1;
ret = nand_torture(nand, off);
if (ret) {
...
A size parameter could probably be added to nand_torture() instead of handling the range in the command, so that the direct usages of nand_torture() (in or out of tree) can also benefit from this enhancement.
I disagree here. Likely one uses the extended functionality only during HW bringup and only interactively. If one would want to test multiple blocks from code one would also want to know the testresult of each individual block rather than only having a return parameter indicating a 'all good' or 'at least one block failed'. Even here in the interactive 'nand torture' cmd the printf of a failed block in the loop is a usecase of this.
Makes sense. Agreed.
Best regards, Benoît
participants (2)
-
Benoît Thébaudeau
-
Max Krummenacher