(no subject)

[Stanley.Miao]

During some BSP development process, I found some tools in mtd-utils
contain legacy interfaces and it is not suitable with the current linux
kernel. I post my patches here in case somebody else need them.

[PATCH 1/5] clean up the legacy interfaces in nandwrite.c

[Stanley.Miao]

The ioctl command "MEMSETOOBSEL" doesn't exist in the current linux kernel
anymore, and the arguments "forcelegacy", "jffs2", "yaffs", "noecc" are not
suitable with the current linux kernel, so clean them up.

Although the ioctl comand "MEMGETOOBSEL" still exist in the current linux
kernel, it is not suitable with some platforms with the NAND ECC data longer
than 32 bytes, so replace it with the new command "ECCGETLAYOUT"

Signed-off-by: Stanley.Miao <stanley.miao@windriver.com>
---
 nandwrite.c |  181 +++++-----------------------------------------------------
 1 files changed, 16 insertions(+), 165 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index b77edd6..c66eda0 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -45,26 +45,6 @@
 #define MAX_PAGE_SIZE	4096
 #define MAX_OOB_SIZE	128
 
-// oob layouts to pass into the kernel as default
-static struct nand_oobinfo none_oobinfo = {
-	.useecc = MTD_NANDECC_OFF,
-};
-
-static struct nand_oobinfo jffs2_oobinfo = {
-	.useecc = MTD_NANDECC_PLACE,
-	.eccbytes = 6,
-	.eccpos = { 0, 1, 2, 3, 6, 7 }
-};
-
-static struct nand_oobinfo yaffs_oobinfo = {
-	.useecc = MTD_NANDECC_PLACE,
-	.eccbytes = 6,
-	.eccpos = { 8, 9, 10, 13, 14, 15}
-};
-
-static struct nand_oobinfo autoplace_oobinfo = {
-	.useecc = MTD_NANDECC_AUTOPLACE
-};
 
 static void display_help (void)
 {
@@ -72,13 +52,7 @@ static void display_help (void)
 "Usage: nandwrite [OPTION] MTD_DEVICE [INPUTFILE|-]\n"
 "Writes to the specified MTD device.\n"
 "\n"
-"  -a, --autoplace         Use auto oob layout\n"
-"  -j, --jffs2             Force jffs2 oob layout (legacy support)\n"
-"  -y, --yaffs             Force yaffs oob layout (legacy support)\n"
-"  -f, --forcelegacy       Force legacy support on autoplacement-enabled mtd\n"
-"                          device\n"
 "  -m, --markbad           Mark blocks bad if write fails\n"
-"  -n, --noecc             Write without ecc\n"
 "  -o, --oob               Image contains oob data\n"
 "  -s addr, --start=addr   Set start address (default is 0)\n"
 "  -p, --pad               Pad to page size\n"
@@ -110,12 +84,7 @@ static const char	*mtd_device, *img;
 static int		mtdoffset = 0;
 static bool		quiet = false;
 static bool		writeoob = false;
-static bool		autoplace = false;
 static bool		markbad = false;
-static bool		forcejffs2 = false;
-static bool		forceyaffs = false;
-static bool		forcelegacy = false;
-static bool		noecc = false;
 static bool		pad = false;
 static int		blockalign = 1; /*default to using 16K block size */
 
@@ -125,21 +94,16 @@ static void process_options (int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "ab:fjmnopqs:y";
+		static const char *short_options = "b:mopqs:";
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
-			{"autoplace", no_argument, 0, 'a'},
 			{"blockalign", required_argument, 0, 'b'},
-			{"forcelegacy", no_argument, 0, 'f'},
-			{"jffs2", no_argument, 0, 'j'},
 			{"markbad", no_argument, 0, 'm'},
-			{"noecc", no_argument, 0, 'n'},
 			{"oob", no_argument, 0, 'o'},
 			{"pad", no_argument, 0, 'p'},
 			{"quiet", no_argument, 0, 'q'},
 			{"start", required_argument, 0, 's'},
-			{"yaffs", no_argument, 0, 'y'},
 			{0, 0, 0, 0},
 		};
 
@@ -163,21 +127,6 @@ static void process_options (int argc, char * const argv[])
 			case 'q':
 				quiet = true;
 				break;
-			case 'a':
-				autoplace = true;
-				break;
-			case 'j':
-				forcejffs2 = true;
-				break;
-			case 'y':
-				forceyaffs = true;
-				break;
-			case 'f':
-				forcelegacy = true;
-				break;
-			case 'n':
-				noecc = true;
-				break;
 			case 'm':
 				markbad = true;
 				break;
@@ -251,8 +200,7 @@ int main(int argc, char * const argv[])
 	struct mtd_oob_buf oob;
 	loff_t offs;
 	int ret;
-	int oobinfochanged = 0;
-	struct nand_oobinfo old_oobinfo;
+	struct nand_ecclayout ecclayout;
 	bool failed = true;
 	// contains all the data read from the file so far for the current eraseblock
 	unsigned char *filebuf = NULL;
@@ -300,86 +248,16 @@ int main(int argc, char * const argv[])
 		exit (EXIT_FAILURE);
 	}
 
-	if (autoplace) {
-		/* Read the current oob info */
-		if (ioctl (fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
-			perror ("MEMGETOOBSEL");
-			close (fd);
-			exit (EXIT_FAILURE);
-		}
-
-		// autoplace ECC ?
-		if (autoplace && (old_oobinfo.useecc != MTD_NANDECC_AUTOPLACE)) {
-
-			if (ioctl (fd, MEMSETOOBSEL, &autoplace_oobinfo) != 0) {
-				perror ("MEMSETOOBSEL");
-				close (fd);
-				exit (EXIT_FAILURE);
-			}
-			oobinfochanged = 1;
-		}
-	}
-
-	if (noecc)  {
-		ret = ioctl(fd, MTDFILEMODE, (void *) MTD_MODE_RAW);
-		if (ret == 0) {
-			oobinfochanged = 2;
-		} else {
-			switch (errno) {
-			case ENOTTY:
-				if (ioctl (fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
-					perror ("MEMGETOOBSEL");
-					close (fd);
-					exit (EXIT_FAILURE);
-				}
-				if (ioctl (fd, MEMSETOOBSEL, &none_oobinfo) != 0) {
-					perror ("MEMSETOOBSEL");
-					close (fd);
-					exit (EXIT_FAILURE);
-				}
-				oobinfochanged = 1;
-				break;
-			default:
-				perror ("MTDFILEMODE");
-				close (fd);
-				exit (EXIT_FAILURE);
-			}
-		}
-	}
-
-	/*
-	 * force oob layout for jffs2 or yaffs ?
-	 * Legacy support
-	 */
-	if (forcejffs2 || forceyaffs) {
-		struct nand_oobinfo *oobsel = forcejffs2 ? &jffs2_oobinfo : &yaffs_oobinfo;
 
-		if (autoplace) {
-			fprintf(stderr, "Autoplacement is not possible for legacy -j/-y options\n");
-			goto restoreoob;
-		}
-		if ((old_oobinfo.useecc == MTD_NANDECC_AUTOPLACE) && !forcelegacy) {
-			fprintf(stderr, "Use -f option to enforce legacy placement on autoplacement enabled mtd device\n");
-			goto restoreoob;
-		}
-		if (meminfo.oobsize == 8) {
-			if (forceyaffs) {
-				fprintf (stderr, "YAFSS cannot operate on 256 Byte page size");
-				goto restoreoob;
-			}
-			/* Adjust number of ecc bytes */
-			jffs2_oobinfo.eccbytes = 3;
-		}
+	oob.length = meminfo.oobsize;
+	oob.ptr =  oobbuf;
 
-		if (ioctl (fd, MEMSETOOBSEL, oobsel) != 0) {
-			perror ("MEMSETOOBSEL");
-			goto restoreoob;
-		}
+	if (ioctl(fd, ECCGETLAYOUT, &ecclayout) != 0) {
+		perror("ECCGETLAYOUT");
+		close(fd);
+		exit(EXIT_FAILURE);
 	}
 
-	oob.length = meminfo.oobsize;
-	oob.ptr = noecc ? oobreadbuf : oobbuf;
-
 	/* Determine if we are reading from standard input or from a file. */
 	if (strcmp(img, standard_input) == 0) {
 		ifd = STDIN_FILENO;
@@ -543,6 +421,7 @@ int main(int argc, char * const argv[])
 		}
 
 		if (writeoob) {
+			int i, start, len;
 			oobreadbuf = writebuf + meminfo.writesize;
 
 			// Read more data for the OOB from the input if there isn't enough in the buffer
@@ -579,34 +458,13 @@ int main(int argc, char * const argv[])
 				}
 			}
 
-			if (noecc) {
-				oob.ptr = oobreadbuf;
-			} else {
-				int i, start, len;
-				/*
-				 *  We use autoplacement and have the oobinfo with the autoplacement
-				 * information from the kernel available
-				 *
-				 * Modified to support out of order oobfree segments,
-				 * such as the layout used by diskonchip.c
-				 */
-				if (!oobinfochanged && (old_oobinfo.useecc == MTD_NANDECC_AUTOPLACE)) {
-					for (i = 0;old_oobinfo.oobfree[i][1]; i++) {
-						/* Set the reserved bytes to 0xff */
-						start = old_oobinfo.oobfree[i][0];
-						len = old_oobinfo.oobfree[i][1];
-						memcpy(oobbuf + start,
-								oobreadbuf + start,
-								len);
-					}
-				} else {
-					/* Set at least the ecc byte positions to 0xff */
-					start = old_oobinfo.eccbytes;
-					len = meminfo.oobsize - start;
-					memcpy(oobbuf + start,
-							oobreadbuf + start,
-							len);
-				}
+			for (i = 0; ecclayout.oobfree[i].length; i++) {
+				/* Set the reserved bytes to 0xff */
+				start = ecclayout.oobfree[i].offset;
+				len = ecclayout.oobfree[i].length;
+				memcpy(oobbuf + start,
+						oobreadbuf + start,
+						len);
 			}
 			/* Write OOB data first, as ecc will be placed in there*/
 			oob.start = mtdoffset;
@@ -666,13 +524,6 @@ closeall:
 	close(ifd);
 
 restoreoob:
-	if (oobinfochanged == 1) {
-		if (ioctl (fd, MEMSETOOBSEL, &old_oobinfo) != 0) {
-			perror ("MEMSETOOBSEL");
-			close (fd);
-			exit (EXIT_FAILURE);
-		}
-	}
 
 	close(fd);
 
-- 
1.5.4.3

[PATCH 2/5] check if the start address is page-aligned.

[Stanley.Miao]

Only page-aligned address is permitted in NAND subsystem.

Signed-off-by: Stanley.Miao <stanley.miao@windriver.com>
---
 nandwrite.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index c66eda0..c66ab54 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -248,6 +248,13 @@ int main(int argc, char * const argv[])
 		exit (EXIT_FAILURE);
 	}
 
+	if (mtdoffset & (meminfo.writesize - 1)) {
+		fprintf(stderr, "The start address is not page-aligned !\n"
+				"The pagesize of this NAND Flash is 0x%x.\n",
+				meminfo.writesize);
+		close(fd);
+		exit(EXIT_FAILURE);
+	}
 
 	oob.length = meminfo.oobsize;
 	oob.ptr =  oobbuf;
-- 
1.5.4.3

[PATCH 3/5] Fix the bug of writing a yaffs2 image to NAND

[Stanley.Miao]

The tool mkyaffs2image doesn't know the oob layout of a NAND flash, so it
puts the yaffs2 tags at the offset 0 of oob area. When nandwrite writes the
image into NAND flash, it should put the yaffs2 tags at the right position
according to the NAND oob layout.

Signed-off-by: Stanley.Miao <stanley.miao@windriver.com>
---
 nandwrite.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index c66ab54..35f1e4d 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -429,6 +429,8 @@ int main(int argc, char * const argv[])
 
 		if (writeoob) {
 			int i, start, len;
+			int tags_pos = 0;
+
 			oobreadbuf = writebuf + meminfo.writesize;
 
 			// Read more data for the OOB from the input if there isn't enough in the buffer
@@ -470,8 +472,8 @@ int main(int argc, char * const argv[])
 				start = ecclayout.oobfree[i].offset;
 				len = ecclayout.oobfree[i].length;
 				memcpy(oobbuf + start,
-						oobreadbuf + start,
-						len);
+						oobreadbuf + tags_pos, len);
+				tags_pos += len;
 			}
 			/* Write OOB data first, as ecc will be placed in there*/
 			oob.start = mtdoffset;
-- 
1.5.4.3

[PATCH 4/5] Discard the legacy interface MEMGETOOBSEL in flash_eraseall

[Stanley.Miao]

The ioctl command "MEMGETOOBSEL" is not suitable with some platforms with
the NAND ECC data longer than 32 bytes, so replace it with the new command
"ECCGETLAYOUT".

Signed-off-by: Stanley.Miao <stanley.miao@windriver.com>
---
 flash_eraseall.c |   42 ++++++++++++------------------------------
 1 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/flash_eraseall.c b/flash_eraseall.c
index a22fc49..1842906 100644
--- a/flash_eraseall.c
+++ b/flash_eraseall.c
@@ -84,41 +84,23 @@ int main (int argc, char *argv[])
 		if (!isNAND)
 			cleanmarker.totlen = cpu_to_je32 (sizeof (struct jffs2_unknown_node));
 		else {
-			struct nand_oobinfo oobinfo;
+			struct nand_ecclayout ecclayout;
 
-			if (ioctl(fd, MEMGETOOBSEL, &oobinfo) != 0) {
-				fprintf(stderr, "%s: %s: unable to get NAND oobinfo\n", exe_name, mtd_device);
+			if (ioctl(fd, ECCGETLAYOUT, &ecclayout) != 0) {
+				fprintf(stderr,	"%s: %s: unable to get NAND oob layout\n",
+						exe_name, mtd_device);
 				return 1;
 			}
 
-			/* Check for autoplacement */
-			if (oobinfo.useecc == MTD_NANDECC_AUTOPLACE) {
-				/* Get the position of the free bytes */
-				if (!oobinfo.oobfree[0][1]) {
-					fprintf (stderr, " Eeep. Autoplacement selected and no empty space in oob\n");
-					return 1;
-				}
-				clmpos = oobinfo.oobfree[0][0];
-				clmlen = oobinfo.oobfree[0][1];
-				if (clmlen > 8)
-					clmlen = 8;
-			} else {
-				/* Legacy mode */
-				switch (meminfo.oobsize) {
-					case 8:
-						clmpos = 6;
-						clmlen = 2;
-						break;
-					case 16:
-						clmpos = 8;
-						clmlen = 8;
-						break;
-					case 64:
-						clmpos = 16;
-						clmlen = 8;
-						break;
-				}
+			/* Get the position of the free bytes */
+			if (!ecclayout.oobfree[0].length) {
+				fprintf(stderr, " Eeep. Autoplacement selected and no empty space in oob\n");
+				return 1;
 			}
+			clmpos = ecclayout.oobfree[0].offset;
+			clmlen = ecclayout.oobfree[0].length;
+			if (clmlen > 8)
+				clmlen = 8;
 			cleanmarker.totlen = cpu_to_je32(8);
 		}
 		cleanmarker.hdr_crc =  cpu_to_je32 (crc32 (0, &cleanmarker,  sizeof (struct jffs2_unknown_node) - 4));
-- 
1.5.4.3

[PATCH 5/5] Place the cleanmarker in OOB area according to the mode MTD_OOB_AUTO

[Stanley.Miao]

In the current linux kernel, Jffs2 writes the cleanmarker according to the
mode MTD_OOB_AUTO, so modify the layout of the cleanmarker from
MTD_OOB_PLACE to MTD_OOB_AUTO in flash_eraseall.

Signed-off-by: Stanley.Miao <stanley.miao@windriver.com>
---
 flash_eraseall.c |   38 ++++++++++++++++++++------------------
 1 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/flash_eraseall.c b/flash_eraseall.c
index 1842906..d2d0bcb 100644
--- a/flash_eraseall.c
+++ b/flash_eraseall.c
@@ -34,7 +34,7 @@
 #include <time.h>
 #include <getopt.h>
 #include <sys/ioctl.h>
-#include <sys/mount.h>
+#include <sys/param.h>
 #include "crc32.h"
 
 #include <mtd/mtd-user.h>
@@ -58,9 +58,11 @@ int target_endian = __BYTE_ORDER;
 int main (int argc, char *argv[])
 {
 	mtd_info_t meminfo;
-	int fd, clmpos = 0, clmlen = 8;
+	int fd;
 	erase_info_t erase;
 	int isNAND, bbtest = 1;
+	unsigned char oobbuf[128];
+	memset(oobbuf, 0xFF, 128);
 
 	process_options(argc, argv);
 
@@ -81,29 +83,29 @@ int main (int argc, char *argv[])
 	if (jffs2) {
 		cleanmarker.magic = cpu_to_je16 (JFFS2_MAGIC_BITMASK);
 		cleanmarker.nodetype = cpu_to_je16 (JFFS2_NODETYPE_CLEANMARKER);
-		if (!isNAND)
+		if (!isNAND) {
 			cleanmarker.totlen = cpu_to_je32 (sizeof (struct jffs2_unknown_node));
-		else {
+			cleanmarker.hdr_crc = cpu_to_je32(crc32(0, &cleanmarker,  sizeof(struct jffs2_unknown_node) - 4));
+		} else {
+			int already_read, num, i, start;
 			struct nand_ecclayout ecclayout;
 
+			cleanmarker.totlen = cpu_to_je32(8);
+			cleanmarker.hdr_crc = cpu_to_je32(crc32(0, &cleanmarker,  sizeof(struct jffs2_unknown_node) - 4));
+
 			if (ioctl(fd, ECCGETLAYOUT, &ecclayout) != 0) {
 				fprintf(stderr,	"%s: %s: unable to get NAND oob layout\n",
 						exe_name, mtd_device);
 				return 1;
 			}
-
-			/* Get the position of the free bytes */
-			if (!ecclayout.oobfree[0].length) {
-				fprintf(stderr, " Eeep. Autoplacement selected and no empty space in oob\n");
-				return 1;
+			already_read = 0;
+			for (i = 0; (already_read < 8) && ecclayout.oobfree[i].length; i++) {
+				num = MIN(8 - already_read, ecclayout.oobfree[i].length);
+				start = ecclayout.oobfree[i].offset;
+				memcpy(oobbuf + start, (unsigned char *)&cleanmarker + already_read, num);
+				already_read += num;
 			}
-			clmpos = ecclayout.oobfree[0].offset;
-			clmlen = ecclayout.oobfree[0].length;
-			if (clmlen > 8)
-				clmlen = 8;
-			cleanmarker.totlen = cpu_to_je32(8);
 		}
-		cleanmarker.hdr_crc =  cpu_to_je32 (crc32 (0, &cleanmarker,  sizeof (struct jffs2_unknown_node) - 4));
 	}
 
 	for (erase.start = 0; erase.start < meminfo.size; erase.start += meminfo.erasesize) {
@@ -143,9 +145,9 @@ int main (int argc, char *argv[])
 		/* write cleanmarker */
 		if (isNAND) {
 			struct mtd_oob_buf oob;
-			oob.ptr = (unsigned char *) &cleanmarker;
-			oob.start = erase.start + clmpos;
-			oob.length = clmlen;
+			oob.ptr = oobbuf;
+			oob.start = erase.start;
+			oob.length = meminfo.oobsize;
 			if (ioctl (fd, MEMWRITEOOB, &oob) != 0) {
 				fprintf(stderr, "\n%s: %s: MTD writeoob failure: %s\n", exe_name, mtd_device, strerror(errno));
 				continue;
-- 
1.5.4.3

Re:

[Mike Frysinger]

On Tue, Feb 2, 2010 at 21:56, Stanley.Miao wrote:
> During some BSP development process, I found some tools in mtd-utils
> contain legacy interfaces and it is not suitable with the current linux
> kernel. I post my patches here in case somebody else need them.

these are good, but i think people might want to keep support for
older kernels around.  so you'd have the code try the newer ioctls
first and if those fail, fall back to the old ones.
-mike

Re:

[stanley.miao]

Mike Frysinger wrote:
> On Tue, Feb 2, 2010 at 21:56, Stanley.Miao wrote:
>   
>> During some BSP development process, I found some tools in mtd-utils
>> contain legacy interfaces and it is not suitable with the current linux
>> kernel. I post my patches here in case somebody else need them.
>>     
>
> these are good, but i think people might want to keep support for
> older kernels around.  so you'd have the code try the newer ioctls
> first and if those fail, fall back to the old ones.
> -mike
>
>   
Yeah, we should keep it compatible ASAP. But in the conflict occasion, I 
think, the
new kernel should get the higher priorities than the older kernel. The 
people who
use the old kernels can use the old version mtd-utils(not very old, 
1.3.1 or earlier).

Stanley.