All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] nandwrite: clean out old ioctls
@ 2011-08-19 17:07 Brian Norris
  2011-08-19 17:07 ` [PATCH 01/10] mtd_debug: fixup style Brian Norris
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

Hello,

This series cleans up some coding style, but it primarily deals with
several options to nandwrite that have rotted. Mostly, this deals with
MEMSETOOBSEL, which hasn't existed since kernel 2.6.17 - see commit:

  [MTD] NAND Consolidate oobinfo handling
  Sat May 27 20:36:12 2006 +0200
  commit ff268fb8791cf18df536113355d7184007c269d9

For reference, MEMSETOOBSEL was "replaced" with MTDFILEMODE in the same
kernel release:

  [MTD] NAND Expose the new raw mode function and status info to userspace
  Tue May 30 00:37:34 2006 +0200
  commit f1a28c02843efcfcc41982149880bac3ac180234

Several features depended (in part or entirely) on the MEMSETOOBSEL
ioctl: the -j, -y, and -a options to nandwrite and by extension the -f
(--forcelegacy) option which handled special cases for -j and -y. There
are also pieces of nanddump that relied on this behavior as a backup. We
just remove them, since they have been useless for over 5 years.

There shouldn't be any actual loss of functionality in these patches so
far (at least, not very much); let me know if there's something critical
I'm missing.

On the other hand, there is plenty of value in these edits, since it
leaves room for more rennovation and addition of newer, better kernel
interfaces for writing data and changing OOB modes.

Brian

Brian Norris (10):
  mtd_debug: fixup style
  mtd_debug: replace #defines with enum
  mtd-utils: use __func__ instead of __FUNCTION__
  nandwrite: remove C99 comment style
  nandwrite: remove `autoplace' features
  nandwrite: kill more MEMSETOOBSEL
  nandwrite: kill -j, -y, and -f options
  nandwrite: cleanup "oobinfochanged" leftovers
  nandwrite: refactor "old_oobinfo" code
  nanddump: kill usages of MEMSETOOBSEL ioctl

 mtd_debug.c              |  355 ++++++++++++++++++++++------------------------
 nanddump.c               |   43 +-----
 nandwrite.c              |  174 ++++-------------------
 tests/ubi-tests/common.c |   10 +-
 4 files changed, 205 insertions(+), 377 deletions(-)

-- 
1.7.6

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

* [PATCH 01/10] mtd_debug: fixup style
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 02/10] mtd_debug: replace #defines with enum Brian Norris
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

Remove extraneous spaces.
Put braces on same line as "if", "for", "switch", etc. statements.
No parentheses around return values.
Use "errmsg_die" from common.h.
Replace "exit (1)" with "exit(EXIT_FAILURE)".

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 mtd_debug.c |  343 +++++++++++++++++++++++++++-------------------------------
 1 files changed, 160 insertions(+), 183 deletions(-)

diff --git a/mtd_debug.c b/mtd_debug.c
index b82dabe..a348a4c 100644
--- a/mtd_debug.c
+++ b/mtd_debug.c
@@ -39,263 +39,250 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <mtd/mtd-user.h>
+#include "common.h"
 
 /*
  * MEMGETINFO
  */
-static int getmeminfo (int fd,struct mtd_info_user *mtd)
+static int getmeminfo(int fd, struct mtd_info_user *mtd)
 {
-	return (ioctl (fd,MEMGETINFO,mtd));
+	return ioctl(fd, MEMGETINFO, mtd);
 }
 
 /*
  * MEMERASE
  */
-static int memerase (int fd,struct erase_info_user *erase)
+static int memerase(int fd, struct erase_info_user *erase)
 {
-	return (ioctl (fd,MEMERASE,erase));
+	return ioctl(fd, MEMERASE, erase);
 }
 
 /*
  * MEMGETREGIONCOUNT
  * MEMGETREGIONINFO
  */
-static int getregions (int fd,struct region_info_user *regions,int *n)
+static int getregions(int fd, struct region_info_user *regions, int *n)
 {
-	int i,err;
-	err = ioctl (fd,MEMGETREGIONCOUNT,n);
-	if (err) return (err);
-	for (i = 0; i < *n; i++)
-	{
+	int i, err;
+	err = ioctl(fd, MEMGETREGIONCOUNT, n);
+	if (err)
+		return err;
+	for (i = 0; i < *n; i++) {
 		regions[i].regionindex = i;
-		err = ioctl (fd,MEMGETREGIONINFO,&regions[i]);
-		if (err) return (err);
+		err = ioctl(fd, MEMGETREGIONINFO, &regions[i]);
+		if (err)
+			return err;
 	}
-	return (0);
+	return 0;
 }
 
-int erase_flash (int fd,u_int32_t offset,u_int32_t bytes)
+int erase_flash(int fd, u_int32_t offset, u_int32_t bytes)
 {
 	int err;
 	struct erase_info_user erase;
 	erase.start = offset;
 	erase.length = bytes;
-	err = memerase (fd,&erase);
-	if (err < 0)
-	{
-		perror ("MEMERASE");
-		return (1);
+	err = memerase(fd, &erase);
+	if (err < 0) {
+		perror("MEMERASE");
+		return 1;
 	}
-	fprintf (stderr,"Erased %d bytes from address 0x%.8x in flash\n",bytes,offset);
-	return (0);
+	fprintf(stderr, "Erased %d bytes from address 0x%.8x in flash\n", bytes, offset);
+	return 0;
 }
 
-void printsize (u_int32_t x)
+void printsize(u_int32_t x)
 {
 	int i;
 	static const char *flags = "KMGT";
-	printf ("%u ",x);
-	for (i = 0; x >= 1024 && flags[i] != '\0'; i++) x /= 1024;
+	printf("%u ", x);
+	for (i = 0; x >= 1024 && flags[i] != '\0'; i++)
+		x /= 1024;
 	i--;
-	if (i >= 0) printf ("(%u%c)",x,flags[i]);
+	if (i >= 0)
+		printf("(%u%c)", x, flags[i]);
 }
 
-int flash_to_file (int fd,u_int32_t offset,size_t len,const char *filename)
+int flash_to_file(int fd, u_int32_t offset, size_t len, const char *filename)
 {
 	u_int8_t *buf = NULL;
-	int outfd,err;
-	int size = len * sizeof (u_int8_t);
+	int outfd, err;
+	int size = len * sizeof(u_int8_t);
 	int n = len;
 
-	if (offset != lseek (fd,offset,SEEK_SET))
-	{
-		perror ("lseek()");
+	if (offset != lseek(fd, offset, SEEK_SET)) {
+		perror("lseek()");
 		goto err0;
 	}
-	outfd = creat (filename,0666);
-	if (outfd < 0)
-	{
-		perror ("creat()");
+	outfd = creat(filename, 0666);
+	if (outfd < 0) {
+		perror("creat()");
 		goto err1;
 	}
 
 retry:
-	if ((buf = (u_int8_t *) malloc (size)) == NULL)
-	{
-#define BUF_SIZE	(64 * 1024 * sizeof (u_int8_t))
-		fprintf (stderr, "%s: malloc(%#x)\n", __FUNCTION__, size);
+	if ((buf = (u_int8_t *) malloc(size)) == NULL) {
+#define BUF_SIZE	(64 * 1024 * sizeof(u_int8_t))
+		fprintf(stderr, "%s: malloc(%#x)\n", __FUNCTION__, size);
 		if (size != BUF_SIZE) {
 			size = BUF_SIZE;
-			fprintf (stderr, "%s: trying buffer size %#x\n", __FUNCTION__, size);
+			fprintf(stderr, "%s: trying buffer size %#x\n", __FUNCTION__, size);
 			goto retry;
 		}
-		perror ("malloc()");
+		perror("malloc()");
 		goto err0;
 	}
 	do {
 		if (n <= size)
 			size = n;
-		err = read (fd,buf,size);
-		if (err < 0)
-		{
-			fprintf (stderr, "%s: read, size %#x, n %#x\n", __FUNCTION__, size, n);
-			perror ("read()");
+		err = read(fd, buf, size);
+		if (err < 0) {
+			fprintf(stderr, "%s: read, size %#x, n %#x\n", __FUNCTION__, size, n);
+			perror("read()");
 			goto err2;
 		}
-		err = write (outfd,buf,size);
-		if (err < 0)
-		{
-			fprintf (stderr, "%s: write, size %#x, n %#x\n", __FUNCTION__, size, n);
-			perror ("write()");
+		err = write(outfd, buf, size);
+		if (err < 0) {
+			fprintf(stderr, "%s: write, size %#x, n %#x\n", __FUNCTION__, size, n);
+			perror("write()");
 			goto err2;
 		}
-		if (err != size)
-		{
-			fprintf (stderr,"Couldn't copy entire buffer to %s. (%d/%d bytes copied)\n",filename,err,size);
+		if (err != size) {
+			fprintf(stderr, "Couldn't copy entire buffer to %s. (%d/%d bytes copied)\n", filename, err, size);
 			goto err2;
 		}
 		n -= size;
 	} while (n > 0);
 
 	if (buf != NULL)
-		free (buf);
-	close (outfd);
-	printf ("Copied %zu bytes from address 0x%.8x in flash to %s\n",len,offset,filename);
-	return (0);
+		free(buf);
+	close(outfd);
+	printf("Copied %zu bytes from address 0x%.8x in flash to %s\n", len, offset, filename);
+	return 0;
 
 err2:
-	close (outfd);
+	close(outfd);
 err1:
 	if (buf != NULL)
-		free (buf);
+		free(buf);
 err0:
-	return (1);
+	return 1;
 }
 
-int file_to_flash (int fd,u_int32_t offset,u_int32_t len,const char *filename)
+int file_to_flash(int fd, u_int32_t offset, u_int32_t len, const char *filename)
 {
 	u_int8_t *buf = NULL;
 	FILE *fp;
 	int err;
-	int size = len * sizeof (u_int8_t);
+	int size = len * sizeof(u_int8_t);
 	int n = len;
 
-	if (offset != lseek (fd,offset,SEEK_SET))
-	{
-		perror ("lseek()");
-		return (1);
+	if (offset != lseek(fd, offset, SEEK_SET)) {
+		perror("lseek()");
+		return 1;
 	}
-	if ((fp = fopen (filename,"r")) == NULL)
-	{
-		perror ("fopen()");
-		return (1);
+	if ((fp = fopen(filename, "r")) == NULL) {
+		perror("fopen()");
+		return 1;
 	}
 retry:
-	if ((buf = (u_int8_t *) malloc (size)) == NULL)
-	{
-		fprintf (stderr, "%s: malloc(%#x) failed\n", __FUNCTION__, size);
+	if ((buf = (u_int8_t *) malloc(size)) == NULL) {
+		fprintf(stderr, "%s: malloc(%#x) failed\n", __FUNCTION__, size);
 		if (size != BUF_SIZE) {
 			size = BUF_SIZE;
-			fprintf (stderr, "%s: trying buffer size %#x\n", __FUNCTION__, size);
+			fprintf(stderr, "%s: trying buffer size %#x\n", __FUNCTION__, size);
 			goto retry;
 		}
-		perror ("malloc()");
-		fclose (fp);
-		return (1);
+		perror("malloc()");
+		fclose(fp);
+		return 1;
 	}
 	do {
 		if (n <= size)
 			size = n;
-		if (fread (buf,size,1,fp) != 1 || ferror (fp))
-		{
-			fprintf (stderr, "%s: fread, size %#x, n %#x\n", __FUNCTION__, size, n);
-			perror ("fread()");
-			free (buf);
-			fclose (fp);
-			return (1);
+		if (fread(buf, size, 1, fp) != 1 || ferror(fp)) {
+			fprintf(stderr, "%s: fread, size %#x, n %#x\n", __FUNCTION__, size, n);
+			perror("fread()");
+			free(buf);
+			fclose(fp);
+			return 1;
 		}
-		err = write (fd,buf,size);
-		if (err < 0)
-		{
-			fprintf (stderr, "%s: write, size %#x, n %#x\n", __FUNCTION__, size, n);
-			perror ("write()");
-			free (buf);
-			fclose (fp);
-			return (1);
+		err = write(fd, buf, size);
+		if (err < 0) {
+			fprintf(stderr, "%s: write, size %#x, n %#x\n", __FUNCTION__, size, n);
+			perror("write()");
+			free(buf);
+			fclose(fp);
+			return 1;
 		}
 		n -= size;
 	} while (n > 0);
 
 	if (buf != NULL)
-		free (buf);
-	fclose (fp);
-	printf ("Copied %d bytes from %s to address 0x%.8x in flash\n",len,filename,offset);
-	return (0);
+		free(buf);
+	fclose(fp);
+	printf("Copied %d bytes from %s to address 0x%.8x in flash\n", len, filename, offset);
+	return 0;
 }
 
-int showinfo (int fd)
+int showinfo(int fd)
 {
-	int i,err,n;
+	int i, err, n;
 	struct mtd_info_user mtd;
 	static struct region_info_user region[1024];
 
-	err = getmeminfo (fd,&mtd);
-	if (err < 0)
-	{
-		perror ("MEMGETINFO");
-		return (1);
+	err = getmeminfo(fd, &mtd);
+	if (err < 0) {
+		perror("MEMGETINFO");
+		return 1;
 	}
 
-	err = getregions (fd,region,&n);
-	if (err < 0)
-	{
-		perror ("MEMGETREGIONCOUNT");
-		return (1);
+	err = getregions(fd, region, &n);
+	if (err < 0) {
+		perror("MEMGETREGIONCOUNT");
+		return 1;
 	}
 
-	printf ("mtd.type = ");
-	switch (mtd.type)
-	{
+	printf("mtd.type = ");
+	switch (mtd.type) {
 		case MTD_ABSENT:
-			printf ("MTD_ABSENT");
+			printf("MTD_ABSENT");
 			break;
 		case MTD_RAM:
-			printf ("MTD_RAM");
+			printf("MTD_RAM");
 			break;
 		case MTD_ROM:
-			printf ("MTD_ROM");
+			printf("MTD_ROM");
 			break;
 		case MTD_NORFLASH:
-			printf ("MTD_NORFLASH");
+			printf("MTD_NORFLASH");
 			break;
 		case MTD_NANDFLASH:
-			printf ("MTD_NANDFLASH");
+			printf("MTD_NANDFLASH");
 			break;
 		case MTD_DATAFLASH:
-			printf ("MTD_DATAFLASH");
+			printf("MTD_DATAFLASH");
 			break;
 		case MTD_UBIVOLUME:
-			printf ("MTD_UBIVOLUME");
+			printf("MTD_UBIVOLUME");
 		default:
-			printf ("(unknown type - new MTD API maybe?)");
+			printf("(unknown type - new MTD API maybe?)");
 	}
 
-	printf ("\nmtd.flags = ");
+	printf("\nmtd.flags = ");
 	if (mtd.flags == MTD_CAP_ROM)
-		printf ("MTD_CAP_ROM");
+		printf("MTD_CAP_ROM");
 	else if (mtd.flags == MTD_CAP_RAM)
-		printf ("MTD_CAP_RAM");
+		printf("MTD_CAP_RAM");
 	else if (mtd.flags == MTD_CAP_NORFLASH)
-		printf ("MTD_CAP_NORFLASH");
+		printf("MTD_CAP_NORFLASH");
 	else if (mtd.flags == MTD_CAP_NANDFLASH)
-		printf ("MTD_CAP_NANDFLASH");
+		printf("MTD_CAP_NANDFLASH");
 	else if (mtd.flags == MTD_WRITEABLE)
-		printf ("MTD_WRITEABLE");
-	else
-	{
+		printf("MTD_WRITEABLE");
+	else {
 		int first = 1;
-		static struct
-		{
+		static struct {
 			const char *name;
 			int value;
 		} flags[] =
@@ -306,58 +293,53 @@ int showinfo (int fd)
 			{ "MTD_POWERUP_LOCK", MTD_POWERUP_LOCK },
 			{ NULL, -1 }
 		};
-		for (i = 0; flags[i].name != NULL; i++)
-			if (mtd.flags & flags[i].value)
-			{
-				if (first)
-				{
-					printf ("%s", flags[i].name);
+		for (i = 0; flags[i].name != NULL; i++) {
+			if (mtd.flags & flags[i].value) {
+				if (first) {
+					printf("%s", flags[i].name);
 					first = 0;
+				} else {
+					printf(" | %s", flags[i].name);
 				}
-				else printf (" | %s",flags[i].name);
 			}
+		}
 	}
 
-	printf ("\nmtd.size = ");
-	printsize (mtd.size);
+	printf("\nmtd.size = ");
+	printsize(mtd.size);
 
-	printf ("\nmtd.erasesize = ");
-	printsize (mtd.erasesize);
+	printf("\nmtd.erasesize = ");
+	printsize(mtd.erasesize);
 
-	printf ("\nmtd.writesize = ");
-	printsize (mtd.writesize);
+	printf("\nmtd.writesize = ");
+	printsize(mtd.writesize);
 
-	printf ("\nmtd.oobsize = ");
-	printsize (mtd.oobsize);
+	printf("\nmtd.oobsize = ");
+	printsize(mtd.oobsize);
 
-	printf ("\n"
-			"regions = %d\n"
-			"\n",
-			n);
+	printf("\nregions = %d\n\n", n);
 
-	for (i = 0; i < n; i++)
-	{
-		printf ("region[%d].offset = 0x%.8x\n"
+	for (i = 0; i < n; i++) {
+		printf("region[%d].offset = 0x%.8x\n"
 				"region[%d].erasesize = ",
-				i,region[i].offset,i);
-		printsize (region[i].erasesize);
-		printf ("\nregion[%d].numblocks = %d\n"
+				i, region[i].offset, i);
+		printsize(region[i].erasesize);
+		printf("\nregion[%d].numblocks = %d\n"
 				"region[%d].regionindex = %d\n",
-				i,region[i].numblocks,
-				i,region[i].regionindex);
+				i, region[i].numblocks,
+				i, region[i].regionindex);
 	}
-	return (0);
+	return 0;
 }
 
 void showusage(void)
 {
-	fprintf (stderr,
-			"usage: %1$s info <device>\n"
+	fprintf(stderr, "usage: %1$s info <device>\n"
 			"       %1$s read <device> <offset> <len> <dest-filename>\n"
 			"       %1$s write <device> <offset> <len> <source-filename>\n"
 			"       %1$s erase <device> <offset> <len>\n",
 			PROGRAM_NAME);
-	exit (1);
+	exit(EXIT_FAILURE);
 }
 
 #define OPT_INFO	1
@@ -365,51 +347,46 @@ void showusage(void)
 #define OPT_WRITE	3
 #define OPT_ERASE	4
 
-int main (int argc,char *argv[])
+int main(int argc, char *argv[])
 {
-	int err = 0,fd,option = OPT_INFO;
+	int err = 0, fd, option = OPT_INFO;
 	int open_flag;
 
 	/* parse command-line options */
-	if (argc == 3 && !strcmp (argv[1],"info"))
+	if (argc == 3 && !strcmp(argv[1], "info"))
 		option = OPT_INFO;
-	else if (argc == 6 && !strcmp (argv[1],"read"))
+	else if (argc == 6 && !strcmp(argv[1], "read"))
 		option = OPT_READ;
-	else if (argc == 6 && !strcmp (argv[1],"write"))
+	else if (argc == 6 && !strcmp(argv[1], "write"))
 		option = OPT_WRITE;
-	else if (argc == 5 && !strcmp (argv[1],"erase"))
+	else if (argc == 5 && !strcmp(argv[1], "erase"))
 		option = OPT_ERASE;
 	else
 		showusage();
 
 	/* open device */
-	open_flag = (option==OPT_INFO || option==OPT_READ) ? O_RDONLY : O_RDWR;
-	if ((fd = open (argv[2],O_SYNC | open_flag)) < 0)
-	{
-		perror ("open()");
-		exit (1);
-	}
+	open_flag = (option == OPT_INFO || option == OPT_READ) ? O_RDONLY : O_RDWR;
+	if ((fd = open(argv[2], O_SYNC | open_flag)) < 0)
+		errmsg_die("open()");
 
-	switch (option)
-	{
+	switch (option) {
 		case OPT_INFO:
-			showinfo (fd);
+			showinfo(fd);
 			break;
 		case OPT_READ:
-			err = flash_to_file (fd,strtol (argv[3],NULL,0),strtol (argv[4],NULL,0),argv[5]);
+			err = flash_to_file(fd, strtol(argv[3], NULL, 0), strtol(argv[4], NULL, 0), argv[5]);
 			break;
 		case OPT_WRITE:
-			err = file_to_flash (fd,strtol (argv[3],NULL,0),strtol (argv[4],NULL,0),argv[5]);
+			err = file_to_flash(fd, strtol(argv[3], NULL, 0), strtol(argv[4], NULL, 0), argv[5]);
 			break;
 		case OPT_ERASE:
-			err = erase_flash (fd,strtol (argv[3],NULL,0),strtol (argv[4],NULL,0));
+			err = erase_flash(fd, strtol(argv[3], NULL, 0), strtol(argv[4], NULL, 0));
 			break;
 	}
 
 	/* close device */
-	if (close (fd) < 0)
-		perror ("close()");
+	if (close(fd) < 0)
+		errmsg_die("close()");
 
-	exit (err);
+	return err;
 }
-
-- 
1.7.6

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

* [PATCH 02/10] mtd_debug: replace #defines with enum
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
  2011-08-19 17:07 ` [PATCH 01/10] mtd_debug: fixup style Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 03/10] mtd-utils: use __func__ instead of __FUNCTION__ Brian Norris
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

enum provides a cleaner mechanism that creating single-purpose `#define`s.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 mtd_debug.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/mtd_debug.c b/mtd_debug.c
index a348a4c..b98de50 100644
--- a/mtd_debug.c
+++ b/mtd_debug.c
@@ -342,16 +342,18 @@ void showusage(void)
 	exit(EXIT_FAILURE);
 }
 
-#define OPT_INFO	1
-#define OPT_READ	2
-#define OPT_WRITE	3
-#define OPT_ERASE	4
-
 int main(int argc, char *argv[])
 {
-	int err = 0, fd, option = OPT_INFO;
+	int err = 0, fd;
 	int open_flag;
 
+	enum {
+		OPT_INFO,
+		OPT_READ,
+		OPT_WRITE,
+		OPT_ERASE
+	} option = OPT_INFO;
+
 	/* parse command-line options */
 	if (argc == 3 && !strcmp(argv[1], "info"))
 		option = OPT_INFO;
-- 
1.7.6

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

* [PATCH 03/10] mtd-utils: use __func__ instead of __FUNCTION__
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
  2011-08-19 17:07 ` [PATCH 01/10] mtd_debug: fixup style Brian Norris
  2011-08-19 17:07 ` [PATCH 02/10] mtd_debug: replace #defines with enum Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 04/10] nandwrite: remove C99 comment style Brian Norris
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

__func__ is more portable

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 mtd_debug.c              |   16 ++++++++--------
 tests/ubi-tests/common.c |   10 +++++-----
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/mtd_debug.c b/mtd_debug.c
index b98de50..2d307a9 100644
--- a/mtd_debug.c
+++ b/mtd_debug.c
@@ -123,10 +123,10 @@ int flash_to_file(int fd, u_int32_t offset, size_t len, const char *filename)
 retry:
 	if ((buf = (u_int8_t *) malloc(size)) == NULL) {
 #define BUF_SIZE	(64 * 1024 * sizeof(u_int8_t))
-		fprintf(stderr, "%s: malloc(%#x)\n", __FUNCTION__, size);
+		fprintf(stderr, "%s: malloc(%#x)\n", __func__, size);
 		if (size != BUF_SIZE) {
 			size = BUF_SIZE;
-			fprintf(stderr, "%s: trying buffer size %#x\n", __FUNCTION__, size);
+			fprintf(stderr, "%s: trying buffer size %#x\n", __func__, size);
 			goto retry;
 		}
 		perror("malloc()");
@@ -137,13 +137,13 @@ retry:
 			size = n;
 		err = read(fd, buf, size);
 		if (err < 0) {
-			fprintf(stderr, "%s: read, size %#x, n %#x\n", __FUNCTION__, size, n);
+			fprintf(stderr, "%s: read, size %#x, n %#x\n", __func__, size, n);
 			perror("read()");
 			goto err2;
 		}
 		err = write(outfd, buf, size);
 		if (err < 0) {
-			fprintf(stderr, "%s: write, size %#x, n %#x\n", __FUNCTION__, size, n);
+			fprintf(stderr, "%s: write, size %#x, n %#x\n", __func__, size, n);
 			perror("write()");
 			goto err2;
 		}
@@ -187,10 +187,10 @@ int file_to_flash(int fd, u_int32_t offset, u_int32_t len, const char *filename)
 	}
 retry:
 	if ((buf = (u_int8_t *) malloc(size)) == NULL) {
-		fprintf(stderr, "%s: malloc(%#x) failed\n", __FUNCTION__, size);
+		fprintf(stderr, "%s: malloc(%#x) failed\n", __func__, size);
 		if (size != BUF_SIZE) {
 			size = BUF_SIZE;
-			fprintf(stderr, "%s: trying buffer size %#x\n", __FUNCTION__, size);
+			fprintf(stderr, "%s: trying buffer size %#x\n", __func__, size);
 			goto retry;
 		}
 		perror("malloc()");
@@ -201,7 +201,7 @@ retry:
 		if (n <= size)
 			size = n;
 		if (fread(buf, size, 1, fp) != 1 || ferror(fp)) {
-			fprintf(stderr, "%s: fread, size %#x, n %#x\n", __FUNCTION__, size, n);
+			fprintf(stderr, "%s: fread, size %#x, n %#x\n", __func__, size, n);
 			perror("fread()");
 			free(buf);
 			fclose(fp);
@@ -209,7 +209,7 @@ retry:
 		}
 		err = write(fd, buf, size);
 		if (err < 0) {
-			fprintf(stderr, "%s: write, size %#x, n %#x\n", __FUNCTION__, size, n);
+			fprintf(stderr, "%s: write, size %#x, n %#x\n", __func__, size, n);
 			perror("write()");
 			free(buf);
 			fclose(fp);
diff --git a/tests/ubi-tests/common.c b/tests/ubi-tests/common.c
index 05cbecc..a64ea75 100644
--- a/tests/ubi-tests/common.c
+++ b/tests/ubi-tests/common.c
@@ -54,24 +54,24 @@ int __initial_check(const char *test, int argc, char * const argv[])
 	 * check this.
 	 */
 	if (argc < 2) {
-		__errmsg(test, __FUNCTION__, __LINE__,
+		__errmsg(test, __func__, __LINE__,
 			 "UBI character device node is not specified");
 		return -1;
 	}
 
 	libubi = libubi_open();
 	if (libubi == NULL) {
-		__failed(test, __FUNCTION__, __LINE__, "libubi_open");
+		__failed(test, __func__, __LINE__, "libubi_open");
 		return -1;
 	}
 
 	if (ubi_get_dev_info(libubi, argv[1], &dev_info)) {
-		__failed(test, __FUNCTION__, __LINE__, "ubi_get_dev_info");
+		__failed(test, __func__, __LINE__, "ubi_get_dev_info");
 		goto close;
 	}
 
 	if (dev_info.avail_lebs < MIN_AVAIL_EBS) {
-		__errmsg(test, __FUNCTION__, __LINE__,
+		__errmsg(test, __func__, __LINE__,
 			 "insufficient available eraseblocks %d on UBI "
 			 "device, required %d",
 			 dev_info.avail_lebs, MIN_AVAIL_EBS);
@@ -79,7 +79,7 @@ int __initial_check(const char *test, int argc, char * const argv[])
 	}
 
 	if (dev_info.vol_count != 0) {
-		__errmsg(test, __FUNCTION__, __LINE__,
+		__errmsg(test, __func__, __LINE__,
 			 "device %s is not empty", argv[1]);
 		goto close;
 	}
-- 
1.7.6

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

* [PATCH 04/10] nandwrite: remove C99 comment style
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
                   ` (2 preceding siblings ...)
  2011-08-19 17:07 ` [PATCH 03/10] mtd-utils: use __func__ instead of __FUNCTION__ Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 05/10] nandwrite: remove `autoplace' features Brian Norris
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index 1700d61..f58bbfa 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -274,13 +274,13 @@ int main(int argc, char * const argv[])
 	int oobinfochanged = 0;
 	struct nand_oobinfo old_oobinfo;
 	bool failed = true;
-	// contains all the data read from the file so far for the current eraseblock
+	/* contains all the data read from the file so far for the current eraseblock */
 	unsigned char *filebuf = NULL;
 	size_t filebuf_max = 0;
 	size_t filebuf_len = 0;
-	// points to the current page inside filebuf
+	/* points to the current page inside filebuf */
 	unsigned char *writebuf = NULL;
-	// points to the OOB for the current page in filebuf
+	/* points to the OOB for the current page in filebuf */
 	unsigned char *oobreadbuf = NULL;
 	unsigned char *oobbuf = NULL;
 	libmtd_t mtd_desc;
@@ -429,14 +429,14 @@ int main(int argc, char * const argv[])
 	    lseek(ifd, 0, SEEK_SET);
 	}
 
-	// Check, if file is page-aligned
+	/* Check, if file is page-aligned */
 	if ((!pad) && ((imglen % pagelen) != 0)) {
 		fprintf(stderr, "Input file is not page-aligned. Use the padding "
 				 "option.\n");
 		goto closeall;
 	}
 
-	// Check, if length fits into device
+	/* Check, if length fits into device */
 	if (((imglen / pagelen) * mtd.min_io_size) > (mtd.size - mtdoffset)) {
 		fprintf(stderr, "Image %d bytes, NAND page %d bytes, OOB area %d"
 				" bytes, device size %lld bytes\n",
@@ -479,8 +479,10 @@ int main(int argc, char * const argv[])
 			blockstart = mtdoffset & (~ebsize_aligned + 1);
 			offs = blockstart;
 
-			// if writebuf == filebuf, we are rewinding so we must not
-			// reset the buffer but just replay it
+			/*
+			 * if writebuf == filebuf, we are rewinding so we must
+			 * not reset the buffer but just replay it
+			 */
 			if (writebuf != filebuf) {
 				erase_buffer(filebuf, filebuf_len);
 				filebuf_len = 0;
@@ -515,7 +517,7 @@ int main(int argc, char * const argv[])
 
 		}
 
-		// Read more data from the input if there isn't enough in the buffer
+		/* Read more data from the input if there isn't enough in the buffer */
 		if ((writebuf + mtd.min_io_size) > (filebuf + filebuf_len)) {
 			int readlen = mtd.min_io_size;
 
@@ -524,7 +526,7 @@ int main(int argc, char * const argv[])
 
 			while (tinycnt < readlen) {
 				cnt = read(ifd, writebuf + tinycnt, readlen - tinycnt);
-				if (cnt == 0) { // EOF
+				if (cnt == 0) { /* EOF */
 					break;
 				} else if (cnt < 0) {
 					perror("File I/O error on input");
@@ -570,7 +572,7 @@ int main(int argc, char * const argv[])
 		if (writeoob) {
 			oobreadbuf = writebuf + mtd.min_io_size;
 
-			// Read more data for the OOB from the input if there isn't enough in the buffer
+			/* Read more data for the OOB from the input if there isn't enough in the buffer */
 			if ((oobreadbuf + mtd.oob_size) > (filebuf + filebuf_len)) {
 				int readlen = mtd.oob_size;
 				int alreadyread = (filebuf + filebuf_len) - oobreadbuf;
@@ -578,7 +580,7 @@ int main(int argc, char * const argv[])
 
 				while (tinycnt < readlen) {
 					cnt = read(ifd, oobreadbuf + tinycnt, readlen - tinycnt);
-					if (cnt == 0) { // EOF
+					if (cnt == 0) { /* EOF */
 						break;
 					} else if (cnt < 0) {
 						perror("File I/O error on input");
-- 
1.7.6

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

* [PATCH 05/10] nandwrite: remove `autoplace' features
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
                   ` (3 preceding siblings ...)
  2011-08-19 17:07 ` [PATCH 04/10] nandwrite: remove C99 comment style Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 06/10] nandwrite: kill more MEMSETOOBSEL Brian Norris
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

The `autoplace' option was meant to force an MTD_OOB_AUTO layout while
writing to flash. This option has not been properly supported for a very
long time, as the necessary ioctl, MEMSETOOBSEL, has been removed.
Apparently nobody uses this option. Kill it.

Other code depends on the availability of the `old_oobinfo' data, so we
move some code within a block that handles autoplacement if it's
*already* enabled. This, however, is inconsistent and should be cleaned
up shortly.

Note: this option may be re-implemented in the near future with the
addition of a new ioctl that allows on-the-fly chaning of MTD OOB modes.

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

diff --git a/nandwrite.c b/nandwrite.c
index f58bbfa..2700975 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -60,17 +60,12 @@ static struct nand_oobinfo yaffs_oobinfo = {
 	.eccpos = { 8, 9, 10, 13, 14, 15}
 };
 
-static struct nand_oobinfo autoplace_oobinfo = {
-	.useecc = MTD_NANDECC_AUTOPLACE
-};
-
 static void display_help(void)
 {
 	printf(
 "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"
@@ -114,7 +109,6 @@ static bool		quiet = false;
 static bool		writeoob = false;
 static bool		rawoob = false;
 static bool		onlyoob = false;
-static bool		autoplace = false;
 static bool		markbad = false;
 static bool		forcejffs2 = false;
 static bool		forceyaffs = false;
@@ -130,11 +124,10 @@ static void process_options(int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "ab:fjmnNoOpqrs:y";
+		static const char *short_options = "b:fjmnNoOpqrs:y";
 		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'},
@@ -171,9 +164,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;
@@ -321,25 +311,6 @@ 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 (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, MTD_MODE_RAW);
 		if (ret == 0) {
@@ -374,10 +345,6 @@ int main(int argc, char * const argv[])
 	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;
@@ -608,6 +575,14 @@ int main(int argc, char * const argv[])
 			if (!noecc) {
 				int i, start, len;
 				int tags_pos = 0;
+
+				/* Read the current oob info */
+				if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
+					perror("MEMGETOOBSEL");
+					close(fd);
+					exit(EXIT_FAILURE);
+				}
+
 				/*
 				 * We use autoplacement and have the oobinfo with the autoplacement
 				 * information from the kernel available
-- 
1.7.6

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

* [PATCH 06/10] nandwrite: kill more MEMSETOOBSEL
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
                   ` (4 preceding siblings ...)
  2011-08-19 17:07 ` [PATCH 05/10] nandwrite: remove `autoplace' features Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 07/10] nandwrite: kill -j, -y, and -f options Brian Norris
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |   26 +-------------------------
 1 files changed, 1 insertions(+), 25 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index 2700975..22e9b7f 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -44,10 +44,6 @@
 #include <libmtd.h>
 
 // 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,
@@ -318,18 +314,7 @@ int main(int argc, char * const argv[])
 		} 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;
+				errmsg_die("ioctl MTDFILEMODE is missing");
 			default:
 				perror("MTDFILEMODE");
 				close(fd);
@@ -670,15 +655,6 @@ restoreoob:
 	libmtd_close(mtd_desc);
 	free(filebuf);
 	free(oobbuf);
-
-	if (oobinfochanged == 1) {
-		if (ioctl(fd, MEMSETOOBSEL, &old_oobinfo) != 0) {
-			perror("MEMSETOOBSEL");
-			close(fd);
-			exit(EXIT_FAILURE);
-		}
-	}
-
 	close(fd);
 
 	if (failed
-- 
1.7.6

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

* [PATCH 07/10] nandwrite: kill -j, -y, and -f options
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
                   ` (5 preceding siblings ...)
  2011-08-19 17:07 ` [PATCH 06/10] nandwrite: kill more MEMSETOOBSEL Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 08/10] nandwrite: cleanup "oobinfochanged" leftovers Brian Norris
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

The legacy -j (--jffs2) and -y (--yaffs) options haven't been
operational for a long time, since MEMSETOOBSEL was killed. I don't
think anybody will miss these options (correct me if I'm wrong). They
can be replaced by proper usage of MTD_OOB_AUTO modes.

The -f (--forcelegacy) option went hand in hand with -j and -y. Now that
-j and -y are gone, there's no use for -f. Kill it.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |   60 +----------------------------------------------------------
 1 files changed, 1 insertions(+), 59 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index 22e9b7f..a0f2596 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -43,29 +43,12 @@
 #include "common.h"
 #include <libmtd.h>
 
-// oob layouts to pass into the kernel as default
-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 void display_help(void)
 {
 	printf(
 "Usage: nandwrite [OPTION] MTD_DEVICE [INPUTFILE|-]\n"
 "Writes to the specified MTD device.\n"
 "\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"
 "  -N, --noskipbad         Write without bad block skipping\n"
@@ -106,9 +89,6 @@ static bool		writeoob = false;
 static bool		rawoob = false;
 static bool		onlyoob = false;
 static bool		markbad = false;
-static bool		forcejffs2 = false;
-static bool		forceyaffs = false;
-static bool		forcelegacy = false;
 static bool		noecc = false;
 static bool		noskipbad = false;
 static bool		pad = false;
@@ -120,13 +100,11 @@ static void process_options(int argc, char * const argv[])
 
 	for (;;) {
 		int option_index = 0;
-		static const char *short_options = "b:fjmnNoOpqrs:y";
+		static const char *short_options = "b:mnNoOpqrs:";
 		static const struct option long_options[] = {
 			{"help", no_argument, 0, 0},
 			{"version", no_argument, 0, 0},
 			{"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'},
 			{"noskipbad", no_argument, 0, 'N'},
@@ -136,7 +114,6 @@ static void process_options(int argc, char * const argv[])
 			{"quiet", no_argument, 0, 'q'},
 			{"raw", no_argument, 0, 'r'},
 			{"start", required_argument, 0, 's'},
-			{"yaffs", no_argument, 0, 'y'},
 			{0, 0, 0, 0},
 		};
 
@@ -160,15 +137,6 @@ static void process_options(int argc, char * const argv[])
 			case 'q':
 				quiet = true;
 				break;
-			case 'j':
-				forcejffs2 = true;
-				break;
-			case 'y':
-				forceyaffs = true;
-				break;
-			case 'f':
-				forcelegacy = true;
-				break;
 			case 'n':
 				noecc = true;
 				break;
@@ -323,32 +291,6 @@ int main(int argc, char * const argv[])
 		}
 	}
 
-	/*
-	 * force oob layout for jffs2 or yaffs ?
-	 * Legacy support
-	 */
-	if (forcejffs2 || forceyaffs) {
-		struct nand_oobinfo *oobsel = forcejffs2 ? &jffs2_oobinfo : &yaffs_oobinfo;
-
-		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 (mtd.oob_size == 8) {
-			if (forceyaffs) {
-				fprintf(stderr, "YAFSS cannot operate on 256 Byte page size");
-				goto restoreoob;
-			}
-			/* Adjust number of ecc bytes */
-			jffs2_oobinfo.eccbytes = 3;
-		}
-
-		if (ioctl(fd, MEMSETOOBSEL, oobsel) != 0) {
-			perror("MEMSETOOBSEL");
-			goto restoreoob;
-		}
-	}
-
 	/* Determine if we are reading from standard input or from a file. */
 	if (strcmp(img, standard_input) == 0) {
 		ifd = STDIN_FILENO;
-- 
1.7.6

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

* [PATCH 08/10] nandwrite: cleanup "oobinfochanged" leftovers
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
                   ` (6 preceding siblings ...)
  2011-08-19 17:07 ` [PATCH 07/10] nandwrite: kill -j, -y, and -f options Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 09/10] nandwrite: refactor "old_oobinfo" code Brian Norris
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

We don't really use oobinfochanged anymore, since all the calls to the
MEMSETOOBSEL ioctls are gone. The remaining usage of it is superfluous
now, as the only case where it is changed is under the "noecc" condition
and the only case where it is tested is under the "!noecc" condition.

We also no longer need the "restoreoob" label and can instead simply
close everything done with the single remaining label, "closeall". Note
that `close(-1)' is legal, although useless.

Finally, we move `old_oobinfo' into the only block of code in which it's
used now.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandwrite.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/nandwrite.c b/nandwrite.c
index a0f2596..078e2ff 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -225,8 +225,6 @@ int main(int argc, char * const argv[])
 	struct mtd_dev_info mtd;
 	long long offs;
 	int ret;
-	int oobinfochanged = 0;
-	struct nand_oobinfo old_oobinfo;
 	bool failed = true;
 	/* contains all the data read from the file so far for the current eraseblock */
 	unsigned char *filebuf = NULL;
@@ -277,9 +275,7 @@ int main(int argc, char * const argv[])
 
 	if (noecc)  {
 		ret = ioctl(fd, MTDFILEMODE, MTD_MODE_RAW);
-		if (ret == 0) {
-			oobinfochanged = 2;
-		} else {
+		if (ret) {
 			switch (errno) {
 			case ENOTTY:
 				errmsg_die("ioctl MTDFILEMODE is missing");
@@ -300,7 +296,7 @@ int main(int argc, char * const argv[])
 
 	if (ifd == -1) {
 		perror(img);
-		goto restoreoob;
+		goto closeall;
 	}
 
 	pagelen = mtd.min_io_size + ((writeoob) ? mtd.oob_size : 0);
@@ -502,6 +498,7 @@ int main(int argc, char * const argv[])
 			if (!noecc) {
 				int i, start, len;
 				int tags_pos = 0;
+				struct nand_oobinfo old_oobinfo;
 
 				/* Read the current oob info */
 				if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
@@ -517,7 +514,7 @@ int main(int argc, char * const argv[])
 				 * Modified to support out of order oobfree segments,
 				 * such as the layout used by diskonchip.c
 				 */
-				if (!oobinfochanged && (old_oobinfo.useecc == MTD_NANDECC_AUTOPLACE)) {
+				if (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];
@@ -592,8 +589,6 @@ int main(int argc, char * const argv[])
 
 closeall:
 	close(ifd);
-
-restoreoob:
 	libmtd_close(mtd_desc);
 	free(filebuf);
 	free(oobbuf);
-- 
1.7.6

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

* [PATCH 09/10] nandwrite: refactor "old_oobinfo" code
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
                   ` (7 preceding siblings ...)
  2011-08-19 17:07 ` [PATCH 08/10] nandwrite: cleanup "oobinfochanged" leftovers Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-19 17:07 ` [PATCH 10/10] nanddump: kill usages of MEMSETOOBSEL ioctl Brian Norris
  2011-08-23  6:29 ` [PATCH 00/10] nandwrite: clean out old ioctls Artem Bityutskiy
  10 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

Move variable within conditional and remove duplicated code.

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

diff --git a/nandwrite.c b/nandwrite.c
index 078e2ff..e782aeb 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -497,7 +497,6 @@ int main(int argc, char * const argv[])
 
 			if (!noecc) {
 				int i, start, len;
-				int tags_pos = 0;
 				struct nand_oobinfo old_oobinfo;
 
 				/* Read the current oob info */
@@ -515,16 +514,13 @@ int main(int argc, char * const argv[])
 				 * such as the layout used by diskonchip.c
 				 */
 				if (old_oobinfo.useecc == MTD_NANDECC_AUTOPLACE) {
+					int tags_pos = 0, tmp_ofs;
 					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];
-						if (rawoob)
-							memcpy(oobbuf + start,
-									oobreadbuf + start, len);
-						else
-							memcpy(oobbuf + start,
-									oobreadbuf + tags_pos, len);
+						tmp_ofs = rawoob ? start : tags_pos;
+						memcpy(oobbuf + start, oobreadbuf + tmp_ofs, len);
 						tags_pos += len;
 					}
 				} else {
-- 
1.7.6

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

* [PATCH 10/10] nanddump: kill usages of MEMSETOOBSEL ioctl
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
                   ` (8 preceding siblings ...)
  2011-08-19 17:07 ` [PATCH 09/10] nandwrite: refactor "old_oobinfo" code Brian Norris
@ 2011-08-19 17:07 ` Brian Norris
  2011-08-23  6:31   ` Artem Bityutskiy
  2011-08-23  6:29 ` [PATCH 00/10] nandwrite: clean out old ioctls Artem Bityutskiy
  10 siblings, 1 reply; 14+ messages in thread
From: Brian Norris @ 2011-08-19 17:07 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Kevin Cernekee, Brian Norris, linux-mtd, Mike Frysinger

The ioctl MEMSETOOBSEL hasn't existed for a long time. Using it as a
backup to MTDFILEMODE is pointless, so just remove every time it is
used.

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

diff --git a/nanddump.c b/nanddump.c
index 0b931db..7a24c0d 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -34,10 +34,6 @@
 #include "common.h"
 #include <libmtd.h>
 
-static struct nand_oobinfo none_oobinfo = {
-	.useecc = MTD_NANDECC_OFF,
-};
-
 static void display_help(void)
 {
 	printf(
@@ -309,11 +305,10 @@ int main(int argc, char * const argv[])
 {
 	long long ofs, end_addr = 0;
 	long long blockstart = 1;
-	int ret, i, fd, ofd = 0, bs, badblock = 0;
+	int i, fd, ofd = 0, bs, badblock = 0;
 	struct mtd_dev_info mtd;
 	char pretty_buf[PRETTY_BUF_LEN];
-	int oobinfochanged = 0, firstblock = 1;
-	struct nand_oobinfo old_oobinfo;
+	int firstblock = 1;
 	struct mtd_ecc_stats stat1, stat2;
 	bool eccstats = false;
 	unsigned char *readbuf = NULL, *oobbuf = NULL;
@@ -341,26 +336,9 @@ int main(int argc, char * const argv[])
 	readbuf = xmalloc(sizeof(readbuf) * mtd.min_io_size);
 
 	if (noecc)  {
-		ret = ioctl(fd, MTDFILEMODE, MTD_MODE_RAW);
-		if (ret == 0) {
-			oobinfochanged = 2;
-		} else {
-			switch (errno) {
-			case ENOTTY:
-				if (ioctl(fd, MEMGETOOBSEL, &old_oobinfo) != 0) {
-					perror("MEMGETOOBSEL");
-					goto closeall;
-				}
-				if (ioctl(fd, MEMSETOOBSEL, &none_oobinfo) != 0) {
-					perror("MEMSETOOBSEL");
-					goto closeall;
-				}
-				oobinfochanged = 1;
-				break;
-			default:
+		if (ioctl(fd, MTDFILEMODE, MTD_MODE_RAW) != 0) {
 				perror("MTDFILEMODE");
 				goto closeall;
-			}
 		}
 	} else {
 		/* check if we can read ecc stats */
@@ -498,15 +476,6 @@ int main(int argc, char * const argv[])
 			write(ofd, oobbuf, mtd.oob_size);
 	}
 
-	/* reset oobinfo */
-	if (oobinfochanged == 1) {
-		if (ioctl(fd, MEMSETOOBSEL, &old_oobinfo) != 0) {
-			perror("MEMSETOOBSEL");
-			close(fd);
-			close(ofd);
-			return EXIT_FAILURE;
-		}
-	}
 	/* Close the output file and MTD device, free memory */
 	close(fd);
 	close(ofd);
@@ -517,12 +486,6 @@ int main(int argc, char * const argv[])
 	return EXIT_SUCCESS;
 
 closeall:
-	/* The new mode change is per file descriptor ! */
-	if (oobinfochanged == 1) {
-		if (ioctl(fd, MEMSETOOBSEL, &old_oobinfo) != 0)  {
-			perror("MEMSETOOBSEL");
-		}
-	}
 	close(fd);
 	close(ofd);
 	free(oobbuf);
-- 
1.7.6

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

* Re: [PATCH 00/10] nandwrite: clean out old ioctls
  2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
                   ` (9 preceding siblings ...)
  2011-08-19 17:07 ` [PATCH 10/10] nanddump: kill usages of MEMSETOOBSEL ioctl Brian Norris
@ 2011-08-23  6:29 ` Artem Bityutskiy
  10 siblings, 0 replies; 14+ messages in thread
From: Artem Bityutskiy @ 2011-08-23  6:29 UTC (permalink / raw)
  To: Brian Norris; +Cc: Kevin Cernekee, linux-mtd, Mike Frysinger

On Fri, 2011-08-19 at 10:07 -0700, Brian Norris wrote:
> Hello,
> 
> This series cleans up some coding style, but it primarily deals with
> several options to nandwrite that have rotted. Mostly, this deals with
> MEMSETOOBSEL, which hasn't existed since kernel 2.6.17 - see commit:

Thanks for cleaning the crap which others wouldn't touch with a 10 foot
pole :-)

Pushed with pleasure to mtd-utils.

-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCH 10/10] nanddump: kill usages of MEMSETOOBSEL ioctl
  2011-08-19 17:07 ` [PATCH 10/10] nanddump: kill usages of MEMSETOOBSEL ioctl Brian Norris
@ 2011-08-23  6:31   ` Artem Bityutskiy
  2011-08-23 16:19     ` Brian Norris
  0 siblings, 1 reply; 14+ messages in thread
From: Artem Bityutskiy @ 2011-08-23  6:31 UTC (permalink / raw)
  To: Brian Norris; +Cc: Kevin Cernekee, linux-mtd, Mike Frysinger

On Fri, 2011-08-19 at 10:07 -0700, Brian Norris wrote:
> The ioctl MEMSETOOBSEL hasn't existed for a long time. Using it as a
> backup to MTDFILEMODE is pointless, so just remove every time it is
> used.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Should we kill the macro definition from the kernel as well?

-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCH 10/10] nanddump: kill usages of MEMSETOOBSEL ioctl
  2011-08-23  6:31   ` Artem Bityutskiy
@ 2011-08-23 16:19     ` Brian Norris
  0 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2011-08-23 16:19 UTC (permalink / raw)
  To: dedekind1; +Cc: Kevin Cernekee, linux-mtd, Mike Frysinger

On Mon, Aug 22, 2011 at 11:31 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> Should we kill the macro definition from the kernel as well?

Probably. To be extra safe, we just may want to ensure that we don't
reuse its number.

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

end of thread, other threads:[~2011-08-23 16:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-19 17:07 [PATCH 00/10] nandwrite: clean out old ioctls Brian Norris
2011-08-19 17:07 ` [PATCH 01/10] mtd_debug: fixup style Brian Norris
2011-08-19 17:07 ` [PATCH 02/10] mtd_debug: replace #defines with enum Brian Norris
2011-08-19 17:07 ` [PATCH 03/10] mtd-utils: use __func__ instead of __FUNCTION__ Brian Norris
2011-08-19 17:07 ` [PATCH 04/10] nandwrite: remove C99 comment style Brian Norris
2011-08-19 17:07 ` [PATCH 05/10] nandwrite: remove `autoplace' features Brian Norris
2011-08-19 17:07 ` [PATCH 06/10] nandwrite: kill more MEMSETOOBSEL Brian Norris
2011-08-19 17:07 ` [PATCH 07/10] nandwrite: kill -j, -y, and -f options Brian Norris
2011-08-19 17:07 ` [PATCH 08/10] nandwrite: cleanup "oobinfochanged" leftovers Brian Norris
2011-08-19 17:07 ` [PATCH 09/10] nandwrite: refactor "old_oobinfo" code Brian Norris
2011-08-19 17:07 ` [PATCH 10/10] nanddump: kill usages of MEMSETOOBSEL ioctl Brian Norris
2011-08-23  6:31   ` Artem Bityutskiy
2011-08-23 16:19     ` Brian Norris
2011-08-23  6:29 ` [PATCH 00/10] nandwrite: clean out old ioctls Artem Bityutskiy

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.