[PATCH 1/6] jffs2: make lzo optional at build time

[PATCH 1/6] jffs2: make lzo optional at build time

[Mike Frysinger]

The external lzo dep can be a pain to deal with when cross-compiling,
so make it optional for jffs2.  This is useful if people aren't even
using the functionality, or for quicker development.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 Makefile    |    9 +++++++--
 compr_lzo.c |   15 +++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 6a65258..8bdba8e 100644
--- a/Makefile
+++ b/Makefile
@@ -6,6 +6,11 @@ CPPFLAGS += -I./include $(ZLIBCPPFLAGS) $(LZOCPPFLAGS)
 ifeq ($(WITHOUT_XATTR), 1)
   CPPFLAGS += -DWITHOUT_XATTR
 endif
+ifeq ($(WITHOUT_LZO), 1)
+  CPPFLAGS += -DWITHOUT_LZO
+else
+  LZOLDLIBS = -llzo2
+endif
 
 SUBDIRS = lib ubi-utils mkfs.ubifs
 TESTS = tests
@@ -48,11 +53,11 @@ $(BUILDDIR)/mkfs.jffs2: $(addprefix $(BUILDDIR)/,\
 	compr_rtime.o mkfs.jffs2.o compr_zlib.o compr_lzo.o \
 	compr.o rbtree.o)
 LDFLAGS_mkfs.jffs2 = $(ZLIBLDFLAGS) $(LZOLDFLAGS)
-LDLIBS_mkfs.jffs2  = -lz -llzo2
+LDLIBS_mkfs.jffs2  = -lz $(LZOLDLIBS)
 
 $(BUILDDIR)/jffs2reader: $(BUILDDIR)/jffs2reader.o
 LDFLAGS_jffs2reader = $(ZLIBLDFLAGS) $(LZOLDFLAGS)
-LDLIBS_jffs2reader  = -lz -llzo2
+LDLIBS_jffs2reader  = -lz $(LZOLDLIBS)
 
 $(BUILDDIR)/lib/libmtd.a: subdirs_lib_all ;
 
diff --git a/compr_lzo.c b/compr_lzo.c
index d0f0ed7..d2e2afc 100644
--- a/compr_lzo.c
+++ b/compr_lzo.c
@@ -24,6 +24,8 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
+
+#ifndef WITHOUT_LZO
 #include <asm/types.h>
 #include <linux/jffs2.h>
 #include <lzo/lzo1x.h>
@@ -118,3 +120,16 @@ void jffs2_lzo_exit(void)
 	free(lzo_compress_buf);
 	free(lzo_mem);
 }
+
+#else
+
+int jffs2_lzo_init(void)
+{
+	return 0;
+}
+
+void jffs2_lzo_exit(void)
+{
+}
+
+#endif
-- 
1.7.5.3

[PATCH 2/6] mtdinfo: send help/version info to stdout

[Mike Frysinger]

Usage/version information should go to stdout when it is expected behavior
(i.e. the user requested it explicitly).  This info should go to stderr
only when the usage info is being shown as a result of incorrect options.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 ubi-utils/src/mtdinfo.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
index c9f6f58..f20fe49 100644
--- a/ubi-utils/src/mtdinfo.c
+++ b/ubi-utils/src/mtdinfo.c
@@ -110,13 +110,13 @@ static int parse_opt(int argc, char * const argv[])
 			break;
 
 		case 'h':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
-			fprintf(stderr, "%s\n", PROGRAM_VERSION);
+			printf("%s\n", PROGRAM_VERSION);
 			exit(EXIT_SUCCESS);
 
 		case ':':
-- 
1.7.5.3

[PATCH 3/6] libmtd: use O_CLOEXEC

[Mike Frysinger]

Not strictly necessary, but this is good library behavior and
should carry no runtime overhead.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 include/common.h |    5 +++++
 lib/libmtd.c     |   10 +++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/include/common.h b/include/common.h
index c37660c..7ea282c 100644
--- a/include/common.h
+++ b/include/common.h
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 #include <ctype.h>
 #include <string.h>
+#include <fcntl.h>
 #include <errno.h>
 
 #ifndef PROGRAM_NAME
@@ -42,6 +43,10 @@ extern "C" {
 #define min(a, b) MIN(a, b) /* glue for linux kernel source */
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
 
+#ifndef O_CLOEXEC
+#define O_CLOEXEC 0
+#endif
+
 /* Verbose messages */
 #define bareverbose(verbose, fmt, ...) do {                        \
 	if (verbose)                                               \
diff --git a/lib/libmtd.c b/lib/libmtd.c
index 7fabd80..a651808 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -77,7 +77,7 @@ static int read_data(const char *file, void *buf, int buf_len)
 {
 	int fd, rd, tmp, tmp1;
 
-	fd = open(file, O_RDONLY);
+	fd = open(file, O_RDONLY | O_CLOEXEC);
 	if (fd == -1)
 		return -1;
 
@@ -201,7 +201,7 @@ static int read_hex_ll(const char *file, long long *value)
 	int fd, rd;
 	char buf[50];
 
-	fd = open(file, O_RDONLY);
+	fd = open(file, O_RDONLY | O_CLOEXEC);
 	if (fd == -1)
 		return -1;
 
@@ -253,7 +253,7 @@ static int read_pos_ll(const char *file, long long *value)
 	int fd, rd;
 	char buf[50];
 
-	fd = open(file, O_RDONLY);
+	fd = open(file, O_RDONLY | O_CLOEXEC);
 	if (fd == -1)
 		return -1;
 
@@ -538,7 +538,7 @@ static int sysfs_is_supported(struct libmtd *lib)
 		return 0;
 
 	sprintf(file, lib->mtd_name, num);
-	fd = open(file, O_RDONLY);
+	fd = open(file, O_RDONLY | O_CLOEXEC);
 	if (fd == -1)
 		return 0;
 
@@ -1193,7 +1193,7 @@ int mtd_write_img(const struct mtd_dev_info *mtd, int fd, int eb, int offs,
 		return -1;
 	}
 
-	in_fd = open(img_name, O_RDONLY);
+	in_fd = open(img_name, O_RDONLY | O_CLOEXEC);
 	if (in_fd == -1)
 		return sys_errmsg("cannot open \"%s\"", img_name);
 
-- 
1.7.5.3

[PATCH 4/6] libmtd: add helper funcs for getting fds, regioninfo, and locked info

[Mike Frysinger]

This extends the libmtd with the helper functions:
	mtd_open_dev1: open a MTD device by its number
	mtd_regioninfo: interface to MEMGETREGIONINFO
	mtd_islocked: interface to MEMISLOCKED

Users of these functions will follow shortly ...

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 include/libmtd.h |   37 +++++++++++++++++++++++++++++++++++++
 lib/libmtd.c     |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/include/libmtd.h b/include/libmtd.h
index e30c8a6..ba1348a 100644
--- a/include/libmtd.h
+++ b/include/libmtd.h
@@ -35,6 +35,9 @@ extern "C" {
 /* MTD library descriptor */
 typedef void * libmtd_t;
 
+/* Forward decls */
+struct region_info_user;
+
 /**
  * @mtd_dev_cnt: count of MTD devices in system
  * @lowest_mtd_num: lowest MTD device number in system
@@ -138,6 +141,15 @@ int mtd_get_dev_info(libmtd_t desc, const char *node, struct mtd_dev_info *mtd);
 int mtd_get_dev_info1(libmtd_t desc, int mtd_num, struct mtd_dev_info *mtd);
 
 /**
+ * mtd_open_dev1 - get a fd to an MTD device.
+ * @desc: MTD library descriptor
+ * @mtd_num: MTD device number to fetch information about
+ *
+ * Based on a MTD device number, attempt to open the corresponding node.
+ */
+int mtd_open_dev1(int mtd_num);
+
+/**
  * mtd_lock - lock eraseblocks.
  * @desc: MTD library descriptor
  * @mtd: MTD device description object
@@ -174,6 +186,31 @@ int mtd_unlock(const struct mtd_dev_info *mtd, int fd, int eb);
 int mtd_erase(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb);
 
 /**
+ * mtd_regioninfo - get information about an erase region.
+ * @fd: MTD device node file descriptor
+ * @regidx: index of region to look up
+ * @reginfo: the region information is returned here
+ *
+ * This function gets information about an erase region defined by the
+ * @regidx index and saves this information in the @reginfo object.
+ * Returns %0 in case of success and %-1 in case of failure. If the
+ * @regidx is not valid or unavailable, errno is set to @ENODEV.
+ */
+int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo);
+
+/**
+ * mtd_islocked - see if the specified eraseblock is locked.
+ * @mtd: MTD device description object
+ * @fd: MTD device node file descriptor
+ * @eb: eraseblock to check
+ *
+ * This function checks to see if eraseblock @eb of MTD device described
+ * by @fd is locked. Returns %0 if it is unlocked, %1 if it is locked, and
+ * %-1 in case of failure.
+ */
+int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb);
+
+/**
  * mtd_torture - torture an eraseblock.
  * @desc: MTD library descriptor
  * @mtd: MTD device description object
diff --git a/lib/libmtd.c b/lib/libmtd.c
index a651808..3be28c4 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -787,6 +787,25 @@ int mtd_get_dev_info(libmtd_t desc, const char *node, struct mtd_dev_info *mtd)
 	return mtd_get_dev_info1(desc, mtd_num, mtd);
 }
 
+int mtd_open_dev1(int mtd_num)
+{
+	char devpath[128];
+	int fd;
+
+	sprintf(devpath, "/dev/mtd%i", mtd_num);
+	fd = open(devpath, O_RDONLY | O_CLOEXEC);
+	if (fd < 0)
+		return sys_errmsg("cannot open \"%s\"", devpath);
+
+	/*
+	 * XXX: We could do a stat on the fd and make sure the dev
+	 *      major/minor matches what is expected ... but no real
+	 *      way for us to recover from a mismatch.
+	 */
+
+	return fd;
+}
+
 static inline int mtd_ioctl_error(const struct mtd_dev_info *mtd, int eb,
 				  const char *sreq)
 {
@@ -883,6 +902,38 @@ int mtd_erase(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb)
 	return 0;
 }
 
+int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo)
+{
+	int ret;
+
+	if (regidx < 0) {
+		errno = ENODEV;
+		return -1;
+	}
+
+	ret = ioctl(fd, MEMGETREGIONINFO, reginfo);
+	if (ret < 0)
+		return sys_errmsg("%s ioctl failed for erase region %d",
+			"MEMGETREGIONINFO", regidx);
+
+	return 0;
+}
+
+int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb)
+{
+	int ret;
+	erase_info_t ei;
+
+	ei.start = eb * mtd->eb_size;
+	ei.length = mtd->eb_size;
+
+	ret = ioctl(fd, MEMISLOCKED, &ei);
+	if (ret < 0)
+		return mtd_ioctl_error(mtd, eb, "MEMISLOCKED");
+
+	return ret;
+}
+
 /* Patterns to write to a physical eraseblock when torturing it */
 static uint8_t patterns[] = {0xa5, 0x5a, 0x0};
 
-- 
1.7.5.3

[PATCH 5/6] mtdinfo: add regioninfo/sectormap display

[Mike Frysinger]

This brings the mtdinfo utility in line with the functionality
of the flash_info utility.  It dumps the erase regioninfo (if
the devices has it) as well as showing a handy sector map (if
the user has requested it).  The sector map also utilizes the
MEMISLOCKED ioctl in order to show which sectors exactly are
locked.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 ubi-utils/src/mtdinfo.c |   75 +++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
index f20fe49..3dab27a 100644
--- a/ubi-utils/src/mtdinfo.c
+++ b/ubi-utils/src/mtdinfo.c
@@ -29,6 +29,7 @@
 #include <getopt.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <mtd/mtd-user.h>
 
 #include <libubigen.h>
@@ -41,6 +42,7 @@ struct args {
 	int mtdn;
 	unsigned int all:1;
 	unsigned int ubinfo:1;
+	unsigned int sectormap:1;
 	const char *node;
 };
 
@@ -58,14 +60,15 @@ static const char optionsstr[] =
 "-m, --mtdn=<MTD device number>  MTD device number to get information about\n"
 "-u, --ubi-info                  print what would UBI layout be if it was put\n"
 "                                on this MTD device\n"
+"-s, --sector-map                print sector map\n"
 "-a, --all                       print information about all MTD devices\n"
 "-h, --help                      print help message\n"
 "-V, --version                   print program version";
 
 static const char usage[] =
-"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-h] [-V] [--mtdn <MTD device number>]\n"
+"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-s] [-h] [-V] [--mtdn <MTD device number>]\n"
 "\t\t[--ubi-info] [--help] [--version]\n"
-"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-h] [-V] [--ubi-info] [--help]\n"
+"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-s] [-h] [-V] [--ubi-info] [--help]\n"
 "\t\t[--version]\n"
 "Example 1: " PROGRAM_NAME " - (no arguments) print general MTD information\n"
 "Example 2: " PROGRAM_NAME " -m 1 - print information about MTD device number 1\n"
@@ -78,6 +81,7 @@ static const char usage[] =
 static const struct option long_options[] = {
 	{ .name = "mtdn",      .has_arg = 1, .flag = NULL, .val = 'm' },
 	{ .name = "ubi-info",  .has_arg = 0, .flag = NULL, .val = 'u' },
+	{ .name = "sector-map",.has_arg = 0, .flag = NULL, .val = 's' },
 	{ .name = "all",       .has_arg = 0, .flag = NULL, .val = 'a' },
 	{ .name = "help",      .has_arg = 0, .flag = NULL, .val = 'h' },
 	{ .name = "version",   .has_arg = 0, .flag = NULL, .val = 'V' },
@@ -89,7 +93,7 @@ static int parse_opt(int argc, char * const argv[])
 	while (1) {
 		int key, error = 0;
 
-		key = getopt_long(argc, argv, "am:uhV", long_options, NULL);
+		key = getopt_long(argc, argv, "am:ushV", long_options, NULL);
 		if (key == -1)
 			break;
 
@@ -109,6 +113,10 @@ static int parse_opt(int argc, char * const argv[])
 
 			break;
 
+		case 's':
+			args.sectormap = 1;
+			break;
+
 		case 'h':
 			printf("%s\n\n", doc);
 			printf("%s\n\n", usage);
@@ -159,9 +167,36 @@ static int translate_dev(libmtd_t libmtd, const char *node)
 	return 0;
 }
 
+static void print_sector_map(const struct mtd_dev_info *mtd, int fd,
+			     const region_info_t *reginfo)
+{
+	unsigned long start;
+	int i, width;
+
+	printf("Sector map:\n");
+
+	/* figure out the number of spaces to pad w/out libm */
+	for (i = 1, width = 0; i < reginfo->numblocks; i *= 10, ++width)
+		continue;
+
+	for (i = 0; i < reginfo->numblocks; ++i) {
+		start = reginfo->offset + i * reginfo->erasesize;
+		printf(" %*i: %08lx ", width, i, start);
+		if (mtd_islocked(mtd, fd, i) == 1)
+			printf("RO ");
+		else
+			printf("   ");
+		if (((i + 1) % 4) == 0)
+			printf("\n");
+	}
+	if (i % 4)
+		printf("\n");
+}
+
 static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int mtdn)
 {
-	int err;
+	int err, fd = -1;
+	region_info_t reginfo;
 	struct mtd_dev_info mtd;
 	struct ubigen_info ui;
 
@@ -195,8 +230,25 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
 	if (mtd.oob_size > 0)
 		printf("OOB size:                       %d bytes\n",
 		       mtd.oob_size);
-	if (mtd.region_cnt > 0)
+	if (mtd.region_cnt > 0) {
 		printf("Additional erase regions:       %d\n", mtd.oob_size);
+		fd = mtd_open_dev1(mtdn);
+		if (fd != -1) {
+			int r;
+
+			for (r = 0; r < mtd.region_cnt; ++r) {
+				printf(" region %i: ", r);
+				if (mtd_regioninfo(fd, r, &reginfo) == 0) {
+					printf(" offset: %#x size: %#x numblocks: %#x\n",
+						reginfo.offset, reginfo.erasesize,
+						reginfo.numblocks);
+					if (args.sectormap)
+						print_sector_map(&mtd, fd, &reginfo);
+				} else
+					printf(" info is unavailable\n");
+			}
+		}
+	}
 	if (mtd_info->sysfs_supported)
 		printf("Character device major/minor:   %d:%d\n",
 		       mtd.major, mtd.minor);
@@ -224,6 +276,19 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
 	printf("Maximum UBI volumes count:      %d\n", ui.max_volumes);
 
 out:
+	if (args.sectormap && mtd.region_cnt == 0) {
+		if (fd == -1)
+			fd = mtd_open_dev1(mtdn);
+		reginfo.offset = 0;
+		reginfo.erasesize = mtd.eb_size;
+		reginfo.numblocks = mtd.eb_cnt;
+		reginfo.regionindex = 0;
+		print_sector_map(&mtd, fd, &reginfo);
+	}
+
+	if (fd != -1)
+		close(fd);
+
 	printf("\n");
 	return 0;
 }
-- 
1.7.5.3

[PATCH 6/6] flash_info: punt in favor of mtdinfo

[Mike Frysinger]

Now that the mtdinfo utility can do everything that flash_info
could do (and better), punt it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 Makefile     |    2 +-
 flash_info.c |  141 ----------------------------------------------------------
 2 files changed, 1 insertions(+), 142 deletions(-)
 delete mode 100644 flash_info.c

diff --git a/Makefile b/Makefile
index 8bdba8e..0c8eb9f 100644
--- a/Makefile
+++ b/Makefile
@@ -16,7 +16,7 @@ SUBDIRS = lib ubi-utils mkfs.ubifs
 TESTS = tests
 
 TARGETS = ftl_format flash_erase nanddump doc_loadbios \
-	ftl_check mkfs.jffs2 flash_lock flash_unlock flash_info \
+	ftl_check mkfs.jffs2 flash_lock flash_unlock \
 	flash_otp_info flash_otp_dump mtd_debug flashcp nandwrite nandtest \
 	jffs2dump \
 	nftldump nftl_format docfdisk \
diff --git a/flash_info.c b/flash_info.c
deleted file mode 100644
index 0e056f3..0000000
--- a/flash_info.c
+++ /dev/null
@@ -1,141 +0,0 @@
-/*
- * flash_info.c -- print info about a MTD device
- */
-
-#define PROGRAM_NAME "flash_info"
-
-#include <unistd.h>
-#include <stdlib.h>
-#include <stdio.h>
-#include <fcntl.h>
-#include <time.h>
-#include <sys/ioctl.h>
-#include <sys/mount.h>
-
-#include "common.h"
-#include <mtd/mtd-user.h>
-
-static void usage(int status)
-{
-	fprintf(status ? stderr : stdout,
-		"Usage: %s <device> [devices]\n",
-		PROGRAM_NAME);
-	exit(status);
-}
-
-#define T(t) [MTD_##t] = #t
-static const char * const mtdtypes[] = {
-	T(ABSENT),
-	T(RAM),
-	T(ABSENT),
-	T(RAM),
-	T(ROM),
-	T(NORFLASH),
-	T(NANDFLASH),
-	T(DATAFLASH),
-	T(UBIVOLUME),
-};
-static const char *mtdtype(int type)
-{
-	if (type < ARRAY_SIZE(mtdtypes) && mtdtypes[type])
-		return mtdtypes[type];
-	return "UNKNOWN";
-}
-
-/* Show a pretty map of all the sectors on this device */
-static void show_sector_map(int fd, const region_info_t *reginfo)
-{
-	erase_info_t einfo;
-	int i, width;
-
-	printf(" sector map:\n");
-
-	einfo.length = reginfo->erasesize;
-	/* figure out the number of spaces to pad w/out libm */
-	for (i = 1, width = 0; i < reginfo->numblocks; i *= 10, ++width)
-		continue;
-
-	for (i = 0; i < reginfo->numblocks; ++i) {
-		einfo.start = reginfo->offset + i * reginfo->erasesize;
-		printf(" %*i: %08x ", width, i, einfo.start);
-		if (ioctl(fd, MEMISLOCKED, &einfo))
-			printf("RO ");
-		else
-			printf("   ");
-		if (((i + 1) % 4) == 0)
-			printf("\n");
-	}
-	if (i % 4)
-		printf("\n");
-}
-
-int main(int argc, char *argv[])
-{
-	int fd, i, regcount;
-
-	if (argc < 2)
-		usage(1);
-	if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
-		usage(0);
-
-	for (i = 1; i < argc; ++i) {
-		const char *dev = argv[i];
-		int r;
-		region_info_t reginfo;
-		mtd_info_t mtdinfo;
-
-		/* Open and size the device */
-		fd = open(dev, O_RDONLY);
-		if (fd < 0) {
-			sys_errmsg("could not open: %s", dev);
-			continue;
-		}
-
-		/* Print out general device info first */
-		if (ioctl(fd, MEMGETINFO, &mtdinfo)) {
-			sys_errmsg("could not get mtd info: %s", dev);
-			continue;
-		}
-
-		printf("%s: type: %i (%s)\n flags: %#x ( ",
-			dev, mtdinfo.type, mtdtype(mtdinfo.type), mtdinfo.flags);
-		if (mtdinfo.flags & MTD_POWERUP_LOCK)
-			printf("powerup_lock ");
-		if (mtdinfo.flags & MTD_NO_ERASE)
-			printf("no_erase ");
-		if (mtdinfo.flags & MTD_BIT_WRITEABLE)
-			printf("bit_writeable ");
-		if (mtdinfo.flags & MTD_WRITEABLE)
-			printf("writeable ");
-		printf(")\n sizes: total %#x write %#x erase %#x oob %#x\n",
-			mtdinfo.size, mtdinfo.writesize,
-			mtdinfo.erasesize, mtdinfo.oobsize);
-
-		/* Print out the region info (if the device has any) */
-		if (ioctl(fd, MEMGETREGIONCOUNT, &regcount) == 0) {
-			printf(" erase regions: %i\n", regcount);
-			for (r = 0; r < regcount; ++r) {
-				reginfo.regionindex = r;
-				if (ioctl(fd, MEMGETREGIONINFO, &reginfo) == 0) {
-					printf(" region %i: offset: %#x size: %#x numblocks: %#x\n",
-						r, reginfo.offset, reginfo.erasesize,
-						reginfo.numblocks);
-					show_sector_map(fd, &reginfo);
-				} else
-					warnmsg(" region %i: can not read region info!?", r);
-			}
-		} else
-			regcount = 0;
-
-		/* If the flash has no regions, then show the whole thing */
-		if (regcount == 0) {
-			reginfo.offset = 0;
-			reginfo.erasesize = mtdinfo.erasesize;
-			reginfo.numblocks = mtdinfo.size / mtdinfo.erasesize,
-			reginfo.regionindex = 0;
-			show_sector_map(fd, &reginfo);
-		}
-	}
-
-	return 0;
-}
-- 
1.7.5.3

Re: [PATCH 1/6] jffs2: make lzo optional at build time

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> The external lzo dep can be a pain to deal with when cross-compiling,
> so make it optional for jffs2.  This is useful if people aren't even
> using the functionality, or for quicker development.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

How about mkfs.ubifs - it requires lzo as well. And then mkfs.jffs2
wants libacl which might be the same PITA.

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

Re: [PATCH 2/6] mtdinfo: send help/version info to stdout

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> Usage/version information should go to stdout when it is expected behavior
> (i.e. the user requested it explicitly).  This info should go to stderr
> only when the usage info is being shown as a result of incorrect options.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

This is fine, but all other ubi utils need this change then as well. I
think this piece of code was copypasted in all ubi-utils, so you should
easily find it and change.

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

Re: [PATCH 3/6] libmtd: use O_CLOEXEC

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> Not strictly necessary, but this is good library behavior and
> should carry no runtime overhead.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  include/common.h |    5 +++++
>  lib/libmtd.c     |   10 +++++-----
>  2 files changed, 10 insertions(+), 5 deletions(-)

Pushed this one, thanks!

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

Re: [PATCH 4/6] libmtd: add helper funcs for getting fds, regioninfo, and locked info

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote: 
> +int mtd_open_dev1(int mtd_num)
> +{
> +	char devpath[128];
> +	int fd;
> +
> +	sprintf(devpath, "/dev/mtd%i", mtd_num);
> +	fd = open(devpath, O_RDONLY | O_CLOEXEC);
> +	if (fd < 0)
> +		return sys_errmsg("cannot open \"%s\"", devpath);
> +

Could we avoid hard-coding device node names?

We indeed have -m option in mtdutils which opens a device by its number.
But I think it was a mistake. Generally, MTD devices can have any name,
it is up to udev configuration. So I'd rather deprecate and would
require a list of device node names.

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

Re: [PATCH 4/6] libmtd: add helper funcs for getting fds, regioninfo, and locked info

[Artem Bityutskiy]

On Tue, 2011-06-07 at 09:50 +0300, Artem Bityutskiy wrote:
> We indeed have -m option in mtdutils which opens a device by its number.
> But I think it was a mistake. Generally, MTD devices can have any name,
> it is up to udev configuration. So I'd rather deprecate and would
> require a list of device node names.

Sorry, I was not clear. I wanted to say that supports -m and does
hard-code the mtd device name. But this was a mistake because it gives a
bad example, and adding more hard-coded names is bad.

So I think this option can be deprecated and removed, and we should only
accept full device node names.

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

Re: [PATCH 1/6] jffs2: make lzo optional at build time

[Mike Frysinger]

On Tue, Jun 7, 2011 at 02:34, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
>> The external lzo dep can be a pain to deal with when cross-compiling,
>> so make it optional for jffs2.  This is useful if people aren't even
>> using the functionality, or for quicker development.
>
> How about mkfs.ubifs - it requires lzo as well. And then mkfs.jffs2
> wants libacl which might be the same PITA.

libacl already has a knob.  i didnt bother with mkfs.ubifs because it
additionally requires libuuid which comes from util-linux.  also, it
seemed like lzo is the only compression supported by UBI, so making it
optional didnt seem like it'd result in anything useful.  at least
jffs2 can operate without it by using other compression schemes.
-mike

Re: [PATCH 4/6] libmtd: add helper funcs for getting fds, regioninfo, and locked info

[Mike Frysinger]

On Tue, Jun 7, 2011 at 02:56, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 09:50 +0300, Artem Bityutskiy wrote:
>> We indeed have -m option in mtdutils which opens a device by its number.
>> But I think it was a mistake. Generally, MTD devices can have any name,
>> it is up to udev configuration. So I'd rather deprecate and would
>> require a list of device node names.
>
> Sorry, I was not clear. I wanted to say that supports -m and does
> hard-code the mtd device name. But this was a mistake because it gives a
> bad example, and adding more hard-coded names is bad.
>
> So I think this option can be deprecated and removed, and we should only
> accept full device node names.

i only added this because mtdinfo has the -m option.  i have no
problem with punting that and going back to requiring people to
specify the path to the device nodes.  i'm off now though, so i'll get
to it tomorrow.
-mike

Re: [PATCH 1/6] jffs2: make lzo optional at build time

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> +
> +#else
> +
> +int jffs2_lzo_init(void)
> +{
> +	return 0;
> +}

This function should return -1, I think, so that mkfs.jffs2 would fail
if lzo was not compiled-in.

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

Re: [PATCH 1/6] jffs2: make lzo optional at build time

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:59 -0400, Mike Frysinger wrote:
> On Tue, Jun 7, 2011 at 02:34, Artem Bityutskiy wrote:
> > On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> >> The external lzo dep can be a pain to deal with when cross-compiling,
> >> so make it optional for jffs2.  This is useful if people aren't even
> >> using the functionality, or for quicker development.
> >
> > How about mkfs.ubifs - it requires lzo as well. And then mkfs.jffs2
> > wants libacl which might be the same PITA.
> 
> libacl already has a knob.

OK.

>   i didnt bother with mkfs.ubifs because it
> additionally requires libuuid which comes from util-linux.

Yeah. The only reason for this is that it wants to generate an UUID for
the new file-system. We could just use pseudo-random numbers as a
work-around.

>   also, it
> seemed like lzo is the only compression supported by UBI, so making it
> optional didnt seem like it'd result in anything useful.

Not really, UBIFS currently supports lzo, zlib, and no compression. But
lzo is the default.

>   at least
> jffs2 can operate without it by using other compression schemes.

UBIFS as well.

But please, do not bother with UBIFS if you do not need this. Just
verify that mkfs.jffs2 fails if you have not compiled lzo and try to use
it. I mean, mkfs.jffs2 should not silently use zlib in this case.

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

Re: [PATCH 4/6] libmtd: add helper funcs for getting fds, regioninfo, and locked info

[Artem Bityutskiy]

On Tue, 2011-06-07 at 03:04 -0400, Mike Frysinger wrote:
> On Tue, Jun 7, 2011 at 02:56, Artem Bityutskiy wrote:
> > On Tue, 2011-06-07 at 09:50 +0300, Artem Bityutskiy wrote:
> >> We indeed have -m option in mtdutils which opens a device by its number.
> >> But I think it was a mistake. Generally, MTD devices can have any name,
> >> it is up to udev configuration. So I'd rather deprecate and would
> >> require a list of device node names.
> >
> > Sorry, I was not clear. I wanted to say that supports -m and does
> > hard-code the mtd device name. But this was a mistake because it gives a
> > bad example, and adding more hard-coded names is bad.
> >
> > So I think this option can be deprecated and removed, and we should only
> > accept full device node names.
> 
> i only added this because mtdinfo has the -m option.  i have no
> problem with punting that and going back to requiring people to
> specify the path to the device nodes.  i'm off now though, so i'll get
> to it tomorrow.

Thanks! I've just pushed the following patch which deprecates it:

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Date: Tue, 7 Jun 2011 10:36:26 +0300
Subject: [PATCH] mtdinfo: deprecate the -m option

... because mtd device node name do not have to follow the "/dev/mtd%d"
pattern.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 feature-removal-schedule.txt |    9 +++++++--
 ubi-utils/src/mtdinfo.c      |    3 ++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/feature-removal-schedule.txt b/feature-removal-schedule.txt
index 30f0403..6e2bfc9 100644
--- a/feature-removal-schedule.txt
+++ b/feature-removal-schedule.txt
@@ -20,11 +20,16 @@ neither --squash-rino-perm nor --nosquash-rino-perm were used,
 mkfs.ubifs printed a warning. This was introduced in mtd-utils-1.4.0 (13 Sep 2010).
 
 Now we have removed a warning and made --nosquash-rino-perm to be the
-default. Also, both options are declared depricated, so users should try
+default. Also, both options are declared deprecated, so users should try
 to stop using them.
 
 The further step is to remove both of them.
 
 ---------------------------
-2.
+2. Kill -m parameter of mtdinfo
+
+We cannot assume that mtd device names follow the "/dev/mtd%d" pattern,
+because it is up to udev rules to name the devices. So we are removing
+the -m option. For now, we just have a warning, but the option will be
+removed in release 1.4.6.
 ---------------------------
diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
index c9f6f58..820c16d 100644
--- a/ubi-utils/src/mtdinfo.c
+++ b/ubi-utils/src/mtdinfo.c
@@ -56,6 +56,7 @@ static const char doc[] = PROGRAM_NAME " version " PROGRAM_VERSION
 
 static const char optionsstr[] =
 "-m, --mtdn=<MTD device number>  MTD device number to get information about\n"
+"                                (deprecated option, will be removed, do not use)\n"
 "-u, --ubi-info                  print what would UBI layout be if it was put\n"
 "                                on this MTD device\n"
 "-a, --all                       print information about all MTD devices\n"
@@ -106,7 +107,7 @@ static int parse_opt(int argc, char * const argv[])
 			args.mtdn = simple_strtoul(optarg, &error);
 			if (error || args.mtdn < 0)
 				return errmsg("bad MTD device number: \"%s\"", optarg);
-
+			warnmsg("-m/--mtdn is depecated, will be removed in mtd-utils-1.4.6");
 			break;
 
 		case 'h':
-- 
1.7.2.3



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

Re: [PATCH 5/6] mtdinfo: add regioninfo/sectormap display

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> @@ -58,14 +60,15 @@ static const char optionsstr[] =
>  "-m, --mtdn=<MTD device number>  MTD device number to get information about\n"
>  "-u, --ubi-info                  print what would UBI layout be if it was put\n"
>  "                                on this MTD device\n"
> +"-s, --sector-map                print sector map\n"
>  "-a, --all                       print information about all MTD devices\n"
>  "-h, --help                      print help message\n"
>  "-V, --version                   print program version";
>  
>  static const char usage[] =
> -"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-h] [-V] [--mtdn <MTD device number>]\n"
> +"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-s] [-h] [-V] [--mtdn <MTD device number>]\n"
>  "\t\t[--ubi-info] [--help] [--version]\n"
> -"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-h] [-V] [--ubi-info] [--help]\n"
> +"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-s] [-h] [-V] [--ubi-info] [--help]\n"
>  "\t\t[--version]\n"
>  "Example 1: " PROGRAM_NAME " - (no arguments) print general MTD information\n"
>  "Example 2: " PROGRAM_NAME " -m 1 - print information about MTD device number 1\n"
> @@ -78,6 +81,7 @@ static const char usage[] =
>  static const struct option long_options[] = {
>  	{ .name = "mtdn",      .has_arg = 1, .flag = NULL, .val = 'm' },
>  	{ .name = "ubi-info",  .has_arg = 0, .flag = NULL, .val = 'u' },
> +	{ .name = "sector-map",.has_arg = 0, .flag = NULL, .val = 's' },

Is sector-map a good name? May be we should call this feature "lock
info" or something which suggests this is about detailed lock
information? In this case you should also print an error message if the
ISLOCKED ioctl is not supported. Probably adding a "islocked_supported"
flag to 'struct mtd_info' (similarly to sysfs_supported) makes sense?
Then you could just check this flag and refuse -s option?

Also, could we avoid using term sector because it is overloaded and
confusing. Could you please use term "eraseblock" instead (in prints and
in internal names)?

Or you meant that this is like general per-sector info? So if we have
more information to print, we add it there? In this case, could you
please print "bad" or something for bad eraseblocks as well?

Thanks!

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

Re: [PATCH 6/6] flash_info: punt in favor of mtdinfo

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> Now that the mtdinfo utility can do everything that flash_info
> could do (and better), punt it.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  Makefile     |    2 +-
>  flash_info.c |  141 ----------------------------------------------------------
>  2 files changed, 1 insertions(+), 142 deletions(-)
>  delete mode 100644 flash_info.c

Let's give it some deprecation period. Would it be possible to add an
entry to feature-removal-schedule.txt instead, and print a warning in
flash_info that it will be removed in mtd-utils-1.4.6. So we'll let it
be in 1.4.5 with this warning. Then I'll remove it myself in 1.4.6.

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

Re: [PATCH 2/6] mtdinfo: send help/version info to stdout

[Florian Fainelli]

On Tuesday 07 June 2011 08:19:04 Mike Frysinger wrote:
> Usage/version information should go to stdout when it is expected behavior
> (i.e. the user requested it explicitly).  This info should go to stderr
> only when the usage info is being shown as a result of incorrect options.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  ubi-utils/src/mtdinfo.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
> index c9f6f58..f20fe49 100644
> --- a/ubi-utils/src/mtdinfo.c
> +++ b/ubi-utils/src/mtdinfo.c
> @@ -110,13 +110,13 @@ static int parse_opt(int argc, char * const argv[])
>  			break;
> 
>  		case 'h':
> -			fprintf(stderr, "%s\n\n", doc);
> -			fprintf(stderr, "%s\n\n", usage);
> -			fprintf(stderr, "%s\n", optionsstr);
> +			printf("%s\n\n", doc);
> +			printf("%s\n\n", usage);
> +			printf("%s\n", optionsstr);

Why not use fprintf(stdout ...), instead of printf()? to be consistent with was is existing?

>  			exit(EXIT_SUCCESS);
> 
>  		case 'V':
> -			fprintf(stderr, "%s\n", PROGRAM_VERSION);
> +			printf("%s\n", PROGRAM_VERSION);
>  			exit(EXIT_SUCCESS);
> 
>  		case ':':

Re: [PATCH 2/6] mtdinfo: send help/version info to stdout

[Mike Frysinger]

On Tue, Jun 7, 2011 at 02:36, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
>> Usage/version information should go to stdout when it is expected behavior
>> (i.e. the user requested it explicitly).  This info should go to stderr
>> only when the usage info is being shown as a result of incorrect options.
>
> This is fine, but all other ubi utils need this change then as well. I
> think this piece of code was copypasted in all ubi-utils, so you should
> easily find it and change.

you just like making me work :p
-mike

[PATCH v2] ubi-utils: send help/version info to stdout

[Mike Frysinger]

Usage/version information should go to stdout when it is expected behavior
(i.e. the user requested it explicitly).  This info should go to stderr
only when the usage info is being shown as a result of incorrect options.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- convert all ubi-utils

 ubi-utils/src/mtdinfo.c      |    8 ++++----
 ubi-utils/src/ubiattach.c    |    6 +++---
 ubi-utils/src/ubicrc32.c     |    6 +++---
 ubi-utils/src/ubidetach.c    |    6 +++---
 ubi-utils/src/ubiformat.c    |    6 +++---
 ubi-utils/src/ubimkvol.c     |    6 +++---
 ubi-utils/src/ubinfo.c       |    6 +++---
 ubi-utils/src/ubinize.c      |    8 ++++----
 ubi-utils/src/ubirmvol.c     |    6 +++---
 ubi-utils/src/ubirsvol.c     |    6 +++---
 ubi-utils/src/ubiupdatevol.c |    6 +++---
 11 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
index 820c16d..666d739 100644
--- a/ubi-utils/src/mtdinfo.c
+++ b/ubi-utils/src/mtdinfo.c
@@ -111,13 +111,13 @@ static int parse_opt(int argc, char * const argv[])
 			break;
 
 		case 'h':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
-			fprintf(stderr, "%s\n", PROGRAM_VERSION);
+			printf("%s\n", PROGRAM_VERSION);
 			exit(EXIT_SUCCESS);
 
 		case ':':
diff --git a/ubi-utils/src/ubiattach.c b/ubi-utils/src/ubiattach.c
index 9297b56..4f18e99 100644
--- a/ubi-utils/src/ubiattach.c
+++ b/ubi-utils/src/ubiattach.c
@@ -123,9 +123,9 @@ static int parse_opt(int argc, char * const argv[])
 			break;
 
 		case 'h':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
diff --git a/ubi-utils/src/ubicrc32.c b/ubi-utils/src/ubicrc32.c
index a09f053..73ec595 100644
--- a/ubi-utils/src/ubicrc32.c
+++ b/ubi-utils/src/ubicrc32.c
@@ -64,9 +64,9 @@ static int parse_opt(int argc, char * const argv[])
 
 		switch (key) {
 		case 'h':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
diff --git a/ubi-utils/src/ubidetach.c b/ubi-utils/src/ubidetach.c
index 5ee55f1..668f1bd 100644
--- a/ubi-utils/src/ubidetach.c
+++ b/ubi-utils/src/ubidetach.c
@@ -107,9 +107,9 @@ static int parse_opt(int argc, char * const argv[])
 			break;
 
 		case 'h':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
diff --git a/ubi-utils/src/ubiformat.c b/ubi-utils/src/ubiformat.c
index 6e5cdb8..c4b944a 100644
--- a/ubi-utils/src/ubiformat.c
+++ b/ubi-utils/src/ubiformat.c
@@ -211,9 +211,9 @@ static int parse_opt(int argc, char * const argv[])
 
 		case 'h':
 		case '?':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case ':':
diff --git a/ubi-utils/src/ubimkvol.c b/ubi-utils/src/ubimkvol.c
index 935f068..25065e3 100644
--- a/ubi-utils/src/ubimkvol.c
+++ b/ubi-utils/src/ubimkvol.c
@@ -167,9 +167,9 @@ static int parse_opt(int argc, char * const argv[])
 
 		case 'h':
 		case '?':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
diff --git a/ubi-utils/src/ubinfo.c b/ubi-utils/src/ubinfo.c
index 2bfee16..8e14e6e 100644
--- a/ubi-utils/src/ubinfo.c
+++ b/ubi-utils/src/ubinfo.c
@@ -118,9 +118,9 @@ static int parse_opt(int argc, char * const argv[])
 			break;
 
 		case 'h':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
diff --git a/ubi-utils/src/ubinize.c b/ubi-utils/src/ubinize.c
index 52a193f..3085b66 100644
--- a/ubi-utils/src/ubinize.c
+++ b/ubi-utils/src/ubinize.c
@@ -227,10 +227,10 @@ static int parse_opt(int argc, char * const argv[])
 			break;
 
 		case 'h':
-			ubiutils_print_text(stderr, doc, 80);
-			fprintf(stderr, "\n%s\n\n", ini_doc);
-			fprintf(stderr, "%s\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			ubiutils_print_text(stdout, doc, 80);
+			printf("\n%s\n\n", ini_doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
diff --git a/ubi-utils/src/ubirmvol.c b/ubi-utils/src/ubirmvol.c
index 5a7217a..5725d90 100644
--- a/ubi-utils/src/ubirmvol.c
+++ b/ubi-utils/src/ubirmvol.c
@@ -111,9 +111,9 @@ static int parse_opt(int argc, char * const argv[])
 
 		case 'h':
 		case '?':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
diff --git a/ubi-utils/src/ubirsvol.c b/ubi-utils/src/ubirsvol.c
index 34321b8..65f579c 100644
--- a/ubi-utils/src/ubirsvol.c
+++ b/ubi-utils/src/ubirsvol.c
@@ -140,9 +140,9 @@ static int parse_opt(int argc, char * const argv[])
 
 		case 'h':
 		case '?':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
diff --git a/ubi-utils/src/ubiupdatevol.c b/ubi-utils/src/ubiupdatevol.c
index 62f140b..24f38fe 100644
--- a/ubi-utils/src/ubiupdatevol.c
+++ b/ubi-utils/src/ubiupdatevol.c
@@ -97,9 +97,9 @@ static int parse_opt(int argc, char * const argv[])
 
 		case 'h':
 		case '?':
-			fprintf(stderr, "%s\n\n", doc);
-			fprintf(stderr, "%s\n\n", usage);
-			fprintf(stderr, "%s\n", optionsstr);
+			printf("%s\n\n", doc);
+			printf("%s\n\n", usage);
+			printf("%s\n", optionsstr);
 			exit(EXIT_SUCCESS);
 
 		case 'V':
-- 
1.7.5.3

[PATCH v2] flash_info: deprecate

[Mike Frysinger]

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- just deprecate for now rather than straight up removal

 feature-removal-schedule.txt |    7 +++++++
 flash_info.c                 |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/feature-removal-schedule.txt b/feature-removal-schedule.txt
index 6e2bfc9..a5980f7 100644
--- a/feature-removal-schedule.txt
+++ b/feature-removal-schedule.txt
@@ -32,4 +32,11 @@ We cannot assume that mtd device names follow the "/dev/mtd%d" pattern,
 because it is up to udev rules to name the devices. So we are removing
 the -m option. For now, we just have a warning, but the option will be
 removed in release 1.4.6.
+
+---------------------------
+3. flash_info utility
+
+This is duplicating behavior with the mtdinfo utility.  Now the util
+warns when people use it, but it'll be removed in release 1.4.6.
+
 ---------------------------
diff --git a/flash_info.c b/flash_info.c
index b710034..b15e0a9 100644
--- a/flash_info.c
+++ b/flash_info.c
@@ -73,6 +73,8 @@ int main(int argc, char *argv[])
 {
 	int fd, i, regcount;
 
+	warnmsg("this is deprecated in favor of `mtdinfo`");
+
 	if (argc < 2)
 		usage(1);
 	if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
-- 
1.7.5.3

Re: [PATCH 1/6] jffs2: make lzo optional at build time

[Mike Frysinger]

On Tue, Jun 7, 2011 at 03:16, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
>> +int jffs2_lzo_init(void)
>> +{
>> +     return 0;
>> +}
>
> This function should return -1, I think, so that mkfs.jffs2 would fail
> if lzo was not compiled-in.

i didnt do that because the func is always called and i was afraid
that it'd prevent normal execution.  the runtime code should already
fail when lzo is requested because the compression routines are looked
up on the fly and when the requested compressor isnt found, it fails
at that time.  (at least that's my understanding of things)
-mike

[PATCH v2] libmtd: add helper funcs for getting regioninfo and locked info

[Mike Frysinger]

This extends the libmtd with the helper functions:
	mtd_regioninfo: interface to MEMGETREGIONINFO
	mtd_islocked: interface to MEMISLOCKED

Users of these functions will follow shortly ...

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- drop the open-by-mtd-num func

 include/libmtd.h |   28 ++++++++++++++++++++++++++++
 lib/libmtd.c     |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/include/libmtd.h b/include/libmtd.h
index e30c8a6..7275246 100644
--- a/include/libmtd.h
+++ b/include/libmtd.h
@@ -35,6 +35,9 @@ extern "C" {
 /* MTD library descriptor */
 typedef void * libmtd_t;
 
+/* Forward decls */
+struct region_info_user;
+
 /**
  * @mtd_dev_cnt: count of MTD devices in system
  * @lowest_mtd_num: lowest MTD device number in system
@@ -174,6 +177,31 @@ int mtd_unlock(const struct mtd_dev_info *mtd, int fd, int eb);
 int mtd_erase(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb);
 
 /**
+ * mtd_regioninfo - get information about an erase region.
+ * @fd: MTD device node file descriptor
+ * @regidx: index of region to look up
+ * @reginfo: the region information is returned here
+ *
+ * This function gets information about an erase region defined by the
+ * @regidx index and saves this information in the @reginfo object.
+ * Returns %0 in case of success and %-1 in case of failure. If the
+ * @regidx is not valid or unavailable, errno is set to @ENODEV.
+ */
+int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo);
+
+/**
+ * mtd_islocked - see if the specified eraseblock is locked.
+ * @mtd: MTD device description object
+ * @fd: MTD device node file descriptor
+ * @eb: eraseblock to check
+ *
+ * This function checks to see if eraseblock @eb of MTD device described
+ * by @fd is locked. Returns %0 if it is unlocked, %1 if it is locked, and
+ * %-1 in case of failure.
+ */
+int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb);
+
+/**
  * mtd_torture - torture an eraseblock.
  * @desc: MTD library descriptor
  * @mtd: MTD device description object
diff --git a/lib/libmtd.c b/lib/libmtd.c
index a651808..0086a85 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -883,6 +883,38 @@ int mtd_erase(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb)
 	return 0;
 }
 
+int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo)
+{
+	int ret;
+
+	if (regidx < 0) {
+		errno = ENODEV;
+		return -1;
+	}
+
+	ret = ioctl(fd, MEMGETREGIONINFO, reginfo);
+	if (ret < 0)
+		return sys_errmsg("%s ioctl failed for erase region %d",
+			"MEMGETREGIONINFO", regidx);
+
+	return 0;
+}
+
+int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb)
+{
+	int ret;
+	erase_info_t ei;
+
+	ei.start = eb * mtd->eb_size;
+	ei.length = mtd->eb_size;
+
+	ret = ioctl(fd, MEMISLOCKED, &ei);
+	if (ret < 0)
+		return mtd_ioctl_error(mtd, eb, "MEMISLOCKED");
+
+	return ret;
+}
+
 /* Patterns to write to a physical eraseblock when torturing it */
 static uint8_t patterns[] = {0xa5, 0x5a, 0x0};
 
-- 
1.7.5.3

Re: [PATCH 5/6] mtdinfo: add regioninfo/sectormap display

[Mike Frysinger]

On Tue, Jun 7, 2011 at 03:41, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
>> @@ -58,14 +60,15 @@ static const char optionsstr[] =
>>  "-m, --mtdn=<MTD device number>  MTD device number to get information about\n"
>>  "-u, --ubi-info                  print what would UBI layout be if it was put\n"
>>  "                                on this MTD device\n"
>> +"-s, --sector-map                print sector map\n"
>>  "-a, --all                       print information about all MTD devices\n"
>>  "-h, --help                      print help message\n"
>>  "-V, --version                   print program version";
>>
>>  static const char usage[] =
>> -"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-h] [-V] [--mtdn <MTD device number>]\n"
>> +"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-s] [-h] [-V] [--mtdn <MTD device number>]\n"
>>  "\t\t[--ubi-info] [--help] [--version]\n"
>> -"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-h] [-V] [--ubi-info] [--help]\n"
>> +"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-s] [-h] [-V] [--ubi-info] [--help]\n"
>>  "\t\t[--version]\n"
>>  "Example 1: " PROGRAM_NAME " - (no arguments) print general MTD information\n"
>>  "Example 2: " PROGRAM_NAME " -m 1 - print information about MTD device number 1\n"
>> @@ -78,6 +81,7 @@ static const char usage[] =
>>  static const struct option long_options[] = {
>>       { .name = "mtdn",      .has_arg = 1, .flag = NULL, .val = 'm' },
>>       { .name = "ubi-info",  .has_arg = 0, .flag = NULL, .val = 'u' },
>> +     { .name = "sector-map",.has_arg = 0, .flag = NULL, .val = 's' },
>
> Is sector-map a good name?

it's what i'm used to calling it ;)

> May be we should call this feature "lock
> info" or something which suggests this is about detailed lock
> information? In this case you should also print an error message if the
> ISLOCKED ioctl is not supported. Probably adding a "islocked_supported"
> flag to 'struct mtd_info' (similarly to sysfs_supported) makes sense?
> Then you could just check this flag and refuse -s option?

honestly, i was more interested in the sector map than the ISLOCKED
info.  that was just a bonus for me.

> Also, could we avoid using term sector because it is overloaded and
> confusing. Could you please use term "eraseblock" instead (in prints and
> in internal names)?

ok

> Or you meant that this is like general per-sector info? So if we have
> more information to print, we add it there? In this case, could you
> please print "bad" or something for bad eraseblocks as well?

if MEMGETBADBLOCK provides that info, should be easy to add
-mike

[PATCH v3] libmtd: add helper funcs for getting regioninfo and locked info

[Mike Frysinger]

This extends the libmtd with the helper functions:
	mtd_regioninfo: interface to MEMGETREGIONINFO
	mtd_islocked: interface to MEMISLOCKED

Users of these functions will follow shortly ...

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3
	- add a "islocked_supported" field to handle older kernels nicely

 include/libmtd.h |   28 ++++++++++++++++++++++++++++
 lib/libmtd.c     |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/libmtd_int.h |    1 +
 3 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/include/libmtd.h b/include/libmtd.h
index e30c8a6..2da12c3 100644
--- a/include/libmtd.h
+++ b/include/libmtd.h
@@ -35,6 +35,9 @@ extern "C" {
 /* MTD library descriptor */
 typedef void * libmtd_t;
 
+/* Forward decls */
+struct region_info_user;
+
 /**
  * @mtd_dev_cnt: count of MTD devices in system
  * @lowest_mtd_num: lowest MTD device number in system
@@ -174,6 +177,31 @@ int mtd_unlock(const struct mtd_dev_info *mtd, int fd, int eb);
 int mtd_erase(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb);
 
 /**
+ * mtd_regioninfo - get information about an erase region.
+ * @fd: MTD device node file descriptor
+ * @regidx: index of region to look up
+ * @reginfo: the region information is returned here
+ *
+ * This function gets information about an erase region defined by the
+ * @regidx index and saves this information in the @reginfo object.
+ * Returns %0 in case of success and %-1 in case of failure. If the
+ * @regidx is not valid or unavailable, errno is set to @ENODEV.
+ */
+int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo);
+
+/**
+ * mtd_is_locked - see if the specified eraseblock is locked.
+ * @mtd: MTD device description object
+ * @fd: MTD device node file descriptor
+ * @eb: eraseblock to check
+ *
+ * This function checks to see if eraseblock @eb of MTD device described
+ * by @fd is locked. Returns %0 if it is unlocked, %1 if it is locked, and
+ * %-1 in case of failure.
+ */
+int mtd_is_locked(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb);
+
+/**
  * mtd_torture - torture an eraseblock.
  * @desc: MTD library descriptor
  * @mtd: MTD device description object
diff --git a/lib/libmtd.c b/lib/libmtd.c
index a651808..d6094c1 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -557,6 +557,7 @@ libmtd_t libmtd_open(void)
 	lib = xzalloc(sizeof(*lib));
 
 	lib->offs64_ioctls = OFFS64_IOCTLS_UNKNOWN;
+	lib->islocked_ioctl = 1;
 
 	lib->sysfs_mtd = mkpath("/sys", SYSFS_MTD);
 	if (!lib->sysfs_mtd)
@@ -883,6 +884,52 @@ int mtd_erase(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb)
 	return 0;
 }
 
+int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo)
+{
+	int ret;
+
+	if (regidx < 0) {
+		errno = ENODEV;
+		return -1;
+	}
+
+	ret = ioctl(fd, MEMGETREGIONINFO, reginfo);
+	if (ret < 0)
+		return sys_errmsg("%s ioctl failed for erase region %d",
+			"MEMGETREGIONINFO", regidx);
+
+	return 0;
+}
+
+int mtd_is_locked(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb)
+{
+	int ret;
+	struct libmtd *lib = (struct libmtd *)desc;
+	erase_info_t ei;
+
+	if (lib->islocked_ioctl == 0)
+		return -1;
+
+	ei.start = eb * mtd->eb_size;
+	ei.length = mtd->eb_size;
+
+	ret = ioctl(fd, MEMISLOCKED, &ei);
+	if (ret < 0) {
+		if (errno == ENOTTY) {
+			/* An old system w/out MEMISLOCKED */
+			lib->islocked_ioctl = 0;
+			return -1;
+		} else if (errno == EOPNOTSUPP) {
+			/* MTD doesn't support locking */
+			lib->islocked_ioctl = 0;
+			return -1;
+		}
+		return mtd_ioctl_error(mtd, eb, "MEMISLOCKED");
+	}
+
+	return ret;
+}
+
 /* Patterns to write to a physical eraseblock when torturing it */
 static uint8_t patterns[] = {0xa5, 0x5a, 0x0};
 
diff --git a/lib/libmtd_int.h b/lib/libmtd_int.h
index bb48d35..87e7f12 100644
--- a/lib/libmtd_int.h
+++ b/lib/libmtd_int.h
@@ -91,6 +91,7 @@ struct libmtd
 	char *mtd_region_cnt;
 	char *mtd_flags;
 	unsigned int sysfs_supported:1;
+	unsigned int islocked_ioctl:1;
 	unsigned int offs64_ioctls:2;
 };
 
-- 
1.7.5.3

[PATCH v2] mtdinfo: add regioninfo/eraseblock map display

[Mike Frysinger]

This brings the mtdinfo utility in line with the functionality
of the flash_info utility.  It dumps the erase regioninfo (if
the devices has it) as well as showing a handy eraseblock map
(if the user has requested it).  The eraseblock map also shows
which blocks are locked and which ones are bad.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- rename sector map to eraseblock map
	- include bad block info in map
	- update libmtd api calls

 ubi-utils/src/mtdinfo.c |   82 ++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
index 666d739..5572199 100644
--- a/ubi-utils/src/mtdinfo.c
+++ b/ubi-utils/src/mtdinfo.c
@@ -29,6 +29,7 @@
 #include <getopt.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <mtd/mtd-user.h>
 
 #include <libubigen.h>
@@ -41,6 +42,7 @@ struct args {
 	int mtdn;
 	unsigned int all:1;
 	unsigned int ubinfo:1;
+	unsigned int map:1;
 	const char *node;
 };
 
@@ -59,14 +61,15 @@ static const char optionsstr[] =
 "                                (deprecated option, will be removed, do not use)\n"
 "-u, --ubi-info                  print what would UBI layout be if it was put\n"
 "                                on this MTD device\n"
+"-M, --map                       print eraseblock map\n"
 "-a, --all                       print information about all MTD devices\n"
 "-h, --help                      print help message\n"
 "-V, --version                   print program version";
 
 static const char usage[] =
-"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-h] [-V] [--mtdn <MTD device number>]\n"
+"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-M] [-h] [-V] [--mtdn <MTD device number>]\n"
 "\t\t[--ubi-info] [--help] [--version]\n"
-"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-h] [-V] [--ubi-info] [--help]\n"
+"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-M] [-h] [-V] [--ubi-info] [--help]\n"
 "\t\t[--version]\n"
 "Example 1: " PROGRAM_NAME " - (no arguments) print general MTD information\n"
 "Example 2: " PROGRAM_NAME " -m 1 - print information about MTD device number 1\n"
@@ -79,6 +82,7 @@ static const char usage[] =
 static const struct option long_options[] = {
 	{ .name = "mtdn",      .has_arg = 1, .flag = NULL, .val = 'm' },
 	{ .name = "ubi-info",  .has_arg = 0, .flag = NULL, .val = 'u' },
+	{ .name = "map",       .has_arg = 0, .flag = NULL, .val = 'M' },
 	{ .name = "all",       .has_arg = 0, .flag = NULL, .val = 'a' },
 	{ .name = "help",      .has_arg = 0, .flag = NULL, .val = 'h' },
 	{ .name = "version",   .has_arg = 0, .flag = NULL, .val = 'V' },
@@ -90,7 +94,7 @@ static int parse_opt(int argc, char * const argv[])
 	while (1) {
 		int key, error = 0;
 
-		key = getopt_long(argc, argv, "am:uhV", long_options, NULL);
+		key = getopt_long(argc, argv, "am:uMhV", long_options, NULL);
 		if (key == -1)
 			break;
 
@@ -110,6 +114,10 @@ static int parse_opt(int argc, char * const argv[])
 			warnmsg("-m/--mtdn is depecated, will be removed in mtd-utils-1.4.6");
 			break;
 
+		case 'M':
+			args.map = 1;
+			break;
+
 		case 'h':
 			printf("%s\n\n", doc);
 			printf("%s\n\n", usage);
@@ -160,9 +168,40 @@ static int translate_dev(libmtd_t libmtd, const char *node)
 	return 0;
 }
 
+static void print_map(libmtd_t libmtd, const struct mtd_dev_info *mtd,
+		      int fd, const region_info_t *reginfo)
+{
+	unsigned long start;
+	int i, width;
+
+	printf("Eraseblock map:\n");
+
+	/* figure out the number of spaces to pad w/out libm */
+	for (i = 1, width = 0; i < reginfo->numblocks; i *= 10, ++width)
+		continue;
+
+	for (i = 0; i < reginfo->numblocks; ++i) {
+		start = reginfo->offset + i * reginfo->erasesize;
+		printf(" %*i: %08lx ", width, i, start);
+		if (mtd_is_locked(libmtd, mtd, fd, i) == 1)
+			printf("RO ");
+		else
+			printf("   ");
+		if (mtd_is_bad(mtd, fd, i) == 1)
+			printf("BAD ");
+		else
+			printf("    ");
+		if (((i + 1) % 4) == 0)
+			printf("\n");
+	}
+	if (i % 4)
+		printf("\n");
+}
+
 static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int mtdn)
 {
-	int err;
+	int err, fd = -1;
+	region_info_t reginfo;
 	struct mtd_dev_info mtd;
 	struct ubigen_info ui;
 
@@ -196,8 +235,26 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
 	if (mtd.oob_size > 0)
 		printf("OOB size:                       %d bytes\n",
 		       mtd.oob_size);
-	if (mtd.region_cnt > 0)
+	if (mtd.region_cnt > 0) {
 		printf("Additional erase regions:       %d\n", mtd.oob_size);
+		if (args.node)
+			fd = open(args.node, O_RDONLY | O_CLOEXEC);
+		if (fd != -1) {
+			int r;
+
+			for (r = 0; r < mtd.region_cnt; ++r) {
+				printf(" region %i: ", r);
+				if (mtd_regioninfo(fd, r, &reginfo) == 0) {
+					printf(" offset: %#x size: %#x numblocks: %#x\n",
+						reginfo.offset, reginfo.erasesize,
+						reginfo.numblocks);
+					if (args.map)
+						print_map(libmtd, &mtd, fd, &reginfo);
+				} else
+					printf(" info is unavailable\n");
+			}
+		}
+	}
 	if (mtd_info->sysfs_supported)
 		printf("Character device major/minor:   %d:%d\n",
 		       mtd.major, mtd.minor);
@@ -225,6 +282,21 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
 	printf("Maximum UBI volumes count:      %d\n", ui.max_volumes);
 
 out:
+	if (args.map && mtd.region_cnt == 0 && args.node) {
+		if (fd == -1)
+			fd = open(args.node, O_RDONLY | O_CLOEXEC);
+		if (fd != -1) {
+			reginfo.offset = 0;
+			reginfo.erasesize = mtd.eb_size;
+			reginfo.numblocks = mtd.eb_cnt;
+			reginfo.regionindex = 0;
+			print_map(libmtd, &mtd, fd, &reginfo);
+		}
+	}
+
+	if (fd != -1)
+		close(fd);
+
 	printf("\n");
 	return 0;
 }
-- 
1.7.5.3

Re: [PATCH 2/6] mtdinfo: send help/version info to stdout

[Artem Bityutskiy]

On Tue, 2011-06-07 at 11:00 -0400, Mike Frysinger wrote:
> On Tue, Jun 7, 2011 at 02:36, Artem Bityutskiy wrote:
> > On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> >> Usage/version information should go to stdout when it is expected behavior
> >> (i.e. the user requested it explicitly).  This info should go to stderr
> >> only when the usage info is being shown as a result of incorrect options.
> >
> > This is fine, but all other ubi utils need this change then as well. I
> > think this piece of code was copypasted in all ubi-utils, so you should
> > easily find it and change.
> 
> you just like making me work :p

It is difficult to not like this - you are doing very good job! :-)

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

Re: [PATCH v2] ubi-utils: send help/version info to stdout

[Artem Bityutskiy]

On Tue, 2011-06-07 at 11:02 -0400, Mike Frysinger wrote:
> Usage/version information should go to stdout when it is expected behavior
> (i.e. the user requested it explicitly).  This info should go to stderr
> only when the usage info is being shown as a result of incorrect options.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Pushed to mtd-utils, thanks!

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

Re: [PATCH v2] flash_info: deprecate

[Artem Bityutskiy]

On Tue, 2011-06-07 at 11:11 -0400, Mike Frysinger wrote:
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v2
> 	- just deprecate for now rather than straight up removal
> 
>  feature-removal-schedule.txt |    7 +++++++
>  flash_info.c                 |    2 ++
>  2 files changed, 9 insertions(+), 0 deletions(-)

Pushed to mtd-utils.git, thanks!

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

Re: [PATCH 1/6] jffs2: make lzo optional at build time

[Artem Bityutskiy]

On Tue, 2011-06-07 at 02:19 -0400, Mike Frysinger wrote:
> The external lzo dep can be a pain to deal with when cross-compiling,
> so make it optional for jffs2.  This is useful if people aren't even
> using the functionality, or for quicker development.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Pushed to l2-mtd-2.6.git, thanks.

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

Re: [PATCH v3] libmtd: add helper funcs for getting regioninfo and locked info

[Artem Bityutskiy]

On Tue, 2011-06-07 at 11:52 -0400, Mike Frysinger wrote:
> This extends the libmtd with the helper functions:
> 	mtd_regioninfo: interface to MEMGETREGIONINFO
> 	mtd_islocked: interface to MEMISLOCKED
> 
> Users of these functions will follow shortly ...
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v3
> 	- add a "islocked_supported" field to handle older kernels nicely

Umm, I meant a user-visible flag, so that the user could check it and
not even try to print the map. There does not seem to be much point
having this flag otherewise - we can just call the ioctl, and return
error if it fails, without printing any error message.

I'll stick with v2 of this patch, I think we better live without this
flag.


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

Re: [PATCH v2] libmtd: add helper funcs for getting regioninfo and locked info

[Artem Bityutskiy]

On Tue, 2011-06-07 at 11:28 -0400, Mike Frysinger wrote:
> This extends the libmtd with the helper functions:
> 	mtd_regioninfo: interface to MEMGETREGIONINFO
> 	mtd_islocked: interface to MEMISLOCKED
> 
> Users of these functions will follow shortly ...
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Pushed with a small tweak, thanks!

> +int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb)
> +{
> +	int ret;
> +	erase_info_t ei;
> +
> +	ei.start = eb * mtd->eb_size;
> +	ei.length = mtd->eb_size;
> +
> +	ret = ioctl(fd, MEMISLOCKED, &ei);
> +	if (ret < 0)
> +		return mtd_ioctl_error(mtd, eb, "MEMISLOCKED");
> +

I've removed this error message - if we fail, better return an error
code silently. At least your next patch is built a way that it will keep
iterating and executing this function.

We might as well print an error message if (!ENOTTY && !ENOTSUPP), but I
did not do this. I expect you to send a correction patch if you do not
like this :-)

Thanks!

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

Re: [PATCH v2] libmtd: add helper funcs for getting regioninfo and locked info

[Artem Bityutskiy]

On Wed, 2011-06-08 at 14:52 +0300, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 11:28 -0400, Mike Frysinger wrote:
> > This extends the libmtd with the helper functions:
> > 	mtd_regioninfo: interface to MEMGETREGIONINFO
> > 	mtd_islocked: interface to MEMISLOCKED
> > 
> > Users of these functions will follow shortly ...
> > 
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> 
> Pushed with a small tweak, thanks!
> 
> > +int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb)
> > +{
> > +	int ret;
> > +	erase_info_t ei;
> > +
> > +	ei.start = eb * mtd->eb_size;
> > +	ei.length = mtd->eb_size;
> > +
> > +	ret = ioctl(fd, MEMISLOCKED, &ei);
> > +	if (ret < 0)
> > +		return mtd_ioctl_error(mtd, eb, "MEMISLOCKED");
> > +
> 
> I've removed this error message - if we fail, better return an error
> code silently. At least your next patch is built a way that it will keep
> iterating and executing this function.
> 
> We might as well print an error message if (!ENOTTY && !ENOTSUPP), but I
> did not do this. I expect you to send a correction patch if you do not
> like this :-)

I've just pushed a correction patch on top of this:

>From 92a06994b7c3f146cbdb770b30780e32f021118f Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Date: Wed, 8 Jun 2011 15:30:14 +0300
Subject: [PATCH] libmtd: improve mtd_islocked interface

This patch first of all, re-names 'mtd_islocked()' into 'mtd_is_locked()' since
this seems to be the name Mike wanted, and it looks a bit nicer.

This patch also makes 'mtd_is_locked()' print an error message if it fails. I'm
not sure if it is good idea for a library to do so, but all functions do this,
so it certainly _not_ a good idea to be inconsistent.

However, for the special case, when the the "is locked" ioctl is not supported
or is not valid for this device, we do not print an error message and return
ENOTSUPP error code.

Thus, the user can distinguish between real errors and non-fatal "not
supported" cases.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 include/libmtd.h |    8 +++++---
 lib/libmtd.c     |   13 +++++++++++--
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/libmtd.h b/include/libmtd.h
index 7275246..9efccbc 100644
--- a/include/libmtd.h
+++ b/include/libmtd.h
@@ -190,16 +190,18 @@ int mtd_erase(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb);
 int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo);
 
 /**
- * mtd_islocked - see if the specified eraseblock is locked.
+ * mtd_is_locked - see if the specified eraseblock is locked.
  * @mtd: MTD device description object
  * @fd: MTD device node file descriptor
  * @eb: eraseblock to check
  *
  * This function checks to see if eraseblock @eb of MTD device described
  * by @fd is locked. Returns %0 if it is unlocked, %1 if it is locked, and
- * %-1 in case of failure.
+ * %-1 in case of failure. If the ioctl is not supported (support was added in
+ * Linux kernel 2.6.36) or this particular device does not support it, errno is
+ * set to @ENOTSUPP.
  */
-int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb);
+int mtd_is_locked(const struct mtd_dev_info *mtd, int fd, int eb);
 
 /**
  * mtd_torture - torture an eraseblock.
diff --git a/lib/libmtd.c b/lib/libmtd.c
index 2573cc7..c34874e 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -900,14 +900,23 @@ int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo)
 	return 0;
 }
 
-int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb)
+int mtd_is_locked(const struct mtd_dev_info *mtd, int fd, int eb)
 {
+	int ret;
 	erase_info_t ei;
 
 	ei.start = eb * mtd->eb_size;
 	ei.length = mtd->eb_size;
 
-	return ioctl(fd, MEMISLOCKED, &ei);
+	ret = ioctl(fd, MEMISLOCKED, &ei);
+	if (ret < 0) {
+		if (errno != ENOTTY && errno != EOPNOTSUPP)
+			return mtd_ioctl_error(mtd, eb, "MEMISLOCKED");
+		else
+			errno = EOPNOTSUPP;
+	}
+
+	return ret;
 }
 
 /* Patterns to write to a physical eraseblock when torturing it */
-- 
1.7.2.3



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

Re: [PATCH v2] mtdinfo: add regioninfo/eraseblock map display

[Artem Bityutskiy]

On Tue, 2011-06-07 at 11:53 -0400, Mike Frysinger wrote:
> This brings the mtdinfo utility in line with the functionality
> of the flash_info utility.  It dumps the erase regioninfo (if
> the devices has it) as well as showing a handy eraseblock map
> (if the user has requested it).  The eraseblock map also shows
> which blocks are locked and which ones are bad.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v2
> 	- rename sector map to eraseblock map
> 	- include bad block info in map
> 	- update libmtd api calls

Sorry, but I still have some nit-picks.

> @@ -110,6 +114,10 @@ static int parse_opt(int argc, char * const argv[])
>  			warnmsg("-m/--mtdn is depecated, will be removed in mtd-utils-1.4.6");
>  			break;
>  
> +		case 'M':
> +			args.map = 1;
> +			break;
> +
>  		case 'h':
>  			printf("%s\n\n", doc);
>  			printf("%s\n\n", usage);

Would you also please add the following code near the end of the
'parse_opt()' function:

+       if (args.map && !args.node)
+               return errmsg("-M requires MTD device node name");

This will anyway be true when we remove -m, a the checks like "if
(args.node)" will be not needed.

> +static void print_map(libmtd_t libmtd, const struct mtd_dev_info *mtd,
> +		      int fd, const region_info_t *reginfo)
> +{
> +	unsigned long start;
> +	int i, width;
> +
> +	printf("Eraseblock map:\n");
> +
> +	/* figure out the number of spaces to pad w/out libm */
> +	for (i = 1, width = 0; i < reginfo->numblocks; i *= 10, ++width)
> +		continue;
> +
> +	for (i = 0; i < reginfo->numblocks; ++i) {
> +		start = reginfo->offset + i * reginfo->erasesize;
> +		printf(" %*i: %08lx ", width, i, start);
> +		if (mtd_is_locked(libmtd, mtd, fd, i) == 1)
> +			printf("RO ");
> +		else
> +			printf("   ");

I think this should be something like:

ret = mtd_is_locked()
if (ret < 0 && errno != ENOTSUPP)
	return;
if (ret == 1)
	printf("RO ");
else if (ret == 0)
	printf("   ");

> +		if (mtd_is_bad(mtd, fd, i) == 1)
> +			printf("BAD ");
> +		else
> +			printf("    ");

And similarly:

ret = mtd_is_bad()
if (ret < 0)
	return;
if (ret)
	printf("BAD ")l
else
	printf("    ");

> @@ -196,8 +235,26 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
>  	if (mtd.oob_size > 0)
>  		printf("OOB size:                       %d bytes\n",
>  		       mtd.oob_size);
> -	if (mtd.region_cnt > 0)
> +	if (mtd.region_cnt > 0) {
>  		printf("Additional erase regions:       %d\n", mtd.oob_size);
> +		if (args.node)
> +			fd = open(args.node, O_RDONLY | O_CLOEXEC);

We do want to print error message if open fails. E.g., I use -M and I do
not see any eraseblock map - because I am not root!

If open fails, we need to print something like "cannot fetch information
about regions - error blah (blah blah)". And may be we can just
continue.

> +		if (fd != -1) {
> +			int r;
> +
> +			for (r = 0; r < mtd.region_cnt; ++r) {
> +				printf(" region %i: ", r);
> +				if (mtd_regioninfo(fd, r, &reginfo) == 0) {
> +					printf(" offset: %#x size: %#x numblocks: %#x\n",
> +						reginfo.offset, reginfo.erasesize,
> +						reginfo.numblocks);
> +					if (args.map)
> +						print_map(libmtd, &mtd, fd, &reginfo);
> +				} else
> +					printf(" info is unavailable\n");
> +			}
> +		}
> +	}

OK, this is not very consistent. If we have more than one region and use
-M - we print sectors map _before_ things like "Bad blocks are allowed"
etc.

But if we have zero regions and use -M - we print the map at the very
end.

Not nice. Please, let's not call print_map here at all.


>  	if (mtd_info->sysfs_supported)
>  		printf("Character device major/minor:   %d:%d\n",
>  		       mtd.major, mtd.minor);
> @@ -225,6 +282,21 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
>  	printf("Maximum UBI volumes count:      %d\n", ui.max_volumes);
>  
>  out:
> +	if (args.map && mtd.region_cnt == 0 && args.node) {
> +		if (fd == -1)
> +			fd = open(args.node, O_RDONLY | O_CLOEXEC);
> +		if (fd != -1) {
> +			reginfo.offset = 0;
> +			reginfo.erasesize = mtd.eb_size;
> +			reginfo.numblocks = mtd.eb_cnt;
> +			reginfo.regionindex = 0;
> +			print_map(libmtd, &mtd, fd, &reginfo);
> +		}
> +	}
> +
> +	if (fd != -1)
> +		close(fd);
> +

Let's print the map here for all situations, not only  mtd.region_cnt ==
0.

I'd created one functions:

int print_regions(mtd, map).

The 'map' parameter makes it also print the map. So you call this
function twice:

above:
if (mtd.region_cnt > 0
	print_regions(mtd, 0);

and here:
print_regions(mtd, 1);

Hide the file opening there. For -M if we cannot open the file - we have
to die. For just regions printing - we can continue if the file cannot
be opened.


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

Re: [PATCH 5/6] mtdinfo: add regioninfo/sectormap display

[Artem Bityutskiy]

I've just pushed this little patch to make sure the code is more
readable when you add your changes. Otherwise this "out:" label looks
ugly.

>From bc1f5c0902ce4e10f062d6e37f3f8773506e6915 Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Date: Wed, 8 Jun 2011 16:21:40 +0300
Subject: [PATCH] mtdinfo: separate out ubi information printing

We are going to add some more code which prints eraseblocks map, so we need to
make 'print_dev_info()' a bit smaller to keep the code readable.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 ubi-utils/src/mtdinfo.c |   41 +++++++++++++++++++++++------------------
 1 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
index 666d739..ca0ad71 100644
--- a/ubi-utils/src/mtdinfo.c
+++ b/ubi-utils/src/mtdinfo.c
@@ -160,6 +160,27 @@ static int translate_dev(libmtd_t libmtd, const char *node)
 	return 0;
 }
 
+static void print_ubi_info(const struct mtd_info *mtd_info,
+			   const struct mtd_dev_info *mtd)
+{
+	struct ubigen_info ui;
+
+	if (!mtd_info->sysfs_supported) {
+		errmsg("cannot provide UBI info, becasue sub-page size is "
+		       "not known");
+		return;
+	}
+
+	ubigen_info_init(&ui, mtd->eb_size, mtd->min_io_size, mtd->subpage_size,
+			 0, 1, 0);
+	printf("Default UBI VID header offset:  %d\n", ui.vid_hdr_offs);
+	printf("Default UBI data offset:        %d\n", ui.data_offs);
+	printf("Default UBI LEB size:           ");
+	ubiutils_print_bytes(ui.leb_size, 0);
+	printf("\n");
+	printf("Maximum UBI volumes count:      %d\n", ui.max_volumes);
+}
+
 static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int mtdn)
 {
 	int err;
@@ -206,25 +227,9 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
 	printf("Device is writable:             %s\n",
 	      mtd.writable ? "true" : "false");
 
-	if (!args.ubinfo)
-		goto out;
-
-	if (!mtd_info->sysfs_supported) {
-		errmsg("cannot provide UBI info, becasue sub-page size is "
-		       "not known");
-		goto out;
-	}
-
-	ubigen_info_init(&ui, mtd.eb_size, mtd.min_io_size, mtd.subpage_size,
-			 0, 1, 0);
-	printf("Default UBI VID header offset:  %d\n", ui.vid_hdr_offs);
-	printf("Default UBI data offset:        %d\n", ui.data_offs);
-	printf("Default UBI LEB size:           ");
-	ubiutils_print_bytes(ui.leb_size, 0);
-	printf("\n");
-	printf("Maximum UBI volumes count:      %d\n", ui.max_volumes);
+	if (args.ubinfo)
+		print_ubi_info(mtd_info, &mtd);
 
-out:
 	printf("\n");
 	return 0;
 }
-- 
1.7.2.3


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

Re: [PATCH v3] libmtd: add helper funcs for getting regioninfo and locked info

[Mike Frysinger]

On Wed, Jun 8, 2011 at 07:47, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 11:52 -0400, Mike Frysinger wrote:
>> This extends the libmtd with the helper functions:
>>       mtd_regioninfo: interface to MEMGETREGIONINFO
>>       mtd_islocked: interface to MEMISLOCKED
>>
>> Users of these functions will follow shortly ...
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>> v3
>>       - add a "islocked_supported" field to handle older kernels nicely
>
> Umm, I meant a user-visible flag, so that the user could check it and
> not even try to print the map.

there is an option to not print the map ... "-M --map".  not sure what
you mean other than that.

> There does not seem to be much point having this flag otherewise
> - we can just call the ioctl, and return
> error if it fails, without printing any error message.

built-in optimization.  no point in doing the ioctl() over and over if
you know it's going to fail.

although thinking about it a bit more, i think the optimization is
incorrect.  it binds the status to the libmtd_t handle which is
per-library, not per-mtd.  so using it on different mtd devices would
result in misbehavior.
-mike

Re: [PATCH v2] libmtd: add helper funcs for getting regioninfo and locked info

[Mike Frysinger]

On Wed, Jun 8, 2011 at 08:27, Artem Bityutskiy wrote:
> On Wed, 2011-06-08 at 14:52 +0300, Artem Bityutskiy wrote:
>> On Tue, 2011-06-07 at 11:28 -0400, Mike Frysinger wrote:
>> > This extends the libmtd with the helper functions:
>> >     mtd_regioninfo: interface to MEMGETREGIONINFO
>> >     mtd_islocked: interface to MEMISLOCKED
>> >
>> > Users of these functions will follow shortly ...
>> >
>> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>
>> Pushed with a small tweak, thanks!
>>
>> > +int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb)
>> > +{
>> > +   int ret;
>> > +   erase_info_t ei;
>> > +
>> > +   ei.start = eb * mtd->eb_size;
>> > +   ei.length = mtd->eb_size;
>> > +
>> > +   ret = ioctl(fd, MEMISLOCKED, &ei);
>> > +   if (ret < 0)
>> > +           return mtd_ioctl_error(mtd, eb, "MEMISLOCKED");
>> > +
>>
>> I've removed this error message - if we fail, better return an error
>> code silently. At least your next patch is built a way that it will keep
>> iterating and executing this function.
>>
>> We might as well print an error message if (!ENOTTY && !ENOTSUPP), but I
>> did not do this. I expect you to send a correction patch if you do not
>> like this :-)
>
> I've just pushed a correction patch on top of this:
>
> From 92a06994b7c3f146cbdb770b30780e32f021118f Mon Sep 17 00:00:00 2001
> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Date: Wed, 8 Jun 2011 15:30:14 +0300
> Subject: [PATCH] libmtd: improve mtd_islocked interface
>
> This patch first of all, re-names 'mtd_islocked()' into 'mtd_is_locked()' since
> this seems to be the name Mike wanted, and it looks a bit nicer.
>
> This patch also makes 'mtd_is_locked()' print an error message if it fails. I'm
> not sure if it is good idea for a library to do so, but all functions do this,
> so it certainly _not_ a good idea to be inconsistent.
>
> However, for the special case, when the the "is locked" ioctl is not supported
> or is not valid for this device, we do not print an error message and return
> ENOTSUPP error code.
>
> Thus, the user can distinguish between real errors and non-fatal "not
> supported" cases.
>
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> ---
>  include/libmtd.h |    8 +++++---
>  lib/libmtd.c     |   13 +++++++++++--
>  2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/include/libmtd.h b/include/libmtd.h
> index 7275246..9efccbc 100644
> --- a/include/libmtd.h
> +++ b/include/libmtd.h
> @@ -190,16 +190,18 @@ int mtd_erase(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb);
>  int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo);
>
>  /**
> - * mtd_islocked - see if the specified eraseblock is locked.
> + * mtd_is_locked - see if the specified eraseblock is locked.
>  * @mtd: MTD device description object
>  * @fd: MTD device node file descriptor
>  * @eb: eraseblock to check
>  *
>  * This function checks to see if eraseblock @eb of MTD device described
>  * by @fd is locked. Returns %0 if it is unlocked, %1 if it is locked, and
> - * %-1 in case of failure.
> + * %-1 in case of failure. If the ioctl is not supported (support was added in
> + * Linux kernel 2.6.36) or this particular device does not support it, errno is
> + * set to @ENOTSUPP.
>  */
> -int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb);
> +int mtd_is_locked(const struct mtd_dev_info *mtd, int fd, int eb);
>
>  /**
>  * mtd_torture - torture an eraseblock.
> diff --git a/lib/libmtd.c b/lib/libmtd.c
> index 2573cc7..c34874e 100644
> --- a/lib/libmtd.c
> +++ b/lib/libmtd.c
> @@ -900,14 +900,23 @@ int mtd_regioninfo(int fd, int regidx, struct region_info_user *reginfo)
>        return 0;
>  }
>
> -int mtd_islocked(const struct mtd_dev_info *mtd, int fd, int eb)
> +int mtd_is_locked(const struct mtd_dev_info *mtd, int fd, int eb)
>  {
> +       int ret;
>        erase_info_t ei;
>
>        ei.start = eb * mtd->eb_size;
>        ei.length = mtd->eb_size;
>
> -       return ioctl(fd, MEMISLOCKED, &ei);
> +       ret = ioctl(fd, MEMISLOCKED, &ei);
> +       if (ret < 0) {
> +               if (errno != ENOTTY && errno != EOPNOTSUPP)
> +                       return mtd_ioctl_error(mtd, eb, "MEMISLOCKED");
> +               else
> +                       errno = EOPNOTSUPP;
> +       }
> +
> +       return ret;
>  }

if you remove the new "islocked_supported" flag from my v3, this is
almost what i had already.  so i'll drop any further work here (and
focus on `mtdinfo`) since we've come to feature parity.
-mike

Re: [PATCH 5/6] mtdinfo: add regioninfo/sectormap display

[Mike Frysinger]

On Wed, Jun 8, 2011 at 09:41, Artem Bityutskiy wrote:
> I've just pushed this little patch to make sure the code is more
> readable when you add your changes. Otherwise this "out:" label looks
> ugly.

np; i probably should have done this myself
-mike

Re: [PATCH v2] mtdinfo: add regioninfo/eraseblock map display

[Mike Frysinger]

On Wed, Jun 8, 2011 at 09:35, Artem Bityutskiy wrote:
> On Tue, 2011-06-07 at 11:53 -0400, Mike Frysinger wrote:
>> @@ -110,6 +114,10 @@ static int parse_opt(int argc, char * const argv[])
>>                       warnmsg("-m/--mtdn is depecated, will be removed in mtd-utils-1.4.6");
>>                       break;
>>
>> +             case 'M':
>> +                     args.map = 1;
>> +                     break;
>> +
>>               case 'h':
>>                       printf("%s\n\n", doc);
>>                       printf("%s\n\n", usage);
>
> Would you also please add the following code near the end of the
> 'parse_opt()' function:
>
> +       if (args.map && !args.node)
> +               return errmsg("-M requires MTD device node name");
>
> This will anyway be true when we remove -m, a the checks like "if
> (args.node)" will be not needed.

the -a/--all option will need reworking, but that'll have to happen
anyways once we punt -m

>> @@ -196,8 +235,26 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
>>       if (mtd.oob_size > 0)
>>               printf("OOB size:                       %d bytes\n",
>>                      mtd.oob_size);
>> -     if (mtd.region_cnt > 0)
>> +     if (mtd.region_cnt > 0) {
>>               printf("Additional erase regions:       %d\n", mtd.oob_size);
>> +             if (args.node)
>> +                     fd = open(args.node, O_RDONLY | O_CLOEXEC);
>
> We do want to print error message if open fails. E.g., I use -M and I do
> not see any eraseblock map - because I am not root!
>
> If open fails, we need to print something like "cannot fetch information
> about regions - error blah (blah blah)". And may be we can just
> continue.

i can add a warning when the open() fails

i have some ideas for the rest, so i'll just send a new patch which
should make us both happy i think
-mike

[PATCH v3] mtdinfo: add regioninfo/eraseblock map display

[Mike Frysinger]

This brings the mtdinfo utility in line with the functionality
of the flash_info utility.  It dumps the erase regioninfo (if
the devices has it) as well as showing a handy eraseblock map
(if the user has requested it).  The eraseblock map also shows
which blocks are locked and which ones are bad.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3
	- unify region and map printing

 ubi-utils/src/mtdinfo.c |  119 +++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 116 insertions(+), 3 deletions(-)

diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
index 38b63ca..045f5c9 100644
--- a/ubi-utils/src/mtdinfo.c
+++ b/ubi-utils/src/mtdinfo.c
@@ -29,6 +29,7 @@
 #include <getopt.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <mtd/mtd-user.h>
 
 #include <libubigen.h>
@@ -41,6 +42,7 @@ struct args {
 	int mtdn;
 	unsigned int all:1;
 	unsigned int ubinfo:1;
+	unsigned int map:1;
 	const char *node;
 };
 
@@ -59,14 +61,15 @@ static const char optionsstr[] =
 "                                (deprecated option, will be removed, do not use)\n"
 "-u, --ubi-info                  print what would UBI layout be if it was put\n"
 "                                on this MTD device\n"
+"-M, --map                       print eraseblock map\n"
 "-a, --all                       print information about all MTD devices\n"
 "-h, --help                      print help message\n"
 "-V, --version                   print program version";
 
 static const char usage[] =
-"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-h] [-V] [--mtdn <MTD device number>]\n"
+"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-M] [-h] [-V] [--mtdn <MTD device number>]\n"
 "\t\t[--ubi-info] [--help] [--version]\n"
-"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-h] [-V] [--ubi-info] [--help]\n"
+"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-M] [-h] [-V] [--ubi-info] [--help]\n"
 "\t\t[--version]\n"
 "Example 1: " PROGRAM_NAME " - (no arguments) print general MTD information\n"
 "Example 2: " PROGRAM_NAME " -m 1 - print information about MTD device number 1\n"
@@ -79,6 +82,7 @@ static const char usage[] =
 static const struct option long_options[] = {
 	{ .name = "mtdn",      .has_arg = 1, .flag = NULL, .val = 'm' },
 	{ .name = "ubi-info",  .has_arg = 0, .flag = NULL, .val = 'u' },
+	{ .name = "map",       .has_arg = 0, .flag = NULL, .val = 'M' },
 	{ .name = "all",       .has_arg = 0, .flag = NULL, .val = 'a' },
 	{ .name = "help",      .has_arg = 0, .flag = NULL, .val = 'h' },
 	{ .name = "version",   .has_arg = 0, .flag = NULL, .val = 'V' },
@@ -90,7 +94,7 @@ static int parse_opt(int argc, char * const argv[])
 	while (1) {
 		int key, error = 0;
 
-		key = getopt_long(argc, argv, "am:uhV", long_options, NULL);
+		key = getopt_long(argc, argv, "am:uMhV", long_options, NULL);
 		if (key == -1)
 			break;
 
@@ -110,6 +114,10 @@ static int parse_opt(int argc, char * const argv[])
 			warnmsg("-m/--mtdn is depecated, will be removed in mtd-utils-1.4.6");
 			break;
 
+		case 'M':
+			args.map = 1;
+			break;
+
 		case 'h':
 			printf("%s\n\n", doc);
 			printf("%s\n\n", usage);
@@ -139,6 +147,9 @@ static int parse_opt(int argc, char * const argv[])
 		args.node = NULL;
 	}
 
+	if (args.map && !args.node)
+		return errmsg("-M requires MTD device node name");
+
 	return 0;
 }
 
@@ -181,6 +192,105 @@ static void print_ubi_info(const struct mtd_info *mtd_info,
 	printf("Maximum UBI volumes count:      %d\n", ui.max_volumes);
 }
 
+static void print_region_map(const struct mtd_dev_info *mtd, int fd,
+			     const region_info_t *reginfo)
+{
+	unsigned long start;
+	int i, width;
+	int ret_locked, errno_locked, ret_bad, errno_bad;
+
+	printf("Eraseblock map:\n");
+
+	/* figure out the number of spaces to pad w/out libm */
+	for (i = 1, width = 0; i < reginfo->numblocks; i *= 10, ++width)
+		continue;
+
+	/* if we don't have a fd to query, just show the bare map */
+	if (fd == -1) {
+		ret_locked = ret_bad = -1;
+		errno_locked = errno_bad = ENODEV;
+	} else
+		ret_locked = ret_bad = errno_locked = errno_bad = 0;
+
+	for (i = 0; i < reginfo->numblocks; ++i) {
+		start = reginfo->offset + i * reginfo->erasesize;
+		printf(" %*i: %08lx ", width, i, start);
+
+		if (ret_locked != -1) {
+			ret_locked = mtd_is_locked(mtd, fd, i);
+			if (ret_locked == 1)
+				printf("RO ");
+			else
+				errno_locked = errno;
+		}
+		if (ret_locked != 1)
+			printf("   ");
+
+		if (ret_bad != -1) {
+			ret_bad = mtd_is_bad(mtd, fd, i);
+			if (ret_bad == 1)
+				printf("BAD ");
+			else
+				errno_bad = errno;
+		}
+		if (ret_bad != 1)
+			printf("    ");
+
+		if (((i + 1) % 4) == 0)
+			printf("\n");
+	}
+	if (i % 4)
+		printf("\n");
+
+	if (ret_locked == -1 && errno_locked != EOPNOTSUPP) {
+		errno = errno_locked;
+		sys_errmsg("could not read locked block info");
+	}
+
+	if (mtd->bb_allowed && ret_bad == -1 && errno_bad != EOPNOTSUPP) {
+		errno = errno_bad;
+		sys_errmsg("could not read bad block info");
+	}
+}
+
+static void print_region_info(const struct mtd_dev_info *mtd)
+{
+	region_info_t reginfo;
+	int r, fd;
+
+	/* First open the device so we can query it */
+	fd = open(args.node, O_RDONLY | O_CLOEXEC);
+	if (fd == -1) {
+		sys_errmsg("couldn't open MTD dev: %s", args.node);
+		if (mtd->region_cnt)
+			return;
+	}
+
+	/* walk all the regions and show the map for them */
+	if (mtd->region_cnt) {
+		for (r = 0; r < mtd->region_cnt; ++r) {
+			printf("Eraseblock region %i: ", r);
+			if (mtd_regioninfo(fd, r, &reginfo) == 0) {
+				printf(" offset: %#x size: %#x numblocks: %#x\n",
+					reginfo.offset, reginfo.erasesize,
+					reginfo.numblocks);
+				if (args.map)
+					print_region_map(mtd, fd, &reginfo);
+			} else
+				printf(" info is unavailable\n");
+		}
+	} else {
+		reginfo.offset = 0;
+		reginfo.erasesize = mtd->eb_size;
+		reginfo.numblocks = mtd->eb_cnt;
+		reginfo.regionindex = 0;
+		print_region_map(mtd, fd, &reginfo);
+	}
+
+	if (fd != -1)
+		close(fd);
+}
+
 static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int mtdn)
 {
 	int err;
@@ -229,6 +339,9 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
 	if (args.ubinfo)
 		print_ubi_info(mtd_info, &mtd);
 
+	if (args.map)
+		print_region_info(&mtd);
+
 	printf("\n");
 	return 0;
 }
-- 
1.7.5.3

[PATCH v4] mtdinfo: add regioninfo/eraseblock map display

[Mike Frysinger]

This brings the mtdinfo utility in line with the functionality
of the flash_info utility.  It dumps the erase regioninfo (if
the devices has it) as well as showing a handy eraseblock map
(if the user has requested it).  The eraseblock map also shows
which blocks are locked and which ones are bad.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v4
	- still show region info when map isn't requested

 ubi-utils/src/mtdinfo.c |  122 +++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 119 insertions(+), 3 deletions(-)

diff --git a/ubi-utils/src/mtdinfo.c b/ubi-utils/src/mtdinfo.c
index 38b63ca..e4b9f70 100644
--- a/ubi-utils/src/mtdinfo.c
+++ b/ubi-utils/src/mtdinfo.c
@@ -29,6 +29,7 @@
 #include <getopt.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <mtd/mtd-user.h>
 
 #include <libubigen.h>
@@ -41,6 +42,7 @@ struct args {
 	int mtdn;
 	unsigned int all:1;
 	unsigned int ubinfo:1;
+	unsigned int map:1;
 	const char *node;
 };
 
@@ -59,14 +61,15 @@ static const char optionsstr[] =
 "                                (deprecated option, will be removed, do not use)\n"
 "-u, --ubi-info                  print what would UBI layout be if it was put\n"
 "                                on this MTD device\n"
+"-M, --map                       print eraseblock map\n"
 "-a, --all                       print information about all MTD devices\n"
 "-h, --help                      print help message\n"
 "-V, --version                   print program version";
 
 static const char usage[] =
-"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-h] [-V] [--mtdn <MTD device number>]\n"
+"Usage 1: " PROGRAM_NAME " [-m <MTD device number>] [-u] [-M] [-h] [-V] [--mtdn <MTD device number>]\n"
 "\t\t[--ubi-info] [--help] [--version]\n"
-"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-h] [-V] [--ubi-info] [--help]\n"
+"Usage 2: " PROGRAM_NAME " <MTD device node file name> [-u] [-M] [-h] [-V] [--ubi-info] [--help]\n"
 "\t\t[--version]\n"
 "Example 1: " PROGRAM_NAME " - (no arguments) print general MTD information\n"
 "Example 2: " PROGRAM_NAME " -m 1 - print information about MTD device number 1\n"
@@ -79,6 +82,7 @@ static const char usage[] =
 static const struct option long_options[] = {
 	{ .name = "mtdn",      .has_arg = 1, .flag = NULL, .val = 'm' },
 	{ .name = "ubi-info",  .has_arg = 0, .flag = NULL, .val = 'u' },
+	{ .name = "map",       .has_arg = 0, .flag = NULL, .val = 'M' },
 	{ .name = "all",       .has_arg = 0, .flag = NULL, .val = 'a' },
 	{ .name = "help",      .has_arg = 0, .flag = NULL, .val = 'h' },
 	{ .name = "version",   .has_arg = 0, .flag = NULL, .val = 'V' },
@@ -90,7 +94,7 @@ static int parse_opt(int argc, char * const argv[])
 	while (1) {
 		int key, error = 0;
 
-		key = getopt_long(argc, argv, "am:uhV", long_options, NULL);
+		key = getopt_long(argc, argv, "am:uMhV", long_options, NULL);
 		if (key == -1)
 			break;
 
@@ -110,6 +114,10 @@ static int parse_opt(int argc, char * const argv[])
 			warnmsg("-m/--mtdn is depecated, will be removed in mtd-utils-1.4.6");
 			break;
 
+		case 'M':
+			args.map = 1;
+			break;
+
 		case 'h':
 			printf("%s\n\n", doc);
 			printf("%s\n\n", usage);
@@ -139,6 +147,9 @@ static int parse_opt(int argc, char * const argv[])
 		args.node = NULL;
 	}
 
+	if (args.map && !args.node)
+		return errmsg("-M requires MTD device node name");
+
 	return 0;
 }
 
@@ -181,6 +192,109 @@ static void print_ubi_info(const struct mtd_info *mtd_info,
 	printf("Maximum UBI volumes count:      %d\n", ui.max_volumes);
 }
 
+static void print_region_map(const struct mtd_dev_info *mtd, int fd,
+			     const region_info_t *reginfo)
+{
+	unsigned long start;
+	int i, width;
+	int ret_locked, errno_locked, ret_bad, errno_bad;
+
+	printf("Eraseblock map:\n");
+
+	/* figure out the number of spaces to pad w/out libm */
+	for (i = 1, width = 0; i < reginfo->numblocks; i *= 10, ++width)
+		continue;
+
+	/* if we don't have a fd to query, just show the bare map */
+	if (fd == -1) {
+		ret_locked = ret_bad = -1;
+		errno_locked = errno_bad = ENODEV;
+	} else
+		ret_locked = ret_bad = errno_locked = errno_bad = 0;
+
+	for (i = 0; i < reginfo->numblocks; ++i) {
+		start = reginfo->offset + i * reginfo->erasesize;
+		printf(" %*i: %08lx ", width, i, start);
+
+		if (ret_locked != -1) {
+			ret_locked = mtd_is_locked(mtd, fd, i);
+			if (ret_locked == 1)
+				printf("RO ");
+			else
+				errno_locked = errno;
+		}
+		if (ret_locked != 1)
+			printf("   ");
+
+		if (ret_bad != -1) {
+			ret_bad = mtd_is_bad(mtd, fd, i);
+			if (ret_bad == 1)
+				printf("BAD ");
+			else
+				errno_bad = errno;
+		}
+		if (ret_bad != 1)
+			printf("    ");
+
+		if (((i + 1) % 4) == 0)
+			printf("\n");
+	}
+	if (i % 4)
+		printf("\n");
+
+	if (ret_locked == -1 && errno_locked != EOPNOTSUPP) {
+		errno = errno_locked;
+		sys_errmsg("could not read locked block info");
+	}
+
+	if (mtd->bb_allowed && ret_bad == -1 && errno_bad != EOPNOTSUPP) {
+		errno = errno_bad;
+		sys_errmsg("could not read bad block info");
+	}
+}
+
+static void print_region_info(const struct mtd_dev_info *mtd)
+{
+	region_info_t reginfo;
+	int r, fd;
+
+	/* If we don't have any region info, just return */
+	if (!args.map && mtd->region_cnt == 0)
+		return;
+
+	/* First open the device so we can query it */
+	fd = open(args.node, O_RDONLY | O_CLOEXEC);
+	if (fd == -1) {
+		sys_errmsg("couldn't open MTD dev: %s", args.node);
+		if (mtd->region_cnt)
+			return;
+	}
+
+	/* Walk all the regions and show the map for them */
+	if (mtd->region_cnt) {
+		for (r = 0; r < mtd->region_cnt; ++r) {
+			printf("Eraseblock region %i: ", r);
+			if (mtd_regioninfo(fd, r, &reginfo) == 0) {
+				printf(" offset: %#x size: %#x numblocks: %#x\n",
+					reginfo.offset, reginfo.erasesize,
+					reginfo.numblocks);
+				if (args.map)
+					print_region_map(mtd, fd, &reginfo);
+			} else
+				printf(" info is unavailable\n");
+		}
+	} else {
+		reginfo.offset = 0;
+		reginfo.erasesize = mtd->eb_size;
+		reginfo.numblocks = mtd->eb_cnt;
+		reginfo.regionindex = 0;
+		print_region_map(mtd, fd, &reginfo);
+	}
+
+	if (fd != -1)
+		close(fd);
+}
+
 static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int mtdn)
 {
 	int err;
@@ -229,6 +343,8 @@ static int print_dev_info(libmtd_t libmtd, const struct mtd_info *mtd_info, int
 	if (args.ubinfo)
 		print_ubi_info(mtd_info, &mtd);
 
+	print_region_info(&mtd);
+
 	printf("\n");
 	return 0;
 }
-- 
1.7.5.3

Re: [PATCH v4] mtdinfo: add regioninfo/eraseblock map display

[Artem Bityutskiy]

On Wed, 2011-06-08 at 15:11 -0400, Mike Frysinger wrote:
> This brings the mtdinfo utility in line with the functionality
> of the flash_info utility.  It dumps the erase regioninfo (if
> the devices has it) as well as showing a handy eraseblock map
> (if the user has requested it).  The eraseblock map also shows
> which blocks are locked and which ones are bad.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v4
> 	- still show region info when map isn't requested
> 

Looks good, quick test with nandsim also looks good, pushed, thanks a
lot!

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