All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/4] introduce nand write.trimffs
@ 2011-05-24 14:18 Ben Gardiner
  2011-05-24 14:18 ` [U-Boot] [PATCH 1/4] [v3] nand_base: trivial: fix comment read/write comment Ben Gardiner
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Ben Gardiner @ 2011-05-24 14:18 UTC (permalink / raw)
  To: u-boot

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(-)

-- 
1.7.4.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/4] [v3] nand_base: trivial: fix comment read/write comment
  2011-05-24 14:18 [U-Boot] [PATCH v3 0/4] introduce nand write.trimffs Ben Gardiner
@ 2011-05-24 14:18 ` Ben Gardiner
  2011-06-06 21:32   ` Scott Wood
  2011-05-24 14:18 ` [U-Boot] [PATCH 2/4] [v3] nand_util: convert nand_write_skip_bad() to flags Ben Gardiner
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Ben Gardiner @ 2011-05-24 14:18 UTC (permalink / raw)
  To: u-boot

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)
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 2/4] [v3] nand_util: convert nand_write_skip_bad() to flags
  2011-05-24 14:18 [U-Boot] [PATCH v3 0/4] introduce nand write.trimffs Ben Gardiner
  2011-05-24 14:18 ` [U-Boot] [PATCH 1/4] [v3] nand_base: trivial: fix comment read/write comment Ben Gardiner
@ 2011-05-24 14:18 ` Ben Gardiner
  2011-06-06 21:32   ` Scott Wood
  2011-05-24 14:18 ` [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested Ben Gardiner
  2011-05-24 14:18 ` [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command Ben Gardiner
  3 siblings, 1 reply; 19+ messages in thread
From: Ben Gardiner @ 2011-05-24 14:18 UTC (permalink / raw)
  To: u-boot

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
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested
  2011-05-24 14:18 [U-Boot] [PATCH v3 0/4] introduce nand write.trimffs Ben Gardiner
  2011-05-24 14:18 ` [U-Boot] [PATCH 1/4] [v3] nand_base: trivial: fix comment read/write comment Ben Gardiner
  2011-05-24 14:18 ` [U-Boot] [PATCH 2/4] [v3] nand_util: convert nand_write_skip_bad() to flags Ben Gardiner
@ 2011-05-24 14:18 ` Ben Gardiner
  2011-05-27  9:43   ` Detlev Zundel
  2011-06-06 20:58   ` Scott Wood
  2011-05-24 14:18 ` [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command Ben Gardiner
  3 siblings, 2 replies; 19+ messages in thread
From: Ben Gardiner @ 2011-05-24 14:18 UTC (permalink / raw)
  To: u-boot

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 at realitydiluted.com)
  *			       and Thomas Gleixner (tglx at 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);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command
  2011-05-24 14:18 [U-Boot] [PATCH v3 0/4] introduce nand write.trimffs Ben Gardiner
                   ` (2 preceding siblings ...)
  2011-05-24 14:18 ` [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested Ben Gardiner
@ 2011-05-24 14:18 ` Ben Gardiner
  2011-05-27  9:41   ` Detlev Zundel
  2011-06-06 21:00   ` Scott Wood
  3 siblings, 2 replies; 19+ messages in thread
From: Ben Gardiner @ 2011-05-24 14:18 UTC (permalink / raw)
  To: u-boot

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
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command
  2011-05-24 14:18 ` [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command Ben Gardiner
@ 2011-05-27  9:41   ` Detlev Zundel
  2011-06-06 21:00   ` Scott Wood
  1 sibling, 0 replies; 19+ messages in thread
From: Detlev Zundel @ 2011-05-27  9:41 UTC (permalink / raw)
  To: u-boot

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

-- 
More than any other time in history, mankind faces a crossroads.  One
path leads  to despair  and utter  hopelessness.   The other to total
extinction.  Let us pray, we have the wisdom to choose correctly.
                                        -- Woody Allen
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested
  2011-05-24 14:18 ` [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested Ben Gardiner
@ 2011-05-27  9:43   ` Detlev Zundel
  2011-06-06 20:58   ` Scott Wood
  1 sibling, 0 replies; 19+ messages in thread
From: Detlev Zundel @ 2011-05-27  9:43 UTC (permalink / raw)
  To: u-boot

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

-- 
Der Kluge tue gleich anfangs, was der Dumme erst am Ende.
                                    --- Baltasar Gracian
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested
  2011-05-24 14:18 ` [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested Ben Gardiner
  2011-05-27  9:43   ` Detlev Zundel
@ 2011-06-06 20:58   ` Scott Wood
  2011-06-07 13:09     ` Ben Gardiner
  1 sibling, 1 reply; 19+ messages in thread
From: Scott Wood @ 2011-06-06 20:58 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command
  2011-05-24 14:18 ` [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command Ben Gardiner
  2011-05-27  9:41   ` Detlev Zundel
@ 2011-06-06 21:00   ` Scott Wood
  2011-06-07 13:09     ` Ben Gardiner
  1 sibling, 1 reply; 19+ messages in thread
From: Scott Wood @ 2011-06-06 21:00 UTC (permalink / raw)
  To: u-boot

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 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.

-Scott

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/4] [v3] nand_base: trivial: fix comment read/write comment
  2011-05-24 14:18 ` [U-Boot] [PATCH 1/4] [v3] nand_base: trivial: fix comment read/write comment Ben Gardiner
@ 2011-06-06 21:32   ` Scott Wood
  0 siblings, 0 replies; 19+ messages in thread
From: Scott Wood @ 2011-06-06 21:32 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 2/4] [v3] nand_util: convert nand_write_skip_bad() to flags
  2011-05-24 14:18 ` [U-Boot] [PATCH 2/4] [v3] nand_util: convert nand_write_skip_bad() to flags Ben Gardiner
@ 2011-06-06 21:32   ` Scott Wood
  0 siblings, 0 replies; 19+ messages in thread
From: Scott Wood @ 2011-06-06 21:32 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested
  2011-06-06 20:58   ` Scott Wood
@ 2011-06-07 13:09     ` Ben Gardiner
  2011-06-07 16:34       ` Scott Wood
  0 siblings, 1 reply; 19+ messages in thread
From: Ben Gardiner @ 2011-06-07 13:09 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command
  2011-06-06 21:00   ` Scott Wood
@ 2011-06-07 13:09     ` Ben Gardiner
  0 siblings, 0 replies; 19+ messages in thread
From: Ben Gardiner @ 2011-06-07 13:09 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested
  2011-06-07 13:09     ` Ben Gardiner
@ 2011-06-07 16:34       ` Scott Wood
  2011-06-14 20:35         ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Ben Gardiner
  0 siblings, 1 reply; 19+ messages in thread
From: Scott Wood @ 2011-06-07 16:34 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs
  2011-06-07 16:34       ` Scott Wood
@ 2011-06-14 20:35         ` Ben Gardiner
  2011-06-14 20:35           ` [U-Boot] [PATCH 1/3] [v4] nand_util: treat WITH_YAFFS_OOB as a mode Ben Gardiner
                             ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Ben Gardiner @ 2011-06-14 20:35 UTC (permalink / raw)
  To: u-boot

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(-)

-- 
1.7.4.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/3] [v4] nand_util: treat WITH_YAFFS_OOB as a mode
  2011-06-14 20:35         ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Ben Gardiner
@ 2011-06-14 20:35           ` Ben Gardiner
  2011-06-14 20:35           ` [U-Boot] [PATCH 2/3] [v4] nand_util: drop trailing all-0xff pages if requested Ben Gardiner
                             ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Ben Gardiner @ 2011-06-14 20:35 UTC (permalink / raw)
  To: u-boot

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);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 2/3] [v4] nand_util: drop trailing all-0xff pages if requested
  2011-06-14 20:35         ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Ben Gardiner
  2011-06-14 20:35           ` [U-Boot] [PATCH 1/3] [v4] nand_util: treat WITH_YAFFS_OOB as a mode Ben Gardiner
@ 2011-06-14 20:35           ` Ben Gardiner
  2011-06-14 20:35           ` [U-Boot] [PATCH 3/3] [v4] cmd_nand: add nand write.trimffs command Ben Gardiner
  2011-06-15 23:05           ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Scott Wood
  3 siblings, 0 replies; 19+ messages in thread
From: Ben Gardiner @ 2011-06-14 20:35 UTC (permalink / raw)
  To: u-boot

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 at realitydiluted.com)
  *			       and Thomas Gleixner (tglx at 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);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] [v4] cmd_nand: add nand write.trimffs command
  2011-06-14 20:35         ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Ben Gardiner
  2011-06-14 20:35           ` [U-Boot] [PATCH 1/3] [v4] nand_util: treat WITH_YAFFS_OOB as a mode Ben Gardiner
  2011-06-14 20:35           ` [U-Boot] [PATCH 2/3] [v4] nand_util: drop trailing all-0xff pages if requested Ben Gardiner
@ 2011-06-14 20:35           ` Ben Gardiner
  2011-06-15 23:05           ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Scott Wood
  3 siblings, 0 replies; 19+ messages in thread
From: Ben Gardiner @ 2011-06-14 20:35 UTC (permalink / raw)
  To: u-boot

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
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs
  2011-06-14 20:35         ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Ben Gardiner
                             ` (2 preceding siblings ...)
  2011-06-14 20:35           ` [U-Boot] [PATCH 3/3] [v4] cmd_nand: add nand write.trimffs command Ben Gardiner
@ 2011-06-15 23:05           ` Scott Wood
  3 siblings, 0 replies; 19+ messages in thread
From: Scott Wood @ 2011-06-15 23:05 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2011-06-15 23:05 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-24 14:18 [U-Boot] [PATCH v3 0/4] introduce nand write.trimffs Ben Gardiner
2011-05-24 14:18 ` [U-Boot] [PATCH 1/4] [v3] nand_base: trivial: fix comment read/write comment Ben Gardiner
2011-06-06 21:32   ` Scott Wood
2011-05-24 14:18 ` [U-Boot] [PATCH 2/4] [v3] nand_util: convert nand_write_skip_bad() to flags Ben Gardiner
2011-06-06 21:32   ` Scott Wood
2011-05-24 14:18 ` [U-Boot] [PATCH 3/4] [v3] nand_util: drop trailing all-0xff pages if requested Ben Gardiner
2011-05-27  9:43   ` Detlev Zundel
2011-06-06 20:58   ` Scott Wood
2011-06-07 13:09     ` Ben Gardiner
2011-06-07 16:34       ` Scott Wood
2011-06-14 20:35         ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Ben Gardiner
2011-06-14 20:35           ` [U-Boot] [PATCH 1/3] [v4] nand_util: treat WITH_YAFFS_OOB as a mode Ben Gardiner
2011-06-14 20:35           ` [U-Boot] [PATCH 2/3] [v4] nand_util: drop trailing all-0xff pages if requested Ben Gardiner
2011-06-14 20:35           ` [U-Boot] [PATCH 3/3] [v4] cmd_nand: add nand write.trimffs command Ben Gardiner
2011-06-15 23:05           ` [U-Boot] [PATCH v4 0/3] introduce nand write.trimffs Scott Wood
2011-05-24 14:18 ` [U-Boot] [PATCH 4/4] [v3] cmd_nand: add nand write.trimffs command Ben Gardiner
2011-05-27  9:41   ` Detlev Zundel
2011-06-06 21:00   ` Scott Wood
2011-06-07 13:09     ` Ben Gardiner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.