Hi Max, On Thu, Jun 9, 2016 at 3:19 PM, Max Krummenacher <max.oss...@gmail.com> wrote: > Hi Benoît, > > 2016-06-09 1:41 GMT+02:00 Benoît Thébaudeau <benoit.thebaudeau....@gmail.com>: >> On Tue, Jun 7, 2016 at 1:46 PM, Max Krummenacher <max.oss...@gmail.com> >> wrote: >>> diff --git a/cmd/nand.c b/cmd/nand.c >>> index 583a18f..8ade5e2 100644 >>> --- a/cmd/nand.c >>> +++ b/cmd/nand.c [...] >> + if (argc > 3 && !str2off(argv[3], &size)) { > > Here I prefer having that in 2 if() as the stuff tested is only loosely > related.
Usually, we keep things as compact as possible, which also limits the number of indentation levels, but that's fine if you prefer otherwise. I don't think it's a strong rule. > I guess keeping it like this would also require parantheses around (argc > 3). No: `>` has higher precedence than `&&`. > Will revert to two if's in v3 > >> + puts("Size is not a valid number\n"); >> + return 1; >> + } >> >> - return ret == 0 ? 0 : 1; >> + endoff = off + size; >> + if (endoff > mtd->size) { >> + puts("Arguments beyond end of NAND\n"); >> + return 1; >> + } >> + >> + off = round_down(off, mtd->erasesize); >> + endoff = round_up(endoff, mtd->erasesize); >> + size = endoff - off; >> + printf("\nNAND torture: device %d offset 0x%llx size 0x%llx " >> + "(block size 0x%x)\n", > > patman.py/checkpatch.py warn here to keep quoted strings on one line > even when the line length exceeds 80 characters. > Will remove the line break / string concatenation for v3. Normally, this rule is for grep-ability. Here, it's more complicated with the '%' in-between, but it's still makes sense with a regular expression, so OK. >> + dev, off, size, mtd->erasesize); >> + while (off < endoff) { >> + ret = nand_torture(mtd, off); >> + if (ret) { >> + failed++; >> + printf(" block at 0x%llx failed\n", off); >> + } else { >> + passed++; >> + } >> + off += mtd->erasesize; >> + } >> + printf(" Passed: %u, failed: %u\n", passed, failed); >> + return failed != 0; >> >>> } >>> #endif > > Apart from above comments I merged your proposal. > >>> >>> @@ -775,7 +792,8 @@ static char nand_help_text[] = > ... >>> diff --git a/doc/README.nand b/doc/README.nand >>> index 96ffc48..5136f31 100644 >>> --- a/doc/README.nand >>> +++ b/doc/README.nand >>> @@ -307,7 +307,7 @@ Miscellaneous and testing commands: >>> DANGEROUS!!! Factory set bad blocks will be lost. Use only >>> to remove artificial bad blocks created with the "markbad" command. >>> >>> - "torture offset" >>> + "torture offset [size]" >>> Torture block to determine if it is still reliable. >>> Enabled by the CONFIG_CMD_NAND_TORTURE configuration option. >>> This command returns 0 if the block is still reliable, else 1. >>> @@ -324,6 +324,10 @@ Miscellaneous and testing commands: >>> automate actions following a nand->write() error. This would e.g. be >>> required >>> in order to program or update safely firmware to NAND, especially for >>> the UBI >>> part of such firmware. >>> + Optionally a second parameter size can be given to test multiple blocks >>> with >> >> "Optionally," >> >>> + one call. If size is not a multiple of the NAND's erasesize then the >>> block >> >> "erase size, then" >> >>> + which contains offset + size will be tested in full. If used with size >>> this >> >> "that", not "which". >> "size, this" >> > > Agreed. Will add that to v3. Thanks. Best regards, Benoît _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot