[U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too
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(-)
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)
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
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
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);
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"
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
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);
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
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 :)
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:
- 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.
- 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
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:
- 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.
- 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
participants (3)
-
Artem Bityutskiy -
Ben Gardiner -
Detlev Zundel