* [U-Boot] [PATCH 1/5] NAND: Really ignore bad blocks when scrubbing
2011-09-08 20:39 [U-Boot] [PATCH 0/5] Random NAND fixes and improvements Marek Vasut
@ 2011-09-08 20:39 ` Marek Vasut
2011-09-08 20:39 ` [U-Boot] [PATCH 2/5] NAND: Add nand read.raw and write.raw commands Marek Vasut
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Marek Vasut @ 2011-09-08 20:39 UTC (permalink / raw)
To: u-boot
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
---
drivers/mtd/nand/nand_base.c | 2 +-
drivers/mtd/nand/nand_util.c | 22 +++++-----------------
include/linux/mtd/mtd.h | 1 +
3 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 1a95a91..d8d30e3 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2224,7 +2224,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
/*
* heck if we have a bad block, we do not erase bad blocks !
*/
- if (nand_block_checkbad(mtd, ((loff_t) page) <<
+ if (!instr->scrub && nand_block_checkbad(mtd, ((loff_t) page) <<
chip->page_shift, 0, allowbbt)) {
printk(KERN_WARNING "nand_erase: attempt to erase a "
"bad block at page 0x%08x\n", page);
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index 81bf366..0c3b7f7 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -57,12 +57,6 @@ typedef struct mtd_info mtd_info_t;
#define cpu_to_je16(x) (x)
#define cpu_to_je32(x) (x)
-/*****************************************************************************/
-static int nand_block_bad_scrub(struct mtd_info *mtd, loff_t ofs, int getchip)
-{
- return 0;
-}
-
/**
* nand_erase_opts: - erase NAND flash with support for various options
* (jffs2 formating)
@@ -82,10 +76,10 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts)
int bbtest = 1;
int result;
int percent_complete = -1;
- int (*nand_block_bad_old)(struct mtd_info *, loff_t, int) = NULL;
const char *mtd_device = meminfo->name;
struct mtd_oob_ops oob_opts;
struct nand_chip *chip = meminfo->priv;
+ struct nand_chip *priv_nand = meminfo->priv;
if ((opts->offset & (meminfo->writesize - 1)) != 0) {
printf("Attempt to erase non page aligned data\n");
@@ -110,11 +104,9 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts)
* and disable bad block table while erasing.
*/
if (opts->scrub) {
- struct nand_chip *priv_nand = meminfo->priv;
-
- nand_block_bad_old = priv_nand->block_bad;
- priv_nand->block_bad = nand_block_bad_scrub;
- /* we don't need the bad block table anymore...
+ erase.scrub = opts->scrub;
+ /*
+ * We don't need the bad block table anymore...
* after scrub, there are no bad blocks left!
*/
if (priv_nand->bbt) {
@@ -204,12 +196,8 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts)
if (!opts->quiet)
printf("\n");
- if (nand_block_bad_old) {
- struct nand_chip *priv_nand = meminfo->priv;
-
- priv_nand->block_bad = nand_block_bad_old;
+ if (opts->scrub)
priv_nand->scan_bbt(meminfo);
- }
return 0;
}
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index d36d584..141c960 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -55,6 +55,7 @@ struct erase_info {
u_long priv;
u_char state;
struct erase_info *next;
+ int scrub;
};
struct mtd_erase_region_info {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [U-Boot] [PATCH 2/5] NAND: Add nand read.raw and write.raw commands
2011-09-08 20:39 [U-Boot] [PATCH 0/5] Random NAND fixes and improvements Marek Vasut
2011-09-08 20:39 ` [U-Boot] [PATCH 1/5] NAND: Really ignore bad blocks when scrubbing Marek Vasut
@ 2011-09-08 20:39 ` Marek Vasut
2011-09-08 20:39 ` [U-Boot] [PATCH 3/5] NAND: Allow per-buffer allocation Marek Vasut
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Marek Vasut @ 2011-09-08 20:39 UTC (permalink / raw)
To: u-boot
These commands should work around various "hardware" ECC and BCH methods.
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
---
common/cmd_nand.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 66e06a5..a1c8dfd 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -606,6 +606,20 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
ret = nand->read_oob(nand, off, &ops);
else
ret = nand->write_oob(nand, off, &ops);
+ } else if (!strcmp(s, ".raw")) {
+ /* Raw access */
+ mtd_oob_ops_t ops = {
+ .datbuf = (u8 *)addr,
+ .oobbuf = ((u8 *)addr) + nand->writesize,
+ .len = nand->writesize,
+ .ooblen = nand->oobsize,
+ .mode = MTD_OOB_RAW
+ };
+
+ if (read)
+ ret = nand->read_oob(nand, off, &ops);
+ else
+ ret = nand->write_oob(nand, off, &ops);
} else {
printf("Unknown nand command suffix '%s'.\n", s);
return 1;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [U-Boot] [PATCH 3/5] NAND: Allow per-buffer allocation
2011-09-08 20:39 [U-Boot] [PATCH 0/5] Random NAND fixes and improvements Marek Vasut
2011-09-08 20:39 ` [U-Boot] [PATCH 1/5] NAND: Really ignore bad blocks when scrubbing Marek Vasut
2011-09-08 20:39 ` [U-Boot] [PATCH 2/5] NAND: Add nand read.raw and write.raw commands Marek Vasut
@ 2011-09-08 20:39 ` Marek Vasut
2011-09-08 20:39 ` [U-Boot] [PATCH 4/5] NAND: Make page, erase, oob size available via cmd_nand Marek Vasut
2011-09-08 20:39 ` [U-Boot] [PATCH 5/5] NAND: Add scrub.quiet command option Marek Vasut
4 siblings, 0 replies; 11+ messages in thread
From: Marek Vasut @ 2011-09-08 20:39 UTC (permalink / raw)
To: u-boot
Don't allocate NAND buffers as one block, but allocate them separately. This
allows systems where DMA to buffers happen to allocate these buffers properly
aligned.
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
---
drivers/mtd/nand/nand_base.c | 30 +++++++++++++++++++++++-------
include/linux/mtd/nand.h | 7 ++++---
2 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index d8d30e3..3093067 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2749,13 +2749,27 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
*/
int nand_scan_tail(struct mtd_info *mtd)
{
- int i;
+ int i, bufsize;
+ uint8_t *buf;
struct nand_chip *chip = mtd->priv;
- if (!(chip->options & NAND_OWN_BUFFERS))
- chip->buffers = kmalloc(sizeof(*chip->buffers), GFP_KERNEL);
- if (!chip->buffers)
- return -ENOMEM;
+ if (!(chip->options & NAND_OWN_BUFFERS)) {
+ chip->buffers = malloc(sizeof(struct nand_buffers));
+ if (!chip->buffers)
+ return -ENOMEM;
+
+ bufsize = NAND_MAX_PAGESIZE + (3 * NAND_MAX_OOBSIZE);
+ buf = malloc(bufsize);
+ if (!buf) {
+ free(chip->buffers);
+ return -ENOMEM;
+ }
+
+ chip->buffers->buffer = buf;
+ chip->buffers->ecccalc = buf;
+ chip->buffers->ecccode = buf + NAND_MAX_OOBSIZE;
+ chip->buffers->databuf = buf + (2 * NAND_MAX_OOBSIZE);
+ }
/* Set the internal oob buffer location, just after the page data */
chip->oob_poi = chip->buffers->databuf + mtd->writesize;
@@ -2996,6 +3010,8 @@ void nand_release(struct mtd_info *mtd)
/* Free bad block table memory */
kfree(chip->bbt);
- if (!(chip->options & NAND_OWN_BUFFERS))
- kfree(chip->buffers);
+ if (!(chip->options & NAND_OWN_BUFFERS)) {
+ free(chip->buffers->buffer);
+ free(chip->buffers);
+ }
}
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 987a2ec..c3449a9 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -370,9 +370,10 @@ struct nand_ecc_ctrl {
* consecutive order.
*/
struct nand_buffers {
- uint8_t ecccalc[NAND_MAX_OOBSIZE];
- uint8_t ecccode[NAND_MAX_OOBSIZE];
- uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
+ uint8_t *buffer;
+ uint8_t *ecccalc;
+ uint8_t *ecccode;
+ uint8_t *databuf;
};
/**
--
1.7.5.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [U-Boot] [PATCH 4/5] NAND: Make page, erase, oob size available via cmd_nand
2011-09-08 20:39 [U-Boot] [PATCH 0/5] Random NAND fixes and improvements Marek Vasut
` (2 preceding siblings ...)
2011-09-08 20:39 ` [U-Boot] [PATCH 3/5] NAND: Allow per-buffer allocation Marek Vasut
@ 2011-09-08 20:39 ` Marek Vasut
2011-09-08 20:39 ` [U-Boot] [PATCH 5/5] NAND: Add scrub.quiet command option Marek Vasut
4 siblings, 0 replies; 11+ messages in thread
From: Marek Vasut @ 2011-09-08 20:39 UTC (permalink / raw)
To: u-boot
The "nand info" and "nand device" now set shell/environment variables:
nand_writesize ... nand page size
nand_oobsize ..... nand oob area size
nand_erasesize ... nand erase block size
The shell variables are only set if HUSH is enabled.
Also, the "nand info" command now displays this info.
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
---
common/cmd_nand.c | 42 +++++++++++++++++++++++++++++++++++++++---
1 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index a1c8dfd..5b7e83d 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -27,6 +27,9 @@
#include <asm/byteorder.h>
#include <jffs2/jffs2.h>
#include <nand.h>
+#ifdef CONFIG_SYS_HUSH_PARSER
+#include <hush.h>
+#endif
#if defined(CONFIG_CMD_MTDPARTS)
@@ -362,15 +365,48 @@ usage:
#endif
-static void nand_print_info(int idx)
+static void nand_print_and_set_info(int idx)
{
nand_info_t *nand = &nand_info[idx];
struct nand_chip *chip = nand->priv;
+ const int bufsz = 32;
+ char buf[bufsz];
+
printf("Device %d: ", idx);
if (chip->numchips > 1)
printf("%dx ", chip->numchips);
printf("%s, sector size %u KiB\n",
nand->name, nand->erasesize >> 10);
+ printf(" Page size %8d b\n", nand->writesize);
+ printf(" OOB size %8d b\n", nand->oobsize);
+ printf(" Erase size %8d b\n", nand->erasesize);
+
+ /* Set geometry info */
+#ifdef CONFIG_SYS_HUSH_PARSER
+ memset(buf, 0, bufsz);
+ sprintf(buf, "nand_writesize=%x", nand->writesize);
+ set_local_var(buf, 0);
+
+ memset(buf, 0, bufsz);
+ sprintf(buf, "nand_oobsize=%x", nand->oobsize);
+ set_local_var(buf, 0);
+
+ memset(buf, 0, bufsz);
+ sprintf(buf, "nand_erasesize=%x", nand->erasesize);
+ set_local_var(buf, 0);
+#else
+ memset(buf, 0, bufsz);
+ sprintf(buf, "%x", nand->writesize);
+ setenv("nand_writesize", buf);
+
+ memset(buf, 0, bufsz);
+ sprintf(buf, "%x", nand->oobsize);
+ setenv("nand_oobsize", buf);
+
+ memset(buf, 0, bufsz);
+ sprintf(buf, "%x", nand->erasesize);
+ setenv("nand_erasesize", buf);
+#endif
}
int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
@@ -407,7 +443,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
putc('\n');
for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
if (nand_info[i].name)
- nand_print_info(i);
+ nand_print_and_set_info(i);
}
return 0;
}
@@ -418,7 +454,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE)
puts("no devices available\n");
else
- nand_print_info(dev);
+ nand_print_and_set_info(dev);
return 0;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [U-Boot] [PATCH 5/5] NAND: Add scrub.quiet command option
2011-09-08 20:39 [U-Boot] [PATCH 0/5] Random NAND fixes and improvements Marek Vasut
` (3 preceding siblings ...)
2011-09-08 20:39 ` [U-Boot] [PATCH 4/5] NAND: Make page, erase, oob size available via cmd_nand Marek Vasut
@ 2011-09-08 20:39 ` Marek Vasut
2011-09-09 15:39 ` Detlev Zundel
4 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2011-09-08 20:39 UTC (permalink / raw)
To: u-boot
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.vasut@gmail.com>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@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;
+
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;
+
ret = nand_erase_opts(nand, &opts);
printf("%s\n", ret ? "ERROR" : "OK");
--
1.7.5.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [U-Boot] [PATCH 5/5] NAND: Add scrub.quiet command option
2011-09-08 20:39 ` [U-Boot] [PATCH 5/5] NAND: Add scrub.quiet command option Marek Vasut
@ 2011-09-09 15:39 ` Detlev Zundel
2011-09-10 20:54 ` Marek Vasut
0 siblings, 1 reply; 11+ messages in thread
From: Detlev Zundel @ 2011-09-09 15:39 UTC (permalink / raw)
To: u-boot
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.vasut@gmail.com>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Detlev Zundel <dzu@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.
> 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
if (scrub) {
if (!scrub_quiet) {
} else {
}
}
Cheers
Detlev
--
In the topologic hell the beer is packed in Klein's bottles.
--
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: dzu at denx.de
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] [PATCH 5/5] NAND: Add scrub.quiet command option
2011-09-09 15:39 ` Detlev Zundel
@ 2011-09-10 20:54 ` Marek Vasut
2011-09-12 9:49 ` Detlev Zundel
0 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2011-09-10 20:54 UTC (permalink / raw)
To: u-boot
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.vasut@gmail.com>
> > Cc: Scott Wood <scottwood@freescale.com>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Detlev Zundel <dzu@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).
>
> > 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.
Cheers!
>
> if (scrub) {
> if (!scrub_quiet) {
> } else {
> }
> }
>
> Cheers
> Detlev
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] [PATCH 5/5] NAND: Add scrub.quiet command option
2011-09-10 20:54 ` Marek Vasut
@ 2011-09-12 9:49 ` Detlev Zundel
0 siblings, 0 replies; 11+ messages in thread
From: Detlev Zundel @ 2011-09-12 9:49 UTC (permalink / raw)
To: u-boot
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.vasut@gmail.com>
>> > Cc: Scott Wood <scottwood@freescale.com>
>> > Cc: Stefano Babic <sbabic@denx.de>
>> > Cc: Wolfgang Denk <wd@denx.de>
>> > Cc: Detlev Zundel <dzu@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: dzu at denx.de
^ permalink raw reply [flat|nested] 11+ messages in thread