Hi Marek,
> On Friday, September 09, 2011 05:39:07 PM Detlev Zundel wrote:
>> Hi Marek,
>>
>> > This allows the scrub command to scrub without asking the user if he
>> > really wants to scrub the area. Useful in scripts.
>> >
>> > Signed-off-by: Marek Vasut <marek.va...@gmail.com>
>> > Cc: Scott Wood <scottw...@freescale.com>
>> > Cc: Stefano Babic <sba...@denx.de>
>> > Cc: Wolfgang Denk <w...@denx.de>
>> > Cc: Detlev Zundel <d...@denx.de>
>> > ---
>> >
>> > common/cmd_nand.c | 14 +++++++++++++-
>> > 1 files changed, 13 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/common/cmd_nand.c b/common/cmd_nand.c
>> > index 5b7e83d..45179e9 100644
>> > --- a/common/cmd_nand.c
>> > +++ b/common/cmd_nand.c
>> > @@ -502,11 +502,19 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc,
>> > char * const argv[])
>> >
>> > int clean = argc > 2 && !strcmp("clean", argv[2]);
>> > int o = clean ? 3 : 2;
>> > int scrub = !strncmp(cmd, "scrub", 5);
>> >
>> > + int scrub_quiet = !strncmp(cmd, "scrub.quiet", 11);
>> >
>> > int part = 0;
>> > int chip = 0;
>> > int spread = 0;
>> > int args = 2;
>> >
>> > + /*
>> > + * Quiet scrub is a special option only for the scrub command,
>> > + * ignore it in the following construction.
>> > + */
>> > + if (scrub_quiet)
>> > + cmd[5] = 0;
>> > +
>>
>> This is _really_ hackish and makes an effort not to fit into the coding
>> style at hand. Please use the available code and extend the construct
>> below to match the ".quiet" suffix. It is not that different.
>
> Right, I got a bit wild in here. Though still, it'll be a special case, like
>
> } else if (!strncmp(cmd, "scrub.quiet", 11)) {
>
> in the block below, because quiet should only work for scrub (it's no use for
> other commands).
Ok.
>> > if (cmd[5] != 0) {
>> >
>> > if (!strcmp(&cmd[5], ".spread")) {
>> >
>> > spread = 1;
>> >
>> > @@ -543,7 +551,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc,
>> > char * const argv[])
>> >
>> > opts.quiet = quiet;
>> > opts.spread = spread;
>> >
>> > - if (scrub) {
>> > + if (scrub && !scrub_quiet) {
>> >
>> > puts("Warning: "
>> >
>> > "scrub option will erase all factory set "
>> > "bad blocks!\n"
>> >
>> > @@ -569,6 +577,10 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc,
>> > char * const argv[])
>> >
>> > return -1;
>> >
>> > }
>> >
>> > }
>> >
>> > +
>> > + if (scrub_quiet)
>> > + opts.scrub = 1;
>> > +
>>
>> Urgh. Please turn this into
>
> What about:
>
> if (scrub && scrub.quiet) {
> opts.scrub = 1;
> } else if (scrub) {
> ...
> }
>
> To avoid fragmenting the code too much and avoid too deep indent.
>>
>> if (scrub) {
>> if (!scrub_quiet) {
>> } else {
>> }
>> }
I think it expresses the intention less clear, but I don't care enough
to nak such a thing - it's still better than the current version.
Cherrs
Detlev
--
Irrationality is the square root of all evil.
-- Douglas Hofstadter
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
