diff mbox

Add --skip-all-ffs option to mtd-utils nandwrite

Message ID 82cbfdbc-c79b-01b0-bc64-a4259d7a150e@aimvalley.nl
State Superseded
Headers show

Commit Message

Kees Trommel Dec. 5, 2016, 4:14 p.m. UTC 
All,

Would it be possible to add an option --skip-all-ffs option to mtd-utils 
nandwrite?

With this option pages that contain only 0xFF bytes will be skipped and 
not written to flash.

This option is useful when you want to write using nandwrite an UBI 
image to NAND devices with a HW ECC. Without this option the OOB of a 
page that is written with all 0xFFs is no longer erased because the HW 
adds an non 0xFFs ECC to the OOB. If the data of the page contains only 
0xFFs then UBI/UBIFS assumes that the page is erased and writes to it 
without erasing the page first. This causes that a read of this page 
fails with unrecoverable ECC errors and a subsequent corruption of the 
UBIFS.

Attached is a patch file that adds the --skip-all-ffs option. A 
nandwrite of UBI image with this option will be successful while it 
fails without this option.

Regards,

Kees Trommel
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Comments

David Oberhollenzer Dec. 5, 2016, 7:13 p.m. UTC | #1 
Hi,


On 12/05/2016 05:14 PM, Kees Trommel wrote:
> Would it be possible to add an option --skip-all-ffs option to mtd-utils nandwrite?

> 

> With this option pages that contain only 0xFF bytes will be skipped and not written to flash.

> 

> This option is useful when you want to write using nandwrite an UBI image to NAND devices with a HW ECC. Without this option the OOB of a page that is written with all 0xFFs is no longer erased because the HW adds an non 0xFFs ECC to the OOB. If the data of the page contains only 0xFFs then UBI/UBIFS assumes that the page is erased and writes to it without erasing the page first. This causes that a read of this page fails with unrecoverable ECC errors and a subsequent corruption of the UBIFS.


Yes, that sounds reasonable.
 
> Attached is a patch file that adds the --skip-all-ffs option. A nandwrite of UBI image with this option will be successful while it fails without this option.


Your patch looks good to me, however even after fixing the paths to nandwrite.c (the
source tree got restructured over a year ago, see commit 7d81790ced) it doesn't apply
to the current mtd-utils master branch. Could you please rebase your patch onto the
current mtd-utils master branch (git://git.infradead.org/mtd-utils.git) and resubmit it?


Thanks,

David



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
diff mbox

Patch

From f5dd8f13b45a37f0375f01a05a3ed41b39b50beb Mon Sep 17 00:00:00 2001
From: Kees Trommel <ctrommel@linvm302.aimsys.nl>
Date: Mon, 5 Dec 2016 16:37:11 +0100
Subject: [PATCH] skip-all-ff-pages-option

Signed-off-by: Kees Trommel <ctrommel@linvm302.aimsys.nl>
---
 nandwrite.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index 9c3fe8f..722b1bd 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -49,6 +49,7 @@  static void display_help(int status)
 "Writes to the specified MTD device.\n"
 "\n"
 "  -a, --autoplace         Use auto OOB layout\n"
+"  -k, --skip-all-ffs      Skip pages that contain only ff bytes\n"
 "  -m, --markbad           Mark blocks bad if write fails\n"
 "  -n, --noecc             Write without ecc\n"
 "  -N, --noskipbad         Write without bad block skipping\n"
@@ -93,6 +94,7 @@  static bool		onlyoob = false;
 static bool		markbad = false;
 static bool		noecc = false;
 static bool		autoplace = false;
+static bool		skipallffs = false;
 static bool		noskipbad = false;
 static bool		pad = false;
 static int		blockalign = 1; /* default to using actual block size */
@@ -103,7 +105,7 @@  static void process_options(int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char short_options[] = "hb:mnNoOpqs:a";
+		static const char short_options[] = "hb:mnNoOpqs:ak";
 		static const struct option long_options[] = {
 			/* Order of these args with val==0 matters; see option_index. */
 			{"version", no_argument, 0, 0},
@@ -120,6 +122,7 @@  static void process_options(int argc, char * const argv[])
 			{"quiet", no_argument, 0, 'q'},
 			{"start", required_argument, 0, 's'},
 			{"autoplace", no_argument, 0, 'a'},
+			{"skip-all-ffs", no_argument, 0, 'k'},
 			{0, 0, 0, 0},
 		};
 
@@ -173,6 +176,9 @@  static void process_options(int argc, char * const argv[])
 		case 'a':
 			autoplace = true;
 			break;
+		case 'k':
+			skipallffs = true;
+			break;
 		case 'h':
 			display_help(EXIT_SUCCESS);
 			break;
@@ -231,6 +237,8 @@  static void erase_buffer(void *buffer, size_t size)
  */
 int main(int argc, char * const argv[])
 {
+	int allffs;
+	int ii;
 	int fd = -1;
 	int ifd = -1;
 	int pagelen;
@@ -516,14 +524,29 @@  int main(int argc, char * const argv[])
 			}
 		}
 
-		/* Write out data */
-		ret = mtd_write(mtd_desc, &mtd, fd, mtdoffset / mtd.eb_size,
-				mtdoffset % mtd.eb_size,
-				onlyoob ? NULL : writebuf,
-				onlyoob ? 0 : mtd.min_io_size,
-				writeoob ? oobbuf : NULL,
-				writeoob ? mtd.oob_size : 0,
-				write_mode);
+		allffs = 0;
+		if (skipallffs) 
+		{
+			for (ii = 0; ii < mtd.min_io_size; ii += sizeof(uint32_t))
+			{
+				if (*(uint32_t*)(writebuf + ii) != 0xffffffff)
+					break;
+			}
+			if (ii == mtd.min_io_size)
+				allffs = 1;
+		}
+		if (!allffs) {
+			/* Write out data */
+			ret = mtd_write(mtd_desc, &mtd, fd, mtdoffset / mtd.eb_size,
+					mtdoffset % mtd.eb_size,
+					onlyoob ? NULL : writebuf,
+					onlyoob ? 0 : mtd.min_io_size,
+					writeoob ? oobbuf : NULL,
+					writeoob ? mtd.oob_size : 0,
+					write_mode);
+		} else  {
+			ret = 0;
+		}
 		if (ret) {
 			long long i;
 			if (errno != EIO) {
-- 
2.5.5