[U-Boot] [PATCH v3 0/4] introduce nand write.trimffs
This series adds a nand write variant which implements the procedure reccomended in the UBI FAQ [1] of dropping trailing pages of eraseblocks containing entirely 0xff's.
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
Changes since v2: * dropped WITH_DEFAULTS in favour of '0' * moved copyright header to nand_util patch * added write.trimffs variant to README.nand
Changes since v1: * renamed to 'trimffs' from 'ubi' * wrapped the new feature in #ifdefs * don't make it default for jffs -- patch dropped * attribution of the drop_ffs() function from mtd-utils to Artem
Ben Gardiner (4): [v3] nand_base: trivial: fix comment read/write comment [v3] nand_util: convert nand_write_skip_bad() to flags [v3] nand_util: drop trailing all-0xff pages if requested [v3] cmd_nand: add nand write.trimffs command
common/cmd_nand.c | 19 +++++++++++++++++- doc/README.nand | 10 +++++++++ drivers/mtd/nand/nand_base.c | 2 +- drivers/mtd/nand/nand_util.c | 43 +++++++++++++++++++++++++++++++++++------ include/nand.h | 6 ++++- 5 files changed, 70 insertions(+), 10 deletions(-)
Replace an incorrect 'read' with 'write' in a comment.
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca Acked-by: Detlev Zundel dzu@denx.de
--- Changes since v2: * added Detlev's Acked-by Changes since v1: * rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options") --- 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)
On Tue, May 24, 2011 at 10:18:34AM -0400, Ben Gardiner wrote:
Replace an incorrect 'read' with 'write' in a comment.
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca Acked-by: Detlev Zundel dzu@denx.de
Changes since v2:
- added Detlev's Acked-by
Changes since v1:
- rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options")
drivers/mtd/nand/nand_base.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Applied to u-boot-nand-flash next
-Scott
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_YAFFS_OOB | WITH_OTHER);
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca Acked-by: Detlev Zundel dzu@denx.de
--- Changes since v2: * Added Detlev's Acked-by * removed the WITH_DEFAULTS flag -- zero is fine (Detlev Zundel) * fixed typo 'mmofying' to 'modifying' in comment Changes since v1: * rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options") * renamed the flag from WITH_OOB to WITH_YAFFS_OOB (Detlev Zundel) * introduce 'WITH_DEFAULTS' flag defined as 0 so as to convert also the remaining nand_write_skip_bad() call (Detlev Zundel) --- 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..e7db4c9 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_YAFFS_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..762ac53 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 modifying 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_YAFFS_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_YAFFS_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..b0a31b8 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_YAFFS_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
On Tue, May 24, 2011 at 10:18:35AM -0400, Ben Gardiner wrote:
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_YAFFS_OOB | WITH_OTHER);
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca Acked-by: Detlev Zundel dzu@denx.de
Changes since v2:
- Added Detlev's Acked-by
- removed the WITH_DEFAULTS flag -- zero is fine (Detlev Zundel)
- fixed typo 'mmofying' to 'modifying' in comment
Changes since v1:
- rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options")
- renamed the flag from WITH_OOB to WITH_YAFFS_OOB (Detlev Zundel)
- introduce 'WITH_DEFAULTS' flag defined as 0 so as to convert also the remaining nand_write_skip_bad() call (Detlev Zundel)
common/cmd_nand.c | 3 ++- drivers/mtd/nand/nand_util.c | 8 ++++---- include/nand.h | 5 ++++- 3 files changed, 10 insertions(+), 6 deletions(-)
Applied to u-boot-nand-flash next
-Scott
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 [1]
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca CC: Artem Bityutskiy dedekind1@gmail.com CC: Detlev Zundel dzu@denx.de
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
---
This behaviour was found to fix both UBI and JFFS2 images written to cleanly erased NAND partitions on da850evm.
Changes since v2: * moved the copyright header addition of nand_util to this patch from patch 'cmd_nand: add nand write.trimffs command' * Did not add Detlev's Acked-by because of movement of copyright header Changes since v1: * rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options") * wrap the new functionality in a CONFIG_CMD_NAND_TRIMFFS ifdef to reduce size impact of new feature --- drivers/mtd/nand/nand_util.c | 35 ++++++++++++++++++++++++++++++++--- include/nand.h | 1 + 2 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 762ac53..82b41a0 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -11,6 +11,9 @@ * nandwrite.c by Steven J. Hill (sjhill@realitydiluted.com) * and Thomas Gleixner (tglx@linutronix.de) * + * Copyright (C) 2008 Nokia Corporation: drop_ffs() function by + * Artem Bityutskiy dedekind1@gmail.com from mtd-utils + * * See file CREDITS for list of people who contributed to this * project. * @@ -436,6 +439,24 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length) return ret; }
+#ifdef CONFIG_CMD_NAND_TRIMFFS +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; +} +#endif + /** * nand_write_skip_bad: * @@ -499,7 +520,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 +533,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 +579,15 @@ 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); + truncated_write_size = write_size; +#ifdef CONFIG_CMD_NAND_TRIMFFS + if (flags & WITH_DROP_FFS) + truncated_write_size = drop_ffs(nand, p_buffer, + &write_size); +#endif + + 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 b0a31b8..5c8a189 100644 --- a/include/nand.h +++ b/include/nand.h @@ -116,6 +116,7 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, u_char *buffer);
#define WITH_YAFFS_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);
Hi Ben,
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 [1]
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca CC: Artem Bityutskiy dedekind1@gmail.com CC: Detlev Zundel dzu@denx.de
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
Looks good -
Acked-by: Detlev Zundel dzu@denx.de
Cheers Detlev
On Tue, May 24, 2011 at 10:18:36AM -0400, Ben Gardiner wrote:
+#ifdef CONFIG_CMD_NAND_TRIMFFS +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;
This cast looks unnecessary.
- /* 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;
We allow unaligned lengths (the rest of the page gets padded with 0xff, see nand_do_page_write-ops). The input length might be unaligned -- this adjustment could cause you to read beyond the end of the supplied buffer.
+} +#endif
/**
- nand_write_skip_bad:
@@ -499,7 +520,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;
Why not call drop_ffs before this point?
@@ -512,7 +533,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 +579,15 @@ 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);
truncated_write_size = write_size;+#ifdef CONFIG_CMD_NAND_TRIMFFS
if (flags & WITH_DROP_FFS)truncated_write_size = drop_ffs(nand, p_buffer,&write_size);+#endif
What if both WITH_DROP_FFS and WITH_YAFFS_OOB are specified?
-Scott
Hi Scott,
Thanks for the review.
On Mon, Jun 6, 2011 at 4:58 PM, Scott Wood scottwood@freescale.com wrote:
On Tue, May 24, 2011 at 10:18:36AM -0400, Ben Gardiner wrote:
+#ifdef CONFIG_CMD_NAND_TRIMFFS +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;
This cast looks unnecessary.
You're absolutely right. It will be gone in v4.
- /* 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;
We allow unaligned lengths (the rest of the page gets padded with 0xff, see nand_do_page_write-ops). The input length might be unaligned -- this adjustment could cause you to read beyond the end of the supplied buffer.
Right. Sorry I missed that. In v4 I will drop also any trailling 0xff which do not make-up a full page since they would be padded out to trailing 0xff.
+} +#endif
/** * nand_write_skip_bad: * @@ -499,7 +520,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;
Why not call drop_ffs before this point?
To achieve the desired effect, drop_ffs must be called on each eraseblock sized chunk being written; so it seemed the simplest way was to force a block-by-block pass with the !WITH_DROP_FFS to enter
while (left_to_write > 0) {
I'll leave this as-is in v4.
@@ -512,7 +533,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 +579,15 @@ 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);
- truncated_write_size = write_size;
+#ifdef CONFIG_CMD_NAND_TRIMFFS
- if (flags & WITH_DROP_FFS)
- truncated_write_size = drop_ffs(nand, p_buffer,
- &write_size);
+#endif
What if both WITH_DROP_FFS and WITH_YAFFS_OOB are specified?
I didn't plan for that or intend for it to be supported.
Previous to my introduction of WITH_DROP_FFS; using the YAFFS oob mode was mutually exclusive with the 'usual' way of writing. The introduction of WITH_DROP_FFs respects this precedent.
If both flags were set 1) cmd_nand.c would need to be changed ( :) ) and 2) the WITH_YAFFS_OOB behaviour would override.
In v4 I will add a -EINVAL if WITH_YAFFS_OOB flag is used with any other flag.
Best Regards, Ben Gardiner
--- Nanometrics Inc. http://www.nanometrics.ca
On Tue, 7 Jun 2011 09:09:07 -0400 Ben Gardiner bengardiner@nanometrics.ca wrote:
Why not call drop_ffs before this point?
To achieve the desired effect, drop_ffs must be called on each eraseblock sized chunk being written; so it seemed the simplest way was to force a block-by-block pass with the !WITH_DROP_FFS to enter
Ah, I missed that it was within each erase block.
In v4 I will add a -EINVAL if WITH_YAFFS_OOB flag is used with any other flag.
OK, I just didn't want it silently ignored, with no documentation of the limitation.
-Scott
This series adds a nand write variant which implements the procedure reccomended in the UBI FAQ [1] of dropping trailing pages of eraseblocks containing entirely 0xff's.
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
Changes since v3: * rebased to nand-flash/next where patches 1/4 and 2/4 [v3] were already applied * added patch 1/3 [v4] to treat WITH_YAFFS_OOB as a mode * renumbered 3/4 and 4/4 [v3] to 1/3 and 3/3 [v4], respectively * remove uneccessary cast * wrapped README at 80 columns * prevent access past input buffer end
Changes since v2: * dropped WITH_DEFAULTS in favour of '0' * moved copyright header to nand_util patch * added write.trimffs variant to README.nand
Changes since v1: * renamed to 'trimffs' from 'ubi' * wrapped the new feature in #ifdefs * don't make it default for jffs -- patch dropped * attribution of the drop_ffs() function from mtd-utils to Artem
Ben Gardiner (3): [v4] nand_util: treat WITH_YAFFS_OOB as a mode [v4] nand_util: drop trailing all-0xff pages if requested [v4] cmd_nand: add nand write.trimffs command
common/cmd_nand.c | 16 +++++++++++++++ doc/README.nand | 10 +++++++++ drivers/mtd/nand/nand_util.c | 43 +++++++++++++++++++++++++++++++++++++++-- include/nand.h | 5 +++- 4 files changed, 70 insertions(+), 4 deletions(-)
When specified in the flags argument of nand_write, WITH_YAFFS_OOB causes an operation which is mutually exclusive with the 'usual' way of writing.
Add a check that client code does not specify WITH_YAFFS_OOB along with any other flags and add a comment indicating that the WITH_YAFFS_OOB flag should not be mixed with other flags.
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca CC: Scott Wood scottwood@freescale.com
---
Changes since v3: * none. This patch was introduced in v4 to adress Scott Wood's v3 review comments.
--- drivers/mtd/nand/nand_util.c | 3 +++ include/nand.h | 4 +++- 2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 762ac53..e4ef858 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -461,6 +461,9 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
#ifdef CONFIG_CMD_NAND_YAFFS if (flags & WITH_YAFFS_OOB) { + if (flags & ~WITH_YAFFS_OOB) + return -EINVAL; + int pages; pages = nand->erasesize / nand->writesize; blocksize = (pages * nand->oobsize) + nand->erasesize; diff --git a/include/nand.h b/include/nand.h index b0a31b8..c5818f1 100644 --- a/include/nand.h +++ b/include/nand.h @@ -115,7 +115,9 @@ 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_YAFFS_OOB (1 << 0) /* whether write with yaffs format */ +#define WITH_YAFFS_OOB (1 << 0) /* whether write with yaffs format. This flag + * is a 'mode' meaning it cannot be mixed with + * other flags */
int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, u_char *buffer, int flags);
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 [1]
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca CC: Artem Bityutskiy dedekind1@gmail.com Acked-by: Detlev Zundel dzu@denx.de CC: Scott Wood scottwood@freescale.com
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
---
Scott,
I did not add your Acked-by because to handle the possible unaligned input length I opted to return min() instead of dropping the alignment fixup entire as had been agreed upon in correspondence. I found in testing that removing the alignment resulted in corruption of the NAND.
This behaviour was found to fix both UBI and JFFS2 images written to cleanly erased NAND partitions on da850evm.
Changes since v3: * rebased onto nand-flash/next * remove uneccessary cast (Scott Wood) * added Detlev's Acked-by * prevent access past end of buffer using min() (Scott Wood) Changes since v2: * moved the copyright header addition of nand_util to this patch from patch 'cmd_nand: add nand write.trimffs command' * Did not add Detlev's Acked-by because of movement of copyright header Changes since v1: * rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options") * wrap the new functionality in a CONFIG_CMD_NAND_TRIMFFS ifdef to reduce size impact of new feature
--- drivers/mtd/nand/nand_util.c | 40 +++++++++++++++++++++++++++++++++++++--- include/nand.h | 1 + 2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index e4ef858..81bf366 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -11,6 +11,9 @@ * nandwrite.c by Steven J. Hill (sjhill@realitydiluted.com) * and Thomas Gleixner (tglx@linutronix.de) * + * Copyright (C) 2008 Nokia Corporation: drop_ffs() function by + * Artem Bityutskiy dedekind1@gmail.com from mtd-utils + * * See file CREDITS for list of people who contributed to this * project. * @@ -436,6 +439,29 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length) return ret; }
+#ifdef CONFIG_CMD_NAND_TRIMFFS +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 (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; + + /* + * since the input length may be unaligned, prevent access past the end + * of the buffer + */ + return min(l, *len); +} +#endif + /** * nand_write_skip_bad: * @@ -502,7 +528,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; @@ -515,7 +541,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 ();
@@ -561,7 +587,15 @@ 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); + truncated_write_size = write_size; +#ifdef CONFIG_CMD_NAND_TRIMFFS + if (flags & WITH_DROP_FFS) + truncated_write_size = drop_ffs(nand, p_buffer, + &write_size); +#endif + + 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 c5818f1..8d94b5c 100644 --- a/include/nand.h +++ b/include/nand.h @@ -118,6 +118,7 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, #define WITH_YAFFS_OOB (1 << 0) /* whether write with yaffs format. This flag * is a 'mode' meaning it cannot be mixed with * other flags */ +#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, trimffs. This command will request of nand_write_skip_bad() that all trailing all-0xff pages will be dropped from eraseblocks when they are written to flash as-per the reccommended behaviour of the UBI FAQ [1].
The function that implements this timming is the drop_ffs() function by Artem Bityutskiy, ported from the mtd-utils tree.
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca CC: Artem Bityutskiy dedekind1@gmail.com CC: Detlev Zundel dzu@denx.de Acked-by: Scott Wood scottwood@freescale.com
---
Changes since v3: * rebased to nand-flash/next * wrap README at 80 columns including EOL (Scott Wood) Changes since v2: * added nand write.trimffs to the README.nand file * moved the nand_util copyright header addition to patch 'nand_util: drop trailing all-0xff pages if requested' Changes since v1: * rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options") * renamed the command variant to '.trimffs' from '.ubi' (Detlev Zundel) * added attribution to mtd-utils and Artem Bityutskiy in both the source comments and commit message * wrapped the new command in a new ifdef, CONFIG_CMD_NAND_TRIMFFS, to reduce the size impact of this new feature
--- common/cmd_nand.c | 16 ++++++++++++++++ doc/README.nand | 10 ++++++++++ 2 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index 27a8879..b767cd2 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -575,6 +575,16 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) else ret = nand_write_skip_bad(nand, off, &rwsize, (u_char *)addr, 0); +#ifdef CONFIG_CMD_NAND_TRIMFFS + } else if (!strcmp(s, ".trimffs")) { + if (read) { + printf("Unknown nand command suffix '%s'\n", s); + return 1; + } + ret = nand_write_skip_bad(nand, off, &rwsize, + (u_char *)addr, + WITH_DROP_FFS); +#endif #ifdef CONFIG_CMD_NAND_YAFFS } else if (!strcmp(s, ".yaffs")) { if (read) { @@ -689,6 +699,12 @@ 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" +#ifdef CONFIG_CMD_NAND_TRIMFFS + "nand write.trimffs - addr off|partition size\n" + " write 'size' bytes starting at offset 'off' from memory address\n" + " 'addr', skipping bad blocks and dropping any pages at the end\n" + " of eraseblocks that contain only 0xFF\n" +#endif #ifdef CONFIG_CMD_NAND_YAFFS "nand write.yaffs - addr off|partition size\n" " write 'size' bytes starting at offset 'off' with yaffs format\n" diff --git a/doc/README.nand b/doc/README.nand index 8eedb6c..751b693 100644 --- a/doc/README.nand +++ b/doc/README.nand @@ -78,6 +78,16 @@ Commands: should work well, but loading an image copied from another flash is going to be trouble if there are any bad blocks.
+ nand write.trimffs addr ofs|partition size + Enabled by the CONFIG_CMD_NAND_TRIMFFS macro. This command will write to + the NAND flash in a manner identical to the 'nand write' command + described above -- with the additional check that all pages at the end + of eraseblocks which contain only 0xff data will not be written to the + NAND flash. This behaviour is required when flashing UBI images + containing UBIFS volumes as per the UBI FAQ[1]. + + [1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo + nand write.oob addr ofs|partition size Write `size' bytes from `addr' to the out-of-band data area corresponding to `ofs' in NAND flash. This is limited to the 16 bytes
On Tue, Jun 14, 2011 at 04:35:04PM -0400, Ben Gardiner wrote:
This series adds a nand write variant which implements the procedure reccomended in the UBI FAQ [1] of dropping trailing pages of eraseblocks containing entirely 0xff's.
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
Changes since v3:
- rebased to nand-flash/next where patches 1/4 and 2/4 [v3] were already applied
- added patch 1/3 [v4] to treat WITH_YAFFS_OOB as a mode
- renumbered 3/4 and 4/4 [v3] to 1/3 and 3/3 [v4], respectively
- remove uneccessary cast
- wrapped README at 80 columns
- prevent access past input buffer end
Applied to u-boot-nand-flash next
-Scott
Add another nand write. variant, trimffs. This command will request of nand_write_skip_bad() that all trailing all-0xff pages will be dropped from eraseblocks when they are written to flash as-per the reccommended behaviour of the UBI FAQ [1].
The function that implements this timming is the drop_ffs() function by Artem Bityutskiy, ported from the mtd-utils tree.
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca CC: Artem Bityutskiy dedekind1@gmail.com CC: Detlev Zundel dzu@denx.de
---
Changes since v2: * added nand write.trimffs to the README.nand file * moved the nand_util copyright header addition to patch 'nand_util: drop trailing all-0xff pages if requested' Changes since v1: * rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 ("env_nand: zero-initialize variable nand_erase_options") * renamed the command variant to '.trimffs' from '.ubi' (Detlev Zundel) * added attribution to mtd-utils and Artem Bityutskiy in both the source comments and commit message * wrapped the new command in a new ifdef, CONFIG_CMD_NAND_TRIMFFS, to reduce the size impact of this new feature --- common/cmd_nand.c | 16 ++++++++++++++++ doc/README.nand | 10 ++++++++++ 2 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index e7db4c9..786f179 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -575,6 +575,16 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) else ret = nand_write_skip_bad(nand, off, &rwsize, (u_char *)addr, 0); +#ifdef CONFIG_CMD_NAND_TRIMFFS + } else if (!strcmp(s, ".trimffs")) { + if (read) { + printf("Unknown nand command suffix '%s'\n", s); + return 1; + } + ret = nand_write_skip_bad(nand, off, &rwsize, + (u_char *)addr, + WITH_DROP_FFS); +#endif #ifdef CONFIG_CMD_NAND_YAFFS } else if (!strcmp(s, ".yaffs")) { if (read) { @@ -689,6 +699,12 @@ 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" +#ifdef CONFIG_CMD_NAND_TRIMFFS + "nand write.trimffs - addr off|partition size\n" + " write 'size' bytes starting at offset 'off' from memory address\n" + " 'addr', skipping bad blocks and dropping any pages at the end\n" + " of eraseblocks that contain only 0xFF\n" +#endif #ifdef CONFIG_CMD_NAND_YAFFS "nand write.yaffs - addr off|partition size\n" " write 'size' bytes starting at offset 'off' with yaffs format\n" diff --git a/doc/README.nand b/doc/README.nand index 8eedb6c..ca62f00 100644 --- a/doc/README.nand +++ b/doc/README.nand @@ -78,6 +78,16 @@ Commands: should work well, but loading an image copied from another flash is going to be trouble if there are any bad blocks.
+ nand write.trimffs addr ofs|partition size + Enabled by the CONFIG_CMD_NAND_TRIMFFS macro. This command will write to + the NAND flash in a manner identical to the 'nand write' command described + above -- with the additional check that all pages at the end of + eraseblocks which contain only 0xff data will not be written to the NAND + flash. This behaviour is required when flashing UBI images containing + UBIFS volumes as per the UBI FAQ[1]. + + [1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo + nand write.oob addr ofs|partition size Write `size' bytes from `addr' to the out-of-band data area corresponding to `ofs' in NAND flash. This is limited to the 16 bytes
Hi Ben,
Add another nand write. variant, trimffs. This command will request of nand_write_skip_bad() that all trailing all-0xff pages will be dropped from eraseblocks when they are written to flash as-per the reccommended behaviour of the UBI FAQ [1].
The function that implements this timming is the drop_ffs() function by Artem Bityutskiy, ported from the mtd-utils tree.
[1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
Signed-off-by: Ben Gardiner bengardiner@nanometrics.ca CC: Artem Bityutskiy dedekind1@gmail.com CC: Detlev Zundel dzu@denx.de
Thanks - looks good now.
Acked-by: Detlev Zundel dzu@denx.de
Cheers Detlev
On Tue, May 24, 2011 at 10:18:37AM -0400, Ben Gardiner wrote:
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index e7db4c9..786f179 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -575,6 +575,16 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) else ret = nand_write_skip_bad(nand, off, &rwsize, (u_char *)addr, 0); +#ifdef CONFIG_CMD_NAND_TRIMFFS
} else if (!strcmp(s, ".trimffs")) {if (read) {printf("Unknown nand command suffix '%s'\n", s);return 1;}ret = nand_write_skip_bad(nand, off, &rwsize,(u_char *)addr,WITH_DROP_FFS);+#endif #ifdef CONFIG_CMD_NAND_YAFFS } else if (!strcmp(s, ".yaffs")) {
Should these really be modes rather than flags?
- nand write.trimffs addr ofs|partition size
Enabled by the CONFIG_CMD_NAND_TRIMFFS macro. This command will write tothe NAND flash in a manner identical to the 'nand write' command describedabove -- with the additional check that all pages at the end oferaseblocks which contain only 0xff data will not be written to the NANDflash. This behaviour is required when flashing UBI images containingUBIFS volumes as per the UBI FAQ[1].
Please wrap at 80 columns.
-Scott
Hi Scott,
Thanks for the reviews.
On Mon, Jun 6, 2011 at 5:00 PM, Scott Wood scottwood@freescale.com wrote:
On Tue, May 24, 2011 at 10:18:37AM -0400, Ben Gardiner wrote:
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index e7db4c9..786f179 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -575,6 +575,16 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) else ret = nand_write_skip_bad(nand, off, &rwsize, (u_char *)addr, 0); +#ifdef CONFIG_CMD_NAND_TRIMFFS
- } else if (!strcmp(s, ".trimffs")) {
- if (read) {
- printf("Unknown nand command suffix '%s'\n", s);
- return 1;
- }
- ret = nand_write_skip_bad(nand, off, &rwsize,
- (u_char *)addr,
- WITH_DROP_FFS);
+#endif #ifdef CONFIG_CMD_NAND_YAFFS } else if (!strcmp(s, ".yaffs")) {
Should these really be modes rather than flags?
For the two currently possible values of 'int flags' -- yes. But that's because the WITH_YAFFS_OOB causes a specific exuction path which is mutually exclusive to the usual path; whereas the WITH_DROP_FFS option does an operation in addition to the usual execution. So the latter is (to me at least) a flag whereas the former is a mode.
I see you have already applied the patches which convert to a flag -- so I will leave as is in v4. I will, as noted in the previous email -- add a return -EINVAL if WITH_YAFFS_OOB is used with any other flags.
- nand write.trimffs addr ofs|partition size
- Enabled by the CONFIG_CMD_NAND_TRIMFFS macro. This command will write to
- the NAND flash in a manner identical to the 'nand write' command described
- above -- with the additional check that all pages at the end of
- eraseblocks which contain only 0xff data will not be written to the NAND
- flash. This behaviour is required when flashing UBI images containing
- UBIFS volumes as per the UBI FAQ[1].
Please wrap at 80 columns.
Ok. Will do in v4.
checkpatch.pl did not complain about " the NAND flash in a manner identical to the 'nand write' command described" -- which is 81 characters including the \n but it did not complain about a line over 85 characters in that README either so I guess READMEs aren't enforced by that script.
Best Regards, Ben Gardiner
--- Nanometrics Inc. http://www.nanometrics.ca
participants (3)
-
Ben Gardiner -
Detlev Zundel -
Scott Wood