[U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too

[U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too

[Ben Gardiner]

It was found that on da850evm, where the NAND ECC used does not map all 0xff
data to 0xff ECC, that flashing UBI and JFFS2 image from U-boot with nand
write[.e] command resulted in alot of ECC errors... for UBI the result was
an unmountable filesystem on second attach from linux. For JFFS2 the result was
a multitude of ECC errors printed on the cosole on the second mount in Linux --
the filesystem remains mountable for awhile but eventually collapses.

By implementing the procedure reccomended in the UBI FAQ of dropping trailing
pages containing entirely 0xff both UBI images and JFFS2 filesystems flashed
with from u-boot were found to survive past the second time around.

Ben Gardiner (5):
  nand_base: trivial: fix comment read/write comment
  nand_util: convert nand_write_skip_bad() to flags
  nand_util: drop trailing all-0xff pages if requested
  cmd_nand: add nand write.ubi command
  cmd_nand: also drop 0xff pages for jffs2

 common/cmd_nand.c            |   15 ++++++++++++---
 drivers/mtd/nand/nand_base.c |    2 +-
 drivers/mtd/nand/nand_util.c |   37 ++++++++++++++++++++++++++++++-------
 include/nand.h               |    6 +++++-
 4 files changed, 48 insertions(+), 12 deletions(-)

[U-Boot] [PATCH 1/5] nand_base: trivial: fix comment read/write comment

[Ben Gardiner]

Replace an incorrect 'read' with 'write' in a comment.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 drivers/mtd/nand/nand_base.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 52f8575..1a95a91 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1950,7 +1950,7 @@ static int nand_write(struct mtd_info *mtd, loff_t to, size_t len,
 	struct nand_chip *chip = mtd->priv;
 	int ret;
 
-	/* Do not allow reads past end of device */
+	/* Do not allow writes past end of device */
 	if ((to + len) > mtd->size)
 		return -EINVAL;
 	if (!len)
-- 
1.7.1

[U-Boot] [PATCH 2/5] nand_util: convert nand_write_skip_bad() to flags

[Ben Gardiner]

In a future commit the behaviour of nand_write_skip_bad()
will be further extended.

Convert the only flag currently passed to the nand_write_
skip_bad() function to a bitfield of only one allocated
member. This should avoid an explosion of int's at the
end of the parameter list or the ambiguous calls like

nand_write_skip_bad(info, offset, len, buf, 0, 1, 1);
nand_write_skip_bad(info, offset, len, buf, 0, 1, 0);

Instead there will be:

nand_write_skip_bad(info, offset, len, buf, WITH_OOB |
			WITH_OTHER);

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 common/cmd_nand.c            |    3 ++-
 drivers/mtd/nand/nand_util.c |    8 ++++----
 include/nand.h               |    5 ++++-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 7bd37de..69b2acc 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -581,7 +581,8 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 				printf("Unknown nand command suffix '%s'.\n", s);
 				return 1;
 			}
-			ret = nand_write_skip_bad(nand, off, &rwsize, (u_char *)addr, 1);
+			ret = nand_write_skip_bad(nand, off, &rwsize,
+						(u_char *)addr, WITH_OOB);
 #endif
 		} else if (!strcmp(s, ".oob")) {
 			/* out-of-band data */
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index 5a6f7ae..2bd8758 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -448,11 +448,11 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length)
  * @param offset	offset in flash
  * @param length	buffer length
  * @param buffer        buffer to read from
- * @param withoob	whether write with yaffs format
+ * @param flags		flags mmofying the behaviour of the write to NAND
  * @return		0 in case of success
  */
 int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
-			u_char *buffer, int withoob)
+			u_char *buffer, int flags)
 {
 	int rval = 0, blocksize;
 	size_t left_to_write = *length;
@@ -460,7 +460,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 	int need_skip;
 
 #ifdef CONFIG_CMD_NAND_YAFFS
-	if (withoob) {
+	if (flags & WITH_OOB) {
 		int pages;
 		pages = nand->erasesize / nand->writesize;
 		blocksize = (pages * nand->oobsize) + nand->erasesize;
@@ -529,7 +529,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 			write_size = blocksize - block_offset;
 
 #ifdef CONFIG_CMD_NAND_YAFFS
-		if (withoob) {
+		if (flags & WITH_OOB) {
 			int page, pages;
 			size_t pagesize = nand->writesize;
 			size_t pagesize_oob = pagesize + nand->oobsize;
diff --git a/include/nand.h b/include/nand.h
index 7459bd0..628317a 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -114,8 +114,11 @@ typedef struct nand_erase_options nand_erase_options_t;
 
 int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 		       u_char *buffer);
+
+#define WITH_OOB (1 << 0) /* whether write with yaffs format */
+
 int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
-			u_char *buffer, int withoob);
+			u_char *buffer, int flags);
 int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts);
 
 #define NAND_LOCK_STATUS_TIGHT	0x01
-- 
1.7.1

[U-Boot] [PATCH 2/5] nand_util: convert nand_write_skip_bad() to flags

[Detlev Zundel]

Hi Ben,

> In a future commit the behaviour of nand_write_skip_bad()
> will be further extended.
>
> Convert the only flag currently passed to the nand_write_
> skip_bad() function to a bitfield of only one allocated
> member. This should avoid an explosion of int's at the
> end of the parameter list or the ambiguous calls like
>
> nand_write_skip_bad(info, offset, len, buf, 0, 1, 1);
> nand_write_skip_bad(info, offset, len, buf, 0, 1, 0);
>
> Instead there will be:
>
> nand_write_skip_bad(info, offset, len, buf, WITH_OOB |
> 			WITH_OTHER);
>
> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
> ---
>  common/cmd_nand.c            |    3 ++-
>  drivers/mtd/nand/nand_util.c |    8 ++++----
>  include/nand.h               |    5 ++++-
>  3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> index 7bd37de..69b2acc 100644
> --- a/common/cmd_nand.c
> +++ b/common/cmd_nand.c
> @@ -581,7 +581,8 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>  				printf("Unknown nand command suffix '%s'.\n", s);
>  				return 1;
>  			}
> -			ret = nand_write_skip_bad(nand, off, &rwsize, (u_char *)addr, 1);
> +			ret = nand_write_skip_bad(nand, off, &rwsize,
> +						(u_char *)addr, WITH_OOB);
>  #endif
>  		} else if (!strcmp(s, ".oob")) {
>  			/* out-of-band data */

I see an occurrence of nand_write_skip_bad just above this if block.
Please replace this also.

> diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
> index 5a6f7ae..2bd8758 100644
> --- a/drivers/mtd/nand/nand_util.c
> +++ b/drivers/mtd/nand/nand_util.c
> @@ -448,11 +448,11 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length)
>   * @param offset	offset in flash
>   * @param length	buffer length
>   * @param buffer        buffer to read from
> - * @param withoob	whether write with yaffs format
> + * @param flags		flags mmofying the behaviour of the write to NAND
>   * @return		0 in case of success
>   */
>  int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
> -			u_char *buffer, int withoob)
> +			u_char *buffer, int flags)
>  {
>  	int rval = 0, blocksize;
>  	size_t left_to_write = *length;
> @@ -460,7 +460,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
>  	int need_skip;
>  
>  #ifdef CONFIG_CMD_NAND_YAFFS
> -	if (withoob) {
> +	if (flags & WITH_OOB) {
>  		int pages;
>  		pages = nand->erasesize / nand->writesize;
>  		blocksize = (pages * nand->oobsize) + nand->erasesize;
> @@ -529,7 +529,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
>  			write_size = blocksize - block_offset;
>  
>  #ifdef CONFIG_CMD_NAND_YAFFS
> -		if (withoob) {
> +		if (flags & WITH_OOB) {
>  			int page, pages;
>  			size_t pagesize = nand->writesize;
>  			size_t pagesize_oob = pagesize + nand->oobsize;
> diff --git a/include/nand.h b/include/nand.h
> index 7459bd0..628317a 100644
> --- a/include/nand.h
> +++ b/include/nand.h
> @@ -114,8 +114,11 @@ typedef struct nand_erase_options nand_erase_options_t;
>  
>  int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
>  		       u_char *buffer);
> +
> +#define WITH_OOB (1 << 0) /* whether write with yaffs format */
> +

If this flag is really only relevant for YAFFS, then please include this
in its name, i.e. rename it to "WITH_YAFFS_OOB".

>  int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
> -			u_char *buffer, int withoob);
> +			u_char *buffer, int flags);
>  int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts);
>  
>  #define NAND_LOCK_STATUS_TIGHT	0x01

Cheers
  Detlev

-- 
 Those who do not understand Unix are condemned to reinvent it,
 poorly.
 - Henry Spencer, University of Toronto Unix hack
--
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

[U-Boot] [PATCH 3/5] nand_util: drop trailing all-0xff pages if requested

[Ben Gardiner]

Add a flag to nand_read_skip_bad() such that if true, any trailing
pages in an eraseblock whose contents are entirely 0xff will be
dropped.

The implementation is via a new drop_ffs() function which is
based on the function of the same name from the ubiformat
utility by Artem Bityutskiy.

This is as-per the reccomendations of the UBI FAQ. Without this
behaviour, UBI images flashed to NAND where either 1) the ECC used
does not map all-0xff to 0xff or 2) the number of times a page
can be written is limited by the NAND will not be successfully
attached after flashing.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
CC: Artem Bityutskiy <dedekind1@gmail.com>

---

This behaviour was found to fix both UBI and JFFS2 images written to
cleanly erased NAND partitions on da850evm.
---
 drivers/mtd/nand/nand_util.c |   29 ++++++++++++++++++++++++++---
 include/nand.h               |    3 ++-
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index 2bd8758..c9bb930 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -436,6 +436,22 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length)
 	return ret;
 }
 
+static size_t drop_ffs(const nand_info_t *nand, const u_char *buf,
+			const size_t *len)
+{
+	size_t i, l = *len;
+
+	for (i = l - 1; i >= 0; i--)
+		if (((const uint8_t *)buf)[i] != 0xFF)
+			break;
+
+	/* The resulting length must be aligned to the minimum flash I/O size */
+	l = i + 1;
+	l = (l + nand->writesize - 1) / nand->writesize;
+	l *=  nand->writesize;
+	return l;
+}
+
 /**
  * nand_write_skip_bad:
  *
@@ -499,7 +515,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 		return -EINVAL;
 	}
 
-	if (!need_skip) {
+	if (!need_skip && !(flags & WITH_DROP_FFS)) {
 		rval = nand_write (nand, offset, length, buffer);
 		if (rval == 0)
 			return 0;
@@ -512,7 +528,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 
 	while (left_to_write > 0) {
 		size_t block_offset = offset & (nand->erasesize - 1);
-		size_t write_size;
+		size_t write_size, truncated_write_size;
 
 		WATCHDOG_RESET ();
 
@@ -558,7 +574,14 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 		else
 #endif
 		{
-			rval = nand_write (nand, offset, &write_size, p_buffer);
+			if (flags & WITH_DROP_FFS)
+				truncated_write_size = drop_ffs(nand, p_buffer,
+						&write_size);
+			else
+				truncated_write_size = write_size;
+
+			rval = nand_write(nand, offset, &truncated_write_size,
+					p_buffer);
 			offset += write_size;
 			p_buffer += write_size;
 		}
diff --git a/include/nand.h b/include/nand.h
index 628317a..b58a82d 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -115,7 +115,8 @@ typedef struct nand_erase_options nand_erase_options_t;
 int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 		       u_char *buffer);
 
-#define WITH_OOB (1 << 0) /* whether write with yaffs format */
+#define WITH_OOB	(1 << 0) /* whether write with yaffs format */
+#define WITH_DROP_FFS	(1 << 1) /* drop trailing all-0xff pages */
 
 int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 			u_char *buffer, int flags);
-- 
1.7.1

[U-Boot] [PATCH 4/5] cmd_nand: add nand write.ubi command

[Ben Gardiner]

Add another nand write. variant, ubi. This command will request of
nand_write_skip_bad() that all trailing all-0xff pages will be
dropped from eraseblocks as they are written as-per the
reccommended behaviour of the UBI FAQ.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 common/cmd_nand.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 69b2acc..faece07 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -567,7 +567,11 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		rwsize = size;
 
 		s = strchr(cmd, '.');
-		if (!s || !strcmp(s, ".jffs2") ||
+		if (!strcmp(s, ".ubi")) {
+			ret = nand_write_skip_bad(nand, off, &rwsize,
+						  (u_char *)addr,
+						  WITH_DROP_FFS);
+		} else if (!s || !strcmp(s, ".jffs2") ||
 		    !strcmp(s, ".e") || !strcmp(s, ".i")) {
 			if (read)
 				ret = nand_read_skip_bad(nand, off, &rwsize,
@@ -694,6 +698,11 @@ U_BOOT_CMD(
 	"    write 'size' bytes starting at offset 'off' with yaffs format\n"
 	"    from memory address 'addr', skipping bad blocks.\n"
 #endif
+	"nand write.ubi - addr off|partition size\n"
+	"    write 'size' bytes starting at offset 'off'\n"
+	"    from memory address 'addr', skipping bad blocks and "
+	"dropping any pages at the\n"
+	"    end of eraseblocks that contain only 0xFF\n"
 	"nand erase[.spread] [clean] [off [size]] - erase 'size' bytes "
 	"from offset 'off'\n"
 	"    With '.spread', erase enough for given file size, otherwise,\n"
-- 
1.7.1

[U-Boot] [PATCH 4/5] cmd_nand: add nand write.ubi command

[Detlev Zundel]

Hi Ben,

> Add another nand write. variant, ubi. This command will request of
> nand_write_skip_bad() that all trailing all-0xff pages will be
> dropped from eraseblocks as they are written as-per the
> reccommended behaviour of the UBI FAQ.

If I understand the code correctly, then the assumption is that writing
FFs to an erased flash is essentially a no-op, right?  This is not
really UBI specific, so why don't we use a name like e.g. "trimffs" for
the new functionality?

Moreover now that I think about it, I can imagine a corner case where
the flash is not erased at positions where the image contains ffs.  As I
read your code, the ffs will silently be dropped and no error will be
generated, although the contents of the image will _not_ correpsond to
the contents in flash.

If this is right, then this has potential for great confusion.  Maybe we
should check that the flash is really erased at the positions
corresponding to ffs?

Cheers
  Detlev

-- 
It's bad  civic hygiene to build  technologies that could  someday be used to
facilitate a police state.  No matter what the eavesdroppers and censors say,
these systems put us all at greater risk. Communications systems that have no
inherent  eavesdropping capabilities are more  secure than systems with those
capabilities built in.                                      -- Bruce Schneier
--
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

[U-Boot] [PATCH 5/5] cmd_nand: also drop 0xff pages for jffs2

[Ben Gardiner]

The behaviour of dropping trailing 0xff pages of an eraseblock was
observed to fix JFFS2 images on da850evm which usually resulted in
numerous 'ECC errors.'

Assign also the behaviour of dropping trailing 0xff pages to the
.jffs2 nand write variant as it was to the previously introduced
.ubi variant.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
---
 common/cmd_nand.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index faece07..b9d5ae6 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -567,12 +567,11 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		rwsize = size;
 
 		s = strchr(cmd, '.');
-		if (!strcmp(s, ".ubi")) {
+		if (!strcmp(s, ".ubi") || !strcmp(s, ".jffs2")) {
 			ret = nand_write_skip_bad(nand, off, &rwsize,
 						  (u_char *)addr,
 						  WITH_DROP_FFS);
-		} else if (!s || !strcmp(s, ".jffs2") ||
-		    !strcmp(s, ".e") || !strcmp(s, ".i")) {
+		} else if (!s || !strcmp(s, ".e") || !strcmp(s, ".i")) {
 			if (read)
 				ret = nand_read_skip_bad(nand, off, &rwsize,
 							 (u_char *)addr);
-- 
1.7.1

[U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too

[Detlev Zundel]

Hi Ben,

> It was found that on da850evm, where the NAND ECC used does not map all 0xff
> data to 0xff ECC, that flashing UBI and JFFS2 image from U-boot with nand
> write[.e] command resulted in alot of ECC errors... for UBI the result was
> an unmountable filesystem on second attach from linux. For JFFS2 the result was
> a multitude of ECC errors printed on the cosole on the second mount in Linux --
> the filesystem remains mountable for awhile but eventually collapses.

I am not sure that I can follow you here so I have to ask you to clarify
the problem for me.

I understand that a page of 0xffs does _not_ have an ECC of all 0xffs.
Actually I would be surprised if there was any ECC having this property,
so I doubt that this is da850 specific, correct?

So I am wondering about two things:

- If I erase a page in NAND, will the ECC be updated by someone or will
  it be 0xffs?  If the latter, then is it "normal" to have ECC errors on
  freshly erased pages?

- If we (correctly) "write" 0xffs even to an erased page, a generated
  ECC should match this content, so I do not see where ECC errors should
  come from in this setting.

Summarily, I do not understand where the ECC errors came from in your
setup and what the faulting party in that scenario actually is/was.

Can you please enlighten me?

Thanks
  Detlev

-- 
SWYM XYZ (Sympathize with your machinery).  [..] In fact it is often
called a no-op, because it performs no operation.  It does, however,
keep the machine running smoothly, just as real-world swimming helps
to keep programmers healthy.             -- Donald Knuth, Fascicle 1
--
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

[U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too

[Artem Bityutskiy]

On Fri, 2011-04-29 at 13:58 +0200, Detlev Zundel wrote:
> Hi Ben,
> 
> > It was found that on da850evm, where the NAND ECC used does not map all 0xff
> > data to 0xff ECC, that flashing UBI and JFFS2 image from U-boot with nand
> > write[.e] command resulted in alot of ECC errors... for UBI the result was
> > an unmountable filesystem on second attach from linux. For JFFS2 the result was
> > a multitude of ECC errors printed on the cosole on the second mount in Linux --
> > the filesystem remains mountable for awhile but eventually collapses.
> 
> I am not sure that I can follow you here so I have to ask you to clarify
> the problem for me.
> 
> I understand that a page of 0xffs does _not_ have an ECC of all 0xffs.
> Actually I would be surprised if there was any ECC having this property,
> so I doubt that this is da850 specific, correct?
> 
> So I am wondering about two things:
> 
> - If I erase a page in NAND, will the ECC be updated by someone or will
>   it be 0xffs?  If the latter, then is it "normal" to have ECC errors on
>   freshly erased pages?
> 
> - If we (correctly) "write" 0xffs even to an erased page, a generated
>   ECC should match this content, so I do not see where ECC errors should
>   come from in this setting.
> 
> Summarily, I do not understand where the ECC errors came from in your
> setup and what the faulting party in that scenario actually is/was.
> 
> Can you please enlighten me?

I do not know why I'm in CC, but I see that the code to skip all 0xFFs
looks like it was copied from UBI. The reason why UBI and UBI user-space
tools skip NAND pages with all 0xFFs when writing is described here:

http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo

If it is too long to read, in short:

1. if we write to flash, we always write to an erased eraseblock,
obviously :-)
2. erased erase block contains all 0xFFs, so skipping 0xFF NAND pages is
harmless.
3. writing 0xFF pages has side-effects - the ECC bytes in OOB will be
used
4. If we are flashing an UBIFS image, UBIFS will use the half-filled
eraseblocks, and if the "free" pages were written with 0xFFs, they'll
become unusable.

So we skip 0xFF pages at the end. Not all, only the last ones. E.g., if
your eraseblock consists of 4 pages, and you write "data, 0xFFs, data,
0xFFs", then we'll only write "data, 0xFFs, data", so that the last NAND
page can be used later.

Sorry for my poor English, and I'm writing in a hurry - have to have to
a pub to farewell several colleagues who are leaving the company :)

-- 
Best Regards,
Artem Bityutskiy (????? ????????)

[U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too

[Detlev Zundel]

Hi Artem,

> On Fri, 2011-04-29 at 13:58 +0200, Detlev Zundel wrote:
>> Hi Ben,
>> 
>> > It was found that on da850evm, where the NAND ECC used does not map all 0xff
>> > data to 0xff ECC, that flashing UBI and JFFS2 image from U-boot with nand
>> > write[.e] command resulted in alot of ECC errors... for UBI the result was
>> > an unmountable filesystem on second attach from linux. For JFFS2 the result was
>> > a multitude of ECC errors printed on the cosole on the second mount in Linux --
>> > the filesystem remains mountable for awhile but eventually collapses.
>> 
>> I am not sure that I can follow you here so I have to ask you to clarify
>> the problem for me.
>> 
>> I understand that a page of 0xffs does _not_ have an ECC of all 0xffs.
>> Actually I would be surprised if there was any ECC having this property,
>> so I doubt that this is da850 specific, correct?
>> 
>> So I am wondering about two things:
>> 
>> - If I erase a page in NAND, will the ECC be updated by someone or will
>>   it be 0xffs?  If the latter, then is it "normal" to have ECC errors on
>>   freshly erased pages?
>> 
>> - If we (correctly) "write" 0xffs even to an erased page, a generated
>>   ECC should match this content, so I do not see where ECC errors should
>>   come from in this setting.
>> 
>> Summarily, I do not understand where the ECC errors came from in your
>> setup and what the faulting party in that scenario actually is/was.
>> 
>> Can you please enlighten me?
>
> I do not know why I'm in CC, but I see that the code to skip all 0xFFs
> looks like it was copied from UBI. The reason why UBI and UBI user-space
> tools skip NAND pages with all 0xFFs when writing is described here:
>
> http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
>
> If it is too long to read, in short:
>
> 1. if we write to flash, we always write to an erased eraseblock,
> obviously :-)

Yes - I simply wasn't sure if the software layers below (in U-Boot)
would do an "erase on demand" before writing.  Reading the code, this
isn't the case (I should have known this), so the skipping should really
be safe.

> 2. erased erase block contains all 0xFFs, so skipping 0xFF NAND pages is
> harmless.
> 3. writing 0xFF pages has side-effects - the ECC bytes in OOB will be
> used
> 4. If we are flashing an UBIFS image, UBIFS will use the half-filled
> eraseblocks, and if the "free" pages were written with 0xFFs, they'll
> become unusable.
>
> So we skip 0xFF pages at the end. Not all, only the last ones. E.g., if
> your eraseblock consists of 4 pages, and you write "data, 0xFFs, data,
> 0xFFs", then we'll only write "data, 0xFFs, data", so that the last NAND
> page can be used later.
>
> Sorry for my poor English, and I'm writing in a hurry - have to have to
> a pub to farewell several colleagues who are leaving the company :)

Thanks for your input.

So I still fail to see where the ECC errors come from.  The closes thing
that makes sense for me, is that Bens problem very likely wasn't ECC
connected at all but the result of the "not capable to write twice".
I.e. his NAND flashes cannot be written to twice.  When he flashed the
images in U-Boot, there were trailing empty blocks that got programmed
and UBIFS assumed that it _could_ write to them so it tried and failed
and somehow got tripped up over this.

If this is the case then we should change the commit message to point to
the real problem that this patch fixes.

Cheers
  Detlev

-- 
``The number of UNIX installations has grown to 10,
with more expected.''     Unix Programmers Manual -- 1972
The number of UNIX variants has grown to dozens,
with more expected.                               -- 2001
--
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

[U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too

[Ben Gardiner]

Hi Detlev, Artem,

I'm very sorry that I haven't been back to reply in over a week.
Please accept my apologies.

On Fri, Apr 29, 2011 at 7:58 AM, Detlev Zundel <dzu@denx.de> wrote:
> Hi Ben,
>
>> It was found that on da850evm, where the NAND ECC used does not map all 0xff
>> data to 0xff ECC, that flashing UBI and JFFS2 image from U-boot with nand
>> write[.e] command resulted in alot of ECC errors... for UBI the result was
>> an unmountable filesystem on second attach from linux. For JFFS2 the result was
>> a multitude of ECC errors printed on the cosole on the second mount in Linux --
>> the filesystem remains mountable for awhile but eventually collapses.
>
> I am not sure that I can follow you here so I have to ask you to clarify
> the problem for me.
>
> I understand that a page of 0xffs does _not_ have an ECC of all 0xffs.
> Actually I would be surprised if there was any ECC having this property,
> so I doubt that this is da850 specific, correct?

Thanks for the review, Detlev. I don't have enough experience with
NAND devices or controllers to say whether this property is unique or
not. I trust that yours is sufficient to say so. I think I will remove
this note from the cover letter in v2.

On Fri, Apr 29, 2011 at 9:59 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> [...]
>> Can you please enlighten me?
>
> I do not know why I'm in CC, but I see that the code to skip all 0xFFs
> looks like it was copied from UBI. The reason why UBI and UBI user-space
> tools skip NAND pages with all 0xFFs when writing is described here:
>
> http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo

Artem, thank you for picking up my slack and answering Detlev's questions.

Yes, the drop_ffs() implementation is copied from ubiformat utility.
I'll try to make this clearer in the commit  messages and cover
letters for v2.

On Mon, May 2, 2011 at 9:14 AM, Detlev Zundel <dzu@denx.de> wrote:
> [...]
>>> Can you please enlighten me?
>> [...]
>> If it is too long to read, in short:
>>
>> 1. if we write to flash, we always write to an erased eraseblock,
>> obviously :-)
>
> Yes - I simply wasn't sure if the software layers below (in U-Boot)
> would do an "erase on demand" before writing. ?Reading the code, this
> isn't the case (I should have known this), so the skipping should really
> be safe.
>
>> 2. erased erase block contains all 0xFFs, so skipping 0xFF NAND pages is
>> harmless.
>> 3. writing 0xFF pages has side-effects - the ECC bytes in OOB will be
>> used
>> 4. If we are flashing an UBIFS image, UBIFS will use the half-filled
>> eraseblocks, and if the "free" pages were written with 0xFFs, they'll
>> become unusable.
>>
>> So we skip 0xFF pages at the end. Not all, only the last ones. E.g., if
>> your eraseblock consists of 4 pages, and you write "data, 0xFFs, data,
>> 0xFFs", then we'll only write "data, 0xFFs, data", so that the last NAND
>> page can be used later.
>>
>> Sorry for my poor English, and I'm writing in a hurry - have to have to
>> a pub to farewell several colleagues who are leaving the company :)
>
> Thanks for your input.
>
> So I still fail to see where the ECC errors come from. ?The closes thing
> that makes sense for me, is that Bens problem very likely wasn't ECC
> connected at all but the result of the "not capable to write twice".
> I.e. his NAND flashes cannot be written to twice. ?When he flashed the
> images in U-Boot, there were trailing empty blocks that got programmed
> and UBIFS assumed that it _could_ write to them so it tried and failed
> and somehow got tripped up over this.
>
> If this is the case then we should change the commit message to point to
> the real problem that this patch fixes.

I am planning to respin v2 next week; I will 1) remove mention of the
ECC mapping 0xff 2) add a link to the faq entry provided by Artem and
3) make clear the origin of the drop_ffs function.

I will also rename to nand write.trimffs, thanks for suggestion.

How do you feel about making the write.jffs2 function use trimffs as
in the last patch of the series?

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca