[PATCH 1/2] mtd-utils: more style fixups

[PATCH 1/2] mtd-utils: more style fixups

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c  |    7 +++----
 nandwrite.c |    6 +++---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index bb649da..6428bc4 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -308,7 +308,7 @@ int main(int argc, char * const argv[])
 	oob.ptr = oobbuf;
 
 	if (noecc)  {
-		ret = ioctl(fd, MTDFILEMODE, (void *) MTD_MODE_RAW);
+		ret = ioctl(fd, MTDFILEMODE, (void *)MTD_MODE_RAW);
 		if (ret == 0) {
 			oobinfochanged = 2;
 		} else {
@@ -330,7 +330,6 @@ int main(int argc, char * const argv[])
 			}
 		}
 	} else {
-
 		/* check if we can read ecc stats */
 		if (!ioctl(fd, ECCGETSTATS, &stat1)) {
 			eccstats = true;
@@ -375,7 +374,6 @@ int main(int argc, char * const argv[])
 	bs = meminfo.writesize;
 
 	/* Print informative message */
-
 	if (!quiet) {
 		fprintf(stderr, "Block size %u, page size %u, OOB size %u\n",
 				meminfo.erasesize, meminfo.writesize, meminfo.oobsize);
@@ -383,6 +381,7 @@ int main(int argc, char * const argv[])
 				"Dumping data starting at 0x%08x and ending at 0x%08x...\n",
 				(unsigned int)start_addr, (unsigned int)end_addr);
 	}
+
 	/* Dump the flash contents */
 	for (ofs = start_addr; ofs < end_addr; ofs += bs) {
 		/* Check for bad block */
@@ -401,7 +400,7 @@ int main(int argc, char * const argv[])
 		if (badblock) {
 			if (omitbad)
 				continue;
-			memset (readbuf, 0xff, bs);
+			memset(readbuf, 0xff, bs);
 		} else {
 			/* Read page data and exit on failure */
 			if (pread(fd, readbuf, bs, ofs) != bs) {
diff --git a/nandwrite.c b/nandwrite.c
index b5745b9..fe03115 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -592,14 +592,14 @@ int main(int argc, char * const argv[])
 				int i, start, len;
 				int tags_pos = 0;
 				/*
-				 *  We use autoplacement and have the oobinfo with the autoplacement
+				 * 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++) {
+					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];
@@ -620,7 +620,7 @@ int main(int argc, char * const argv[])
 							len);
 				}
 			}
-			/* Write OOB data first, as ecc will be placed in there*/
+			/* Write OOB data first, as ecc will be placed in there */
 			oob.start = mtdoffset;
 			if (ioctl(fd, MEMWRITEOOB, &oob) != 0) {
 				perror("ioctl(MEMWRITEOOB)");
-- 
1.7.0.4

[PATCH 2/2] mtd-utils: nanddump: add 64-bit support, utilize libmtd

[Brian Norris]

Adds support for 64-bit offsets (i.e., devices larger than 4GB).
Calls to ioctls are mostly replaced by libmtd interfaces; however,
a few remain and probably should be handled with more robust
interfaces, as some of these ioctls are considered "legacy."

Similar updates to nandwrite will come shortly.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c  |  117 +++++++++++++++++++++++++++++------------------------------
 nandwrite.c |    7 ++--
 2 files changed, 60 insertions(+), 64 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 6428bc4..55dbd37 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -33,6 +33,7 @@
 #include <asm/types.h>
 #include <mtd/mtd-user.h>
 #include "common.h"
+#include <libmtd.h>
 
 static struct nand_oobinfo none_oobinfo = {
 	.useecc = MTD_NANDECC_OFF,
@@ -77,18 +78,18 @@ static void display_version(void)
 
 // Option variables
 
-static bool		pretty_print = false;	// print nice
-static bool		noecc = false;		// don't error correct
-static bool		noskipbad = false;	// don't skip bad blocks
-static bool		omitoob = false;	// omit oob data
-static unsigned long	start_addr;		// start address
-static unsigned long	length;			// dump length
-static const char	*mtddev;		// mtd device name
-static const char	*dumpfile;		// dump file name
-static bool		omitbad = false;
-static bool		quiet = false;		// suppress diagnostic output
-static bool		canonical = false;	// print nice + ascii
-static bool		forcebinary = false;	// force printing binary to tty
+static bool			pretty_print = false;	// print nice
+static bool			noecc = false;		// don't error correct
+static bool			noskipbad = false;	// don't skip bad blocks
+static bool			omitoob = false;	// omit oob data
+static unsigned long long	start_addr;		// start address
+static unsigned long		length;			// dump length
+static const char		*mtddev;		// mtd device name
+static const char		*dumpfile;		// dump file name
+static bool			omitbad = false;
+static bool			quiet = false;		// suppress diagnostic output
+static bool			canonical = false;	// print nice + ascii
+static bool			forcebinary = false;	// force printing binary to tty
 
 static void process_options(int argc, char * const argv[])
 {
@@ -135,7 +136,7 @@ static void process_options(int argc, char * const argv[])
 				omitbad = true;
 				break;
 			case 's':
-				start_addr = strtol(optarg, NULL, 0);
+				start_addr = strtoull(optarg, NULL, 0);
 				break;
 			case 'f':
 				if (!(dumpfile = strdup(optarg))) {
@@ -221,18 +222,16 @@ static void process_options(int argc, char * const argv[])
  */
 static void pretty_dump_to_buffer(const unsigned char *buf, size_t len,
 		char *linebuf, size_t linebuflen, bool pagedump, bool ascii,
-		unsigned int prefix)
+		unsigned long long prefix)
 {
 	static const char hex_asc[] = "0123456789abcdef";
 	unsigned char ch;
-	int j, lx = 0;
-	int ascii_column;
+	unsigned int j, lx = 0, ascii_column;
 
 	if (pagedump)
-		sprintf(linebuf, "0x%.8x: ", prefix);
+		lx += sprintf(linebuf, "0x%.8llx: ", prefix);
 	else
-		sprintf(linebuf, "  OOB Data: ");
-	lx += 12;
+		lx += sprintf(linebuf, "  OOB Data: ");
 
 	if (!len)
 		goto nil;
@@ -253,8 +252,10 @@ static void pretty_dump_to_buffer(const unsigned char *buf, size_t len,
 	if (!ascii)
 		goto nil;
 
-	while (lx < (linebuflen - 1) && lx < (ascii_column - 1))
+	do {
 		linebuf[lx++] = ' ';
+	} while (lx < (linebuflen - 1) && lx < (ascii_column - 1));
+
 	linebuf[lx++] = '|';
 	for (j = 0; (j < len) && (lx + 2) < linebuflen; j++)
 		linebuf[lx++] = (isascii(buf[j]) && isprint(buf[j])) ? buf[j]
@@ -271,20 +272,25 @@ nil:
  */
 int main(int argc, char * const argv[])
 {
-	unsigned long ofs, end_addr = 0;
+	unsigned long long ofs, end_addr = 0;
 	unsigned long long blockstart = 1;
 	int ret, i, fd, ofd = 0, bs, badblock = 0;
-	struct mtd_oob_buf oob;
-	mtd_info_t meminfo;
+	struct mtd_dev_info mtd;
 	char pretty_buf[PRETTY_BUF_LEN];
 	int oobinfochanged = 0, firstblock = 1;
 	struct nand_oobinfo old_oobinfo;
 	struct mtd_ecc_stats stat1, stat2;
 	bool eccstats = false;
 	unsigned char *readbuf = NULL, *oobbuf = NULL;
+	libmtd_t mtd_desc;
 
 	process_options(argc, argv);
 
+	/* Initialize libmtd */
+	mtd_desc = libmtd_open();
+	if (!mtd_desc)
+		return errmsg("can't initialize libmtd");
+
 	/* Open MTD device */
 	if ((fd = open(mtddev, O_RDONLY)) == -1) {
 		perror(mtddev);
@@ -292,20 +298,12 @@ int main(int argc, char * const argv[])
 	}
 
 	/* Fill in MTD device capability structure */
-	if (ioctl(fd, MEMGETINFO, &meminfo) != 0) {
-		perror("MEMGETINFO");
-		close(fd);
-		exit(EXIT_FAILURE);
-	}
+	if (mtd_get_dev_info(mtd_desc, mtddev, &mtd) < 0)
+		return errmsg("mtd_get_dev_info failed");
 
 	/* Allocate buffers */
-	oobbuf = xmalloc(sizeof(oobbuf) * meminfo.oobsize);
-	readbuf = xmalloc(sizeof(readbuf) * meminfo.writesize);
-
-	/* Fill in oob info */
-	oob.start = 0;
-	oob.length = meminfo.oobsize;
-	oob.ptr = oobbuf;
+	oobbuf = xmalloc(sizeof(oobbuf) * mtd.oob_size);
+	readbuf = xmalloc(sizeof(readbuf) * mtd.min_io_size);
 
 	if (noecc)  {
 		ret = ioctl(fd, MTDFILEMODE, (void *)MTD_MODE_RAW);
@@ -359,27 +357,27 @@ int main(int argc, char * const argv[])
 	}
 
 	/* Initialize start/end addresses and block size */
-	if (start_addr & (meminfo.writesize - 1)) {
+	if (start_addr & (mtd.min_io_size - 1)) {
 		fprintf(stderr, "WARNING: The start address is not page-aligned !\n"
 				"The pagesize of this NAND Flash is 0x%x.\n"
 				"nandwrite doesn't allow writes starting at this location.\n"
 				"Future versions of nanddump will fail here.\n",
-				meminfo.writesize);
+				mtd.min_io_size);
 	}
 	if (length)
 		end_addr = start_addr + length;
-	if (!length || end_addr > meminfo.size)
-		end_addr = meminfo.size;
+	if (!length || end_addr > mtd.size)
+		end_addr = mtd.size;
 
-	bs = meminfo.writesize;
+	bs = mtd.min_io_size;
 
 	/* Print informative message */
 	if (!quiet) {
 		fprintf(stderr, "Block size %u, page size %u, OOB size %u\n",
-				meminfo.erasesize, meminfo.writesize, meminfo.oobsize);
+				mtd.eb_size, mtd.min_io_size, mtd.oob_size);
 		fprintf(stderr,
-				"Dumping data starting at 0x%08x and ending at 0x%08x...\n",
-				(unsigned int)start_addr, (unsigned int)end_addr);
+				"Dumping data starting at 0x%08llx and ending at 0x%08llx...\n",
+				start_addr, end_addr);
 	}
 
 	/* Dump the flash contents */
@@ -387,12 +385,12 @@ int main(int argc, char * const argv[])
 		/* Check for bad block */
 		if (noskipbad) {
 			badblock = 0;
-		} else if (blockstart != (ofs & (~meminfo.erasesize + 1)) ||
+		} else if (blockstart != (ofs & (~mtd.eb_size + 1)) ||
 				firstblock) {
-			blockstart = ofs & (~meminfo.erasesize + 1);
+			blockstart = ofs & (~mtd.eb_size + 1);
 			firstblock = 0;
-			if ((badblock = ioctl(fd, MEMGETBADBLOCK, &blockstart)) < 0) {
-				perror("ioctl(MEMGETBADBLOCK)");
+			if ((badblock = mtd_is_bad(&mtd, fd, ofs / mtd.eb_size)) < 0) {
+				errmsg("libmtd: mtd_is_bad");
 				goto closeall;
 			}
 		}
@@ -403,8 +401,8 @@ int main(int argc, char * const argv[])
 			memset(readbuf, 0xff, bs);
 		} else {
 			/* Read page data and exit on failure */
-			if (pread(fd, readbuf, bs, ofs) != bs) {
-				perror("pread");
+			if (mtd_read(&mtd, fd, ofs / mtd.eb_size, ofs % mtd.eb_size, readbuf, bs)) {
+				errmsg("mtd_read");
 				goto closeall;
 			}
 		}
@@ -417,11 +415,11 @@ int main(int argc, char * const argv[])
 			}
 			if (stat1.failed != stat2.failed)
 				fprintf(stderr, "ECC: %d uncorrectable bitflip(s)"
-						" at offset 0x%08lx\n",
+						" at offset 0x%08llx\n",
 						stat2.failed - stat1.failed, ofs);
 			if (stat1.corrected != stat2.corrected)
 				fprintf(stderr, "ECC: %d corrected bitflip(s) at"
-						" offset 0x%08lx\n",
+						" offset 0x%08llx\n",
 						stat2.corrected - stat1.corrected, ofs);
 			stat1 = stat2;
 		}
@@ -429,8 +427,8 @@ int main(int argc, char * const argv[])
 		/* Write out page data */
 		if (pretty_print) {
 			for (i = 0; i < bs; i += PRETTY_ROW_SIZE) {
-				pretty_dump_to_buffer(readbuf+i, PRETTY_ROW_SIZE,
-						pretty_buf, PRETTY_BUF_LEN, true, canonical, ofs+i);
+				pretty_dump_to_buffer(readbuf + i, PRETTY_ROW_SIZE,
+						pretty_buf, PRETTY_BUF_LEN, true, canonical, ofs + i);
 				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
 		} else
@@ -440,25 +438,24 @@ int main(int argc, char * const argv[])
 			continue;
 
 		if (badblock) {
-			memset(oobbuf, 0xff, meminfo.oobsize);
+			memset(oobbuf, 0xff, mtd.oob_size);
 		} else {
 			/* Read OOB data and exit on failure */
-			oob.start = ofs;
-			if (ioctl(fd, MEMREADOOB, &oob) != 0) {
-				perror("ioctl(MEMREADOOB)");
+			if (mtd_read_oob(mtd_desc, &mtd, fd, ofs, mtd.oob_size, oobbuf)) {
+				errmsg("libmtd: mtd_read_oob");
 				goto closeall;
 			}
 		}
 
 		/* Write out OOB data */
 		if (pretty_print) {
-			for (i = 0; i < meminfo.oobsize; i += 16) {
-				pretty_dump_to_buffer(oobbuf+i, meminfo.oobsize-i,
+			for (i = 0; i < mtd.oob_size; i += 16) {
+				pretty_dump_to_buffer(oobbuf + i, mtd.oob_size - i,
 						pretty_buf, PRETTY_BUF_LEN, false, canonical, 0);
 				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
 		} else
-			write(ofd, oobbuf, meminfo.oobsize);
+			write(ofd, oobbuf, mtd.oob_size);
 	}
 
 	/* reset oobinfo */
diff --git a/nandwrite.c b/nandwrite.c
index fe03115..9eb63a4 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -108,7 +108,7 @@ static void display_version(void)
 
 static const char	*standard_input = "-";
 static const char	*mtd_device, *img;
-static int		mtdoffset = 0;
+static unsigned int	mtdoffset = 0;
 static bool		quiet = false;
 static bool		writeoob = false;
 static bool		rawoob = false;
@@ -200,7 +200,7 @@ static void process_options(int argc, char * const argv[])
 				writeoob = true;
 				break;
 			case 's':
-				mtdoffset = strtol(optarg, NULL, 0);
+				mtdoffset = strtoul(optarg, NULL, 0);
 				break;
 			case 'b':
 				blockalign = atoi(optarg);
@@ -318,7 +318,6 @@ int main(int argc, char * const argv[])
 
 		// autoplace ECC ?
 		if (old_oobinfo.useecc != MTD_NANDECC_AUTOPLACE) {
-
 			if (ioctl(fd, MEMSETOOBSEL, &autoplace_oobinfo) != 0) {
 				perror("MEMSETOOBSEL");
 				close(fd);
@@ -480,7 +479,7 @@ int main(int argc, char * const argv[])
 			if (noskipbad)
 				continue;
 			do {
-				if ((ret = ioctl(fd, MEMGETBADBLOCK, &offs)) < 0) {
+				if ((ret = 0 /*ioctl(fd, MEMGETBADBLOCK, &offs)*/) < 0) {
 					perror("ioctl(MEMGETBADBLOCK)");
 					goto closeall;
 				}
-- 
1.7.0.4

Re: [PATCH 1/2] mtd-utils: more style fixups

[Mike Frysinger]

On Wed, Oct 20, 2010 at 02:45, Brian Norris wrote:
> -               ret = ioctl(fd, MTDFILEMODE, (void *) MTD_MODE_RAW);
> +               ret = ioctl(fd, MTDFILEMODE, (void *)MTD_MODE_RAW);

i'd just drop the cast completely.  ioctl() is a vararg func so there
is no need to tweak the type.
-mike

Re: [PATCH 2/2] mtd-utils: nanddump: add 64-bit support, utilize libmtd

[Mike Frysinger]

On Wed, Oct 20, 2010 at 02:45, Brian Norris wrote:
> -static bool            pretty_print = false;   // print nice
> -static bool            noecc = false;          // don't error correct
> -static bool            noskipbad = false;      // don't skip bad blocks
> -static bool            omitoob = false;        // omit oob data
> -static unsigned long   start_addr;             // start address
> -static unsigned long   length;                 // dump length
> -static const char      *mtddev;                // mtd device name
> -static const char      *dumpfile;              // dump file name
> -static bool            omitbad = false;
> -static bool            quiet = false;          // suppress diagnostic output
> -static bool            canonical = false;      // print nice + ascii
> -static bool            forcebinary = false;    // force printing binary to tty
> +static bool                    pretty_print = false;   // print nice
> +static bool                    noecc = false;          // don't error correct
> +static bool                    noskipbad = false;      // don't skip bad blocks
> +static bool                    omitoob = false;        // omit oob data
> +static unsigned long long      start_addr;             // start address
> +static unsigned long           length;                 // dump length
> +static const char              *mtddev;                // mtd device name
> +static const char              *dumpfile;              // dump file name
> +static bool                    omitbad = false;
> +static bool                    quiet = false;          // suppress diagnostic output
> +static bool                    canonical = false;      // print nice + ascii
> +static bool                    forcebinary = false;    // force printing binary to tty

only one of these lines are functional.  please fold the rest into
your "style fixup" patch.

> @@ -318,7 +318,6 @@ int main(int argc, char * const argv[])
>
>                // autoplace ECC ?
>                if (old_oobinfo.useecc != MTD_NANDECC_AUTOPLACE) {
> -
>                        if (ioctl(fd, MEMSETOOBSEL, &autoplace_oobinfo) != 0) {
>                                perror("MEMSETOOBSEL");
>                                close(fd);

same here

> @@ -480,7 +479,7 @@ int main(int argc, char * const argv[])
>                        if (noskipbad)
>                                continue;
>                        do {
> -                               if ((ret = ioctl(fd, MEMGETBADBLOCK, &offs)) < 0) {
> +                               if ((ret = 0 /*ioctl(fd, MEMGETBADBLOCK, &offs)*/) < 0) {
>                                        perror("ioctl(MEMGETBADBLOCK)");
>                                        goto closeall;
>                                }

doesnt seem to belong.  then again, none of the changes to this file
look like they belong in this patch.
-mike

Re: [PATCH 2/2] mtd-utils: nanddump: add 64-bit support, utilize libmtd

[Brian Norris]

Mike,

On 10/20/2010 12:01 AM, Mike Frysinger wrote:
> On Wed, Oct 20, 2010 at 02:45, Brian Norris wrote:
>> ...
>> -static unsigned long   start_addr;             // start address
>> ...
>> -static bool            forcebinary = false;    // force printing binary to tty
...
>> +static unsigned long long      start_addr;             // start address
...
>> +static bool                    forcebinary = false;    // force printing binary to tty
> 
> only one of these lines are functional.  please fold the rest into
> your "style fixup" patch.

True, except that they are not "style fixups" until after start_addr has
a longer data type (pun intended). I'll respin though.

>> @@ -480,7 +479,7 @@ int main(int argc, char * const argv[])
>>                        if (noskipbad)
>>                                continue;
>>                        do {
>> -                               if ((ret = ioctl(fd, MEMGETBADBLOCK, &offs)) < 0) {
>> +                               if ((ret = 0 /*ioctl(fd, MEMGETBADBLOCK, &offs)*/) < 0) {
>>                                        perror("ioctl(MEMGETBADBLOCK)");
>>                                        goto closeall;
>>                                }
> 
> doesnt seem to belong.  then again, none of the changes to this file
> look like they belong in this patch.

Wow, I really thought I had fixed this. Sorry, I will definitely respin.
I must have been in a hurry to send this out.

Still, despite my inability/failure to fully proofread my patches, most
of the patch *is* intentional and *does* belong, IMO. I will try to
document it better in the next patch description, but do you have
specifics on what you meant?

Thanks,
Brian

Re: [PATCH 2/2] mtd-utils: nanddump: add 64-bit support, utilize libmtd

[Mike Frysinger]

On Wed, Oct 20, 2010 at 10:03, Brian Norris wrote:
> but do you have specifics on what you meant?

this patch is titled "nanddump", so you shouldnt be modifying files
outside of nanddump.  nor should you be replacing a call to an ioctl()
with 0 without any comment whatsoever as to why.
-mike

[PATCH v2 0/2] mtd-utils fixups

[Brian Norris]

OK, this should be a decent rewrite. Again, I'm very sorry for the very
previous mangled patches. There was some laziness on my part compounded
by my misreading of Mike's responses.

Please provide any comments you have, especially regarding usage of libmtd
and even tips on replacing straight-up calls to ioctl. I'm sure I haven't
hit everything important.

Also, I'm working through nandwrite at the moment. It seems a tougher
slog though.

Thanks,
Brian

Brian Norris (2):
  mtd-utils: nanddump/nandwrite: style, signed-ness, printing fixups
  mtd-utils: nanddump: add 64-bit support, utilize libmtd

 nanddump.c  |  126 ++++++++++++++++++++++++++++------------------------------
 nandwrite.c |   28 +++++++-------
 2 files changed, 75 insertions(+), 79 deletions(-)

[PATCH v2 1/2] mtd-utils: nanddump/nandwrite: style, signed-ness, printing fixups

[Brian Norris]

There were some signed/unsigned integer comparisons. Their types were
changed for safety. Also, "strtol" was improperly used for unsigned
data types.

Nanddump's pretty print options needed a slight reformat to prepare for
printing offsets that are more than 32 bits (8 hex characters) wide.
This prevents overlap of output and ensures that at least one space is
printed between hex and ascii printouts. Perhaps this could use some
better alignment in the future.

Other fixes:
* Corrected several simple spacing issues
* Changed indentation of some global variable declarations in
  order to prepare for the next patch, which makes those
  declarations longer
* Used macro for PRETTY_ROW_SIZE instead of constant 16
* Reformatted, edited a multi-line comment
* Removed some unnecessary casts

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c  |   55 +++++++++++++++++++++++++++----------------------------
 nandwrite.c |   28 ++++++++++++++--------------
 2 files changed, 41 insertions(+), 42 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index bb649da..013eca0 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -77,18 +77,18 @@ static void display_version(void)
 
 // Option variables
 
-static bool		pretty_print = false;	// print nice
-static bool		noecc = false;		// don't error correct
-static bool		noskipbad = false;	// don't skip bad blocks
-static bool		omitoob = false;	// omit oob data
-static unsigned long	start_addr;		// start address
-static unsigned long	length;			// dump length
-static const char	*mtddev;		// mtd device name
-static const char	*dumpfile;		// dump file name
-static bool		omitbad = false;
-static bool		quiet = false;		// suppress diagnostic output
-static bool		canonical = false;	// print nice + ascii
-static bool		forcebinary = false;	// force printing binary to tty
+static bool			pretty_print = false;	// print nice
+static bool			noecc = false;		// don't error correct
+static bool			noskipbad = false;	// don't skip bad blocks
+static bool			omitoob = false;	// omit oob data
+static unsigned long		start_addr;		// start address
+static unsigned long		length;			// dump length
+static const char		*mtddev;		// mtd device name
+static const char		*dumpfile;		// dump file name
+static bool			omitbad = false;
+static bool			quiet = false;		// suppress diagnostic output
+static bool			canonical = false;	// print nice + ascii
+static bool			forcebinary = false;	// force printing binary to tty
 
 static void process_options(int argc, char * const argv[])
 {
@@ -135,7 +135,7 @@ static void process_options(int argc, char * const argv[])
 				omitbad = true;
 				break;
 			case 's':
-				start_addr = strtol(optarg, NULL, 0);
+				start_addr = strtoul(optarg, NULL, 0);
 				break;
 			case 'f':
 				if (!(dumpfile = strdup(optarg))) {
@@ -144,7 +144,7 @@ static void process_options(int argc, char * const argv[])
 				}
 				break;
 			case 'l':
-				length = strtol(optarg, NULL, 0);
+				length = strtoul(optarg, NULL, 0);
 				break;
 			case 'o':
 				omitoob = true;
@@ -225,14 +225,12 @@ static void pretty_dump_to_buffer(const unsigned char *buf, size_t len,
 {
 	static const char hex_asc[] = "0123456789abcdef";
 	unsigned char ch;
-	int j, lx = 0;
-	int ascii_column;
+	unsigned int j, lx = 0, ascii_column;
 
 	if (pagedump)
-		sprintf(linebuf, "0x%.8x: ", prefix);
+		lx += sprintf(linebuf, "0x%.8x: ", prefix);
 	else
-		sprintf(linebuf, "  OOB Data: ");
-	lx += 12;
+		lx += sprintf(linebuf, "  OOB Data: ");
 
 	if (!len)
 		goto nil;
@@ -253,8 +251,10 @@ static void pretty_dump_to_buffer(const unsigned char *buf, size_t len,
 	if (!ascii)
 		goto nil;
 
-	while (lx < (linebuflen - 1) && lx < (ascii_column - 1))
+	do {
 		linebuf[lx++] = ' ';
+	} while (lx < (linebuflen - 1) && lx < (ascii_column - 1));
+
 	linebuf[lx++] = '|';
 	for (j = 0; (j < len) && (lx + 2) < linebuflen; j++)
 		linebuf[lx++] = (isascii(buf[j]) && isprint(buf[j])) ? buf[j]
@@ -308,7 +308,7 @@ int main(int argc, char * const argv[])
 	oob.ptr = oobbuf;
 
 	if (noecc)  {
-		ret = ioctl(fd, MTDFILEMODE, (void *) MTD_MODE_RAW);
+		ret = ioctl(fd, MTDFILEMODE, MTD_MODE_RAW);
 		if (ret == 0) {
 			oobinfochanged = 2;
 		} else {
@@ -330,7 +330,6 @@ int main(int argc, char * const argv[])
 			}
 		}
 	} else {
-
 		/* check if we can read ecc stats */
 		if (!ioctl(fd, ECCGETSTATS, &stat1)) {
 			eccstats = true;
@@ -375,7 +374,6 @@ int main(int argc, char * const argv[])
 	bs = meminfo.writesize;
 
 	/* Print informative message */
-
 	if (!quiet) {
 		fprintf(stderr, "Block size %u, page size %u, OOB size %u\n",
 				meminfo.erasesize, meminfo.writesize, meminfo.oobsize);
@@ -383,6 +381,7 @@ int main(int argc, char * const argv[])
 				"Dumping data starting at 0x%08x and ending at 0x%08x...\n",
 				(unsigned int)start_addr, (unsigned int)end_addr);
 	}
+
 	/* Dump the flash contents */
 	for (ofs = start_addr; ofs < end_addr; ofs += bs) {
 		/* Check for bad block */
@@ -401,7 +400,7 @@ int main(int argc, char * const argv[])
 		if (badblock) {
 			if (omitbad)
 				continue;
-			memset (readbuf, 0xff, bs);
+			memset(readbuf, 0xff, bs);
 		} else {
 			/* Read page data and exit on failure */
 			if (pread(fd, readbuf, bs, ofs) != bs) {
@@ -430,8 +429,8 @@ int main(int argc, char * const argv[])
 		/* Write out page data */
 		if (pretty_print) {
 			for (i = 0; i < bs; i += PRETTY_ROW_SIZE) {
-				pretty_dump_to_buffer(readbuf+i, PRETTY_ROW_SIZE,
-						pretty_buf, PRETTY_BUF_LEN, true, canonical, ofs+i);
+				pretty_dump_to_buffer(readbuf + i, PRETTY_ROW_SIZE,
+						pretty_buf, PRETTY_BUF_LEN, true, canonical, ofs + i);
 				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
 		} else
@@ -453,8 +452,8 @@ int main(int argc, char * const argv[])
 
 		/* Write out OOB data */
 		if (pretty_print) {
-			for (i = 0; i < meminfo.oobsize; i += 16) {
-				pretty_dump_to_buffer(oobbuf+i, meminfo.oobsize-i,
+			for (i = 0; i < meminfo.oobsize; i += PRETTY_ROW_SIZE) {
+				pretty_dump_to_buffer(oobbuf + i, meminfo.oobsize - i,
 						pretty_buf, PRETTY_BUF_LEN, false, canonical, 0);
 				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
diff --git a/nandwrite.c b/nandwrite.c
index b5745b9..b0c4366 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -318,7 +318,6 @@ int main(int argc, char * const argv[])
 
 		// autoplace ECC ?
 		if (old_oobinfo.useecc != MTD_NANDECC_AUTOPLACE) {
-
 			if (ioctl(fd, MEMSETOOBSEL, &autoplace_oobinfo) != 0) {
 				perror("MEMSETOOBSEL");
 				close(fd);
@@ -329,7 +328,7 @@ int main(int argc, char * const argv[])
 	}
 
 	if (noecc)  {
-		ret = ioctl(fd, MTDFILEMODE, (void *)MTD_MODE_RAW);
+		ret = ioctl(fd, MTDFILEMODE, MTD_MODE_RAW);
 		if (ret == 0) {
 			oobinfochanged = 2;
 		} else {
@@ -428,7 +427,7 @@ int main(int argc, char * const argv[])
 	}
 
 	// Check, if length fits into device
-	if ( ((imglen / pagelen) * meminfo.writesize) > (meminfo.size - mtdoffset)) {
+	if (((imglen / pagelen) * meminfo.writesize) > (meminfo.size - mtdoffset)) {
 		fprintf(stderr, "Image %d bytes, NAND page %d bytes, OOB area %u bytes, device size %u bytes\n",
 				imglen, pagelen, meminfo.writesize, meminfo.size);
 		perror("Input file does not fit into device");
@@ -451,14 +450,15 @@ int main(int argc, char * const argv[])
 	 * length or zero.
 	 */
 	while (((imglen > 0) || (writebuf < (filebuf + filebuf_len)))
-		&& (mtdoffset < meminfo.size))
-	{
-		// new eraseblock , check for bad block(s)
-		// Stay in the loop to be sure if the mtdoffset changes because
-		// of a bad block, that the next block that will be written to
-		// is also checked. Thus avoiding errors if the block(s) after the
-		// skipped block(s) is also bad (number of blocks depending on
-		// the blockalign
+		&& (mtdoffset < meminfo.size)) {
+		/*
+		 * New eraseblock, check for bad block(s)
+		 * Stay in the loop to be sure that, if mtdoffset changes because
+		 * of a bad block, the next block that will be written to
+		 * is also checked. Thus, we avoid errors if the block(s) after the
+		 * skipped block(s) is also bad (number of blocks depending on
+		 * the blockalign).
+		 */
 		while (blockstart != (mtdoffset & (~meminfo.erasesize + 1))) {
 			blockstart = mtdoffset & (~meminfo.erasesize + 1);
 			offs = blockstart;
@@ -592,14 +592,14 @@ int main(int argc, char * const argv[])
 				int i, start, len;
 				int tags_pos = 0;
 				/*
-				 *  We use autoplacement and have the oobinfo with the autoplacement
+				 * 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++) {
+					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];
@@ -620,7 +620,7 @@ int main(int argc, char * const argv[])
 							len);
 				}
 			}
-			/* Write OOB data first, as ecc will be placed in there*/
+			/* Write OOB data first, as ecc will be placed in there */
 			oob.start = mtdoffset;
 			if (ioctl(fd, MEMWRITEOOB, &oob) != 0) {
 				perror("ioctl(MEMWRITEOOB)");
-- 
1.7.0.4

[PATCH v2 2/2] mtd-utils: nanddump: add 64-bit support, utilize libmtd

[Brian Norris]

Adds support for 64-bit offsets (i.e., devices larger than 4GB).
Utilizes the "unsigned long long" data type as the standard type
for 64-bit offsets throughout. Reformats a few printf statements
to avoid casting and to properly handle "long long."

Calls to ioctls are mostly replaced by libmtd interfaces (which should
choose the proper ioctls for us); however, a few remain and probably
should be handled with more robust interfaces, as some of these
ioctls are considered "legacy."

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nanddump.c |   83 +++++++++++++++++++++++++++++-------------------------------
 1 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 013eca0..86a71c9 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -33,6 +33,7 @@
 #include <asm/types.h>
 #include <mtd/mtd-user.h>
 #include "common.h"
+#include <libmtd.h>
 
 static struct nand_oobinfo none_oobinfo = {
 	.useecc = MTD_NANDECC_OFF,
@@ -81,7 +82,7 @@ static bool			pretty_print = false;	// print nice
 static bool			noecc = false;		// don't error correct
 static bool			noskipbad = false;	// don't skip bad blocks
 static bool			omitoob = false;	// omit oob data
-static unsigned long		start_addr;		// start address
+static unsigned long long	start_addr;		// start address
 static unsigned long		length;			// dump length
 static const char		*mtddev;		// mtd device name
 static const char		*dumpfile;		// dump file name
@@ -135,7 +136,7 @@ static void process_options(int argc, char * const argv[])
 				omitbad = true;
 				break;
 			case 's':
-				start_addr = strtoul(optarg, NULL, 0);
+				start_addr = strtoull(optarg, NULL, 0);
 				break;
 			case 'f':
 				if (!(dumpfile = strdup(optarg))) {
@@ -144,7 +145,7 @@ static void process_options(int argc, char * const argv[])
 				}
 				break;
 			case 'l':
-				length = strtoul(optarg, NULL, 0);
+				length = strtoull(optarg, NULL, 0);
 				break;
 			case 'o':
 				omitoob = true;
@@ -221,14 +222,14 @@ static void process_options(int argc, char * const argv[])
  */
 static void pretty_dump_to_buffer(const unsigned char *buf, size_t len,
 		char *linebuf, size_t linebuflen, bool pagedump, bool ascii,
-		unsigned int prefix)
+		unsigned long long prefix)
 {
 	static const char hex_asc[] = "0123456789abcdef";
 	unsigned char ch;
 	unsigned int j, lx = 0, ascii_column;
 
 	if (pagedump)
-		lx += sprintf(linebuf, "0x%.8x: ", prefix);
+		lx += sprintf(linebuf, "0x%.8llx: ", prefix);
 	else
 		lx += sprintf(linebuf, "  OOB Data: ");
 
@@ -271,20 +272,25 @@ nil:
  */
 int main(int argc, char * const argv[])
 {
-	unsigned long ofs, end_addr = 0;
+	unsigned long long ofs, end_addr = 0;
 	unsigned long long blockstart = 1;
 	int ret, i, fd, ofd = 0, bs, badblock = 0;
-	struct mtd_oob_buf oob;
-	mtd_info_t meminfo;
+	struct mtd_dev_info mtd;
 	char pretty_buf[PRETTY_BUF_LEN];
 	int oobinfochanged = 0, firstblock = 1;
 	struct nand_oobinfo old_oobinfo;
 	struct mtd_ecc_stats stat1, stat2;
 	bool eccstats = false;
 	unsigned char *readbuf = NULL, *oobbuf = NULL;
+	libmtd_t mtd_desc;
 
 	process_options(argc, argv);
 
+	/* Initialize libmtd */
+	mtd_desc = libmtd_open();
+	if (!mtd_desc)
+		return errmsg("can't initialize libmtd");
+
 	/* Open MTD device */
 	if ((fd = open(mtddev, O_RDONLY)) == -1) {
 		perror(mtddev);
@@ -292,20 +298,12 @@ int main(int argc, char * const argv[])
 	}
 
 	/* Fill in MTD device capability structure */
-	if (ioctl(fd, MEMGETINFO, &meminfo) != 0) {
-		perror("MEMGETINFO");
-		close(fd);
-		exit(EXIT_FAILURE);
-	}
+	if (mtd_get_dev_info(mtd_desc, mtddev, &mtd) < 0)
+		return errmsg("mtd_get_dev_info failed");
 
 	/* Allocate buffers */
-	oobbuf = xmalloc(sizeof(oobbuf) * meminfo.oobsize);
-	readbuf = xmalloc(sizeof(readbuf) * meminfo.writesize);
-
-	/* Fill in oob info */
-	oob.start = 0;
-	oob.length = meminfo.oobsize;
-	oob.ptr = oobbuf;
+	oobbuf = xmalloc(sizeof(oobbuf) * mtd.oob_size);
+	readbuf = xmalloc(sizeof(readbuf) * mtd.min_io_size);
 
 	if (noecc)  {
 		ret = ioctl(fd, MTDFILEMODE, MTD_MODE_RAW);
@@ -359,27 +357,27 @@ int main(int argc, char * const argv[])
 	}
 
 	/* Initialize start/end addresses and block size */
-	if (start_addr & (meminfo.writesize - 1)) {
+	if (start_addr & (mtd.min_io_size - 1)) {
 		fprintf(stderr, "WARNING: The start address is not page-aligned !\n"
 				"The pagesize of this NAND Flash is 0x%x.\n"
 				"nandwrite doesn't allow writes starting at this location.\n"
 				"Future versions of nanddump will fail here.\n",
-				meminfo.writesize);
+				mtd.min_io_size);
 	}
 	if (length)
 		end_addr = start_addr + length;
-	if (!length || end_addr > meminfo.size)
-		end_addr = meminfo.size;
+	if (!length || end_addr > mtd.size)
+		end_addr = mtd.size;
 
-	bs = meminfo.writesize;
+	bs = mtd.min_io_size;
 
 	/* Print informative message */
 	if (!quiet) {
 		fprintf(stderr, "Block size %u, page size %u, OOB size %u\n",
-				meminfo.erasesize, meminfo.writesize, meminfo.oobsize);
+				mtd.eb_size, mtd.min_io_size, mtd.oob_size);
 		fprintf(stderr,
-				"Dumping data starting at 0x%08x and ending at 0x%08x...\n",
-				(unsigned int)start_addr, (unsigned int)end_addr);
+				"Dumping data starting at 0x%08llx and ending at 0x%08llx...\n",
+				start_addr, end_addr);
 	}
 
 	/* Dump the flash contents */
@@ -387,12 +385,12 @@ int main(int argc, char * const argv[])
 		/* Check for bad block */
 		if (noskipbad) {
 			badblock = 0;
-		} else if (blockstart != (ofs & (~meminfo.erasesize + 1)) ||
+		} else if (blockstart != (ofs & (~mtd.eb_size + 1)) ||
 				firstblock) {
-			blockstart = ofs & (~meminfo.erasesize + 1);
+			blockstart = ofs & (~mtd.eb_size + 1);
 			firstblock = 0;
-			if ((badblock = ioctl(fd, MEMGETBADBLOCK, &blockstart)) < 0) {
-				perror("ioctl(MEMGETBADBLOCK)");
+			if ((badblock = mtd_is_bad(&mtd, fd, ofs / mtd.eb_size)) < 0) {
+				errmsg("libmtd: mtd_is_bad");
 				goto closeall;
 			}
 		}
@@ -403,8 +401,8 @@ int main(int argc, char * const argv[])
 			memset(readbuf, 0xff, bs);
 		} else {
 			/* Read page data and exit on failure */
-			if (pread(fd, readbuf, bs, ofs) != bs) {
-				perror("pread");
+			if (mtd_read(&mtd, fd, ofs / mtd.eb_size, ofs % mtd.eb_size, readbuf, bs)) {
+				errmsg("mtd_read");
 				goto closeall;
 			}
 		}
@@ -417,11 +415,11 @@ int main(int argc, char * const argv[])
 			}
 			if (stat1.failed != stat2.failed)
 				fprintf(stderr, "ECC: %d uncorrectable bitflip(s)"
-						" at offset 0x%08lx\n",
+						" at offset 0x%08llx\n",
 						stat2.failed - stat1.failed, ofs);
 			if (stat1.corrected != stat2.corrected)
 				fprintf(stderr, "ECC: %d corrected bitflip(s) at"
-						" offset 0x%08lx\n",
+						" offset 0x%08llx\n",
 						stat2.corrected - stat1.corrected, ofs);
 			stat1 = stat2;
 		}
@@ -440,25 +438,24 @@ int main(int argc, char * const argv[])
 			continue;
 
 		if (badblock) {
-			memset(oobbuf, 0xff, meminfo.oobsize);
+			memset(oobbuf, 0xff, mtd.oob_size);
 		} else {
 			/* Read OOB data and exit on failure */
-			oob.start = ofs;
-			if (ioctl(fd, MEMREADOOB, &oob) != 0) {
-				perror("ioctl(MEMREADOOB)");
+			if (mtd_read_oob(mtd_desc, &mtd, fd, ofs, mtd.oob_size, oobbuf)) {
+				errmsg("libmtd: mtd_read_oob");
 				goto closeall;
 			}
 		}
 
 		/* Write out OOB data */
 		if (pretty_print) {
-			for (i = 0; i < meminfo.oobsize; i += PRETTY_ROW_SIZE) {
-				pretty_dump_to_buffer(oobbuf + i, meminfo.oobsize - i,
+			for (i = 0; i < mtd.oob_size; i += PRETTY_ROW_SIZE) {
+				pretty_dump_to_buffer(oobbuf + i, mtd.oob_size - i,
 						pretty_buf, PRETTY_BUF_LEN, false, canonical, 0);
 				write(ofd, pretty_buf, strlen(pretty_buf));
 			}
 		} else
-			write(ofd, oobbuf, meminfo.oobsize);
+			write(ofd, oobbuf, mtd.oob_size);
 	}
 
 	/* reset oobinfo */
-- 
1.7.0.4

Re: [PATCH v2 1/2] mtd-utils: nanddump/nandwrite: style, signed-ness, printing fixups

[Mike Frysinger]

On Thu, Oct 21, 2010 at 03:19, Brian Norris wrote:
> There were some signed/unsigned integer comparisons. Their types were
> changed for safety. Also, "strtol" was improperly used for unsigned
> data types.

dont really like these being merged, but that's obviously Artem's call, not mine

> -       while (lx < (linebuflen - 1) && lx < (ascii_column - 1))
> +       do {
>                linebuf[lx++] = ' ';
> +       } while (lx < (linebuflen - 1) && lx < (ascii_column - 1));

hmm, unrelated to your changes, but i'm pretty sure this can be
rewritten into a sprintf ...
  sprintf(linebuf + lx, "%*s", (linebuflen - 1) - lx, "")
and integrating the ascii_column stuff too.  or maybe that's too complicated.
-mike

Re: [PATCH v2 0/2] mtd-utils fixups

[Mike Frysinger]

looks straight forward to me (other than the one note).  thanks !
-mike

Re: [PATCH v2 1/2] mtd-utils: nanddump/nandwrite: style, signed-ness, printing fixups

[Brian Norris]

Hi,

I just want to be clear in communication this time, so I have a few
clarification questions.

On 10/21/2010 12:37 AM, Mike Frysinger wrote:
> On Thu, Oct 21, 2010 at 03:19, Brian Norris wrote:
>> There were some signed/unsigned integer comparisons. Their types were
>> changed for safety. Also, "strtol" was improperly used for unsigned
>> data types.
> 
> dont really like these being merged, but that's obviously Artem's call, not mine

What exactly do you not like? The use of unsigned types, the use of
strtoul, a lack of consistency, or something else? I'm open to changing
my type usages if you have ideas that make more sense.

>> -       while (lx < (linebuflen - 1) && lx < (ascii_column - 1))
>> +       do {
>>                linebuf[lx++] = ' ';
>> +       } while (lx < (linebuflen - 1) && lx < (ascii_column - 1));
> 
> hmm, unrelated to your changes, but i'm pretty sure this can be
> rewritten into a sprintf ...
>   sprintf(linebuf + lx, "%*s", (linebuflen - 1) - lx, "")
> and integrating the ascii_column stuff too.  or maybe that's too complicated.


I can look try that in the future. You're probably right though. When I
wrote this, I really just copied/pasted/fixed the library hexdump code
from the linux kernel, so I didn't spend time making it the simplest
possible.

Thanks,
Brian

Re: [PATCH v2 1/2] mtd-utils: nanddump/nandwrite: style, signed-ness, printing fixups

[Mike Frysinger]

On Thu, Oct 21, 2010 at 11:54, Brian Norris wrote:
> On 10/21/2010 12:37 AM, Mike Frysinger wrote:
>> On Thu, Oct 21, 2010 at 03:19, Brian Norris wrote:
>>> There were some signed/unsigned integer comparisons. Their types were
>>> changed for safety. Also, "strtol" was improperly used for unsigned
>>> data types.
>>
>> dont really like these being merged, but that's obviously Artem's call, not mine
>
> What exactly do you not like? The use of unsigned types, the use of
> strtoul, a lack of consistency, or something else? I'm open to changing
> my type usages if you have ideas that make more sense.

i dont have a problem with the changes.  i was talking about doing
style and functional (type changes) in one patch rather than two.
-mike

Re: [PATCH v2 1/2] mtd-utils: nanddump/nandwrite: style, signed-ness, printing fixups

[Artem Bityutskiy]

On Thu, 2010-10-21 at 00:19 -0700, Brian Norris wrote:
> There were some signed/unsigned integer comparisons. Their types were
> changed for safety. Also, "strtol" was improperly used for unsigned
> data types.
> 
> Nanddump's pretty print options needed a slight reformat to prepare for
> printing offsets that are more than 32 bits (8 hex characters) wide.
> This prevents overlap of output and ensures that at least one space is
> printed between hex and ascii printouts. Perhaps this could use some
> better alignment in the future.
> 
> Other fixes:
> * Corrected several simple spacing issues
> * Changed indentation of some global variable declarations in
>   order to prepare for the next patch, which makes those
>   declarations longer
> * Used macro for PRETTY_ROW_SIZE instead of constant 16
> * Reformatted, edited a multi-line comment
> * Removed some unnecessary casts
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  nanddump.c  |   55 +++++++++++++++++++++++++++----------------------------
>  nandwrite.c |   28 ++++++++++++++--------------
>  2 files changed, 41 insertions(+), 42 deletions(-)

Mike is right, it is much better to separate functional and cleanup
changes, even split different clean-up changes into several patches.
Maintaining this stuff and reviewing patches takes time, and splitting
helps a lot. Let's push this patch but please, try to splint patches on
more smaller pieces. Yes, it'll be more work for you, but less for me.
But when submitters take more burden and maintainer less - the system
scales better. Take into account that I'm doing this stuff just because
I like MTD and do not like see people's patches lost, so little help in
form of perfectly split micro-patches is appreciated.

Pushed, thanks!

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH v2 2/2] mtd-utils: nanddump: add 64-bit support, utilize libmtd

[Artem Bityutskiy]

On Thu, 2010-10-21 at 00:19 -0700, Brian Norris wrote:
> Adds support for 64-bit offsets (i.e., devices larger than 4GB).
> Utilizes the "unsigned long long" data type as the standard type
> for 64-bit offsets throughout. Reformats a few printf statements
> to avoid casting and to properly handle "long long."
> 
> Calls to ioctls are mostly replaced by libmtd interfaces (which should
> choose the proper ioctls for us); however, a few remain and probably
> should be handled with more robust interfaces, as some of these
> ioctls are considered "legacy."
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  nanddump.c |   83 +++++++++++++++++++++++++++++-------------------------------
>  1 files changed, 40 insertions(+), 43 deletions(-)

Pushed, thanks.

Mike, thanks for reviewing. You did not provide formal "Reviewed-by"
though, so I did not add this tag, assuming you did this for purposes,
because for other patches you provide ready to copy-paste tags.

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH v2 2/2] mtd-utils: nanddump: add 64-bit support, utilize libmtd

[Mike Frysinger]

On Mon, Oct 25, 2010 at 15:20, Artem Bityutskiy wrote:
> Mike, thanks for reviewing. You did not provide formal "Reviewed-by"
> though, so I did not add this tag, assuming you did this for purposes,
> because for other patches you provide ready to copy-paste tags.

i'm not in the habit of using that tag, so i dont remember to post it
-mike

Re: [PATCH v2 2/2] mtd-utils: nanddump: add 64-bit support, utilize libmtd

[Artem Bityutskiy]

On Mon, 2010-10-25 at 15:54 -0400, Mike Frysinger wrote:
> On Mon, Oct 25, 2010 at 15:20, Artem Bityutskiy wrote:
> > Mike, thanks for reviewing. You did not provide formal "Reviewed-by"
> > though, so I did not add this tag, assuming you did this for purposes,
> > because for other patches you provide ready to copy-paste tags.
> 
> i'm not in the habit of using that tag, so i dont remember to post it

Well, you did answer with proper Acked-by recently. But if you do not
provide tag, I cannot make it myself and add it to the patch, because
you might not want it.

Anyway, if you want to have your reviewed-by or acked by tags in any
patch I deal with, send them explicitly. and I'll add them.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH v2 1/2] mtd-utils: nanddump/nandwrite: style, signed-ness, printing fixups

[Brian Norris]

On 10/25/2010 12:15 PM, Artem Bityutskiy wrote:
> Mike is right, it is much better to separate functional and cleanup
> changes, even split different clean-up changes into several patches.
> Maintaining this stuff and reviewing patches takes time, and splitting
> helps a lot. Let's push this patch but please, try to splint patches on
> more smaller pieces. Yes, it'll be more work for you, but less for me.
> But when submitters take more burden and maintainer less - the system
> scales better. Take into account that I'm doing this stuff just because
> I like MTD and do not like see people's patches lost, so little help in
> form of perfectly split micro-patches is appreciated.
> 
> Pushed, thanks!

Right, this makes perfect sense. I will remind myself of this for future
patches. I certainly don't want to make things more difficult than
necessary for you!

Oh, and thanks for the push.

Brian