[U-Boot] [PATCH 2/2] nand: extend nand torture

[Benoît Thébaudeau]

Dear Max Krummenacher,

On Mon, May 30, 2016 at 4:28 PM, Max Krummenacher <max.oss.09 at 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 at 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