All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mtd-utils 00/11] flash_{un,}lock upgrades
@ 2015-08-31 22:34 Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 01/11] flash_{un, }lock: nest optional parameters in help message Brian Norris
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

Hi,

I've done a bit of work to make the flash_lock tool more useful, in order to
help test some new SPI NOR block protection features. Of note:

 * added getopt support
 * further unified flash_unlock and flash_lock -- the only difference now is
   their default feature, and both binaries contain support for all features
 * add support for MEMISLOCKED, so we can query arbitrary regions for
   protection status
 * mtdinfo -M already can dump some block lock information, but you get a
   little more flexibility using flash_lock --info, so both seem worth having
   IMO

Unsolved issues:

 * the MEM{LOCK,UNLOCK,ISLOCKED} ioctls all still use the out-dated 32-bit
   erase_info_user struct, which means they'll have problems supporting >=4GB
   flash. This isn't a mtd-utils problem, per se, and at the moment, Linux NAND
   (the only (?) candidate for >=4GB flash) support doesn't implement any of
   the locking APIs. But just an observation I ran across.
 * Deprecation plan: it would make sense to deprecate and remove flash_unlock
   eventually (in favor of 'flash_lock --unlock'), but this isn't really
   pressing, so I don't see a good reason to spend much effort on it.

Enjoy,
Brian

Brian Norris (11):
  flash_{un,}lock: nest optional parameters in help message
  flash_{un,}lock: switch to getopt library
  flash_{un,}lock: support --version flag
  flash_{un,}lock: document option flags
  flash_{un,}lock: abstract the argument positions
  flash_{un,}lock: move args processing to its own function
  flash_{un,}lock: support both lock/unlock in the same binary
  flash_{un,}lock: add MEMISLOCKED support
  flash_{un,}lock: improve strtol() error handling
  flash_{un,}lock: don't allow "last byte + 1"
  flash_{un,}lock: document block count == -1

 flash_unlock.c | 182 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 156 insertions(+), 26 deletions(-)

-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 01/11] flash_{un, }lock: nest optional parameters in help message
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 02/11] flash_{un,}lock: switch to getopt library Brian Norris
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

block count should be nested within the optional offset listing. That
is, we require offset before we accept a block count.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index 1cc8c2ff2d9b..ee8ac890f61c 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -28,7 +28,7 @@
 static void usage(int status)
 {
 	fprintf(status ? stderr : stdout,
-		"Usage: %s <mtd device> [offset] [block count]\n\n"
+		"Usage: %s <mtd device> [offset [block count]]\n\n"
 		"If offset is not specified, it defaults to 0.\n"
 		"If block count is not specified, it defaults to all blocks.\n",
 		PROGRAM_NAME);
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 02/11] flash_{un,}lock: switch to getopt library
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 01/11] flash_{un, }lock: nest optional parameters in help message Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 03/11] flash_{un,}lock: support --version flag Brian Norris
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

We will be adding some more flags, so the getopt library can help.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index ee8ac890f61c..777e4375a44d 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -13,6 +13,7 @@
 #define FLASH_UNLOCK 0
 #endif
 
+#include <getopt.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -35,6 +36,12 @@ static void usage(int status)
 	exit(status);
 }
 
+static const char short_opts[] = "h";
+static const struct option long_opts[] = {
+	{ "help",	no_argument,	0, 'h' },
+	{ NULL,		0,		0, 0 },
+};
+
 int main(int argc, char *argv[])
 {
 	int fd, request;
@@ -43,11 +50,26 @@ int main(int argc, char *argv[])
 	int count;
 	const char *dev;
 
+	for (;;) {
+		int c;
+
+		c = getopt_long(argc, argv, short_opts, long_opts, NULL);
+		if (c == EOF)
+			break;
+
+		switch (c) {
+		case 'h':
+			usage(0);
+			break;
+		default:
+			usage(1);
+			break;
+		}
+	}
+
 	/* Parse command line options */
 	if (argc < 2 || argc > 4)
 		usage(1);
-	if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
-		usage(0);
 
 	dev = argv[1];
 
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 03/11] flash_{un,}lock: support --version flag
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 01/11] flash_{un, }lock: nest optional parameters in help message Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 02/11] flash_{un,}lock: switch to getopt library Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 04/11] flash_{un,}lock: document option flags Brian Norris
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

Just use the common helper macro.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/flash_unlock.c b/flash_unlock.c
index 777e4375a44d..9a24b5f796e0 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -39,6 +39,7 @@ static void usage(int status)
 static const char short_opts[] = "h";
 static const struct option long_opts[] = {
 	{ "help",	no_argument,	0, 'h' },
+	{ "version",	no_argument,	0, 'v' },
 	{ NULL,		0,		0, 0 },
 };
 
@@ -61,6 +62,9 @@ int main(int argc, char *argv[])
 		case 'h':
 			usage(0);
 			break;
+		case 'v':
+			common_print_version();
+			exit(0);
 		default:
 			usage(1);
 			break;
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 04/11] flash_{un,}lock: document option flags
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (2 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 03/11] flash_{un,}lock: support --version flag Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 05/11] flash_{un, }lock: abstract the argument positions Brian Norris
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index 9a24b5f796e0..27e7df1eac3f 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -29,7 +29,12 @@
 static void usage(int status)
 {
 	fprintf(status ? stderr : stdout,
-		"Usage: %s <mtd device> [offset [block count]]\n\n"
+		"Usage: %s [options] [--] <mtd device> [offset [block count]]\n"
+		"\n"
+		"Options:\n"
+		" -h         --help              Display this help and exit\n"
+		"            --version           Display version information and exit\n"
+		"\n"
 		"If offset is not specified, it defaults to 0.\n"
 		"If block count is not specified, it defaults to all blocks.\n",
 		PROGRAM_NAME);
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 05/11] flash_{un, }lock: abstract the argument positions
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (3 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 04/11] flash_{un,}lock: document option flags Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 06/11] flash_{un, }lock: move args processing to its own function Brian Norris
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

Previously, there were no options (besides stand-alone --help and
--version), so we just used fixed-position argv indexes. Let's change
that.

Also clean up the sanity checks a bit to make them more verbose and
specific.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index 27e7df1eac3f..ce72e49d4ba6 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -54,7 +54,8 @@ int main(int argc, char *argv[])
 	struct mtd_info_user mtdInfo;
 	struct erase_info_user mtdLockInfo;
 	int count;
-	const char *dev;
+	const char *dev, *offs_s, *count_s;
+	int arg_idx;
 
 	for (;;) {
 		int c;
@@ -76,11 +77,31 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	/* Parse command line options */
-	if (argc < 2 || argc > 4)
+	arg_idx = optind;
+
+	/* Sanity checks */
+	if (argc - arg_idx < 1) {
+		errmsg("too few arguments");
+		usage(1);
+	} else if (argc - arg_idx > 3) {
+		errmsg("too many arguments");
 		usage(1);
+	}
+
+	/* First non-option argument */
+	dev = argv[arg_idx++];
 
-	dev = argv[1];
+	/* Second non-option argument */
+	if (arg_idx < argc)
+		offs_s = argv[arg_idx++];
+	else
+		offs_s = NULL;
+
+	/* Third non-option argument */
+	if (arg_idx < argc)
+		count_s = argv[arg_idx++];
+	else
+		count_s = NULL;
 
 	/* Get the device info to compare to command line sizes */
 	fd = open(dev, O_RDWR);
@@ -91,16 +112,16 @@ int main(int argc, char *argv[])
 		sys_errmsg_die("could not get mtd info: %s", dev);
 
 	/* Make sure user options are valid */
-	if (argc > 2)
-		mtdLockInfo.start = strtol(argv[2], NULL, 0);
+	if (offs_s)
+		mtdLockInfo.start = strtol(offs_s, NULL, 0);
 	else
 		mtdLockInfo.start = 0;
 	if (mtdLockInfo.start > mtdInfo.size)
 		errmsg_die("%#x is beyond device size %#x",
 			mtdLockInfo.start, mtdInfo.size);
 
-	if (argc > 3) {
-		count = strtol(argv[3], NULL, 0);
+	if (count_s) {
+		count = strtol(count_s, NULL, 0);
 		if (count == -1)
 			mtdLockInfo.length = mtdInfo.size;
 		else
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 06/11] flash_{un, }lock: move args processing to its own function
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (4 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 05/11] flash_{un, }lock: abstract the argument positions Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 07/11] flash_{un, }lock: support both lock/unlock in the same binary Brian Norris
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index ce72e49d4ba6..67277493d4b3 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -48,13 +48,11 @@ static const struct option long_opts[] = {
 	{ NULL,		0,		0, 0 },
 };
 
-int main(int argc, char *argv[])
+/* Program arguments */
+static const char *dev, *offs_s, *count_s;
+
+static void process_args(int argc, char *argv[])
 {
-	int fd, request;
-	struct mtd_info_user mtdInfo;
-	struct erase_info_user mtdLockInfo;
-	int count;
-	const char *dev, *offs_s, *count_s;
 	int arg_idx;
 
 	for (;;) {
@@ -103,6 +101,17 @@ int main(int argc, char *argv[])
 	else
 		count_s = NULL;
 
+}
+
+int main(int argc, char *argv[])
+{
+	int fd, request;
+	struct mtd_info_user mtdInfo;
+	struct erase_info_user mtdLockInfo;
+	int count;
+
+	process_args(argc, argv);
+
 	/* Get the device info to compare to command line sizes */
 	fd = open(dev, O_RDWR);
 	if (fd < 0)
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 07/11] flash_{un, }lock: support both lock/unlock in the same binary
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (5 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 06/11] flash_{un, }lock: move args processing to its own function Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 08/11] flash_{un,}lock: add MEMISLOCKED support Brian Norris
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

Add new --lock/--unlock flags, so we can do either with the same binary.
This will prepare for the addition of other features, so we don't have
to keep duplicating the same binary via #include "flash_unlock.c".

The defaults still work as expected: flash_unlock will default to
REQUEST_UNLOCK, and flash_lock will default to REQUEST_LOCK.

Eventually, we might deprecate one of the two (flash_unlock, probably),
so we only have to ship one flash_{un,}lock binary.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index 67277493d4b3..be09347faa92 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -4,13 +4,16 @@
  * utilities for locking/unlocking sectors of flash devices
  */
 
+enum flash_lock_request {
+	REQUEST_LOCK,
+	REQUEST_UNLOCK,
+};
+
 #ifndef PROGRAM_NAME
-#define PROGRAM_NAME "flash_unlock"
-#define FLASH_MSG    "unlock"
-#define FLASH_UNLOCK 1
+#define PROGRAM_NAME		"flash_unlock"
+#define DEFAULT_REQUEST		REQUEST_UNLOCK
 #else
-#define FLASH_MSG    "lock"
-#define FLASH_UNLOCK 0
+#define DEFAULT_REQUEST		REQUEST_LOCK
 #endif
 
 #include <getopt.h>
@@ -26,34 +29,48 @@
 #include "common.h"
 #include <mtd/mtd-user.h>
 
+static const char *flash_msg[] = {
+	[ REQUEST_LOCK ]	= "lock",
+	[ REQUEST_UNLOCK ]	= "unlock",
+};
+
 static void usage(int status)
 {
 	fprintf(status ? stderr : stdout,
+		"Utility to lock or unlock the flash. Default action: %s\n"
+		"\n"
 		"Usage: %s [options] [--] <mtd device> [offset [block count]]\n"
 		"\n"
 		"Options:\n"
 		" -h         --help              Display this help and exit\n"
 		"            --version           Display version information and exit\n"
+		" -l         --lock              Lock a region of flash\n"
+		" -u         --unlock            Unlock a region of flash\n"
 		"\n"
 		"If offset is not specified, it defaults to 0.\n"
 		"If block count is not specified, it defaults to all blocks.\n",
+		flash_msg[DEFAULT_REQUEST],
 		PROGRAM_NAME);
 	exit(status);
 }
 
-static const char short_opts[] = "h";
+static const char short_opts[] = "hlu";
 static const struct option long_opts[] = {
 	{ "help",	no_argument,	0, 'h' },
+	{ "lock",	no_argument,	0, 'l' },
+	{ "unlock",	no_argument,	0, 'u' },
 	{ "version",	no_argument,	0, 'v' },
 	{ NULL,		0,		0, 0 },
 };
 
 /* Program arguments */
 static const char *dev, *offs_s, *count_s;
+static enum flash_lock_request req = DEFAULT_REQUEST;
 
 static void process_args(int argc, char *argv[])
 {
 	int arg_idx;
+	int req_set = 0;
 
 	for (;;) {
 		int c;
@@ -66,6 +83,14 @@ static void process_args(int argc, char *argv[])
 		case 'h':
 			usage(0);
 			break;
+		case 'l':
+			req = REQUEST_LOCK;
+			req_set++;
+			break;
+		case 'u':
+			req = REQUEST_UNLOCK;
+			req_set++;
+			break;
 		case 'v':
 			common_print_version();
 			exit(0);
@@ -75,6 +100,11 @@ static void process_args(int argc, char *argv[])
 		}
 	}
 
+	if (req_set > 1) {
+		errmsg("cannot specify more than one lock/unlock option");
+		usage(1);
+	}
+
 	arg_idx = optind;
 
 	/* Sanity checks */
@@ -142,10 +172,20 @@ int main(int argc, char *argv[])
 			mtdLockInfo.start, mtdLockInfo.length, mtdInfo.size);
 
 	/* Finally do the operation */
-	request = FLASH_UNLOCK ? MEMUNLOCK : MEMLOCK;
+	switch (req) {
+	case REQUEST_LOCK:
+		request = MEMLOCK;
+		break;
+	case REQUEST_UNLOCK:
+		request = MEMUNLOCK;
+		break;
+	default:
+		errmsg_die("unknown request type: %d", req);
+		break;
+	}
 	if (ioctl(fd, request, &mtdLockInfo))
 		sys_errmsg_die("could not %s device: %s\n",
-			FLASH_MSG, dev);
+				flash_msg[req], dev);
 
 	return 0;
 }
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 08/11] flash_{un,}lock: add MEMISLOCKED support
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (6 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 07/11] flash_{un, }lock: support both lock/unlock in the same binary Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 09/11] flash_{un, }lock: improve strtol() error handling Brian Norris
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

With the -i / --islocked flags.

Sample output:

  # flash_lock --islocked /dev/mtd0
  Device: /dev/mtd0
  Start: 0
  Len: 0x400000
  Lock status: unlocked
  Return code: 0

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index be09347faa92..644360548601 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -7,6 +7,7 @@
 enum flash_lock_request {
 	REQUEST_LOCK,
 	REQUEST_UNLOCK,
+	REQUEST_ISLOCKED,
 };
 
 #ifndef PROGRAM_NAME
@@ -32,18 +33,21 @@ enum flash_lock_request {
 static const char *flash_msg[] = {
 	[ REQUEST_LOCK ]	= "lock",
 	[ REQUEST_UNLOCK ]	= "unlock",
+	[ REQUEST_ISLOCKED ]	= "check lock status",
 };
 
 static void usage(int status)
 {
 	fprintf(status ? stderr : stdout,
-		"Utility to lock or unlock the flash. Default action: %s\n"
+		"Utility to lock, unlock, or check the lock status of the flash.\n"
+		"Default action: %s\n"
 		"\n"
 		"Usage: %s [options] [--] <mtd device> [offset [block count]]\n"
 		"\n"
 		"Options:\n"
 		" -h         --help              Display this help and exit\n"
 		"            --version           Display version information and exit\n"
+		" -i         --islocked          Check if flash region is locked\n"
 		" -l         --lock              Lock a region of flash\n"
 		" -u         --unlock            Unlock a region of flash\n"
 		"\n"
@@ -54,9 +58,10 @@ static void usage(int status)
 	exit(status);
 }
 
-static const char short_opts[] = "hlu";
+static const char short_opts[] = "hilu";
 static const struct option long_opts[] = {
 	{ "help",	no_argument,	0, 'h' },
+	{ "islocked",	no_argument,	0, 'i' },
 	{ "lock",	no_argument,	0, 'l' },
 	{ "unlock",	no_argument,	0, 'u' },
 	{ "version",	no_argument,	0, 'v' },
@@ -83,6 +88,10 @@ static void process_args(int argc, char *argv[])
 		case 'h':
 			usage(0);
 			break;
+		case 'i':
+			req = REQUEST_ISLOCKED;
+			req_set++;
+			break;
 		case 'l':
 			req = REQUEST_LOCK;
 			req_set++;
@@ -101,7 +110,7 @@ static void process_args(int argc, char *argv[])
 	}
 
 	if (req_set > 1) {
-		errmsg("cannot specify more than one lock/unlock option");
+		errmsg("cannot specify more than one lock/unlock/islocked option");
 		usage(1);
 	}
 
@@ -139,6 +148,7 @@ int main(int argc, char *argv[])
 	struct mtd_info_user mtdInfo;
 	struct erase_info_user mtdLockInfo;
 	int count;
+	int ret;
 
 	process_args(argc, argv);
 
@@ -179,13 +189,25 @@ int main(int argc, char *argv[])
 	case REQUEST_UNLOCK:
 		request = MEMUNLOCK;
 		break;
+	case REQUEST_ISLOCKED:
+		request = MEMISLOCKED;
+		break;
 	default:
 		errmsg_die("unknown request type: %d", req);
 		break;
 	}
-	if (ioctl(fd, request, &mtdLockInfo))
+	ret = ioctl(fd, request, &mtdLockInfo);
+	if (ret < 0)
 		sys_errmsg_die("could not %s device: %s\n",
 				flash_msg[req], dev);
 
+	if (req == REQUEST_ISLOCKED) {
+		printf("Device: %s\n", dev);
+		printf("Start: %#0x\n", mtdLockInfo.start);
+		printf("Len: %#0x\n", mtdLockInfo.length);
+		printf("Lock status: %s\n", ret ? "locked" : "unlocked");
+		printf("Return code: %d\n", ret);
+	}
+
 	return 0;
 }
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 09/11] flash_{un, }lock: improve strtol() error handling
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (7 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 08/11] flash_{un,}lock: add MEMISLOCKED support Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 10/11] flash_{un,}lock: don't allow "last byte + 1" Brian Norris
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

Use the simple_* helpers to improve error checking.

Also fixup brace style at the same time.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index 644360548601..7a7a773d1d76 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -147,8 +147,8 @@ int main(int argc, char *argv[])
 	int fd, request;
 	struct mtd_info_user mtdInfo;
 	struct erase_info_user mtdLockInfo;
-	int count;
-	int ret;
+	long count;
+	int ret = 0;
 
 	process_args(argc, argv);
 
@@ -161,22 +161,28 @@ int main(int argc, char *argv[])
 		sys_errmsg_die("could not get mtd info: %s", dev);
 
 	/* Make sure user options are valid */
-	if (offs_s)
-		mtdLockInfo.start = strtol(offs_s, NULL, 0);
-	else
+	if (offs_s) {
+		mtdLockInfo.start = simple_strtol(offs_s, &ret);
+		if (ret)
+			errmsg_die("bad offset");
+	} else {
 		mtdLockInfo.start = 0;
+	}
 	if (mtdLockInfo.start > mtdInfo.size)
 		errmsg_die("%#x is beyond device size %#x",
 			mtdLockInfo.start, mtdInfo.size);
 
 	if (count_s) {
-		count = strtol(count_s, NULL, 0);
+		count = simple_strtol(count_s, &ret);
+		if (ret)
+			errmsg_die("bad count");
 		if (count == -1)
 			mtdLockInfo.length = mtdInfo.size;
 		else
 			mtdLockInfo.length = mtdInfo.erasesize * count;
-	} else
+	} else {
 		mtdLockInfo.length = mtdInfo.size;
+	}
 	if (mtdLockInfo.start + mtdLockInfo.length > mtdInfo.size)
 		errmsg_die("range is more than device supports: %#x + %#x > %#x",
 			mtdLockInfo.start, mtdLockInfo.length, mtdInfo.size);
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 10/11] flash_{un,}lock: don't allow "last byte + 1"
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (8 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 09/11] flash_{un, }lock: improve strtol() error handling Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-08-31 22:34 ` [PATCH mtd-utils 11/11] flash_{un,}lock: document block count == -1 Brian Norris
  2015-11-11 22:13 ` [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

A lock/unlock/islocked ioctl() should be prevented from anything past
the last byte, inclusive. But we were doing an exclusive check.

This isn't a big deal, as the kernel MTD APIs would be guarding this
anyway, but let's do this for completeness.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index 7a7a773d1d76..d775c0b01950 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -168,7 +168,7 @@ int main(int argc, char *argv[])
 	} else {
 		mtdLockInfo.start = 0;
 	}
-	if (mtdLockInfo.start > mtdInfo.size)
+	if (mtdLockInfo.start >= mtdInfo.size)
 		errmsg_die("%#x is beyond device size %#x",
 			mtdLockInfo.start, mtdInfo.size);
 
-- 
2.5.0.457.gab17608

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

* [PATCH mtd-utils 11/11] flash_{un,}lock: document block count == -1
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (9 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 10/11] flash_{un,}lock: don't allow "last byte + 1" Brian Norris
@ 2015-08-31 22:34 ` Brian Norris
  2015-11-11 22:13 ` [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-08-31 22:34 UTC (permalink / raw)
  To: linux-mtd; +Cc: Brian Norris

These utilities have accepted -1 as a block count to mean "all blocks."
Let's document that.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 flash_unlock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/flash_unlock.c b/flash_unlock.c
index d775c0b01950..f51870aeb69f 100644
--- a/flash_unlock.c
+++ b/flash_unlock.c
@@ -52,7 +52,8 @@ static void usage(int status)
 		" -u         --unlock            Unlock a region of flash\n"
 		"\n"
 		"If offset is not specified, it defaults to 0.\n"
-		"If block count is not specified, it defaults to all blocks.\n",
+		"If block count is not specified, it defaults to all blocks.\n"
+		"A block count of -1 means all blocks.\n",
 		flash_msg[DEFAULT_REQUEST],
 		PROGRAM_NAME);
 	exit(status);
-- 
2.5.0.457.gab17608

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

* Re: [PATCH mtd-utils 00/11] flash_{un,}lock upgrades
  2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
                   ` (10 preceding siblings ...)
  2015-08-31 22:34 ` [PATCH mtd-utils 11/11] flash_{un,}lock: document block count == -1 Brian Norris
@ 2015-11-11 22:13 ` Brian Norris
  11 siblings, 0 replies; 13+ messages in thread
From: Brian Norris @ 2015-11-11 22:13 UTC (permalink / raw)
  To: linux-mtd

On Mon, Aug 31, 2015 at 03:34:21PM -0700, Brian Norris wrote:
> Hi,
> 
> I've done a bit of work to make the flash_lock tool more useful, in order to
> help test some new SPI NOR block protection features. Of note:
> 
>  * added getopt support
>  * further unified flash_unlock and flash_lock -- the only difference now is
>    their default feature, and both binaries contain support for all features
>  * add support for MEMISLOCKED, so we can query arbitrary regions for
>    protection status
>  * mtdinfo -M already can dump some block lock information, but you get a
>    little more flexibility using flash_lock --info, so both seem worth having
>    IMO
> 
> Unsolved issues:
> 
>  * the MEM{LOCK,UNLOCK,ISLOCKED} ioctls all still use the out-dated 32-bit
>    erase_info_user struct, which means they'll have problems supporting >=4GB
>    flash. This isn't a mtd-utils problem, per se, and at the moment, Linux NAND
>    (the only (?) candidate for >=4GB flash) support doesn't implement any of
>    the locking APIs. But just an observation I ran across.
>  * Deprecation plan: it would make sense to deprecate and remove flash_unlock
>    eventually (in favor of 'flash_lock --unlock'), but this isn't really
>    pressing, so I don't see a good reason to spend much effort on it.
> 
> Enjoy,
> Brian

Pushed to mtd-utils.git

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

end of thread, other threads:[~2015-11-11 22:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-31 22:34 [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 01/11] flash_{un, }lock: nest optional parameters in help message Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 02/11] flash_{un,}lock: switch to getopt library Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 03/11] flash_{un,}lock: support --version flag Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 04/11] flash_{un,}lock: document option flags Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 05/11] flash_{un, }lock: abstract the argument positions Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 06/11] flash_{un, }lock: move args processing to its own function Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 07/11] flash_{un, }lock: support both lock/unlock in the same binary Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 08/11] flash_{un,}lock: add MEMISLOCKED support Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 09/11] flash_{un, }lock: improve strtol() error handling Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 10/11] flash_{un,}lock: don't allow "last byte + 1" Brian Norris
2015-08-31 22:34 ` [PATCH mtd-utils 11/11] flash_{un,}lock: document block count == -1 Brian Norris
2015-11-11 22:13 ` [PATCH mtd-utils 00/11] flash_{un,}lock upgrades Brian Norris

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.