diff mbox

[v2] mtd-utils: Add nandpart to add/delete partition

Message ID 1423520026-27068-1-git-send-email-namnguyen@chromium.org
State Changes Requested
Headers show

Commit Message

namnguyen@chromium.org Feb. 9, 2015, 10:13 p.m. UTC 
Add a simple utility to exercise BLKPG ioctl.

Signed-off-by: Nam T. Nguyen <namnguyen@chromium.org>
---
Change from v1:
  * Fix coding style

 Makefile   |   2 +-
 nandpart.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 197 insertions(+), 1 deletion(-)
 create mode 100644 nandpart.c

Comments

Dan Ehrenberg March 4, 2015, 6:45 p.m. UTC | #1 
On Mon, Feb 9, 2015 at 2:13 PM, Nam T. Nguyen <namnguyen@chromium.org> wrote:
> Add a simple utility to exercise BLKPG ioctl.
>
> Signed-off-by: Nam T. Nguyen <namnguyen@chromium.org>

Suggestion: rename to flash_part.c . This utility is not
nand-specific, and flash_ is used for general MTD utilities.

Dan
Brian Norris March 4, 2015, 6:51 p.m. UTC | #2 
On Wed, Mar 04, 2015 at 10:45:44AM -0800, Daniel Ehrenberg wrote:
> On Mon, Feb 9, 2015 at 2:13 PM, Nam T. Nguyen <namnguyen@chromium.org> wrote:
> > Add a simple utility to exercise BLKPG ioctl.
> >
> > Signed-off-by: Nam T. Nguyen <namnguyen@chromium.org>
> 
> Suggestion: rename to flash_part.c . This utility is not
> nand-specific, and flash_ is used for general MTD utilities.

Agreed.

Brian
Brian Norris March 4, 2015, 6:58 p.m. UTC | #3 
On Mon, Feb 09, 2015 at 02:13:46PM -0800, Nam T. Nguyen wrote:
> Add a simple utility to exercise BLKPG ioctl.
> 
> Signed-off-by: Nam T. Nguyen <namnguyen@chromium.org>
> ---
> Change from v1:
>   * Fix coding style
> 
>  Makefile   |   2 +-
>  nandpart.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 197 insertions(+), 1 deletion(-)
>  create mode 100644 nandpart.c
> 
> diff --git a/Makefile b/Makefile
> index eade234..712ff14 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -20,7 +20,7 @@ MTD_BINS = \
>  	ftl_format flash_erase nanddump doc_loadbios \
>  	ftl_check mkfs.jffs2 flash_lock flash_unlock \
>  	flash_otp_info flash_otp_dump flash_otp_lock flash_otp_write \
> -	mtd_debug flashcp nandwrite nandtest \
> +	mtd_debug flashcp nandwrite nandtest nandpart \
>  	jffs2dump \
>  	nftldump nftl_format docfdisk \
>  	rfddump rfdformat \
> diff --git a/nandpart.c b/nandpart.c
> new file mode 100644
> index 0000000..63c4cea
> --- /dev/null
> +++ b/nandpart.c
> @@ -0,0 +1,196 @@
> +/*
> + *  nandpart.c
> + *
> + *  Copyright 2015 The Chromium OS Authors.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + *  Overview:
> + *   This utility adds or removes a partition from an MTD device.
> + */
> +
> +#define PROGRAM_NAME "nandpart"
> +
> +#include <fcntl.h>
> +#include <getopt.h>
> +#include <linux/blkpg.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "common.h"
> +
> +static void display_help(int status)
> +{
> +	fprintf(status == EXIT_SUCCESS ? stdout : stderr,
> +"Usage: %1$s add MTD-device part-name start size\n"
> +"       %1$s del MTD-device part-number\n"

I feel like this help text doesn't match the other tools very well. Take
a look at nandwrite/nanddump/flash_erase/mtdinfo for examples, though
they are not all 100% consistent. (Consider capitalization, <brackets>,
etc.)

> +"Adds a partition to an MTD device, or remove an existing partition from it.\n"
> +"\n"
> +"-h         --help               Display this help and exit\n"
> +"           --version            Output version information and exit\n"
> +"\n"
> +"start location and size of the partition are in bytes. They should align on\n"
> +"eraseblock size.\n",
> +	PROGRAM_NAME
> +	);
> +	exit(status);
> +}
> +
> +static void display_version(void)
> +{
> +	printf("%1$s " VERSION "\n"
> +			"\n"
> +			"%1$s comes with NO WARRANTY\n"
> +			"to the extent permitted by law.\n"
> +			"\n"
> +			"You may redistribute copies of %1$s\n"
> +			"under the terms of the GNU General Public Licence.\n"
> +			"See the file `COPYING' for more information.\n",
> +			PROGRAM_NAME);
> +	exit(EXIT_SUCCESS);
> +}
> +
> +/* Command arguments */
> +
> +typedef enum {
> +	COMMAND_ADD,
> +	COMMAND_DEL
> +} command_type;
> +
> +static command_type		command;		/* add or del */
> +static const char		*mtddev;		/* mtd device name */
> +static const char		*part_name;		/* partition name */
> +static int			part_no;		/* partition number */
> +static long long		start_addr;		/* start address */
> +static long long		length;			/* partition size */
> +
> +static void process_options(int argc, char * const argv[])
> +{
> +	int error = 0;
> +
> +	for (;;) {
> +		int option_index = 0;
> +		static const char short_options[] = "h";
> +		static const struct option long_options[] = {
> +			{"version", no_argument, 0, 0},
> +			{"help", no_argument, 0, 'h'},
> +			{0, 0, 0, 0},
> +		};
> +
> +		int c = getopt_long(argc, argv, short_options,
> +				long_options, &option_index);
> +		if (c == EOF) {
> +			break;
> +		}
> +
> +		switch (c) {
> +			case 0:
> +				display_version();
> +				break;
> +			case 'h':
> +				display_help(EXIT_SUCCESS);
> +				break;
> +			case '?':
> +				error++;
> +				break;
> +		}
> +	}
> +
> +	if ((argc - optind) < 3 || error)
> +		display_help(EXIT_FAILURE);
> +
> +	const char *s_command = argv[optind++];
> +	mtddev = argv[optind++];
> +
> +	if (strcmp(s_command, "del") == 0 && (argc - optind) == 1) {
> +		const char *s_part_no = argv[optind++];
> +
> +		part_no = simple_strtol(s_part_no, &error);
> +		if (part_no < 0) {
> +		       errmsg_die("Can't specify negative partition number: %d",
> +				  part_no);
> +		}
> +
> +		command = COMMAND_DEL;
> +	} else if (strcmp(s_command, "add") == 0 && (argc - optind) == 3) {
> +		part_name = argv[optind++];
> +		const char *s_start = argv[optind++];
> +		const char *s_length = argv[optind++];
> +
> +		if (strlen(part_name) >= BLKPG_DEVNAMELTH)
> +			errmsg_die("Partition name (%s) should be less than %d "
> +				   "characters", part_name, BLKPG_DEVNAMELTH);

It's best not to break up error strings just for the 80-char style.

> +
> +		start_addr = simple_strtoll(s_start, &error);
> +		if (start_addr < 0)
> +		       errmsg_die("Can't specify negative start offset: %lld",
> +				  start_addr);
> +
> +		length = simple_strtoll(s_length, &error);
> +		if (length < 0)
> +		       errmsg_die("Can't specify negative length: %lld",
> +				  length);
> +
> +		command = COMMAND_ADD;
> +	} else
> +		display_help(EXIT_FAILURE);
> +
> +	if (error)
> +		display_help(EXIT_FAILURE);
> +}
> +
> +
> +/*
> + * Main program
> + */
> +int main(int argc, char * const argv[])
> +{
> +	process_options(argc, argv);
> +
> +	/* Open MTD device */
> +	int fd = open(mtddev, O_RDWR | O_CLOEXEC);

The preferred style is to put declarations at the top of the function,
not mixed with code.

> +	if (fd == -1) {

Unnecessary braces.

> +		sys_errmsg_die("Cannot open %s", mtddev);
> +	}
> +
> +	struct blkpg_partition part;

Move to top of main().

> +	memset(&part, 0, sizeof(part));
> +
> +	struct blkpg_ioctl_arg arg;

Ditto.

> +	memset(&arg, 0, sizeof(arg));
> +	arg.datalen = sizeof(part);
> +	arg.data = &part;
> +
> +	switch (command) {
> +		case COMMAND_ADD:
> +			part.start = start_addr;
> +			part.length = length;
> +			strncpy(part.devname, part_name, sizeof(part.devname));
> +			arg.op = BLKPG_ADD_PARTITION;
> +			break;
> +		case COMMAND_DEL:
> +			part.pno = part_no;
> +			arg.op = BLKPG_DEL_PARTITION;
> +			break;
> +	}
> +
> +	if (ioctl(fd, BLKPG, &arg)) {
> +		errmsg("Failed to issue BLKPG ioctl");

sys_errmsg(). Or just sys_errmsg_die(), since exit() would clean up our
resources anyway.

> +		goto closeall;
> +	}
> +
> +	close(fd);
> +
> +	/* Exit happy */
> +	return EXIT_SUCCESS;
> +
> +closeall:
> +	close(fd);
> +	exit(EXIT_FAILURE);
> +}

Brian
Brian Norris March 4, 2015, 7:24 p.m. UTC | #4 
On Wed, Mar 04, 2015 at 10:51:19AM -0800, Brian Norris wrote:
> On Wed, Mar 04, 2015 at 10:45:44AM -0800, Daniel Ehrenberg wrote:
> > On Mon, Feb 9, 2015 at 2:13 PM, Nam T. Nguyen <namnguyen@chromium.org> wrote:
> > > Add a simple utility to exercise BLKPG ioctl.
> > >
> > > Signed-off-by: Nam T. Nguyen <namnguyen@chromium.org>
> > 
> > Suggestion: rename to flash_part.c . This utility is not
> > nand-specific, and flash_ is used for general MTD utilities.
> 
> Agreed.

I'll retract that a bit. I think mtdpart might make more sense.
(Following after mtdinfo, rather than flash_erase.) And technically, not
all MTD's are flash.

Brian
Andrea Adami March 6, 2015, 3:53 p.m. UTC | #5 
On Wed, Mar 4, 2015 at 8:24 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Wed, Mar 04, 2015 at 10:51:19AM -0800, Brian Norris wrote:
>> On Wed, Mar 04, 2015 at 10:45:44AM -0800, Daniel Ehrenberg wrote:
>> > On Mon, Feb 9, 2015 at 2:13 PM, Nam T. Nguyen <namnguyen@chromium.org> wrote:
>> > > Add a simple utility to exercise BLKPG ioctl.
>> > >
>> > > Signed-off-by: Nam T. Nguyen <namnguyen@chromium.org>
>> >
>> > Suggestion: rename to flash_part.c . This utility is not
>> > nand-specific, and flash_ is used for general MTD utilities.
>>
>> Agreed.
>
> I'll retract that a bit. I think mtdpart might make more sense.
> (Following after mtdinfo, rather than flash_erase.) And technically, not
> all MTD's are flash.
>
> Brian
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

While fiddling with BLKPG and mtd, would it be useful to implement
BLKPG_RESIZE_PARTITION to have repartitoning on the fly? Maybe with a
separate tool like i.e. https://patchwork.ozlabs.org/patch/60224/ ?

Regards
Andrea
diff mbox

Patch

diff --git a/Makefile b/Makefile
index eade234..712ff14 100644
--- a/Makefile
+++ b/Makefile
@@ -20,7 +20,7 @@  MTD_BINS = \
 	ftl_format flash_erase nanddump doc_loadbios \
 	ftl_check mkfs.jffs2 flash_lock flash_unlock \
 	flash_otp_info flash_otp_dump flash_otp_lock flash_otp_write \
-	mtd_debug flashcp nandwrite nandtest \
+	mtd_debug flashcp nandwrite nandtest nandpart \
 	jffs2dump \
 	nftldump nftl_format docfdisk \
 	rfddump rfdformat \
diff --git a/nandpart.c b/nandpart.c
new file mode 100644
index 0000000..63c4cea
--- /dev/null
+++ b/nandpart.c
@@ -0,0 +1,196 @@ 
+/*
+ *  nandpart.c
+ *
+ *  Copyright 2015 The Chromium OS Authors.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ *  Overview:
+ *   This utility adds or removes a partition from an MTD device.
+ */
+
+#define PROGRAM_NAME "nandpart"
+
+#include <fcntl.h>
+#include <getopt.h>
+#include <linux/blkpg.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "common.h"
+
+static void display_help(int status)
+{
+	fprintf(status == EXIT_SUCCESS ? stdout : stderr,
+"Usage: %1$s add MTD-device part-name start size\n"
+"       %1$s del MTD-device part-number\n"
+"Adds a partition to an MTD device, or remove an existing partition from it.\n"
+"\n"
+"-h         --help               Display this help and exit\n"
+"           --version            Output version information and exit\n"
+"\n"
+"start location and size of the partition are in bytes. They should align on\n"
+"eraseblock size.\n",
+	PROGRAM_NAME
+	);
+	exit(status);
+}
+
+static void display_version(void)
+{
+	printf("%1$s " VERSION "\n"
+			"\n"
+			"%1$s comes with NO WARRANTY\n"
+			"to the extent permitted by law.\n"
+			"\n"
+			"You may redistribute copies of %1$s\n"
+			"under the terms of the GNU General Public Licence.\n"
+			"See the file `COPYING' for more information.\n",
+			PROGRAM_NAME);
+	exit(EXIT_SUCCESS);
+}
+
+/* Command arguments */
+
+typedef enum {
+	COMMAND_ADD,
+	COMMAND_DEL
+} command_type;
+
+static command_type		command;		/* add or del */
+static const char		*mtddev;		/* mtd device name */
+static const char		*part_name;		/* partition name */
+static int			part_no;		/* partition number */
+static long long		start_addr;		/* start address */
+static long long		length;			/* partition size */
+
+static void process_options(int argc, char * const argv[])
+{
+	int error = 0;
+
+	for (;;) {
+		int option_index = 0;
+		static const char short_options[] = "h";
+		static const struct option long_options[] = {
+			{"version", no_argument, 0, 0},
+			{"help", no_argument, 0, 'h'},
+			{0, 0, 0, 0},
+		};
+
+		int c = getopt_long(argc, argv, short_options,
+				long_options, &option_index);
+		if (c == EOF) {
+			break;
+		}
+
+		switch (c) {
+			case 0:
+				display_version();
+				break;
+			case 'h':
+				display_help(EXIT_SUCCESS);
+				break;
+			case '?':
+				error++;
+				break;
+		}
+	}
+
+	if ((argc - optind) < 3 || error)
+		display_help(EXIT_FAILURE);
+
+	const char *s_command = argv[optind++];
+	mtddev = argv[optind++];
+
+	if (strcmp(s_command, "del") == 0 && (argc - optind) == 1) {
+		const char *s_part_no = argv[optind++];
+
+		part_no = simple_strtol(s_part_no, &error);
+		if (part_no < 0) {
+		       errmsg_die("Can't specify negative partition number: %d",
+				  part_no);
+		}
+
+		command = COMMAND_DEL;
+	} else if (strcmp(s_command, "add") == 0 && (argc - optind) == 3) {
+		part_name = argv[optind++];
+		const char *s_start = argv[optind++];
+		const char *s_length = argv[optind++];
+
+		if (strlen(part_name) >= BLKPG_DEVNAMELTH)
+			errmsg_die("Partition name (%s) should be less than %d "
+				   "characters", part_name, BLKPG_DEVNAMELTH);
+
+		start_addr = simple_strtoll(s_start, &error);
+		if (start_addr < 0)
+		       errmsg_die("Can't specify negative start offset: %lld",
+				  start_addr);
+
+		length = simple_strtoll(s_length, &error);
+		if (length < 0)
+		       errmsg_die("Can't specify negative length: %lld",
+				  length);
+
+		command = COMMAND_ADD;
+	} else
+		display_help(EXIT_FAILURE);
+
+	if (error)
+		display_help(EXIT_FAILURE);
+}
+
+
+/*
+ * Main program
+ */
+int main(int argc, char * const argv[])
+{
+	process_options(argc, argv);
+
+	/* Open MTD device */
+	int fd = open(mtddev, O_RDWR | O_CLOEXEC);
+	if (fd == -1) {
+		sys_errmsg_die("Cannot open %s", mtddev);
+	}
+
+	struct blkpg_partition part;
+	memset(&part, 0, sizeof(part));
+
+	struct blkpg_ioctl_arg arg;
+	memset(&arg, 0, sizeof(arg));
+	arg.datalen = sizeof(part);
+	arg.data = &part;
+
+	switch (command) {
+		case COMMAND_ADD:
+			part.start = start_addr;
+			part.length = length;
+			strncpy(part.devname, part_name, sizeof(part.devname));
+			arg.op = BLKPG_ADD_PARTITION;
+			break;
+		case COMMAND_DEL:
+			part.pno = part_no;
+			arg.op = BLKPG_DEL_PARTITION;
+			break;
+	}
+
+	if (ioctl(fd, BLKPG, &arg)) {
+		errmsg("Failed to issue BLKPG ioctl");
+		goto closeall;
+	}
+
+	close(fd);
+
+	/* Exit happy */
+	return EXIT_SUCCESS;
+
+closeall:
+	close(fd);
+	exit(EXIT_FAILURE);
+}