Skip to contentSkip to conversation

Random NAND fixes and improvements

PATCH New Marek Vasut 5 participants 59 messages Sep 12, 2011 – Sep 27, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover Random NAND fixes and improvements

This is a resend of the NAND fixes and improvements. These are squashed togethere here and sent as a batch.

V2: Fix the scrub.quiet command

Marek Vasut (5):
NAND: Really ignore bad blocks when scrubbing
NAND: Add nand read.raw and write.raw commands
NAND: Allow per-buffer allocation
NAND: Make page, erase, oob size available via cmd_nand
NAND: Add scrub.quiet command option

 common/cmd_nand.c            |   64 +++++++++++++++++++++++++++++++++++++++--
 drivers/mtd/nand/nand_base.c |   32 +++++++++++++++-----
 drivers/mtd/nand/nand_util.c |   22 +++-----------
 include/linux/mtd/mtd.h      |    1 +
 include/linux/mtd/nand.h     |    7 ++--
 5 files changed, 94 insertions(+), 32 deletions(-)
Signature · 1 line
1.7.5.4
#1 lore raw

PATCH v2 1/5 NAND: Really ignore bad blocks when scrubbing

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 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(-)
drivers/mtd/nand/nand_base.c +1 -1
index 1a95a91..d8d30e3 100644
@@ -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);
drivers/mtd/nand/nand_util.c +5 -17
index 81bf366..0c3b7f7 100644
@@ -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;
 }
include/linux/mtd/mtd.h +1 -0
index d36d584..141c960 100644
@@ -55,6 +55,7 @@ struct erase_info {
 	u_long priv;
 	u_char state;
 	struct erase_info *next;
+	int scrub;
 };
 
 struct mtd_erase_region_info {
Signature · 1 line
1.7.5.4
#2 lore raw

PATCH 1/5 NAND: Really ignore bad blocks when scrubbing

Quoted 10 lines · Marek Vasut
Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 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(-)

Applied to u-boot-nand-flash next with this change to remove the chip/priv_nand redundancy:

drivers/mtd/nand/nand_util.c +4 -5
index 0c3b7f7..60c778e 100644
@@ -79,7 +79,6 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts)
 	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");
@@ -109,10 +108,10 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts)
 		 * We don't need the bad block table anymore...
 		 * after scrub, there are no bad blocks left!
 		 */
-		if (priv_nand->bbt) {
-			kfree(priv_nand->bbt);
+		if (chip->bbt) {
+			kfree(chip->bbt);
 		}
-		priv_nand->bbt = NULL;
+		chip->bbt = NULL;
 	}
 
 	for (erased_length = 0;
@@ -197,7 +196,7 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts)
 		printf("\n");
 
 	if (opts->scrub)
-		priv_nand->scan_bbt(meminfo);
+		chip->scan_bbt(meminfo);
 
 	return 0;
 }

-Scott

#3 lore raw

PATCH v2 2/5 NAND: Add nand read.raw and write.raw commands

In reply to Marek Vasut

These commands should work around various "hardware" ECC and BCH methods.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
common/cmd_nand.c +14 -0
index 66e06a5..a1c8dfd 100644
@@ -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;
Signature · 1 line
1.7.5.4
#4 lore raw

PATCH 2/5 NAND: Add nand read.raw and write.raw commands

Quoted 26 lines · Marek Vasut

These commands should work around various "hardware" ECC and BCH methods.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
common/cmd_nand.c +7 -0
index 66e06a5..a1c8dfd 100644
@@ -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,

common/cmd_nand.c int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 625

Quoted 10 lines
common/cmd_nand.c +7 -0
+				.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;

Update help text and doc/README.nand.

Should we support doing multiple pages at once?

-Scott

#5 lore raw

PATCH v2 2/5 NAND: Add nand read.raw and write.raw commands

In reply to Marek Vasut

These commands should work around various "hardware" ECC and BCH methods.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   19 +++++++++++++++++--
 doc/README.nand   |    9 +++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

V2: Add documentation.

common/cmd_nand.c +17 -2
index 66e06a5..72d418c 100644
@@ -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;
@@ -695,10 +709,11 @@ U_BOOT_CMD(
 	"NAND sub-system",
 	"info - show available NAND devices\n"
 	"nand device [dev] - show or set current device\n"
-	"nand read - addr off|partition size\n"
-	"nand write - addr off|partition size\n"
+	"nand read[.raw] - addr off|partition size\n"
+	"nand write[.raw] - addr off|partition size\n"
 	"    read/write 'size' bytes starting at offset 'off'\n"
 	"    to/from memory address 'addr', skipping bad blocks.\n"
+	"    Use read.raw/write.raw to avoid ECC and write the block as-is.\n"
 #ifdef CONFIG_CMD_NAND_TRIMFFS
 	"nand write.trimffs - addr off|partition size\n"
 	"    write 'size' bytes starting at offset 'off' from memory address\n"
doc/README.nand +9 -0
index 751b693..084223a 100644
@@ -94,6 +94,15 @@ Commands:
       of data for one 512-byte page or 2 256-byte pages. There is no check
       for bad blocks.
 
+   nand read.raw addr ofs|partition size
+      Read `size' bytes from `ofs' in NAND flash to `addr'. This reads the raw
+      block, so ECC is avoided and the OOB area is read as well.
+
+   nand write.raw addr ofs|partition size
+      Write `size' bytes from `addr' to `ofs' in NAND flash. This writes the raw
+      block, so ECC is avoided and the OOB area is written as well, making the
+      whole block written as-is.
+
 Configuration Options:
 
    CONFIG_CMD_NAND
Signature · 1 line
1.7.5.4
#6 lore raw

PATCH v2 2/5 NAND: Add nand read.raw and write.raw commands

Quoted 40 lines · Marek Vasut

These commands should work around various "hardware" ECC and BCH methods.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   19 +++++++++++++++++--
 doc/README.nand   |    9 +++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

V2: Add documentation.

common/cmd_nand.c +14 -0
index 66e06a5..72d418c 100644
@@ -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;

common/cmd_nand.c U_BOOT_CMD( · line 716

Quoted 10 lines
common/cmd_nand.c +3 -2
@@ -695,10 +709,11 @@ U_BOOT_CMD(
 	"NAND sub-system",
 	"info - show available NAND devices\n"
 	"nand device [dev] - show or set current device\n"
-	"nand read - addr off|partition size\n"
-	"nand write - addr off|partition size\n"
+	"nand read[.raw] - addr off|partition size\n"
+	"nand write[.raw] - addr off|partition size\n"
 	"    read/write 'size' bytes starting at offset 'off'\n"
 	"    to/from memory address 'addr', skipping bad blocks.\n"
+	"    Use read.raw/write.raw to avoid ECC and write the block as-is.\n"

This says that .raw takes a size parameter, but you assume one page instead.

Quoted 6 lines
doc/README.nand +0 -0
index 751b693..084223a 100644
@@ -94,6 +94,15 @@ Commands:
       of data for one 512-byte page or 2 256-byte pages. There is no check

doc/README.nand Commands: · line 104

Quoted 10 lines
doc/README.nand +8 -0
       for bad blocks.
 
+   nand read.raw addr ofs|partition size
+      Read `size' bytes from `ofs' in NAND flash to `addr'. This reads the raw
+      block, so ECC is avoided and the OOB area is read as well.
+
+   nand write.raw addr ofs|partition size
+      Write `size' bytes from `addr' to `ofs' in NAND flash. This writes the raw
+      block, so ECC is avoided and the OOB area is written as well, making the
+      whole block written as-is.

The current implementation reads/writes a page, not a block, and not 'size' bytes.

Should mention that the OOB is expected to immediately follow the main area in memory.

-Scott

#7 lore raw

PATCH v3 2/5 NAND: Add nand read.raw and write.raw commands

In reply to Marek Vasut

These commands should work around various "hardware" ECC and BCH methods.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   21 +++++++++++++++++++--
 doc/README.nand   |    9 +++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

V2: Add documentation.
V3: Drop size param.

common/cmd_nand.c +19 -2
index 66e06a5..92b6280 100644
@@ -606,6 +606,22 @@ 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
+			};
+
+			rwsize = nand->writesize + nand->oobsize;
+
+			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;
@@ -695,10 +711,11 @@ U_BOOT_CMD(
 	"NAND sub-system",
 	"info - show available NAND devices\n"
 	"nand device [dev] - show or set current device\n"
-	"nand read - addr off|partition size\n"
-	"nand write - addr off|partition size\n"
+	"nand read[.raw] - addr off|partition\n"
+	"nand write[.raw] - addr off|partition\n"
 	"    read/write 'size' bytes starting at offset 'off'\n"
 	"    to/from memory address 'addr', skipping bad blocks.\n"
+	"    Use read.raw/write.raw to avoid ECC and write the block as-is.\n"
 #ifdef CONFIG_CMD_NAND_TRIMFFS
 	"nand write.trimffs - addr off|partition size\n"
 	"    write 'size' bytes starting at offset 'off' from memory address\n"
doc/README.nand +9 -0
index 751b693..62c077e 100644
@@ -94,6 +94,15 @@ Commands:
       of data for one 512-byte page or 2 256-byte pages. There is no check
       for bad blocks.
 
+   nand read.raw addr ofs|partition
+      Read block from `ofs' in NAND flash to `addr'. This reads the raw block,
+      so ECC is avoided and the OOB area is read as well.
+
+   nand write.raw addr ofs|partition
+      Write block from `addr' to `ofs' in NAND flash. This writes the raw block,
+      so ECC is avoided and the OOB area is written as well, making the whole
+      block written as-is.
+
 Configuration Options:
 
    CONFIG_CMD_NAND
Signature · 1 line
1.7.5.4
#8 lore raw

PATCH v3 2/5 NAND: Add nand read.raw and write.raw commands

common/cmd_nand.c U_BOOT_CMD( · line 718

Quoted 11 lines · Marek Vasut
common/cmd_nand.c +3 -2
@@ -695,10 +711,11 @@ U_BOOT_CMD(
 	"NAND sub-system",
 	"info - show available NAND devices\n"
 	"nand device [dev] - show or set current device\n"
-	"nand read - addr off|partition size\n"
-	"nand write - addr off|partition size\n"
+	"nand read[.raw] - addr off|partition\n"
+	"nand write[.raw] - addr off|partition\n"
 	"    read/write 'size' bytes starting at offset 'off'\n"
 	"    to/from memory address 'addr', skipping bad blocks.\n"
+	"    Use read.raw/write.raw to avoid ECC and write the block as-is.\n"

Only the .raw version lacks size -- please do not remove it from the help text for the normal read/write.

Quoted 6 lines
doc/README.nand +0 -0
index 751b693..62c077e 100644
@@ -94,6 +94,15 @@ Commands:
       of data for one 512-byte page or 2 256-byte pages. There is no check

doc/README.nand Commands: · line 104

Quoted 10 lines
doc/README.nand +8 -0
       for bad blocks.
 
+   nand read.raw addr ofs|partition
+      Read block from `ofs' in NAND flash to `addr'. This reads the raw block,
+      so ECC is avoided and the OOB area is read as well.
+
+   nand write.raw addr ofs|partition
+      Write block from `addr' to `ofs' in NAND flash. This writes the raw block,
+      so ECC is avoided and the OOB area is written as well, making the whole
+      block written as-is.

Again, it's operating on a page, not a block.

-Scott

#9 lore raw

PATCH v4 2/5 NAND: Add nand read.raw and write.raw commands

In reply to Marek Vasut

These commands should work around various "hardware" ECC and BCH methods.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   19 +++++++++++++++++++
 doc/README.nand   |    9 +++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

V2: Add documentation.
V3: Drop size param.
V4: Dont drop size of read/write commands, operate on pages

common/cmd_nand.c +19 -0
index 66e06a5..14c7c09 100644
@@ -606,6 +606,22 @@ 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
+			};
+
+			rwsize = nand->writesize + nand->oobsize;
+
+			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;
@@ -699,6 +715,9 @@ U_BOOT_CMD(
 	"nand write - addr off|partition size\n"
 	"    read/write 'size' bytes starting at offset 'off'\n"
 	"    to/from memory address 'addr', skipping bad blocks.\n"
+	"nand read.raw - addr off|partition\n"
+	"nand write.raw - addr off|partition\n"
+	"    Use read.raw/write.raw to avoid ECC and write the page as-is.\n"
 #ifdef CONFIG_CMD_NAND_TRIMFFS
 	"nand write.trimffs - addr off|partition size\n"
 	"    write 'size' bytes starting at offset 'off' from memory address\n"
doc/README.nand +9 -0
index 751b693..023740e 100644
@@ -94,6 +94,15 @@ Commands:
       of data for one 512-byte page or 2 256-byte pages. There is no check
       for bad blocks.
 
+   nand read.raw addr ofs|partition
+      Read page from `ofs' in NAND flash to `addr'. This reads the raw page,
+      so ECC is avoided and the OOB area is read as well.
+
+   nand write.raw addr ofs|partition
+      Write page from `addr' to `ofs' in NAND flash. This writes the raw page,
+      so ECC is avoided and the OOB area is written as well, making the whole
+      page written as-is.
+
 Configuration Options:
 
    CONFIG_CMD_NAND
Signature · 1 line
1.7.5.4
#10 lore raw

PATCH v4 2/5 NAND: Add nand read.raw and write.raw commands

Quoted 7 lines · Marek Vasut

These commands should work around various "hardware" ECC and BCH methods.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
Quoted 8 lines
---
 common/cmd_nand.c |   19 +++++++++++++++++++
 doc/README.nand   |    9 +++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

V2: Add documentation.
V3: Drop size param.
V4: Dont drop size of read/write commands, operate on pages

Applied to u-boot-nand-flash next

common/cmd_nand.c U_BOOT_CMD( · line 720

Quoted 3 lines
common/cmd_nand.c +3 -0
+	"nand read.raw - addr off|partition\n"
+	"nand write.raw - addr off|partition\n"
+	"    Use read.raw/write.raw to avoid ECC and write the page as-is.\n"

...with s/write the page/access the page/

-Scott

#11 lore raw

PATCH v2 3/5 NAND: Allow per-buffer allocation

In reply to Marek Vasut

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.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 drivers/mtd/nand/nand_base.c |   30 +++++++++++++++++++++++-------
 include/linux/mtd/nand.h     |    7 ++++---
 2 files changed, 27 insertions(+), 10 deletions(-)
drivers/mtd/nand/nand_base.c +23 -7
index d8d30e3..3093067 100644
@@ -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);
+	}
 }
include/linux/mtd/nand.h +4 -3
index 987a2ec..c3449a9 100644
@@ -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;
 };
 
 /**
Signature · 1 line
1.7.5.4
#12 lore raw

PATCH 3/5 NAND: Allow per-buffer allocation

Quoted 9 lines · Marek Vasut

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.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
Quoted 4 lines
---
 drivers/mtd/nand/nand_base.c |   30 +++++++++++++++++++++++-------
 include/linux/mtd/nand.h     |    7 ++++---
 2 files changed, 27 insertions(+), 10 deletions(-)

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

-Scott

#13 lore raw

PATCH 3/5 NAND: Allow per-buffer allocation

Dear Scott Wood,

Quoted 3 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Mr. Cole's Axiom:
        The sum of the intelligence on the planet is a constant;
        the population is growing.
#14 lore raw

PATCH 3/5 NAND: Allow per-buffer allocation

Quoted 7 lines · Wolfgang Denk

Dear Scott Wood,

Quoted 3 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Quoted 7 lines

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

Best regards,

Wolfgang Denk

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

-Scott

#15 lore raw

PATCH 3/5 NAND: Allow per-buffer allocation

Dear Stefano & Marek,

can you please provide the requested information?

Quoted 6 lines · In message <[email protected]> Scott Wood wrote:
Quoted 6 lines
Quoted 3 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Quoted 7 lines
Quoted 2 lines

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

Thanks.

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Without facts, the decision cannot be made logically. You  must  rely
on your human intuition.
	-- Spock, "Assignment: Earth", stardate unknown
#16 lore raw

PATCH v2 3/5 NAND: Allow per-buffer allocation

Quoted 12 lines · Wolfgang Denk

Dear Stefano & Marek,

can you please provide the requested information?

Quoted 7 lines · In message <[email protected]> Scott Wood wrote:
Quoted 6 lines
Quoted 2 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

Quoted 8 lines
Quoted 4 lines

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

Thanks.

Wolfgang Denk

This patch is actually optional and right now isn't used by M28. Though someone might find this helpful.

Cheers

#17 lore raw

PATCH 3/5 NAND: Allow per-buffer allocation

In reply to Wolfgang Denk
Quoted 5 lines · Wolfgang Denk

Dear Stefano & Marek,

can you please provide the requested information?

Hi Scott,

Quoted 7 lines
Quoted 6 lines · In message <[email protected]> Scott Wood wrote:
Quoted 6 lines
Quoted 3 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

I tested NAND with the gpmi-nand patches sent to linux-arm by Huang Shije:

	http://www.spinics.net/lists/arm-kernel/msg139526.html

However, I have not seen the option NAND_OWN_BUFFERS in his patches.

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#18 lore raw

PATCH v2 3/5 NAND: Allow per-buffer allocation

Quoted 20 lines · Stefano Babic
Quoted 3 lines · Wolfgang Denk

Dear Stefano & Marek,

can you please provide the requested information?

Hi Scott,

Quoted 5 lines
Quoted 4 lines · In message <[email protected]> Scott Wood wrote:
Quoted 4 lines
Quoted 2 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

Quoted 8 lines

I tested NAND with the gpmi-nand patches sent to linux-arm by Huang Shije:

	http://www.spinics.net/lists/arm-kernel/msg139526.html

However, I have not seen the option NAND_OWN_BUFFERS in his patches.

Best regards,
Stefano

Like I said, this patch is not needed anymore. It's just a convenience measure now. I don't need to for mx28.

#19 lore raw

PATCH 3/5 NAND: Allow per-buffer allocation

Quoted 25 lines · Marek Vasut
Quoted 24 lines · Stefano Babic
Quoted 3 lines · Wolfgang Denk

Dear Stefano & Marek,

can you please provide the requested information?

Hi Scott,

Quoted 5 lines
Quoted 4 lines · In message <[email protected]> Scott Wood wrote:
Quoted 4 lines
Quoted 2 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

I tested NAND with the gpmi-nand patches sent to linux-arm by Huang Shije:

	http://www.spinics.net/lists/arm-kernel/msg139526.html
Quoted 8 lines
Quoted 4 lines

However, I have not seen the option NAND_OWN_BUFFERS in his patches.

Best regards,
Stefano

Like I said, this patch is not needed anymore. It's just a convenience measure now. I don't need to for mx28.

Let's hold off on this patch until it's actually needed, then.

-Scott

#20 lore raw

PATCH v2 3/5 NAND: Allow per-buffer allocation

Quoted 27 lines · Scott Wood
Quoted 26 lines · Marek Vasut
Quoted 25 lines · Stefano Babic
Quoted 3 lines · Wolfgang Denk

Dear Stefano & Marek,

can you please provide the requested information?

Hi Scott,

Quoted 5 lines
Quoted 4 lines · In message <[email protected]> Scott Wood wrote:
Quoted 4 lines
Quoted 2 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

I tested NAND with the gpmi-nand patches sent to linux-arm by Huang Shije:

	http://www.spinics.net/lists/arm-kernel/msg139526.html

However, I have not seen the option NAND_OWN_BUFFERS in his patches.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Best regards,
Stefano

Like I said, this patch is not needed anymore. It's just a convenience measure now. I don't need to for mx28.

Let's hold off on this patch until it's actually needed, then.

Very well then, mind merging the rest then ?

Cheers

Quoted 2 lines

-Scott

#21 lore raw

PATCH 3/5 NAND: Allow per-buffer allocation

Quoted 31 lines · Marek Vasut
Quoted 30 lines · Scott Wood
Quoted 29 lines · Marek Vasut
Quoted 27 lines · Stefano Babic
Quoted 3 lines · Wolfgang Denk

Dear Stefano & Marek,

can you please provide the requested information?

Hi Scott,

Quoted 5 lines
Quoted 4 lines · In message <[email protected]> Scott Wood wrote:
Quoted 4 lines
Quoted 2 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

I tested NAND with the gpmi-nand patches sent to linux-arm by Huang Shije:

	http://www.spinics.net/lists/arm-kernel/msg139526.html

However, I have not seen the option NAND_OWN_BUFFERS in his patches.

Best regards,
Stefano

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Like I said, this patch is not needed anymore. It's just a convenience measure now. I don't need to for mx28.

Let's hold off on this patch until it's actually needed, then.

Very well then, mind merging the rest then ?

Yes, I'll try to get to it soon.

-Scott

#22 lore raw

PATCH v2 3/5 NAND: Allow per-buffer allocation

Quoted 20 lines · Scott Wood
Quoted 19 lines · Marek Vasut
Quoted 18 lines · Scott Wood
Quoted 17 lines · Marek Vasut
Quoted 16 lines · Stefano Babic
Quoted 3 lines · Wolfgang Denk

Dear Stefano & Marek,

can you please provide the requested information?

Hi Scott,

Quoted 9 lines
Quoted 8 lines · In message <[email protected]> Scott Wood wrote:
Quoted 7 lines
Quoted 2 lines · In message <[email protected]> you wrote:

Is this hardware going to be supported in Linux? It would be nice if we could keep this code in sync.

Stefano has submitted patches for the iMX28 based M28 / M28EVK board, so yes, this hardware going to be supported in mainline Linux, too.

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 4 lines

How do the Linux iMX28 patches deal with NAND_OWN_BUFFERS?

I'd like to see this change be submitted to Linux first, or else have an explanation of why a divergence for U-Boot is warranted.

I tested NAND with the gpmi-nand patches sent to linux-arm by Huang

Shije:

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
	http://www.spinics.net/lists/arm-kernel/msg139526.html

However, I have not seen the option NAND_OWN_BUFFERS in his patches.

Best regards,
Stefano

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Like I said, this patch is not needed anymore. It's just a convenience measure now. I don't need to for mx28.

Let's hold off on this patch until it's actually needed, then.

Very well then, mind merging the rest then ?

Yes, I'll try to get to it soon.

Thanks Scott, and thanks for your patience while reviewing, I know I had my moments where I wasn't too nice / was whiny ;-)

Cheers

Quoted 2 lines

-Scott

#23 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

In reply to Marek Vasut

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.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   42 +++++++++++++++++++++++++++++++++++++++---
 1 files changed, 39 insertions(+), 3 deletions(-)
common/cmd_nand.c +39 -3
index a1c8dfd..5b7e83d 100644
@@ -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;
 		}
 
Signature · 1 line
1.7.5.4
#24 lore raw

PATCH 4/5 NAND: Make page, erase, oob size available via cmd_nand

Quoted 8 lines · Marek Vasut

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.

"writesize" is what the internals use for this, but nand_pagesize would be more obvious.

Quoted 14 lines
Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   42 +++++++++++++++++++++++++++++++++++++++---
 1 files changed, 39 insertions(+), 3 deletions(-)
common/cmd_nand.c +0 -0
index a1c8dfd..5b7e83d 100644

common/cmd_nand.c line 32

Quoted 6 lines
common/cmd_nand.c +3 -0
@@ -27,6 +27,9 @@
 #include <asm/byteorder.h>
 #include <jffs2/jffs2.h>
 #include <nand.h>
+#ifdef	CONFIG_SYS_HUSH_PARSER
+#include <hush.h>
+#endif

I don't think you need this ifdef.

Quoted 16 lines
common/cmd_nand.c +16 -0
+	/* 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);

common/cmd_nand.c usage: · line 409

Quoted 10 lines
common/cmd_nand.c +10 -0
+	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

Is there no existing abstraction to add a variable to both the shell (if present) and the environment, without needing such an ifdef in the caller? If not, should one be added?

-Scott

#25 lore raw

PATCH 4/5 NAND: Make page, erase, oob size available via cmd_nand

Dear Scott Wood,

Quoted 6 lines · In message <[email protected]> you wrote:

common/cmd_nand.c line 32

Quoted 4 lines
common/cmd_nand.c +3 -0
+#ifdef	CONFIG_SYS_HUSH_PARSER
+#include <hush.h>
+#endif

I don't think you need this ifdef.

It should be omitted alltogether.

Quoted 4 lines
Quoted 4 lines
common/cmd_nand.c +4 -0
+#ifdef	CONFIG_SYS_HUSH_PARSER
+	memset(buf, 0, bufsz);
+	sprintf(buf, "nand_writesize=%x", nand->writesize);
+	set_local_var(buf, 0);

...

Quoted 4 lines

Is there no existing abstraction to add a variable to both the shell (if present) and the environment, without needing such an ifdef in the caller? If not, should one be added?

While it's a good idea to keep such dynamic variables out of the (persistently stored) environment, I don't want to see such a solitary approach. If we start thinking about this it should be implemented differently, and for all dynamic variables (including things like filesize etc.).

Marek, please get rid of the hush special case and always use plain env variables like we do elsewhere, too.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"He was so narrow minded he could see through  a  keyhole  with  both
eyes ..."
#26 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

In reply to Marek Vasut

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

Also, the "nand info" command now displays this info.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

V2: Remove setup of HUSH local vars.

common/cmd_nand.c +22 -3
index 72d418c..2f8723f 100644
@@ -362,15 +362,34 @@ 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 */
+	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);
 }
 
 int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
@@ -407,7 +426,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 +437,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;
 		}
 
Signature · 1 line
1.7.5.4
#27 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

Quoted 27 lines · Marek Vasut
common/cmd_nand.c +10 -1
index 72d418c..2f8723f 100644
@@ -362,15 +362,34 @@ 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 */
+	memset(buf, 0, bufsz);

common/cmd_nand.c usage: · line 392

Quoted 10 lines
common/cmd_nand.c +10 -0
+	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);

Why the memsets?

-Scott

#28 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

Quoted 40 lines · Scott Wood
Quoted 39 lines · Marek Vasut
common/cmd_nand.c +14 -1
index 72d418c..2f8723f 100644
@@ -362,15 +362,34 @@ 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 */
+	memset(buf, 0, bufsz);
+	sprintf(buf, "%x", nand->writesize);
+	setenv("nand_writesize", buf);
+
+	memset(buf, 0, bufsz);
Quoted 8 lines

common/cmd_nand.c usage: · line 392

Quoted 6 lines
common/cmd_nand.c +6 -0
+	sprintf(buf, "%x", nand->oobsize);
+	setenv("nand_oobsize", buf);
+
+	memset(buf, 0, bufsz);
+	sprintf(buf, "%x", nand->erasesize);
+	setenv("nand_erasesize", buf);

Why the memsets?

To clear the memory from previous usage ?

Quoted 2 lines

-Scott

#29 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

Quoted 17 lines · Marek Vasut
Quoted 15 lines · Scott Wood

common/cmd_nand.c usage: · line 392

Quoted 12 lines · Marek Vasut
common/cmd_nand.c +12 -0
+	/* Set geometry info */
+	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);

Why the memsets?

Quoted 1 line

To clear the memory from previous usage ?

What part of the previous usage will both survive the sprintf() and be looked at by setenv()?

-Scott

#30 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

Quoted 18 lines · Scott Wood
Quoted 17 lines · Marek Vasut
Quoted 15 lines · Scott Wood

common/cmd_nand.c usage: · line 392

Quoted 12 lines · Marek Vasut
common/cmd_nand.c +12 -0
+	/* Set geometry info */
+	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);

Why the memsets?

Quoted 4 lines
Quoted 1 line

To clear the memory from previous usage ?

What part of the previous usage will both survive the sprintf() and be looked at by setenv()?

The part of data that are copied in _do_set_env() ?

Quoted 2 lines

-Scott

#31 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

Quoted 19 lines · Marek Vasut
Quoted 18 lines · Scott Wood
Quoted 17 lines · Marek Vasut
Quoted 15 lines · Scott Wood

common/cmd_nand.c usage: · line 392

Quoted 12 lines · Marek Vasut
common/cmd_nand.c +12 -0
+	/* Set geometry info */
+	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);

Why the memsets?

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

To clear the memory from previous usage ?

What part of the previous usage will both survive the sprintf() and be looked at by setenv()?

The part of data that are copied in _do_set_env() ?

I don't see _do_set_env anywhere -- what tree are you looking at?

In any case, sprintf() produces a zero-terminated string. setenv() consumes a zero-terminated string. setenv() doesn't even know that the buffer containing the string happens to be 32 bytes, much less have any business poking around in that area.

-Scott

#32 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

Quoted 25 lines · Scott Wood
Quoted 24 lines · Marek Vasut
Quoted 22 lines · Scott Wood
Quoted 17 lines · Marek Vasut
Quoted 15 lines · Scott Wood

common/cmd_nand.c usage: · line 392

Quoted 12 lines · Marek Vasut
common/cmd_nand.c +12 -0
+	/* Set geometry info */
+	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);

Why the memsets?

Quoted 1 line

To clear the memory from previous usage ?

What part of the previous usage will both survive the sprintf() and be looked at by setenv()?

Quoted 6 lines
Quoted 1 line

The part of data that are copied in _do_set_env() ?

I don't see _do_set_env anywhere -- what tree are you looking at?

In any case, sprintf() produces a zero-terminated string. setenv() consumes a zero-terminated string.

Correct

Quoted 3 lines

setenv() doesn't even know that the
buffer containing the string happens to be 32 bytes, much less have any business poking around in that area.

True ... but the stuff you call setenv() on is copied to environment. That's about it, it doesn't get lost anywhere.

#33 lore raw

PATCH v2 4/5 NAND: Make page, erase, oob size available via cmd_nand

In reply to Marek Vasut
Quoted 7 lines · Marek Vasut

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

Also, the "nand info" command now displays this info.

Quoted 9 lines
Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

Applied to u-boot-nand-flash next

common/cmd_nand.c usage: · line 392

Quoted 12 lines
common/cmd_nand.c +12 -0
+	/* Set geometry info */
+	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);

...with memsets removed.

-Scott

#34 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

In reply to Marek Vasut

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
common/cmd_nand.c +7 -1
index 5b7e83d..6d66e5d 100644
@@ -502,6 +502,7 @@ 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;
 		int part = 0;
 		int chip = 0;
 		int spread = 0;
@@ -516,6 +517,8 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 			} else if (!strcmp(&cmd[5], ".chip")) {
 				chip = 1;
 				args = 0;
+			} else if (!strncmp(cmd, "scrub.quiet", 11)) {
+				scrub_quiet = 1;
 			} else {
 				goto usage;
 			}
@@ -543,7 +546,9 @@ 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) {
+			opts.scrub = 1;
+		} else if (scrub) {
 			puts("Warning: "
 			     "scrub option will erase all factory set "
 			     "bad blocks!\n"
@@ -569,6 +574,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 				return -1;
 			}
 		}
+
 		ret = nand_erase_opts(nand, &opts);
 		printf("%s\n", ret ? "ERROR" : "OK");
 
Signature · 1 line
1.7.5.4
#35 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

common/cmd_nand.c int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char · line 579

Quoted 7 lines
common/cmd_nand.c +1 -0
@@ -569,6 +574,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char
 * const argv[]) return -1;
 			}
 		}
+
 		ret = nand_erase_opts(nand, &opts);
 		printf("%s\n", ret ? "ERROR" : "OK");

unrelated whitespace changes are frowned upon
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110912/d6d9c825/attachment.pgp

#36 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 6 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

I'd prefer to have this hidden from common users as much as possible.

Quoted 8 lines
Quoted 8 lines
(quoted diff) +0 -0
@@ -569,6 +574,7 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc,
 char * const argv[]) return -1;
 
 			}
 
 		}
 
Quoted 7 lines

common/cmd_nand.c int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 579

Quoted 4 lines
common/cmd_nand.c +1 -0
+
common/cmd_nand.c +0 -0
 		ret = nand_erase_opts(nand, &opts);
 		printf("%s\n", ret ? "ERROR" : "OK");

unrelated whitespace changes are frowned upon
-mike

Thanks for finding this.

Cheers

#37 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

Quoted 9 lines · Marek Vasut
Quoted 6 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

I'd prefer to have this hidden from common users as much as possible.

What's the use case for needing to script this, BTW?

-Scott

#38 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 6 lines · Scott Wood
Quoted 5 lines · Marek Vasut
Quoted 4 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

I'd prefer to have this hidden from common users as much as possible.

What's the use case for needing to script this, BTW?

Update a block in NAND that's not handled by BCH accelerator in the MX28 chip.

The problem is, block 0 has it's own ECC done by bootrom software. That kind of ECC is incompatible with BCH-produced ECC. That's also a reason for needing that write.raw command.

Now, if you try erasing that block, the BCH reads and writes some of it's metadata there. Obviously, since there is different kind of ECC, the metadata aren't there and it chokes, claiming the block is bad and refuses to erase it.

And before you ask why -- that's because the BCH accelerator puts the metadata at random places in the block (every 512 bytes, it puts a few bytes of it's ECC) instead of putting them only to the ECC area. On the other hand, the bootrom ECC puts the whole ECC at offset (1024 + 12) bytes from the start of the block 0.

And finally, we obviously don't want to bother the user by asking "do you really want to scrub your nand" in the update process.

Cheers

Quoted 2 lines

-Scott

#39 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

Quoted 20 lines · Marek Vasut
Quoted 6 lines · Scott Wood
Quoted 5 lines · Marek Vasut
Quoted 4 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

I'd prefer to have this hidden from common users as much as possible.

What's the use case for needing to script this, BTW?

Update a block in NAND that's not handled by BCH accelerator in the MX28 chip.

The problem is, block 0 has it's own ECC done by bootrom software. That kind of ECC is incompatible with BCH-produced ECC. That's also a reason for needing that write.raw command.

Quoted 8 lines

Now, if you try erasing that block, the BCH reads and writes some of it's metadata there. Obviously, since there is different kind of ECC, the metadata aren't there and it chokes, claiming the block is bad and refuses to erase it.

And before you ask why -- that's because the BCH accelerator puts the metadata at random places in the block (every 512 bytes, it puts a few bytes of it's ECC) instead of putting them only to the ECC area. On the other hand, the bootrom ECC puts the whole ECC at offset (1024 + 12) bytes from the start of the block 0.

Would it make sense to have the driver code treat block 0 specially (possibly conditioned on an hwconfig or compile-time config), rather than have it be user-driven?

I'm curious why anything is written on an erase, though, regardless of data format.

-Scott

#40 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 33 lines · Scott Wood
Quoted 26 lines · Marek Vasut
Quoted 6 lines · Scott Wood
Quoted 5 lines · Marek Vasut
Quoted 4 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

I'd prefer to have this hidden from common users as much as possible.

What's the use case for needing to script this, BTW?

Update a block in NAND that's not handled by BCH accelerator in the MX28 chip.

The problem is, block 0 has it's own ECC done by bootrom software. That kind of ECC is incompatible with BCH-produced ECC. That's also a reason for needing that write.raw command.

Now, if you try erasing that block, the BCH reads and writes some of it's metadata there. Obviously, since there is different kind of ECC, the metadata aren't there and it chokes, claiming the block is bad and refuses to erase it.

Quoted 5 lines

And before you ask why -- that's because the BCH accelerator puts the metadata at random places in the block (every 512 bytes, it puts a few bytes of it's ECC) instead of putting them only to the ECC area. On the other hand, the bootrom ECC puts the whole ECC at offset (1024 + 12) bytes from the start of the block 0.

Quoted 3 lines

Would it make sense to have the driver code treat block 0 specially (possibly conditioned on an hwconfig or compile-time config), rather than have it be user-driven?

No! What if (very possible situation actually) the user wants to use the whole NAND because the user is booting from SD/SPI/... ?

Quoted 3 lines

I'm curious why anything is written on an erase, though, regardless of data format.

Badblock markers (some FSL invention) are written always.

Quoted 2 lines

-Scott

#41 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

Quoted 7 lines · Marek Vasut
Quoted 3 lines · Scott Wood

Would it make sense to have the driver code treat block 0 specially (possibly conditioned on an hwconfig or compile-time config), rather than have it be user-driven?

No! What if (very possible situation actually) the user wants to use the whole NAND because the user is booting from SD/SPI/... ?

Well, that's why I suggested it be configurable.

Quoted 4 lines
Quoted 2 lines

I'm curious why anything is written on an erase, though, regardless of data format.

Badblock markers (some FSL invention) are written always.

What's it doing with them? Migrating them is something that should only happen on the first use, as there will later be data in the factory bad block area, right? So it shouldn't be "always". Migration should be an explicitly requested option. Like scrub. :-)

Where is the code that does this? Which driver?

-Scott

#42 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 14 lines · Scott Wood
Quoted 7 lines · Marek Vasut
Quoted 3 lines · Scott Wood

Would it make sense to have the driver code treat block 0 specially (possibly conditioned on an hwconfig or compile-time config), rather than have it be user-driven?

No! What if (very possible situation actually) the user wants to use the whole NAND because the user is booting from SD/SPI/... ?

Well, that's why I suggested it be configurable.

Quoted 3 lines
Quoted 2 lines

I'm curious why anything is written on an erase, though, regardless of data format.

Quoted 8 lines
Quoted 1 line

Badblock markers (some FSL invention) are written always.

What's it doing with them? Migrating them is something that should only happen on the first use, as there will later be data in the factory bad block area, right? So it shouldn't be "always". Migration should be an explicitly requested option. Like scrub. :-)

Where is the code that does this? Which driver?

The BCH accelerator does this.

Quoted 2 lines

-Scott

#43 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

Quoted 9 lines · Marek Vasut
Quoted 6 lines · Scott Wood

What's it doing with them? Migrating them is something that should only happen on the first use, as there will later be data in the factory bad block area, right? So it shouldn't be "always". Migration should be an explicitly requested option. Like scrub. :-)

Where is the code that does this? Which driver?

The BCH accelerator does this.

In hardware? What chip is this, and where is the code that drives this chip?

-Scott

#44 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 7 lines · Scott Wood
Quoted 6 lines · Marek Vasut
Quoted 5 lines · Scott Wood

What's it doing with them? Migrating them is something that should only happen on the first use, as there will later be data in the factory bad block area, right? So it shouldn't be "always". Migration should be an explicitly requested option. Like scrub. :-)

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Where is the code that does this? Which driver?

The BCH accelerator does this.

In hardware? What chip is this, and where is the code that drives this chip?

I think it does something to it, yes. i.MX287, see the patchset [PATCH 00/15 V2] Support for the DENX M28 SoM, [PATCH 09/15] iMX28: Add GPMI NAND driver .

Quoted 2 lines

-Scott

#45 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

Quoted 10 lines · Marek Vasut
Quoted 9 lines · Scott Wood
Quoted 8 lines · Marek Vasut
Quoted 6 lines · Scott Wood

What's it doing with them? Migrating them is something that should only happen on the first use, as there will later be data in the factory bad block area, right? So it shouldn't be "always". Migration should be an explicitly requested option. Like scrub. :-)

Where is the code that does this? Which driver?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

The BCH accelerator does this.

In hardware? What chip is this, and where is the code that drives this chip?

I think it does something to it, yes. i.MX287, see the patchset [PATCH 00/15 V2] Support for the DENX M28 SoM, [PATCH 09/15] iMX28: Add GPMI NAND driver .

I looked at the code and the datasheet, and without getting into it too deeply, I don't see how BCH is involved in an erase operation. What specifically are you seeing happen?

The "Raw NAND Boot Mode" section (12.12 in my copy of the i.MX28 manual) says that it uses BCH for ECC -- is this not the case? Is it some special configuration of BCH?

I tried reading the "Bad Block Handling in the ROM" section and got a headache.

I work for the PowerPC side of Freescale, in case you're wondering why I'm unfamiliar with this. :-)

-Scott

#46 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 24 lines · Scott Wood
Quoted 10 lines · Marek Vasut
Quoted 9 lines · Scott Wood
Quoted 8 lines · Marek Vasut
Quoted 6 lines · Scott Wood

What's it doing with them? Migrating them is something that should only happen on the first use, as there will later be data in the factory bad block area, right? So it shouldn't be "always". Migration should be an explicitly requested option. Like scrub. :-)

Where is the code that does this? Which driver?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

The BCH accelerator does this.

In hardware? What chip is this, and where is the code that drives this chip?

I think it does something to it, yes. i.MX287, see the patchset [PATCH 00/15 V2] Support for the DENX M28 SoM, [PATCH 09/15] iMX28: Add GPMI NAND driver .

I looked at the code and the datasheet, and without getting into it too deeply, I don't see how BCH is involved in an erase operation. What specifically are you seeing happen?

Quoted 6 lines

The "Raw NAND Boot Mode" section (12.12 in my copy of the i.MX28 manual) says that it uses BCH for ECC -- is this not the case? Is it some special configuration of BCH?

I tried reading the "Bad Block Handling in the ROM" section and got a headache.

12.12.1.8 is exactly it. See fig. 12-11. Also section 12.12.1.9 is interesting (at least for the block 0 problem).

For the other problem, you should read chapter 15 and 16. Though they are not completely beefed with information on that matter either.

I got most of these information from the source code FSL provided in their MX28 BSP. The usual flow is the driver uses DMA to send the commands/data to the GPMI NAND driver. The GPMI NAND controller diverts the data to the BCH, which does the bit obscurisation. Then the data are taken from the BCH and written to NAND.

And the other way in the read process ... GPMI NAND controller reads data back from NAND, pushes them through the BCH and then presents them to the user.

The problem with erase here is that the block's ECC is updated on erase. But we want to write a block in our own format, so we need to scrub (wipe the block completely).

Quoted 3 lines

I work for the PowerPC side of Freescale, in case you're wondering why I'm unfamiliar with this. :-)

Interesting ... does everyone work for the PowerPC side of Freescale or is there some other reason why I never met anyone working for the ARM side of Freescale ? ;-)

Quoted 2 lines

-Scott

Cheers

#47 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

Dear Marek Vasut,

Quoted 4 lines · In message <[email protected]> you wrote:

Interesting ... does everyone work for the PowerPC side of Freescale or is there some other reason why I never met anyone working for the ARM side of Freescale ? ;-)

Keep in mind that the i.MX28 is actually not a Freescale design...

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The IQ of the group is the lowest IQ of a member of the group divided
by the number of people in the group.
#48 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 8 lines · Wolfgang Denk

Dear Marek Vasut,

Quoted 3 lines · In message <[email protected]> you wrote:

Interesting ... does everyone work for the PowerPC side of Freescale or is there some other reason why I never met anyone working for the ARM side of Freescale ? ;-)

Keep in mind that the i.MX28 is actually not a Freescale design...

I was just curious here.

Cheers

Quoted 4 lines

Best regards,

Wolfgang Denk

#49 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

In reply to Marek Vasut
Quoted 20 lines · Marek Vasut
Quoted 19 lines · Scott Wood
Quoted 18 lines · Marek Vasut
Quoted 7 lines · Scott Wood
Quoted 6 lines · Marek Vasut
Quoted 5 lines · Scott Wood

What's it doing with them? Migrating them is something that should only happen on the first use, as there will later be data in the factory bad block area, right? So it shouldn't be "always". Migration should be an explicitly requested option. Like scrub. :-)

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Where is the code that does this? Which driver?

The BCH accelerator does this.

In hardware? What chip is this, and where is the code that drives this chip?

I think it does something to it, yes. i.MX287, see the patchset [PATCH 00/15 V2] Support for the DENX M28 SoM, [PATCH 09/15] iMX28: Add GPMI NAND driver .

[snip]

Quoted 1 line

The problem with erase here is that the block's ECC is updated on erase.

I don't understand this -- wouldn't that stop you from being able to later write a different ECC (when you write the actual data)? Is the controller snooping the command bytes you issue for erase, and then generating a program sequence? How would the hardware even know that you told U-Boot to scrub?

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

I looked at the code and the datasheet, and without getting into it too deeply, I don't see how BCH is involved in an erase operation. What specifically are you seeing happen?

Quoted 8 lines
Quoted 8 lines
Quoted 6 lines

The "Raw NAND Boot Mode" section (12.12 in my copy of the i.MX28 manual) says that it uses BCH for ECC -- is this not the case? Is it some special configuration of BCH?

I tried reading the "Bad Block Handling in the ROM" section and got a headache.

12.12.1.8 is exactly it. See fig. 12-11.

I think we have different versions, that's "Layout of Boot Page containing FCB" in my copy, which shows a fixed spare/data/ECC layout that may not match what is programmed for normal BCH operations, but it doesn't say anything about bad block markers.

The bad block marker stuff is in 12.12.1.12 (figure 12-13) in my copy. It says:

Quoted 3 lines

In order to preserve the BI (bad block information), flash updater or gang programmer applications need to swap Bad Block Information (BI) data to byte 0 of metadata area for every page before programming NAND Flash.

Before first programming the flash, you'll need to make sure that all blocks that are marked bad in the normal way get marked bad in the new layout, but you don't need to *swap* anything. The block's bad, the rest of the block doesn't matter. But anyway...

Quoted 3 lines

ROM when loading firmware, copies back
the value at metadata[0] to BI offset in page data. The following figure shows how the factory bad block marker is preserved.

...this is insane. It seems that they want you to swap this byte even in good blocks, so that you put the byte of real data that should go somewhere in the middle of the last 512-byte ECC chunk at offset zero in the page. This means that it will show up as a bad block when normal (but new-layout-using) software looks at it, which is why you need scrub. Ew.

How many blocks are being loaded by this mechanism? Just block zero (which is normally supposed to be guaranteed good anyway...)? Or multiple blocks?

Any chance you could blow the NAND_MEMBLOCK_MARKER_RESERVE fuse? :-) Otherwise, I guess you do need to scrub. Have you complained to Freescale sales/support?

Quoted 2 lines

But we want to write a block in our own format, so we need to scrub (wipe the block completely).

Erase always wipes the block completely, if it erases at all. Scrub in this context just means that U-Boot ignores the bad block indications (marker or table). Otherwise it would avoid erasing bad blocks, so that they stay bad, and you won't have scrubbed the entire region requested.

Quoted 6 lines
Quoted 2 lines

I work for the PowerPC side of Freescale, in case you're wondering why I'm unfamiliar with this. :-)

Interesting ... does everyone work for the PowerPC side of Freescale or is there some other reason why I never met anyone working for the ARM side of Freescale ? ;-)

They exist, but don't seem to engage Open Source development communities to the same degree.

-Scott

#50 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

[...]

Quoted 11 lines
Quoted 3 lines

ROM when loading firmware, copies back
the value at metadata[0] to BI offset in page data. The following figure shows how the factory bad block marker is preserved.

...this is insane. It seems that they want you to swap this byte even in good blocks, so that you put the byte of real data that should go somewhere in the middle of the last 512-byte ECC chunk at offset zero in the page. This means that it will show up as a bad block when normal (but new-layout-using) software looks at it, which is why you need scrub. Ew.

Quoted 3 lines

How many blocks are being loaded by this mechanism? Just block zero (which is normally supposed to be guaranteed good anyway...)? Or multiple blocks?

The first block, then the 64th block, 128th block and 192th block (in default layout).

Quoted 2 lines

Any chance you could blow the NAND_MEMBLOCK_MARKER_RESERVE fuse? :-)

No, they are one-time programable. Delivering a "damaged" chip isn't a good practice.

Quoted 2 lines

Otherwise, I guess you do need to scrub. Have you complained to Freescale sales/support?

In fact no. The BootROM is "broken" and I doubt they will be willing to do anything about it.

Quoted 12 lines
Quoted 3 lines

But we want to write a block in our own format, so we need to scrub (wipe the block completely).

Erase always wipes the block completely, if it erases at all. Scrub in this context just means that U-Boot ignores the bad block indications (marker or table). Otherwise it would avoid erasing bad blocks, so that they stay bad, and you won't have scrubbed the entire region requested.

Quoted 3 lines
Quoted 2 lines

I work for the PowerPC side of Freescale, in case you're wondering why I'm unfamiliar with this. :-)

Quoted 8 lines
Quoted 3 lines

Interesting ... does everyone work for the PowerPC side of Freescale or is there some other reason why I never met anyone working for the ARM side of Freescale ? ;-)

They exist, but don't seem to engage Open Source development communities to the same degree.

-Scott

Cheers

#51 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

Quoted 4 lines · Marek Vasut
Quoted 1 line

Any chance you could blow the NAND_MEMBLOCK_MARKER_RESERVE fuse? :-)

No, they are one-time programable. Delivering a "damaged" chip isn't a good practice.

I'd call it "fixed" rather than "damaged" -- they did throw in that fuse in case of "any defective ROM code in handling bad block marker byte swapping", and the entire concept is defective.

But I agree that if you're delivering them to other developers, rather than as part of a final product, they might want to make that choice themselves. And who knows, blowing the fuse might expose other defects.

Quoted 5 lines
Quoted 2 lines

Otherwise, I guess you do need to scrub. Have you complained to Freescale sales/support?

In fact no. The BootROM is "broken" and I doubt they will be willing to do anything about it.

Probably not, but they should still hear about it, if only for the chance they'll do it right next time.

-Scott

#52 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

In reply to Marek Vasut

Dear Marek Vasut,

Quoted 9 lines · In message <[email protected]> you wrote:
Quoted 6 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

I'd prefer to have this hidden from common users as much as possible.

This is probably well-intentioned, but keep in mind old (and good!) Unix rules like:

"UNIX was not designed to stop you from doing stupid things, because
that would also stop you from doing clever things." - Doug Gwyn

Don't try hiding stuff - others might find it useful.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I can't say I've ever been lost, but I was bewildered once for  three
days.                                     - Daniel Boone (Attributed)
#53 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 13 lines · Wolfgang Denk

Dear Marek Vasut,

Quoted 9 lines · In message <[email protected]> you wrote:
Quoted 6 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

I'd prefer to have this hidden from common users as much as possible.

Quoted 7 lines

This is probably well-intentioned, but keep in mind old (and good!) Unix rules like:

"UNIX was not designed to stop you from doing stupid things, because
that would also stop you from doing clever things." - Doug Gwyn

Don't try hiding stuff - others might find it useful.

You can use the usual scrub command, noone is preventing you from anything.

Using this kind of a scrub.quiet command is really an arguable practice.

Quoted 4 lines

Best regards,

Wolfgang Denk

#54 lore raw

PATCH 5/5 NAND: Add scrub.quiet command option

In reply to Marek Vasut
Quoted 9 lines · On Monday, September 12, 2011 13:45:17 Marek Vasut wrote:
Quoted 6 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

I'd prefer to have this hidden from common users as much as possible.

ok, but i dont follow why that makes a difference to my feedback. "quiet" does not mean "do not prompt the user". however, "scrub -y" does mean "assume yes to the sanity prompts".
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110912/2633d85f/attachment.pgp

#55 lore raw

PATCH v2 5/5 NAND: Add scrub.quiet command option

Quoted 9 lines · Mike Frysinger
Quoted 8 lines · On Monday, September 12, 2011 13:45:17 Marek Vasut wrote:
Quoted 6 lines · Mike Frysinger
Quoted 2 lines · On Monday, September 12, 2011 00:04:10 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

"quiet" and "skip user input" are two different things. can you use a more clean option like accepting "-y" to the "scrub" subcommand ?

Quoted 6 lines
Quoted 1 line

I'd prefer to have this hidden from common users as much as possible.

ok, but i dont follow why that makes a difference to my feedback. "quiet" does not mean "do not prompt the user". however, "scrub -y" does mean "assume yes to the sanity prompts".
-mike

Ok, you convinced me ... I added a -y option.

#56 lore raw

PATCH v2 5/5 NAND: Add -y option to nand scrub command

In reply to Marek Vasut

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)
common/cmd_nand.c +19 -15
index 5b7e83d..c2a5623 100644
@@ -500,12 +500,23 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		nand_erase_options_t opts;
 		/* "clean" at index 2 means request to write cleanmarker */
 		int clean = argc > 2 && !strcmp("clean", argv[2]);
-		int o = clean ? 3 : 2;
+		int scrub_yes = argc > 2 && !strcmp("-y", argv[2]);
+		int o = (clean || scrub_yes) ? 3 : 2;
 		int scrub = !strncmp(cmd, "scrub", 5);
 		int part = 0;
 		int chip = 0;
 		int spread = 0;
 		int args = 2;
+		const char *scrub_warn =
+			"Warning: "
+			"scrub option will erase all factory set bad blocks!\n"
+			"         "
+			"There is no reliable way to recover them.\n"
+			"         "
+			"Use this command only for testing purposes if you\n"
+			"         "
+			"are sure of what you are doing!\n"
+			"\nReally scrub this NAND flash? <y/N>\n";
 
 		if (cmd[5] != 0) {
 			if (!strcmp(&cmd[5], ".spread")) {
@@ -544,19 +555,12 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		opts.spread = spread;
 
 		if (scrub) {
-			puts("Warning: "
-			     "scrub option will erase all factory set "
-			     "bad blocks!\n"
-			     "         "
-			     "There is no reliable way to recover them.\n"
-			     "         "
-			     "Use this command only for testing purposes "
-			     "if you\n"
-			     "         "
-			     "are sure of what you are doing!\n"
-			     "\nReally scrub this NAND flash? <y/N>\n");
-
-			if (getc() == 'y') {
+			if (!scrub_yes)
+				puts(scrub_warn);
+
+			if (scrub_yes)
+				opts.scrub = 1;
+			else if (getc() == 'y') {
 				puts("y");
 				if (getc() == '\r')
 					opts.scrub = 1;
@@ -768,7 +772,7 @@ U_BOOT_CMD(
 	"nand erase.chip [clean] - erase entire chip'\n"
 	"nand bad - show bad blocks\n"
 	"nand dump[.oob] off - dump page\n"
-	"nand scrub off size | scrub.part partition | scrub.chip\n"
+	"nand scrub [-y] off size | scrub.part partition | scrub.chip\n"
 	"    really clean NAND erasing bad blocks (UNSAFE)\n"
 	"nand markbad off [...] - mark bad block(s) at offset (UNSAFE)\n"
 	"nand biterr off - make a bit error at offset (UNSAFE)"
Signature · 1 line
1.7.5.4
#57 lore raw

PATCH 5/5 NAND: Add -y option to nand scrub command

Quoted 2 lines · On Tuesday, September 13, 2011 18:20:35 Marek Vasut wrote:

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110913/962a6cd3/attachment.pgp

#58 lore raw

PATCH 5/5 NAND: Add -y option to nand scrub command

In reply to Marek Vasut
Quoted 11 lines · Marek Vasut

This allows the scrub command to scrub without asking the user if he really wants to scrub the area. Useful in scripts.

Cc Scott Wood <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Detlev Zundel <[email protected]>
---
 common/cmd_nand.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

Applied to u-boot-nand-flash next

-Scott