[PATCH] mtd: cmdlinepart: use cmdline partition parser lib

[PATCH] mtd: cmdlinepart: use cmdline partition parser lib

[Caizhiyong]

From: Cai Zhiyong <caizhiyong@huawei.com>

MTD cmdline partition use cmdline partition parser lib

reference: https://lkml.org/lkml/2013/8/6/550

Signed-off-by: Cai ZhiYong <caizhiyong@huawei.com>
---
 Documentation/block/cmdline-partition.txt |  93 +++++---
 block/cmdline-parser.c                    |   6 +-
 drivers/mtd/Kconfig                       |   1 +
 drivers/mtd/cmdlinepart.c                 | 360 ++++--------------------------
 4 files changed, 101 insertions(+), 359 deletions(-)

diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt
index 651863d..adc5f7a 100644
--- a/Documentation/block/cmdline-partition.txt
+++ b/Documentation/block/cmdline-partition.txt
@@ -1,40 +1,59 @@
 Embedded device command line partition
-=====================================================================
-
-Read block device partition table from command line.
-The partition used for fixed block device (eMMC) embedded device.
-It is no MBR, save storage space. Bootloader can be easily accessed
-by absolute address of data on the block device.
-Users can easily change the partition.
-
-The format for the command line is just like mtdparts:
-
-blkdevparts=<blkdev-def>[;<blkdev-def>]
+Authors: Cai Zhiyong <caizhiyong@huawei.com>
+
+The format for the command line is as follows,
+it is used by MTD device (reference drivers/mtd/cmdlinepart.c) and
+block device (reference block/partitions/cmdline.c)
+
+================================================================================
+For MTD device:
+  mtdparts=<mtddef>[;<mtddef]
+  <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
+  <partdef> := <size>[@<offset>][<name>][ro][lk]
+  <mtd-id>  := unique name used in mapping driver/device (mtd->name)
+  <size>    := standard linux memsize OR "-" to denote all remaining space
+               size is automatically truncated at end of device
+               if specified or trucated size is 0 the part is skipped
+  <offset>  := standard linux memsize
+               if omitted the part will immediately follow the previous part
+               or 0 if the first part
+  <name>    := '(' NAME ')'
+               NAME will appear in /proc/mtd
+
+  <size> and <offset> can be specified such that the parts are out of order
+  in physical memory and may even overlap.
+
+  The parts are assigned MTD numbers in the order they are specified in the
+  command line regardless of their order in physical memory.
+
+  Examples:
+
+  1 NOR Flash, with 1 single writable partition:
+  edb7312-nor:-
+
+  1 NOR Flash with 2 partitions, 1 NAND with one
+  edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home)
+
+================================================================================
+For block device:
+  blkdevparts=<blkdev-def>[;<blkdev-def>]
   <blkdev-def> := <blkdev-id>:<partdef>[,<partdef>]
-    <partdef> := <size>[@<offset>](part-name)
-
-<blkdev-id>
-    block device disk name, embedded device used fixed block device,
-    it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0.
-
-<size>
-    partition size, in bytes, such as: 512, 1m, 1G.
-
-<offset>
-    partition start address, in bytes.
-
-(part-name)
-    partition name, kernel send uevent with "PARTNAME". application can create
-    a link to block device partition with the name "PARTNAME".
-    user space application can access partition by partition name.
-
-Example:
-    eMMC disk name is "mmcblk0" and "mmcblk0boot0"
-
-  bootargs:
+  <partdef> := <size>[@<offset>][<name>]
+  <blkdev-id>
+       block device disk name, embedded device used fixed block device,
+       it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0.
+  <size>    := standard linux memsize OR "-" to denote all remaining space
+               size is automatically truncated at end of device
+               if specified or trucated size is 0 the part is skipped
+  <offset>  := standard linux memsize
+               if omitted the part will immediately follow the previous part
+               or 0 if the first part
+  <name>    := '(' NAME ')'
+        partition name, kernel send uevent with "PARTNAME". application could
+        create a link to block device partition with the name "PARTNAME".
+        user space application can access partition by partition name.
+
+  Example:
+
+    eMMC disk name is "mmcblk0" and "mmcblk0boot0", bootargs:
     'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'
-
-  dmesg:
-    mmcblk0: p1(data0) p2(data1) p3()
-    mmcblk0boot0: p1(boot) p2(kernel)
-
diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c
index 18fb435..7ad2bc2 100644
--- a/block/cmdline-parser.c
+++ b/block/cmdline-parser.c
@@ -4,8 +4,6 @@
  * Written by Cai Zhiyong <caizhiyong@huawei.com>
  *
  */
-#include <linux/buffer_head.h>
-#include <linux/module.h>
 #include <linux/cmdline-parser.h>
 
 static int parse_subpart(struct cmdline_subpart **subpart, char *partdef)
@@ -158,6 +156,7 @@ void cmdline_parts_free(struct cmdline_parts **parts)
 		*parts = next_parts;
 	}
 }
+EXPORT_SYMBOL(cmdline_parts_free);
 
 int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline)
 {
@@ -205,6 +204,7 @@ fail:
 	cmdline_parts_free(parts);
 	goto done;
 }
+EXPORT_SYMBOL(cmdline_parts_parse);
 
 struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
 					 const char *bdev)
@@ -213,6 +213,7 @@ struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
 		parts = parts->next_parts;
 	return parts;
 }
+EXPORT_SYMBOL(cmdline_parts_find);
 
 /*
  *  add_part()
@@ -247,3 +248,4 @@ void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
 			break;
 	}
 }
+EXPORT_SYMBOL(cmdline_parts_set);
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 5fab4e6e..6f7f9ca 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -75,6 +75,7 @@ endif # MTD_REDBOOT_PARTS
 
 config MTD_CMDLINE_PARTS
 	tristate "Command line partition table parsing"
+	select CMDLINE_PARSER
 	depends on MTD
 	---help---
 	  Allow generic configuration of the MTD partition tables via the kernel
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index 721caeb..240b4ac 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -18,34 +18,8 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  *
- * The format for the command line is as follows:
+ * Verbose please reference "Documentation/block/cmdline-partition.txt"
  *
- * mtdparts=<mtddef>[;<mtddef]
- * <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
- * <partdef> := <size>[@<offset>][<name>][ro][lk]
- * <mtd-id>  := unique name used in mapping driver/device (mtd->name)
- * <size>    := standard linux memsize OR "-" to denote all remaining space
- *              size is automatically truncated at end of device
- *              if specified or trucated size is 0 the part is skipped
- * <offset>  := standard linux memsize
- *              if omitted the part will immediately follow the previous part
- *              or 0 if the first part
- * <name>    := '(' NAME ')'
- *              NAME will appear in /proc/mtd
- *
- * <size> and <offset> can be specified such that the parts are out of order
- * in physical memory and may even overlap.
- *
- * The parts are assigned MTD numbers in the order they are specified in the
- * command line regardless of their order in physical memory.
- *
- * Examples:
- *
- * 1 NOR Flash, with 1 single writable partition:
- * edb7312-nor:-
- *
- * 1 NOR Flash with 2 partitions, 1 NAND with one
- * edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home)
  */
 
 #include <linux/kernel.h>
@@ -54,249 +28,43 @@
 #include <linux/mtd/partitions.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/cmdline-parser.h>
 
-/* error message prefix */
-#define ERRP "mtd: "
-
-/* debug macro */
-#if 0
-#define dbg(x) do { printk("DEBUG-CMDLINE-PART: "); printk x; } while(0)
-#else
-#define dbg(x)
-#endif
-
-
-/* special size referring to all the remaining space in a partition */
-#define SIZE_REMAINING ULLONG_MAX
-#define OFFSET_CONTINUOUS ULLONG_MAX
-
-struct cmdline_mtd_partition {
-	struct cmdline_mtd_partition *next;
-	char *mtd_id;
-	int num_parts;
-	struct mtd_partition *parts;
-};
-
-/* mtdpart_setup() parses into here */
-static struct cmdline_mtd_partition *partitions;
-
-/* the command line passed to mtdpart_setup() */
 static char *mtdparts;
-static char *cmdline;
-static int cmdline_parsed;
+static struct cmdline_parts *mtd_cmdline_parts;
 
-/*
- * Parse one partition definition for an MTD. Since there can be many
- * comma separated partition definitions, this function calls itself
- * recursively until no more partition definitions are found. Nice side
- * effect: the memory to keep the mtd_partition structs and the names
- * is allocated upon the last definition being found. At that point the
- * syntax has been verified ok.
- */
-static struct mtd_partition * newpart(char *s,
-				      char **retptr,
-				      int *num_parts,
-				      int this_part,
-				      unsigned char **extra_mem_ptr,
-				      int extra_mem_size)
+static int add_part(int slot, struct cmdline_subpart *subpart, void *param)
 {
-	struct mtd_partition *parts;
-	unsigned long long size, offset = OFFSET_CONTINUOUS;
-	char *name;
-	int name_len;
-	unsigned char *extra_mem;
-	char delim;
-	unsigned int mask_flags;
-
-	/* fetch the partition size */
-	if (*s == '-') {
-		/* assign all remaining space to this partition */
-		size = SIZE_REMAINING;
-		s++;
-	} else {
-		size = memparse(s, &s);
-		if (size < PAGE_SIZE) {
-			printk(KERN_ERR ERRP "partition size too small (%llx)\n",
-			       size);
-			return ERR_PTR(-EINVAL);
-		}
-	}
-
-	/* fetch partition name and flags */
-	mask_flags = 0; /* this is going to be a regular partition */
-	delim = 0;
-
-	/* check for offset */
-	if (*s == '@') {
-		s++;
-		offset = memparse(s, &s);
-	}
-
-	/* now look for name */
-	if (*s == '(')
-		delim = ')';
-
-	if (delim) {
-		char *p;
+	struct mtd_partition *mtdpart = &((struct mtd_partition *)param)[slot];
 
-		name = ++s;
-		p = strchr(name, delim);
-		if (!p) {
-			printk(KERN_ERR ERRP "no closing %c found in partition name\n", delim);
-			return ERR_PTR(-EINVAL);
-		}
-		name_len = p - name;
-		s = p + 1;
-	} else {
-		name = NULL;
-		name_len = 13; /* Partition_000 */
-	}
-
-	/* record name length for memory allocation later */
-	extra_mem_size += name_len + 1;
+	mtdpart->offset = subpart->from;
+	mtdpart->size = subpart->size;
+	mtdpart->name = subpart->name;
+	mtdpart->mask_flags = 0;
 
-	/* test for options */
-	if (strncmp(s, "ro", 2) == 0) {
-		mask_flags |= MTD_WRITEABLE;
-		s += 2;
-	}
+	if (subpart->flags & PF_RDONLY)
+		mtdpart->mask_flags |= MTD_WRITEABLE;
 
 	/* if lk is found do NOT unlock the MTD partition*/
-	if (strncmp(s, "lk", 2) == 0) {
-		mask_flags |= MTD_POWERUP_LOCK;
-		s += 2;
-	}
-
-	/* test if more partitions are following */
-	if (*s == ',') {
-		if (size == SIZE_REMAINING) {
-			printk(KERN_ERR ERRP "no partitions allowed after a fill-up partition\n");
-			return ERR_PTR(-EINVAL);
-		}
-		/* more partitions follow, parse them */
-		parts = newpart(s + 1, &s, num_parts, this_part + 1,
-				&extra_mem, extra_mem_size);
-		if (IS_ERR(parts))
-			return parts;
-	} else {
-		/* this is the last partition: allocate space for all */
-		int alloc_size;
-
-		*num_parts = this_part + 1;
-		alloc_size = *num_parts * sizeof(struct mtd_partition) +
-			     extra_mem_size;
-
-		parts = kzalloc(alloc_size, GFP_KERNEL);
-		if (!parts)
-			return ERR_PTR(-ENOMEM);
-		extra_mem = (unsigned char *)(parts + *num_parts);
-	}
-
-	/* enter this partition (offset will be calculated later if it is zero at this point) */
-	parts[this_part].size = size;
-	parts[this_part].offset = offset;
-	parts[this_part].mask_flags = mask_flags;
-	if (name)
-		strlcpy(extra_mem, name, name_len + 1);
-	else
-		sprintf(extra_mem, "Partition_%03d", this_part);
-	parts[this_part].name = extra_mem;
-	extra_mem += name_len + 1;
+	if (subpart->flags & PF_POWERUP_LOCK)
+		mtdpart->mask_flags |= MTD_POWERUP_LOCK;
 
-	dbg(("partition %d: name <%s>, offset %llx, size %llx, mask flags %x\n",
-	     this_part, parts[this_part].name, parts[this_part].offset,
-	     parts[this_part].size, parts[this_part].mask_flags));
-
-	/* return (updated) pointer to extra_mem memory */
-	if (extra_mem_ptr)
-		*extra_mem_ptr = extra_mem;
-
-	/* return (updated) pointer command line string */
-	*retptr = s;
-
-	/* return partition table */
-	return parts;
+	return 0;
 }
 
 /*
- * Parse the command line.
+ * This is the handler for our kernel parameter, called from
+ * main.c::checksetup(). Note that we can not yet kmalloc() anything,
+ * so we only save the commandline for later processing.
+ *
+ * This function needs to be visible for bootloaders.
  */
-static int mtdpart_setup_real(char *s)
+static int __init mtdpart_setup(char *s)
 {
-	cmdline_parsed = 1;
-
-	for( ; s != NULL; )
-	{
-		struct cmdline_mtd_partition *this_mtd;
-		struct mtd_partition *parts;
-		int mtd_id_len, num_parts;
-		char *p, *mtd_id;
-
-		mtd_id = s;
-
-		/* fetch <mtd-id> */
-		p = strchr(s, ':');
-		if (!p) {
-			printk(KERN_ERR ERRP "no mtd-id\n");
-			return -EINVAL;
-		}
-		mtd_id_len = p - mtd_id;
-
-		dbg(("parsing <%s>\n", p+1));
-
-		/*
-		 * parse one mtd. have it reserve memory for the
-		 * struct cmdline_mtd_partition and the mtd-id string.
-		 */
-		parts = newpart(p + 1,		/* cmdline */
-				&s,		/* out: updated cmdline ptr */
-				&num_parts,	/* out: number of parts */
-				0,		/* first partition */
-				(unsigned char**)&this_mtd, /* out: extra mem */
-				mtd_id_len + 1 + sizeof(*this_mtd) +
-				sizeof(void*)-1 /*alignment*/);
-		if (IS_ERR(parts)) {
-			/*
-			 * An error occurred. We're either:
-			 * a) out of memory, or
-			 * b) in the middle of the partition spec
-			 * Either way, this mtd is hosed and we're
-			 * unlikely to succeed in parsing any more
-			 */
-			 return PTR_ERR(parts);
-		 }
-
-		/* align this_mtd */
-		this_mtd = (struct cmdline_mtd_partition *)
-				ALIGN((unsigned long)this_mtd, sizeof(void *));
-		/* enter results */
-		this_mtd->parts = parts;
-		this_mtd->num_parts = num_parts;
-		this_mtd->mtd_id = (char*)(this_mtd + 1);
-		strlcpy(this_mtd->mtd_id, mtd_id, mtd_id_len + 1);
-
-		/* link into chain */
-		this_mtd->next = partitions;
-		partitions = this_mtd;
-
-		dbg(("mtdid=<%s> num_parts=<%d>\n",
-		     this_mtd->mtd_id, this_mtd->num_parts));
-
-
-		/* EOS - we're done */
-		if (*s == 0)
-			break;
-
-		/* does another spec follow? */
-		if (*s != ';') {
-			printk(KERN_ERR ERRP "bad character after partition (%c)\n", *s);
-			return -EINVAL;
-		}
-		s++;
-	}
-
-	return 0;
+	mtdparts = s;
+	return 1;
 }
+__setup("mtdparts=", mtdpart_setup);
 
 /*
  * Main function to be called from the MTD mapping driver/device to
@@ -309,82 +77,36 @@ static int parse_cmdline_partitions(struct mtd_info *master,
 				    struct mtd_partition **pparts,
 				    struct mtd_part_parser_data *data)
 {
-	unsigned long long offset;
-	int i, err;
-	struct cmdline_mtd_partition *part;
-	const char *mtd_id = master->name;
+	struct cmdline_parts *parts;
 
-	/* parse command line */
-	if (!cmdline_parsed) {
-		err = mtdpart_setup_real(cmdline);
-		if (err)
-			return err;
-	}
+	if (mtdparts) {
+		if (mtd_cmdline_parts)
+			cmdline_parts_free(&mtd_cmdline_parts);
+
+		if (cmdline_parts_parse(&mtd_cmdline_parts, mtdparts)) {
+			mtdparts = NULL;
+			return -EINVAL;
+		}
 
-	/*
-	 * Search for the partition definition matching master->name.
-	 * If master->name is not set, stop at first partition definition.
-	 */
-	for (part = partitions; part; part = part->next) {
-		if ((!mtd_id) || (!strcmp(part->mtd_id, mtd_id)))
-			break;
+		mtdparts = NULL;
 	}
 
-	if (!part)
+	if (!mtd_cmdline_parts)
 		return 0;
 
-	for (i = 0, offset = 0; i < part->num_parts; i++) {
-		if (part->parts[i].offset == OFFSET_CONTINUOUS)
-			part->parts[i].offset = offset;
-		else
-			offset = part->parts[i].offset;
-
-		if (part->parts[i].size == SIZE_REMAINING)
-			part->parts[i].size = master->size - offset;
-
-		if (offset + part->parts[i].size > master->size) {
-			printk(KERN_WARNING ERRP
-			       "%s: partitioning exceeds flash size, truncating\n",
-			       part->mtd_id);
-			part->parts[i].size = master->size - offset;
-		}
-		offset += part->parts[i].size;
-
-		if (part->parts[i].size == 0) {
-			printk(KERN_WARNING ERRP
-			       "%s: skipping zero sized partition\n",
-			       part->mtd_id);
-			part->num_parts--;
-			memmove(&part->parts[i], &part->parts[i + 1],
-				sizeof(*part->parts) * (part->num_parts - i));
-			i--;
-		}
-	}
+	parts = cmdline_parts_find(mtd_cmdline_parts, master->name);
+	if (!parts)
+		return 0;
 
-	*pparts = kmemdup(part->parts, sizeof(*part->parts) * part->num_parts,
-			  GFP_KERNEL);
+	*pparts = kzalloc(sizeof(**pparts) * parts->nr_subparts, GFP_KERNEL);
 	if (!*pparts)
 		return -ENOMEM;
 
-	return part->num_parts;
-}
+	cmdline_parts_set(parts, master->size, 0, add_part, (void *)*pparts);
 
-
-/*
- * This is the handler for our kernel parameter, called from
- * main.c::checksetup(). Note that we can not yet kmalloc() anything,
- * so we only save the commandline for later processing.
- *
- * This function needs to be visible for bootloaders.
- */
-static int __init mtdpart_setup(char *s)
-{
-	cmdline = s;
-	return 1;
+	return parts->nr_subparts;
 }
 
-__setup("mtdparts=", mtdpart_setup);
-
 static struct mtd_part_parser cmdline_parser = {
 	.owner = THIS_MODULE,
 	.parse_fn = parse_cmdline_partitions,
@@ -393,8 +115,6 @@ static struct mtd_part_parser cmdline_parser = {
 
 static int __init cmdline_parser_init(void)
 {
-	if (mtdparts)
-		mtdpart_setup(mtdparts);
 	return register_mtd_parser(&cmdline_parser);
 }
 
-- 
1.8.1.5

[PATCH] mtd: cmdlinepart: use cmdline partition parser lib

[Caizhiyong]

From: Cai Zhiyong <caizhiyong@huawei.com>

MTD cmdline partition use cmdline partition parser lib

reference: https://lkml.org/lkml/2013/8/6/550

Signed-off-by: Cai ZhiYong <caizhiyong@huawei.com>
---
 Documentation/block/cmdline-partition.txt |  93 +++++---
 block/cmdline-parser.c                    |  16 +-
 drivers/mtd/Kconfig                       |   1 +
 drivers/mtd/cmdlinepart.c                 | 361 ++++--------------------------
 include/linux/cmdline-parser.h            |   8 +-
 5 files changed, 111 insertions(+), 368 deletions(-)

diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt
index 651863d..adc5f7a 100644
--- a/Documentation/block/cmdline-partition.txt
+++ b/Documentation/block/cmdline-partition.txt
@@ -1,40 +1,59 @@
 Embedded device command line partition
-=====================================================================
-
-Read block device partition table from command line.
-The partition used for fixed block device (eMMC) embedded device.
-It is no MBR, save storage space. Bootloader can be easily accessed
-by absolute address of data on the block device.
-Users can easily change the partition.
-
-The format for the command line is just like mtdparts:
-
-blkdevparts=<blkdev-def>[;<blkdev-def>]
+Authors: Cai Zhiyong <caizhiyong@huawei.com>
+
+The format for the command line is as follows,
+it is used by MTD device (reference drivers/mtd/cmdlinepart.c) and
+block device (reference block/partitions/cmdline.c)
+
+================================================================================
+For MTD device:
+  mtdparts=<mtddef>[;<mtddef]
+  <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
+  <partdef> := <size>[@<offset>][<name>][ro][lk]
+  <mtd-id>  := unique name used in mapping driver/device (mtd->name)
+  <size>    := standard linux memsize OR "-" to denote all remaining space
+               size is automatically truncated at end of device
+               if specified or trucated size is 0 the part is skipped
+  <offset>  := standard linux memsize
+               if omitted the part will immediately follow the previous part
+               or 0 if the first part
+  <name>    := '(' NAME ')'
+               NAME will appear in /proc/mtd
+
+  <size> and <offset> can be specified such that the parts are out of order
+  in physical memory and may even overlap.
+
+  The parts are assigned MTD numbers in the order they are specified in the
+  command line regardless of their order in physical memory.
+
+  Examples:
+
+  1 NOR Flash, with 1 single writable partition:
+  edb7312-nor:-
+
+  1 NOR Flash with 2 partitions, 1 NAND with one
+  edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home)
+
+================================================================================
+For block device:
+  blkdevparts=<blkdev-def>[;<blkdev-def>]
   <blkdev-def> := <blkdev-id>:<partdef>[,<partdef>]
-    <partdef> := <size>[@<offset>](part-name)
-
-<blkdev-id>
-    block device disk name, embedded device used fixed block device,
-    it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0.
-
-<size>
-    partition size, in bytes, such as: 512, 1m, 1G.
-
-<offset>
-    partition start address, in bytes.
-
-(part-name)
-    partition name, kernel send uevent with "PARTNAME". application can create
-    a link to block device partition with the name "PARTNAME".
-    user space application can access partition by partition name.
-
-Example:
-    eMMC disk name is "mmcblk0" and "mmcblk0boot0"
-
-  bootargs:
+  <partdef> := <size>[@<offset>][<name>]
+  <blkdev-id>
+       block device disk name, embedded device used fixed block device,
+       it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0.
+  <size>    := standard linux memsize OR "-" to denote all remaining space
+               size is automatically truncated at end of device
+               if specified or trucated size is 0 the part is skipped
+  <offset>  := standard linux memsize
+               if omitted the part will immediately follow the previous part
+               or 0 if the first part
+  <name>    := '(' NAME ')'
+        partition name, kernel send uevent with "PARTNAME". application could
+        create a link to block device partition with the name "PARTNAME".
+        user space application can access partition by partition name.
+
+  Example:
+
+    eMMC disk name is "mmcblk0" and "mmcblk0boot0", bootargs:
     'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'
-
-  dmesg:
-    mmcblk0: p1(data0) p2(data1) p3()
-    mmcblk0boot0: p1(boot) p2(kernel)
-
diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c
index 18fb435..8c09db3 100644
--- a/block/cmdline-parser.c
+++ b/block/cmdline-parser.c
@@ -4,8 +4,6 @@
  * Written by Cai Zhiyong <caizhiyong@huawei.com>
  *
  */
-#include <linux/buffer_head.h>
-#include <linux/module.h>
 #include <linux/cmdline-parser.h>
 
 static int parse_subpart(struct cmdline_subpart **subpart, char *partdef)
@@ -158,6 +156,7 @@ void cmdline_parts_free(struct cmdline_parts **parts)
 		*parts = next_parts;
 	}
 }
+EXPORT_SYMBOL(cmdline_parts_free);
 
 int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline)
 {
@@ -205,6 +204,7 @@ fail:
 	cmdline_parts_free(parts);
 	goto done;
 }
+EXPORT_SYMBOL(cmdline_parts_parse);
 
 struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
 					 const char *bdev)
@@ -213,16 +213,17 @@ struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
 		parts = parts->next_parts;
 	return parts;
 }
+EXPORT_SYMBOL(cmdline_parts_find);
 
 /*
  *  add_part()
  *    0 success.
  *    1 can not add so many partitions.
  */
-void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
-		       int slot,
-		       int (*add_part)(int, struct cmdline_subpart *, void *),
-		       void *param)
+int cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
+		      int slot,
+		      int (*add_part)(int, struct cmdline_subpart *, void *),
+		      void *param)
 
 {
 	sector_t from = 0;
@@ -246,4 +247,7 @@ void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
 		if (add_part(slot, subpart, param))
 			break;
 	}
+
+	return slot;
 }
+EXPORT_SYMBOL(cmdline_parts_set);
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 5fab4e6e..6f7f9ca 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -75,6 +75,7 @@ endif # MTD_REDBOOT_PARTS
 
 config MTD_CMDLINE_PARTS
 	tristate "Command line partition table parsing"
+	select CMDLINE_PARSER
 	depends on MTD
 	---help---
 	  Allow generic configuration of the MTD partition tables via the kernel
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index 721caeb..79bd584 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -18,34 +18,8 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  *
- * The format for the command line is as follows:
+ * Verbose please reference "Documentation/block/cmdline-partition.txt"
  *
- * mtdparts=<mtddef>[;<mtddef]
- * <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
- * <partdef> := <size>[@<offset>][<name>][ro][lk]
- * <mtd-id>  := unique name used in mapping driver/device (mtd->name)
- * <size>    := standard linux memsize OR "-" to denote all remaining space
- *              size is automatically truncated at end of device
- *              if specified or trucated size is 0 the part is skipped
- * <offset>  := standard linux memsize
- *              if omitted the part will immediately follow the previous part
- *              or 0 if the first part
- * <name>    := '(' NAME ')'
- *              NAME will appear in /proc/mtd
- *
- * <size> and <offset> can be specified such that the parts are out of order
- * in physical memory and may even overlap.
- *
- * The parts are assigned MTD numbers in the order they are specified in the
- * command line regardless of their order in physical memory.
- *
- * Examples:
- *
- * 1 NOR Flash, with 1 single writable partition:
- * edb7312-nor:-
- *
- * 1 NOR Flash with 2 partitions, 1 NAND with one
- * edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home)
  */
 
 #include <linux/kernel.h>
@@ -54,249 +28,43 @@
 #include <linux/mtd/partitions.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/cmdline-parser.h>
 
-/* error message prefix */
-#define ERRP "mtd: "
-
-/* debug macro */
-#if 0
-#define dbg(x) do { printk("DEBUG-CMDLINE-PART: "); printk x; } while(0)
-#else
-#define dbg(x)
-#endif
-
-
-/* special size referring to all the remaining space in a partition */
-#define SIZE_REMAINING ULLONG_MAX
-#define OFFSET_CONTINUOUS ULLONG_MAX
-
-struct cmdline_mtd_partition {
-	struct cmdline_mtd_partition *next;
-	char *mtd_id;
-	int num_parts;
-	struct mtd_partition *parts;
-};
-
-/* mtdpart_setup() parses into here */
-static struct cmdline_mtd_partition *partitions;
-
-/* the command line passed to mtdpart_setup() */
 static char *mtdparts;
-static char *cmdline;
-static int cmdline_parsed;
+static struct cmdline_parts *mtd_cmdline_parts;
 
-/*
- * Parse one partition definition for an MTD. Since there can be many
- * comma separated partition definitions, this function calls itself
- * recursively until no more partition definitions are found. Nice side
- * effect: the memory to keep the mtd_partition structs and the names
- * is allocated upon the last definition being found. At that point the
- * syntax has been verified ok.
- */
-static struct mtd_partition * newpart(char *s,
-				      char **retptr,
-				      int *num_parts,
-				      int this_part,
-				      unsigned char **extra_mem_ptr,
-				      int extra_mem_size)
+static int add_part(int slot, struct cmdline_subpart *subpart, void *param)
 {
-	struct mtd_partition *parts;
-	unsigned long long size, offset = OFFSET_CONTINUOUS;
-	char *name;
-	int name_len;
-	unsigned char *extra_mem;
-	char delim;
-	unsigned int mask_flags;
-
-	/* fetch the partition size */
-	if (*s == '-') {
-		/* assign all remaining space to this partition */
-		size = SIZE_REMAINING;
-		s++;
-	} else {
-		size = memparse(s, &s);
-		if (size < PAGE_SIZE) {
-			printk(KERN_ERR ERRP "partition size too small (%llx)\n",
-			       size);
-			return ERR_PTR(-EINVAL);
-		}
-	}
+	struct mtd_partition *mtdpart = &((struct mtd_partition *)param)[slot];
 
-	/* fetch partition name and flags */
-	mask_flags = 0; /* this is going to be a regular partition */
-	delim = 0;
+	mtdpart->offset = subpart->from;
+	mtdpart->size = subpart->size;
+	mtdpart->name = subpart->name;
+	mtdpart->mask_flags = 0;
 
-	/* check for offset */
-	if (*s == '@') {
-		s++;
-		offset = memparse(s, &s);
-	}
-
-	/* now look for name */
-	if (*s == '(')
-		delim = ')';
-
-	if (delim) {
-		char *p;
-
-		name = ++s;
-		p = strchr(name, delim);
-		if (!p) {
-			printk(KERN_ERR ERRP "no closing %c found in partition name\n", delim);
-			return ERR_PTR(-EINVAL);
-		}
-		name_len = p - name;
-		s = p + 1;
-	} else {
-		name = NULL;
-		name_len = 13; /* Partition_000 */
-	}
-
-	/* record name length for memory allocation later */
-	extra_mem_size += name_len + 1;
-
-	/* test for options */
-	if (strncmp(s, "ro", 2) == 0) {
-		mask_flags |= MTD_WRITEABLE;
-		s += 2;
-	}
+	if (subpart->flags & PF_RDONLY)
+		mtdpart->mask_flags |= MTD_WRITEABLE;
 
 	/* if lk is found do NOT unlock the MTD partition*/
-	if (strncmp(s, "lk", 2) == 0) {
-		mask_flags |= MTD_POWERUP_LOCK;
-		s += 2;
-	}
-
-	/* test if more partitions are following */
-	if (*s == ',') {
-		if (size == SIZE_REMAINING) {
-			printk(KERN_ERR ERRP "no partitions allowed after a fill-up partition\n");
-			return ERR_PTR(-EINVAL);
-		}
-		/* more partitions follow, parse them */
-		parts = newpart(s + 1, &s, num_parts, this_part + 1,
-				&extra_mem, extra_mem_size);
-		if (IS_ERR(parts))
-			return parts;
-	} else {
-		/* this is the last partition: allocate space for all */
-		int alloc_size;
-
-		*num_parts = this_part + 1;
-		alloc_size = *num_parts * sizeof(struct mtd_partition) +
-			     extra_mem_size;
-
-		parts = kzalloc(alloc_size, GFP_KERNEL);
-		if (!parts)
-			return ERR_PTR(-ENOMEM);
-		extra_mem = (unsigned char *)(parts + *num_parts);
-	}
-
-	/* enter this partition (offset will be calculated later if it is zero at this point) */
-	parts[this_part].size = size;
-	parts[this_part].offset = offset;
-	parts[this_part].mask_flags = mask_flags;
-	if (name)
-		strlcpy(extra_mem, name, name_len + 1);
-	else
-		sprintf(extra_mem, "Partition_%03d", this_part);
-	parts[this_part].name = extra_mem;
-	extra_mem += name_len + 1;
-
-	dbg(("partition %d: name <%s>, offset %llx, size %llx, mask flags %x\n",
-	     this_part, parts[this_part].name, parts[this_part].offset,
-	     parts[this_part].size, parts[this_part].mask_flags));
+	if (subpart->flags & PF_POWERUP_LOCK)
+		mtdpart->mask_flags |= MTD_POWERUP_LOCK;
 
-	/* return (updated) pointer to extra_mem memory */
-	if (extra_mem_ptr)
-		*extra_mem_ptr = extra_mem;
-
-	/* return (updated) pointer command line string */
-	*retptr = s;
-
-	/* return partition table */
-	return parts;
+	return 0;
 }
 
 /*
- * Parse the command line.
+ * This is the handler for our kernel parameter, called from
+ * main.c::checksetup(). Note that we can not yet kmalloc() anything,
+ * so we only save the commandline for later processing.
+ *
+ * This function needs to be visible for bootloaders.
  */
-static int mtdpart_setup_real(char *s)
+static int __init mtdpart_setup(char *s)
 {
-	cmdline_parsed = 1;
-
-	for( ; s != NULL; )
-	{
-		struct cmdline_mtd_partition *this_mtd;
-		struct mtd_partition *parts;
-		int mtd_id_len, num_parts;
-		char *p, *mtd_id;
-
-		mtd_id = s;
-
-		/* fetch <mtd-id> */
-		p = strchr(s, ':');
-		if (!p) {
-			printk(KERN_ERR ERRP "no mtd-id\n");
-			return -EINVAL;
-		}
-		mtd_id_len = p - mtd_id;
-
-		dbg(("parsing <%s>\n", p+1));
-
-		/*
-		 * parse one mtd. have it reserve memory for the
-		 * struct cmdline_mtd_partition and the mtd-id string.
-		 */
-		parts = newpart(p + 1,		/* cmdline */
-				&s,		/* out: updated cmdline ptr */
-				&num_parts,	/* out: number of parts */
-				0,		/* first partition */
-				(unsigned char**)&this_mtd, /* out: extra mem */
-				mtd_id_len + 1 + sizeof(*this_mtd) +
-				sizeof(void*)-1 /*alignment*/);
-		if (IS_ERR(parts)) {
-			/*
-			 * An error occurred. We're either:
-			 * a) out of memory, or
-			 * b) in the middle of the partition spec
-			 * Either way, this mtd is hosed and we're
-			 * unlikely to succeed in parsing any more
-			 */
-			 return PTR_ERR(parts);
-		 }
-
-		/* align this_mtd */
-		this_mtd = (struct cmdline_mtd_partition *)
-				ALIGN((unsigned long)this_mtd, sizeof(void *));
-		/* enter results */
-		this_mtd->parts = parts;
-		this_mtd->num_parts = num_parts;
-		this_mtd->mtd_id = (char*)(this_mtd + 1);
-		strlcpy(this_mtd->mtd_id, mtd_id, mtd_id_len + 1);
-
-		/* link into chain */
-		this_mtd->next = partitions;
-		partitions = this_mtd;
-
-		dbg(("mtdid=<%s> num_parts=<%d>\n",
-		     this_mtd->mtd_id, this_mtd->num_parts));
-
-
-		/* EOS - we're done */
-		if (*s == 0)
-			break;
-
-		/* does another spec follow? */
-		if (*s != ';') {
-			printk(KERN_ERR ERRP "bad character after partition (%c)\n", *s);
-			return -EINVAL;
-		}
-		s++;
-	}
-
-	return 0;
+	mtdparts = s;
+	return 1;
 }
+__setup("mtdparts=", mtdpart_setup);
 
 /*
  * Main function to be called from the MTD mapping driver/device to
@@ -309,82 +77,35 @@ static int parse_cmdline_partitions(struct mtd_info *master,
 				    struct mtd_partition **pparts,
 				    struct mtd_part_parser_data *data)
 {
-	unsigned long long offset;
-	int i, err;
-	struct cmdline_mtd_partition *part;
-	const char *mtd_id = master->name;
+	struct cmdline_parts *parts;
 
-	/* parse command line */
-	if (!cmdline_parsed) {
-		err = mtdpart_setup_real(cmdline);
-		if (err)
-			return err;
-	}
+	if (mtdparts) {
+		if (mtd_cmdline_parts)
+			cmdline_parts_free(&mtd_cmdline_parts);
+
+		if (cmdline_parts_parse(&mtd_cmdline_parts, mtdparts)) {
+			mtdparts = NULL;
+			return -EINVAL;
+		}
 
-	/*
-	 * Search for the partition definition matching master->name.
-	 * If master->name is not set, stop at first partition definition.
-	 */
-	for (part = partitions; part; part = part->next) {
-		if ((!mtd_id) || (!strcmp(part->mtd_id, mtd_id)))
-			break;
+		mtdparts = NULL;
 	}
 
-	if (!part)
+	if (!mtd_cmdline_parts)
 		return 0;
 
-	for (i = 0, offset = 0; i < part->num_parts; i++) {
-		if (part->parts[i].offset == OFFSET_CONTINUOUS)
-			part->parts[i].offset = offset;
-		else
-			offset = part->parts[i].offset;
-
-		if (part->parts[i].size == SIZE_REMAINING)
-			part->parts[i].size = master->size - offset;
-
-		if (offset + part->parts[i].size > master->size) {
-			printk(KERN_WARNING ERRP
-			       "%s: partitioning exceeds flash size, truncating\n",
-			       part->mtd_id);
-			part->parts[i].size = master->size - offset;
-		}
-		offset += part->parts[i].size;
-
-		if (part->parts[i].size == 0) {
-			printk(KERN_WARNING ERRP
-			       "%s: skipping zero sized partition\n",
-			       part->mtd_id);
-			part->num_parts--;
-			memmove(&part->parts[i], &part->parts[i + 1],
-				sizeof(*part->parts) * (part->num_parts - i));
-			i--;
-		}
-	}
+	parts = cmdline_parts_find(mtd_cmdline_parts, master->name);
+	if (!parts)
+		return 0;
 
-	*pparts = kmemdup(part->parts, sizeof(*part->parts) * part->num_parts,
-			  GFP_KERNEL);
+	*pparts = kzalloc(sizeof(**pparts) * parts->nr_subparts, GFP_KERNEL);
 	if (!*pparts)
 		return -ENOMEM;
 
-	return part->num_parts;
-}
-
-
-/*
- * This is the handler for our kernel parameter, called from
- * main.c::checksetup(). Note that we can not yet kmalloc() anything,
- * so we only save the commandline for later processing.
- *
- * This function needs to be visible for bootloaders.
- */
-static int __init mtdpart_setup(char *s)
-{
-	cmdline = s;
-	return 1;
+	return cmdline_parts_set(parts, master->size, 0, add_part,
+				 (void *)*pparts);
 }
 
-__setup("mtdparts=", mtdpart_setup);
-
 static struct mtd_part_parser cmdline_parser = {
 	.owner = THIS_MODULE,
 	.parse_fn = parse_cmdline_partitions,
@@ -393,8 +114,6 @@ static struct mtd_part_parser cmdline_parser = {
 
 static int __init cmdline_parser_init(void)
 {
-	if (mtdparts)
-		mtdpart_setup(mtdparts);
 	return register_mtd_parser(&cmdline_parser);
 }
 
diff --git a/include/linux/cmdline-parser.h b/include/linux/cmdline-parser.h
index 98e892e..43b65a9 100644
--- a/include/linux/cmdline-parser.h
+++ b/include/linux/cmdline-parser.h
@@ -35,9 +35,9 @@ int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline);
 struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
 					 const char *bdev);
 
-void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
-		       int slot,
-		       int (*add_part)(int, struct cmdline_subpart *, void *),
-		       void *param);
+int cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
+		      int slot,
+		      int (*add_part)(int, struct cmdline_subpart *, void *),
+		      void *param);
 
 #endif /* CMDLINEPARSEH */
-- 
1.8.1.5

Re: [PATCH] mtd: cmdlinepart: use cmdline partition parser lib

[Ezequiel Garcia]

Hi Cai,

Sorry for the late review. See below some minor comments.

On Thu, Aug 15, 2013 at 11:19:36AM +0000, Caizhiyong wrote:
> From: Cai Zhiyong <caizhiyong@huawei.com>
> 
> MTD cmdline partition use cmdline partition parser lib
> 
> reference: https://lkml.org/lkml/2013/8/6/550
> 

How about some more verbose commit message explaining the impact
of this change, or clearly stating there's no functionality change
(as it should given the mtdparts parameter _must_ be kept compatible).

> Signed-off-by: Cai ZhiYong <caizhiyong@huawei.com>
> ---
>  Documentation/block/cmdline-partition.txt |  93 +++++---
>  block/cmdline-parser.c                    |  16 +-
>  drivers/mtd/Kconfig                       |   1 +
>  drivers/mtd/cmdlinepart.c                 | 361 ++++--------------------------
>  include/linux/cmdline-parser.h            |   8 +-
>  5 files changed, 111 insertions(+), 368 deletions(-)
> 

This a nice diffstat, nice job!

> diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt
> index 651863d..adc5f7a 100644
> --- a/Documentation/block/cmdline-partition.txt
> +++ b/Documentation/block/cmdline-partition.txt
> @@ -1,40 +1,59 @@
>  Embedded device command line partition
> -=====================================================================
> -
> -Read block device partition table from command line.
> -The partition used for fixed block device (eMMC) embedded device.
> -It is no MBR, save storage space. Bootloader can be easily accessed
> -by absolute address of data on the block device.
> -Users can easily change the partition.
> -
> -The format for the command line is just like mtdparts:
> -
> -blkdevparts=<blkdev-def>[;<blkdev-def>]
> +Authors: Cai Zhiyong <caizhiyong@huawei.com>
> +
> +The format for the command line is as follows,
> +it is used by MTD device (reference drivers/mtd/cmdlinepart.c) and
> +block device (reference block/partitions/cmdline.c)
> +
> +================================================================================
> +For MTD device:
> +  mtdparts=<mtddef>[;<mtddef]
> +  <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
> +  <partdef> := <size>[@<offset>][<name>][ro][lk]
> +  <mtd-id>  := unique name used in mapping driver/device (mtd->name)
> +  <size>    := standard linux memsize OR "-" to denote all remaining space
> +               size is automatically truncated at end of device
> +               if specified or trucated size is 0 the part is skipped
> +  <offset>  := standard linux memsize
> +               if omitted the part will immediately follow the previous part
> +               or 0 if the first part
> +  <name>    := '(' NAME ')'
> +               NAME will appear in /proc/mtd
> +
> +  <size> and <offset> can be specified such that the parts are out of order
> +  in physical memory and may even overlap.
> +
> +  The parts are assigned MTD numbers in the order they are specified in the
> +  command line regardless of their order in physical memory.
> +
> +  Examples:
> +
> +  1 NOR Flash, with 1 single writable partition:
> +  edb7312-nor:-
> +
> +  1 NOR Flash with 2 partitions, 1 NAND with one
> +  edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home)
> +
> +================================================================================
> +For block device:
> +  blkdevparts=<blkdev-def>[;<blkdev-def>]
>    <blkdev-def> := <blkdev-id>:<partdef>[,<partdef>]
> -    <partdef> := <size>[@<offset>](part-name)
> -
> -<blkdev-id>
> -    block device disk name, embedded device used fixed block device,
> -    it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0.
> -
> -<size>
> -    partition size, in bytes, such as: 512, 1m, 1G.
> -
> -<offset>
> -    partition start address, in bytes.
> -
> -(part-name)
> -    partition name, kernel send uevent with "PARTNAME". application can create
> -    a link to block device partition with the name "PARTNAME".
> -    user space application can access partition by partition name.
> -
> -Example:
> -    eMMC disk name is "mmcblk0" and "mmcblk0boot0"
> -
> -  bootargs:
> +  <partdef> := <size>[@<offset>][<name>]
> +  <blkdev-id>
> +       block device disk name, embedded device used fixed block device,
> +       it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0.
> +  <size>    := standard linux memsize OR "-" to denote all remaining space
> +               size is automatically truncated at end of device
> +               if specified or trucated size is 0 the part is skipped
> +  <offset>  := standard linux memsize
> +               if omitted the part will immediately follow the previous part
> +               or 0 if the first part
> +  <name>    := '(' NAME ')'
> +        partition name, kernel send uevent with "PARTNAME". application could
> +        create a link to block device partition with the name "PARTNAME".
> +        user space application can access partition by partition name.
> +
> +  Example:
> +
> +    eMMC disk name is "mmcblk0" and "mmcblk0boot0", bootargs:
>      'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'
> -
> -  dmesg:
> -    mmcblk0: p1(data0) p2(data1) p3()
> -    mmcblk0boot0: p1(boot) p2(kernel)
> -
> diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c
> index 18fb435..8c09db3 100644
> --- a/block/cmdline-parser.c
> +++ b/block/cmdline-parser.c
> @@ -4,8 +4,6 @@
>   * Written by Cai Zhiyong <caizhiyong@huawei.com>
>   *
>   */
> -#include <linux/buffer_head.h>
> -#include <linux/module.h>

Hm.. this header removal seems an unrelated change, right?
I'd suggest to send a separate patch for this.

>  #include <linux/cmdline-parser.h>
>  
>  static int parse_subpart(struct cmdline_subpart **subpart, char *partdef)
> @@ -158,6 +156,7 @@ void cmdline_parts_free(struct cmdline_parts **parts)
>  		*parts = next_parts;
>  	}
>  }
> +EXPORT_SYMBOL(cmdline_parts_free);
>  
>  int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline)
>  {
> @@ -205,6 +204,7 @@ fail:
>  	cmdline_parts_free(parts);
>  	goto done;
>  }
> +EXPORT_SYMBOL(cmdline_parts_parse);
>  
>  struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
>  					 const char *bdev)
> @@ -213,16 +213,17 @@ struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
>  		parts = parts->next_parts;
>  	return parts;
>  }
> +EXPORT_SYMBOL(cmdline_parts_find);
>  
>  /*
>   *  add_part()
>   *    0 success.
>   *    1 can not add so many partitions.
>   */
> -void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
> -		       int slot,
> -		       int (*add_part)(int, struct cmdline_subpart *, void *),
> -		       void *param)
> +int cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
> +		      int slot,
> +		      int (*add_part)(int, struct cmdline_subpart *, void *),
> +		      void *param)
>  
>  {
>  	sector_t from = 0;
> @@ -246,4 +247,7 @@ void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
>  		if (add_part(slot, subpart, param))
>  			break;
>  	}
> +
> +	return slot;
>  }
> +EXPORT_SYMBOL(cmdline_parts_set);
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index 5fab4e6e..6f7f9ca 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -75,6 +75,7 @@ endif # MTD_REDBOOT_PARTS
>  
>  config MTD_CMDLINE_PARTS
>  	tristate "Command line partition table parsing"
> +	select CMDLINE_PARSER
>  	depends on MTD
>  	---help---
>  	  Allow generic configuration of the MTD partition tables via the kernel
> diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
> index 721caeb..79bd584 100644
> --- a/drivers/mtd/cmdlinepart.c
> +++ b/drivers/mtd/cmdlinepart.c
> @@ -18,34 +18,8 @@
>   * along with this program; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>   *
> - * The format for the command line is as follows:
> + * Verbose please reference "Documentation/block/cmdline-partition.txt"
>   *
> - * mtdparts=<mtddef>[;<mtddef]
> - * <mtddef>  := <mtd-id>:<partdef>[,<partdef>]
> - * <partdef> := <size>[@<offset>][<name>][ro][lk]
> - * <mtd-id>  := unique name used in mapping driver/device (mtd->name)
> - * <size>    := standard linux memsize OR "-" to denote all remaining space
> - *              size is automatically truncated at end of device
> - *              if specified or trucated size is 0 the part is skipped
> - * <offset>  := standard linux memsize
> - *              if omitted the part will immediately follow the previous part
> - *              or 0 if the first part
> - * <name>    := '(' NAME ')'
> - *              NAME will appear in /proc/mtd
> - *
> - * <size> and <offset> can be specified such that the parts are out of order
> - * in physical memory and may even overlap.
> - *
> - * The parts are assigned MTD numbers in the order they are specified in the
> - * command line regardless of their order in physical memory.
> - *
> - * Examples:
> - *
> - * 1 NOR Flash, with 1 single writable partition:
> - * edb7312-nor:-
> - *
> - * 1 NOR Flash with 2 partitions, 1 NAND with one
> - * edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home)
>   */
>  
>  #include <linux/kernel.h>
> @@ -54,249 +28,43 @@
>  #include <linux/mtd/partitions.h>
>  #include <linux/module.h>
>  #include <linux/err.h>
> +#include <linux/cmdline-parser.h>
>  
> -/* error message prefix */
> -#define ERRP "mtd: "
> -
> -/* debug macro */
> -#if 0
> -#define dbg(x) do { printk("DEBUG-CMDLINE-PART: "); printk x; } while(0)
> -#else
> -#define dbg(x)
> -#endif
> -
> -
> -/* special size referring to all the remaining space in a partition */
> -#define SIZE_REMAINING ULLONG_MAX
> -#define OFFSET_CONTINUOUS ULLONG_MAX
> -
> -struct cmdline_mtd_partition {
> -	struct cmdline_mtd_partition *next;
> -	char *mtd_id;
> -	int num_parts;
> -	struct mtd_partition *parts;
> -};
> -
> -/* mtdpart_setup() parses into here */
> -static struct cmdline_mtd_partition *partitions;
> -
> -/* the command line passed to mtdpart_setup() */
>  static char *mtdparts;
> -static char *cmdline;
> -static int cmdline_parsed;
> +static struct cmdline_parts *mtd_cmdline_parts;
>  
> -/*
> - * Parse one partition definition for an MTD. Since there can be many
> - * comma separated partition definitions, this function calls itself
> - * recursively until no more partition definitions are found. Nice side
> - * effect: the memory to keep the mtd_partition structs and the names
> - * is allocated upon the last definition being found. At that point the
> - * syntax has been verified ok.
> - */
> -static struct mtd_partition * newpart(char *s,
> -				      char **retptr,
> -				      int *num_parts,
> -				      int this_part,
> -				      unsigned char **extra_mem_ptr,
> -				      int extra_mem_size)
> +static int add_part(int slot, struct cmdline_subpart *subpart, void *param)
>  {
> -	struct mtd_partition *parts;
> -	unsigned long long size, offset = OFFSET_CONTINUOUS;
> -	char *name;
> -	int name_len;
> -	unsigned char *extra_mem;
> -	char delim;
> -	unsigned int mask_flags;
> -
> -	/* fetch the partition size */
> -	if (*s == '-') {
> -		/* assign all remaining space to this partition */
> -		size = SIZE_REMAINING;
> -		s++;
> -	} else {
> -		size = memparse(s, &s);
> -		if (size < PAGE_SIZE) {
> -			printk(KERN_ERR ERRP "partition size too small (%llx)\n",
> -			       size);
> -			return ERR_PTR(-EINVAL);
> -		}
> -	}
> +	struct mtd_partition *mtdpart = &((struct mtd_partition *)param)[slot];
>  
> -	/* fetch partition name and flags */
> -	mask_flags = 0; /* this is going to be a regular partition */
> -	delim = 0;
> +	mtdpart->offset = subpart->from;
> +	mtdpart->size = subpart->size;
> +	mtdpart->name = subpart->name;
> +	mtdpart->mask_flags = 0;
>  
> -	/* check for offset */
> -	if (*s == '@') {
> -		s++;
> -		offset = memparse(s, &s);
> -	}
> -
> -	/* now look for name */
> -	if (*s == '(')
> -		delim = ')';
> -
> -	if (delim) {
> -		char *p;
> -
> -		name = ++s;
> -		p = strchr(name, delim);
> -		if (!p) {
> -			printk(KERN_ERR ERRP "no closing %c found in partition name\n", delim);
> -			return ERR_PTR(-EINVAL);
> -		}
> -		name_len = p - name;
> -		s = p + 1;
> -	} else {
> -		name = NULL;
> -		name_len = 13; /* Partition_000 */
> -	}
> -
> -	/* record name length for memory allocation later */
> -	extra_mem_size += name_len + 1;
> -
> -	/* test for options */
> -	if (strncmp(s, "ro", 2) == 0) {
> -		mask_flags |= MTD_WRITEABLE;
> -		s += 2;
> -	}
> +	if (subpart->flags & PF_RDONLY)
> +		mtdpart->mask_flags |= MTD_WRITEABLE;
>  
>  	/* if lk is found do NOT unlock the MTD partition*/
> -	if (strncmp(s, "lk", 2) == 0) {
> -		mask_flags |= MTD_POWERUP_LOCK;
> -		s += 2;
> -	}
> -
> -	/* test if more partitions are following */
> -	if (*s == ',') {
> -		if (size == SIZE_REMAINING) {
> -			printk(KERN_ERR ERRP "no partitions allowed after a fill-up partition\n");
> -			return ERR_PTR(-EINVAL);
> -		}
> -		/* more partitions follow, parse them */
> -		parts = newpart(s + 1, &s, num_parts, this_part + 1,
> -				&extra_mem, extra_mem_size);
> -		if (IS_ERR(parts))
> -			return parts;
> -	} else {
> -		/* this is the last partition: allocate space for all */
> -		int alloc_size;
> -
> -		*num_parts = this_part + 1;
> -		alloc_size = *num_parts * sizeof(struct mtd_partition) +
> -			     extra_mem_size;
> -
> -		parts = kzalloc(alloc_size, GFP_KERNEL);
> -		if (!parts)
> -			return ERR_PTR(-ENOMEM);
> -		extra_mem = (unsigned char *)(parts + *num_parts);
> -	}
> -
> -	/* enter this partition (offset will be calculated later if it is zero at this point) */
> -	parts[this_part].size = size;
> -	parts[this_part].offset = offset;
> -	parts[this_part].mask_flags = mask_flags;
> -	if (name)
> -		strlcpy(extra_mem, name, name_len + 1);
> -	else
> -		sprintf(extra_mem, "Partition_%03d", this_part);
> -	parts[this_part].name = extra_mem;
> -	extra_mem += name_len + 1;
> -
> -	dbg(("partition %d: name <%s>, offset %llx, size %llx, mask flags %x\n",
> -	     this_part, parts[this_part].name, parts[this_part].offset,
> -	     parts[this_part].size, parts[this_part].mask_flags));
> +	if (subpart->flags & PF_POWERUP_LOCK)
> +		mtdpart->mask_flags |= MTD_POWERUP_LOCK;
>  
> -	/* return (updated) pointer to extra_mem memory */
> -	if (extra_mem_ptr)
> -		*extra_mem_ptr = extra_mem;
> -
> -	/* return (updated) pointer command line string */
> -	*retptr = s;
> -
> -	/* return partition table */
> -	return parts;
> +	return 0;
>  }
>  
>  /*
> - * Parse the command line.
> + * This is the handler for our kernel parameter, called from
> + * main.c::checksetup(). Note that we can not yet kmalloc() anything,
> + * so we only save the commandline for later processing.
> + *
> + * This function needs to be visible for bootloaders.
>   */
> -static int mtdpart_setup_real(char *s)
> +static int __init mtdpart_setup(char *s)
>  {
> -	cmdline_parsed = 1;
> -
> -	for( ; s != NULL; )
> -	{
> -		struct cmdline_mtd_partition *this_mtd;
> -		struct mtd_partition *parts;
> -		int mtd_id_len, num_parts;
> -		char *p, *mtd_id;
> -
> -		mtd_id = s;
> -
> -		/* fetch <mtd-id> */
> -		p = strchr(s, ':');
> -		if (!p) {
> -			printk(KERN_ERR ERRP "no mtd-id\n");
> -			return -EINVAL;
> -		}
> -		mtd_id_len = p - mtd_id;
> -
> -		dbg(("parsing <%s>\n", p+1));
> -
> -		/*
> -		 * parse one mtd. have it reserve memory for the
> -		 * struct cmdline_mtd_partition and the mtd-id string.
> -		 */
> -		parts = newpart(p + 1,		/* cmdline */
> -				&s,		/* out: updated cmdline ptr */
> -				&num_parts,	/* out: number of parts */
> -				0,		/* first partition */
> -				(unsigned char**)&this_mtd, /* out: extra mem */
> -				mtd_id_len + 1 + sizeof(*this_mtd) +
> -				sizeof(void*)-1 /*alignment*/);
> -		if (IS_ERR(parts)) {
> -			/*
> -			 * An error occurred. We're either:
> -			 * a) out of memory, or
> -			 * b) in the middle of the partition spec
> -			 * Either way, this mtd is hosed and we're
> -			 * unlikely to succeed in parsing any more
> -			 */
> -			 return PTR_ERR(parts);
> -		 }
> -
> -		/* align this_mtd */
> -		this_mtd = (struct cmdline_mtd_partition *)
> -				ALIGN((unsigned long)this_mtd, sizeof(void *));
> -		/* enter results */
> -		this_mtd->parts = parts;
> -		this_mtd->num_parts = num_parts;
> -		this_mtd->mtd_id = (char*)(this_mtd + 1);
> -		strlcpy(this_mtd->mtd_id, mtd_id, mtd_id_len + 1);
> -
> -		/* link into chain */
> -		this_mtd->next = partitions;
> -		partitions = this_mtd;
> -
> -		dbg(("mtdid=<%s> num_parts=<%d>\n",
> -		     this_mtd->mtd_id, this_mtd->num_parts));
> -
> -
> -		/* EOS - we're done */
> -		if (*s == 0)
> -			break;
> -
> -		/* does another spec follow? */
> -		if (*s != ';') {
> -			printk(KERN_ERR ERRP "bad character after partition (%c)\n", *s);
> -			return -EINVAL;
> -		}
> -		s++;
> -	}
> -
> -	return 0;
> +	mtdparts = s;
> +	return 1;
>  }
> +__setup("mtdparts=", mtdpart_setup);
>  
>  /*
>   * Main function to be called from the MTD mapping driver/device to
> @@ -309,82 +77,35 @@ static int parse_cmdline_partitions(struct mtd_info *master,
>  				    struct mtd_partition **pparts,
>  				    struct mtd_part_parser_data *data)
>  {
> -	unsigned long long offset;
> -	int i, err;
> -	struct cmdline_mtd_partition *part;
> -	const char *mtd_id = master->name;
> +	struct cmdline_parts *parts;
>  
> -	/* parse command line */
> -	if (!cmdline_parsed) {
> -		err = mtdpart_setup_real(cmdline);
> -		if (err)
> -			return err;
> -	}
> +	if (mtdparts) {
> +		if (mtd_cmdline_parts)
> +			cmdline_parts_free(&mtd_cmdline_parts);
> +
> +		if (cmdline_parts_parse(&mtd_cmdline_parts, mtdparts)) {
> +			mtdparts = NULL;
> +			return -EINVAL;
> +		}
>  
> -	/*
> -	 * Search for the partition definition matching master->name.
> -	 * If master->name is not set, stop at first partition definition.
> -	 */
> -	for (part = partitions; part; part = part->next) {
> -		if ((!mtd_id) || (!strcmp(part->mtd_id, mtd_id)))
> -			break;
> +		mtdparts = NULL;
>  	}
>  
> -	if (!part)
> +	if (!mtd_cmdline_parts)
>  		return 0;
>  
> -	for (i = 0, offset = 0; i < part->num_parts; i++) {
> -		if (part->parts[i].offset == OFFSET_CONTINUOUS)
> -			part->parts[i].offset = offset;
> -		else
> -			offset = part->parts[i].offset;
> -
> -		if (part->parts[i].size == SIZE_REMAINING)
> -			part->parts[i].size = master->size - offset;
> -
> -		if (offset + part->parts[i].size > master->size) {
> -			printk(KERN_WARNING ERRP
> -			       "%s: partitioning exceeds flash size, truncating\n",
> -			       part->mtd_id);
> -			part->parts[i].size = master->size - offset;
> -		}
> -		offset += part->parts[i].size;
> -
> -		if (part->parts[i].size == 0) {
> -			printk(KERN_WARNING ERRP
> -			       "%s: skipping zero sized partition\n",
> -			       part->mtd_id);
> -			part->num_parts--;
> -			memmove(&part->parts[i], &part->parts[i + 1],
> -				sizeof(*part->parts) * (part->num_parts - i));
> -			i--;
> -		}
> -	}
> +	parts = cmdline_parts_find(mtd_cmdline_parts, master->name);
> +	if (!parts)
> +		return 0;
>  
> -	*pparts = kmemdup(part->parts, sizeof(*part->parts) * part->num_parts,
> -			  GFP_KERNEL);
> +	*pparts = kzalloc(sizeof(**pparts) * parts->nr_subparts, GFP_KERNEL);
>  	if (!*pparts)
>  		return -ENOMEM;
>  
> -	return part->num_parts;
> -}
> -
> -
> -/*
> - * This is the handler for our kernel parameter, called from
> - * main.c::checksetup(). Note that we can not yet kmalloc() anything,
> - * so we only save the commandline for later processing.
> - *
> - * This function needs to be visible for bootloaders.
> - */
> -static int __init mtdpart_setup(char *s)
> -{
> -	cmdline = s;
> -	return 1;
> +	return cmdline_parts_set(parts, master->size, 0, add_part,
> +				 (void *)*pparts);
>  }
>  
> -__setup("mtdparts=", mtdpart_setup);
> -
>  static struct mtd_part_parser cmdline_parser = {
>  	.owner = THIS_MODULE,
>  	.parse_fn = parse_cmdline_partitions,
> @@ -393,8 +114,6 @@ static struct mtd_part_parser cmdline_parser = {
>  
>  static int __init cmdline_parser_init(void)
>  {
> -	if (mtdparts)
> -		mtdpart_setup(mtdparts);
>  	return register_mtd_parser(&cmdline_parser);
>  }
>  
> diff --git a/include/linux/cmdline-parser.h b/include/linux/cmdline-parser.h
> index 98e892e..43b65a9 100644
> --- a/include/linux/cmdline-parser.h
> +++ b/include/linux/cmdline-parser.h
> @@ -35,9 +35,9 @@ int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline);
>  struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
>  					 const char *bdev);
>  
> -void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
> -		       int slot,
> -		       int (*add_part)(int, struct cmdline_subpart *, void *),
> -		       void *param);
> +int cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
> +		      int slot,
> +		      int (*add_part)(int, struct cmdline_subpart *, void *),
> +		      void *param);
>  
>  #endif /* CMDLINEPARSEH */
> -- 
> 1.8.1.5
> 

I'd like to review the patch in detail and test it, but being a bit old,
the patch doesn't apply as it is on v3.12-rc5. Care to resend an update?

Brian: I saw you want to move forward with this. I guess we can give
it a test, and if all the possible 'mtdparts' use cases work, then
we shouldn't object the cleanup, right?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 1/2] block: remove unrelated header files and export symbol

[Caizhiyong]

From: CaiZhiyong <caizhiyong@huawei.com>
Subject: block: remove unrelated header files and export symbol

This patch fix up the following items:
 - remove unrelated header files.
 - export interface function.
 - modify function cmdline_parts_parse return value, this will make
   it more friendly for the caller.

Signed-off-by: CaiZhiyong <caizhiyong@huawei.com>
---
 block/cmdline-parser.c         | 18 +++++++++++-------
 include/linux/cmdline-parser.h |  8 ++++----
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c
index cc2637f..9dbc67e 100644
--- a/block/cmdline-parser.c
+++ b/block/cmdline-parser.c
@@ -4,8 +4,7 @@
  * Written by Cai Zhiyong <caizhiyong@huawei.com>
  *
  */
-#include <linux/buffer_head.h>
-#include <linux/module.h>
+#include <linux/export.h>
 #include <linux/cmdline-parser.h>
 
 static int parse_subpart(struct cmdline_subpart **subpart, char *partdef)
@@ -159,6 +158,7 @@ void cmdline_parts_free(struct cmdline_parts **parts)
 		*parts = next_parts;
 	}
 }
+EXPORT_SYMBOL(cmdline_parts_free);
 
 int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline)
 {
@@ -206,6 +206,7 @@ fail:
 	cmdline_parts_free(parts);
 	goto done;
 }
+EXPORT_SYMBOL(cmdline_parts_parse);
 
 struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
 					 const char *bdev)
@@ -214,17 +215,17 @@ struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
 		parts = parts->next_parts;
 	return parts;
 }
+EXPORT_SYMBOL(cmdline_parts_find);
 
 /*
  *  add_part()
  *    0 success.
  *    1 can not add so many partitions.
  */
-void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
-		       int slot,
-		       int (*add_part)(int, struct cmdline_subpart *, void *),
-		       void *param)
-
+int cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
+		      int slot,
+		      int (*add_part)(int, struct cmdline_subpart *, void *),
+		      void *param)
 {
 	sector_t from = 0;
 	struct cmdline_subpart *subpart;
@@ -247,4 +248,7 @@ void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
 		if (add_part(slot, subpart, param))
 			break;
 	}
+
+	return slot;
 }
+EXPORT_SYMBOL(cmdline_parts_set);
diff --git a/include/linux/cmdline-parser.h b/include/linux/cmdline-parser.h
index 98e892e..43b65a9 100644
--- a/include/linux/cmdline-parser.h
+++ b/include/linux/cmdline-parser.h
@@ -35,9 +35,9 @@ int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline);
 struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
 					 const char *bdev);
 
-void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
-		       int slot,
-		       int (*add_part)(int, struct cmdline_subpart *, void *),
-		       void *param);
+int cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size,
+		      int slot,
+		      int (*add_part)(int, struct cmdline_subpart *, void *),
+		      void *param);
 
 #endif /* CMDLINEPARSEH */
-- 
1.8.1.5

[PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib

[Caizhiyong]

From: CaiZhiyong <caizhiyong@huawei.com>
Subject: mtd: cmdlinepart: use cmdline partition parser lib

In the previous version, adjust the cmdline parser code to library-style
code, and move it to a separate file "block/cmdline-parser.c", we can use
it in some client code. there is no any functionality change in the adjusting.

this patch use cmdline parser lib.

For further information, see "https://lkml.org/lkml/2013/8/6/550"

Signed-off-by: CaiZhiyong <caizhiyong@huawei.com>
---
 drivers/mtd/Kconfig       |   1 +
 drivers/mtd/cmdlinepart.c | 343 +++++-----------------------------------------
 2 files changed, 39 insertions(+), 305 deletions(-)

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 5fab4e6e..daf544a 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -75,6 +75,7 @@ endif # MTD_REDBOOT_PARTS
 
 config MTD_CMDLINE_PARTS
 	tristate "Command line partition table parsing"
+	select BLK_CMDLINE_PARSER
 	depends on MTD
 	---help---
 	  Allow generic configuration of the MTD partition tables via the kernel
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index 721caeb..ba934a4 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -47,344 +47,79 @@
  * 1 NOR Flash with 2 partitions, 1 NAND with one
  * edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home)
  */
+ /*
+  * Copyright © 2013 Cai Zhiyong <caizhiyong@huawei.com>
+  * Rewrite the cmdline parser code, adjust it to a library-style code.
+  * this module only use the cmdline parser lib.
+  */
 
 #include <linux/kernel.h>
-#include <linux/slab.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/cmdline-parser.h>
 
-/* error message prefix */
-#define ERRP "mtd: "
-
-/* debug macro */
-#if 0
-#define dbg(x) do { printk("DEBUG-CMDLINE-PART: "); printk x; } while(0)
-#else
-#define dbg(x)
-#endif
-
-
-/* special size referring to all the remaining space in a partition */
-#define SIZE_REMAINING ULLONG_MAX
-#define OFFSET_CONTINUOUS ULLONG_MAX
-
-struct cmdline_mtd_partition {
-	struct cmdline_mtd_partition *next;
-	char *mtd_id;
-	int num_parts;
-	struct mtd_partition *parts;
-};
-
-/* mtdpart_setup() parses into here */
-static struct cmdline_mtd_partition *partitions;
-
-/* the command line passed to mtdpart_setup() */
 static char *mtdparts;
-static char *cmdline;
-static int cmdline_parsed;
+static struct cmdline_parts *mtd_cmdline_parts;
 
-/*
- * Parse one partition definition for an MTD. Since there can be many
- * comma separated partition definitions, this function calls itself
- * recursively until no more partition definitions are found. Nice side
- * effect: the memory to keep the mtd_partition structs and the names
- * is allocated upon the last definition being found. At that point the
- * syntax has been verified ok.
- */
-static struct mtd_partition * newpart(char *s,
-				      char **retptr,
-				      int *num_parts,
-				      int this_part,
-				      unsigned char **extra_mem_ptr,
-				      int extra_mem_size)
+static int add_part(int slot, struct cmdline_subpart *subpart, void *param)
 {
-	struct mtd_partition *parts;
-	unsigned long long size, offset = OFFSET_CONTINUOUS;
-	char *name;
-	int name_len;
-	unsigned char *extra_mem;
-	char delim;
-	unsigned int mask_flags;
-
-	/* fetch the partition size */
-	if (*s == '-') {
-		/* assign all remaining space to this partition */
-		size = SIZE_REMAINING;
-		s++;
-	} else {
-		size = memparse(s, &s);
-		if (size < PAGE_SIZE) {
-			printk(KERN_ERR ERRP "partition size too small (%llx)\n",
-			       size);
-			return ERR_PTR(-EINVAL);
-		}
-	}
-
-	/* fetch partition name and flags */
-	mask_flags = 0; /* this is going to be a regular partition */
-	delim = 0;
-
-	/* check for offset */
-	if (*s == '@') {
-		s++;
-		offset = memparse(s, &s);
-	}
+	struct mtd_partition *mtdpart = &((struct mtd_partition *)param)[slot];
 
-	/* now look for name */
-	if (*s == '(')
-		delim = ')';
+	mtdpart->offset = subpart->from;
+	mtdpart->size = subpart->size;
+	mtdpart->name = subpart->name;
+	mtdpart->mask_flags = 0;
 
-	if (delim) {
-		char *p;
+	if (subpart->flags & PF_RDONLY)
+		mtdpart->mask_flags |= MTD_WRITEABLE;
 
-		name = ++s;
-		p = strchr(name, delim);
-		if (!p) {
-			printk(KERN_ERR ERRP "no closing %c found in partition name\n", delim);
-			return ERR_PTR(-EINVAL);
-		}
-		name_len = p - name;
-		s = p + 1;
-	} else {
-		name = NULL;
-		name_len = 13; /* Partition_000 */
-	}
-
-	/* record name length for memory allocation later */
-	extra_mem_size += name_len + 1;
-
-	/* test for options */
-	if (strncmp(s, "ro", 2) == 0) {
-		mask_flags |= MTD_WRITEABLE;
-		s += 2;
-	}
+	if (subpart->flags & PF_POWERUP_LOCK)
+		mtdpart->mask_flags |= MTD_POWERUP_LOCK;
 
-	/* if lk is found do NOT unlock the MTD partition*/
-	if (strncmp(s, "lk", 2) == 0) {
-		mask_flags |= MTD_POWERUP_LOCK;
-		s += 2;
-	}
-
-	/* test if more partitions are following */
-	if (*s == ',') {
-		if (size == SIZE_REMAINING) {
-			printk(KERN_ERR ERRP "no partitions allowed after a fill-up partition\n");
-			return ERR_PTR(-EINVAL);
-		}
-		/* more partitions follow, parse them */
-		parts = newpart(s + 1, &s, num_parts, this_part + 1,
-				&extra_mem, extra_mem_size);
-		if (IS_ERR(parts))
-			return parts;
-	} else {
-		/* this is the last partition: allocate space for all */
-		int alloc_size;
-
-		*num_parts = this_part + 1;
-		alloc_size = *num_parts * sizeof(struct mtd_partition) +
-			     extra_mem_size;
-
-		parts = kzalloc(alloc_size, GFP_KERNEL);
-		if (!parts)
-			return ERR_PTR(-ENOMEM);
-		extra_mem = (unsigned char *)(parts + *num_parts);
-	}
-
-	/* enter this partition (offset will be calculated later if it is zero at this point) */
-	parts[this_part].size = size;
-	parts[this_part].offset = offset;
-	parts[this_part].mask_flags = mask_flags;
-	if (name)
-		strlcpy(extra_mem, name, name_len + 1);
-	else
-		sprintf(extra_mem, "Partition_%03d", this_part);
-	parts[this_part].name = extra_mem;
-	extra_mem += name_len + 1;
-
-	dbg(("partition %d: name <%s>, offset %llx, size %llx, mask flags %x\n",
-	     this_part, parts[this_part].name, parts[this_part].offset,
-	     parts[this_part].size, parts[this_part].mask_flags));
-
-	/* return (updated) pointer to extra_mem memory */
-	if (extra_mem_ptr)
-		*extra_mem_ptr = extra_mem;
-
-	/* return (updated) pointer command line string */
-	*retptr = s;
-
-	/* return partition table */
-	return parts;
+	return 0;
 }
 
-/*
- * Parse the command line.
- */
-static int mtdpart_setup_real(char *s)
+static int __init mtdpart_setup(char *s)
 {
-	cmdline_parsed = 1;
-
-	for( ; s != NULL; )
-	{
-		struct cmdline_mtd_partition *this_mtd;
-		struct mtd_partition *parts;
-		int mtd_id_len, num_parts;
-		char *p, *mtd_id;
-
-		mtd_id = s;
-
-		/* fetch <mtd-id> */
-		p = strchr(s, ':');
-		if (!p) {
-			printk(KERN_ERR ERRP "no mtd-id\n");
-			return -EINVAL;
-		}
-		mtd_id_len = p - mtd_id;
-
-		dbg(("parsing <%s>\n", p+1));
-
-		/*
-		 * parse one mtd. have it reserve memory for the
-		 * struct cmdline_mtd_partition and the mtd-id string.
-		 */
-		parts = newpart(p + 1,		/* cmdline */
-				&s,		/* out: updated cmdline ptr */
-				&num_parts,	/* out: number of parts */
-				0,		/* first partition */
-				(unsigned char**)&this_mtd, /* out: extra mem */
-				mtd_id_len + 1 + sizeof(*this_mtd) +
-				sizeof(void*)-1 /*alignment*/);
-		if (IS_ERR(parts)) {
-			/*
-			 * An error occurred. We're either:
-			 * a) out of memory, or
-			 * b) in the middle of the partition spec
-			 * Either way, this mtd is hosed and we're
-			 * unlikely to succeed in parsing any more
-			 */
-			 return PTR_ERR(parts);
-		 }
-
-		/* align this_mtd */
-		this_mtd = (struct cmdline_mtd_partition *)
-				ALIGN((unsigned long)this_mtd, sizeof(void *));
-		/* enter results */
-		this_mtd->parts = parts;
-		this_mtd->num_parts = num_parts;
-		this_mtd->mtd_id = (char*)(this_mtd + 1);
-		strlcpy(this_mtd->mtd_id, mtd_id, mtd_id_len + 1);
-
-		/* link into chain */
-		this_mtd->next = partitions;
-		partitions = this_mtd;
-
-		dbg(("mtdid=<%s> num_parts=<%d>\n",
-		     this_mtd->mtd_id, this_mtd->num_parts));
-
-
-		/* EOS - we're done */
-		if (*s == 0)
-			break;
-
-		/* does another spec follow? */
-		if (*s != ';') {
-			printk(KERN_ERR ERRP "bad character after partition (%c)\n", *s);
-			return -EINVAL;
-		}
-		s++;
-	}
-
-	return 0;
+	mtdparts = s;
+	return 1;
 }
+__setup("mtdparts=", mtdpart_setup);
 
-/*
- * Main function to be called from the MTD mapping driver/device to
- * obtain the partitioning information. At this point the command line
- * arguments will actually be parsed and turned to struct mtd_partition
- * information. It returns partitions for the requested mtd device, or
- * the first one in the chain if a NULL mtd_id is passed in.
- */
 static int parse_cmdline_partitions(struct mtd_info *master,
 				    struct mtd_partition **pparts,
 				    struct mtd_part_parser_data *data)
 {
-	unsigned long long offset;
-	int i, err;
-	struct cmdline_mtd_partition *part;
-	const char *mtd_id = master->name;
+	struct cmdline_parts *parts;
 
-	/* parse command line */
-	if (!cmdline_parsed) {
-		err = mtdpart_setup_real(cmdline);
-		if (err)
-			return err;
-	}
+	if (mtdparts) {
+		if (mtd_cmdline_parts)
+			cmdline_parts_free(&mtd_cmdline_parts);
 
-	/*
-	 * Search for the partition definition matching master->name.
-	 * If master->name is not set, stop at first partition definition.
-	 */
-	for (part = partitions; part; part = part->next) {
-		if ((!mtd_id) || (!strcmp(part->mtd_id, mtd_id)))
-			break;
+		if (cmdline_parts_parse(&mtd_cmdline_parts, mtdparts)) {
+			mtdparts = NULL;
+			return -EINVAL;
+		}
+		mtdparts = NULL;
 	}
 
-	if (!part)
+	if (!mtd_cmdline_parts)
 		return 0;
 
-	for (i = 0, offset = 0; i < part->num_parts; i++) {
-		if (part->parts[i].offset == OFFSET_CONTINUOUS)
-			part->parts[i].offset = offset;
-		else
-			offset = part->parts[i].offset;
-
-		if (part->parts[i].size == SIZE_REMAINING)
-			part->parts[i].size = master->size - offset;
-
-		if (offset + part->parts[i].size > master->size) {
-			printk(KERN_WARNING ERRP
-			       "%s: partitioning exceeds flash size, truncating\n",
-			       part->mtd_id);
-			part->parts[i].size = master->size - offset;
-		}
-		offset += part->parts[i].size;
-
-		if (part->parts[i].size == 0) {
-			printk(KERN_WARNING ERRP
-			       "%s: skipping zero sized partition\n",
-			       part->mtd_id);
-			part->num_parts--;
-			memmove(&part->parts[i], &part->parts[i + 1],
-				sizeof(*part->parts) * (part->num_parts - i));
-			i--;
-		}
-	}
+	parts = cmdline_parts_find(mtd_cmdline_parts, master->name);
+	if (!parts)
+		return 0;
 
-	*pparts = kmemdup(part->parts, sizeof(*part->parts) * part->num_parts,
-			  GFP_KERNEL);
+	*pparts = kzalloc(sizeof(**pparts) * parts->nr_subparts, GFP_KERNEL);
 	if (!*pparts)
 		return -ENOMEM;
 
-	return part->num_parts;
+	return cmdline_parts_set(parts, master->size, 0, add_part,
+				 (void *)*pparts);
 }
 
-
-/*
- * This is the handler for our kernel parameter, called from
- * main.c::checksetup(). Note that we can not yet kmalloc() anything,
- * so we only save the commandline for later processing.
- *
- * This function needs to be visible for bootloaders.
- */
-static int __init mtdpart_setup(char *s)
-{
-	cmdline = s;
-	return 1;
-}
-
-__setup("mtdparts=", mtdpart_setup);
-
 static struct mtd_part_parser cmdline_parser = {
 	.owner = THIS_MODULE,
 	.parse_fn = parse_cmdline_partitions,
@@ -393,8 +128,6 @@ static struct mtd_part_parser cmdline_parser = {
 
 static int __init cmdline_parser_init(void)
 {
-	if (mtdparts)
-		mtdpart_setup(mtdparts);
 	return register_mtd_parser(&cmdline_parser);
 }
 
-- 
1.8.1.5

RE: [PATCH] mtd: cmdlinepart: use cmdline partition parser lib

[Caizhiyong]

> 
> I'd like to review the patch in detail and test it, but being a bit old,
> the patch doesn't apply as it is on v3.12-rc5. Care to resend an update?
>
  I have sent you a 2 patch base on v3.12-rc6. Are there any problems? I tested it for a long time and found no problems.

  Are there any plans for supporting synchronous NAND in the future?

 
> Brian: I saw you want to move forward with this. I guess we can give
> it a test, and if all the possible 'mtdparts' use cases work, then
> we shouldn't object the cleanup, right?
> --
> Ezequiel García, Free Electrons
> Embedded Linux, Kernel and Android Engineering
> http://free-electrons.com

Re: [PATCH] mtd: cmdlinepart: use cmdline partition parser lib

[Ezequiel Garcia]

Hi,

On Fri, Oct 25, 2013 at 11:52:14AM +0000, Caizhiyong wrote:
> > 
> > I'd like to review the patch in detail and test it, but being a bit old,
> > the patch doesn't apply as it is on v3.12-rc5. Care to resend an update?
> >
>   I have sent you a 2 patch base on v3.12-rc6. Are there any problems? I tested it for a long time and found no problems.
> 

I haven't had time to look at that yet, thanks for re-sending them!

If you already did testing using those, then I think we would all appreciate
that you post the results.

In particular, we're interested in **seeing** this change parses correctly
all possible combinations of partitions configured by the 'mtdparts' parameter.

This is a big change you're proposing and we need to be sure there won't
be any users experiencing any unexpected result while using 'mtdparts'.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib

[Andrew Morton]

On Tue, 22 Oct 2013 13:14:17 +0000 Caizhiyong <caizhiyong@hisilicon.com> wrote:

> In the previous version, adjust the cmdline parser code to library-style
> code, and move it to a separate file "block/cmdline-parser.c", we can use
> it in some client code. there is no any functionality change in the adjusting.
> 
> this patch use cmdline parser lib.
> 
> For further information, see "https://lkml.org/lkml/2013/8/6/550"

Thanks for doing this.  Could we please get some acked-by's or,
preferably, tested-by's from the MTD people?

Re: [PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib

[Brian Norris]

On Tue, Nov 5, 2013 at 2:43 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 22 Oct 2013 13:14:17 +0000 Caizhiyong <caizhiyong@hisilicon.com> wrote:
>
>> In the previous version, adjust the cmdline parser code to library-style
>> code, and move it to a separate file "block/cmdline-parser.c", we can use
>> it in some client code. there is no any functionality change in the adjusting.
>>
>> this patch use cmdline parser lib.
>>
>> For further information, see "https://lkml.org/lkml/2013/8/6/550"
>
> Thanks for doing this.  Could we please get some acked-by's or,
> preferably, tested-by's from the MTD people?

Nobody has had time to test this on MTD, it seems, and as such, I
strongly recommend you do not force it through -mm. We are perfectly
capable of merging it through the MTD tree if it ever gets proper
vetting by people in MTD (not just on block devices), and I am well
aware of this patch set's existence.

However, the patch really provides little to no benefit to the MTD
subsystem at the moment, at the added cost of reviewing the small
differences in parsing. For some reason, Cai decided to make small
differences in the parser between drivers/mtd/cmdlinepart.c and
block/cmdline-parser.c, and it is not obvious that these differences
retain the same parsing. For instance, according to my code
read-through, the block device parser seems to accept multiple
partitions to be specified with "-" (meaning "fill the remaining
device"). MTD already has code that rejects such a parser string
outright.

So, I'd recommend one of the following:
(1) We (MTD users) make more time to properly test this patch and post
relevant results (i.e., tested partition strings) or
(2) Somebody (Cai?) spend time to actually make block/cmdline-parser.c
fully equivalent to the parser in drivers/mtd/cmdlinepart.c. That
means it should be trivial to compare the two and say "yes, these are
equivalent". That is currently not the case, AFAICT.

Without one of those two, I will give my NAK.

Brian

RE: [PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib

[Caizhiyong]

>> For further information, see "https://lkml.org/lkml/2013/8/6/550"
> 
> Thanks for doing this.  Could we please get some acked-by's or,
> preferably, tested-by's from the MTD people?

Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Acked-by: Andrew Morton <akpm@linux-foundation.org>

RE: [PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib

[Caizhiyong]

> Nobody has had time to test this on MTD, it seems, and as such, I
> strongly recommend you do not force it through -mm. We are perfectly
> capable of merging it through the MTD tree if it ever gets proper
> vetting by people in MTD (not just on block devices), and I am well
> aware of this patch set's existence.
> 
> However, the patch really provides little to no benefit to the MTD
> subsystem at the moment, at the added cost of reviewing the small
> differences in parsing. For some reason, Cai decided to make small
> differences in the parser between drivers/mtd/cmdlinepart.c and
> block/cmdline-parser.c, and it is not obvious that these differences
> retain the same parsing. For instance, according to my code
> read-through, the block device parser seems to accept multiple
> partitions to be specified with "-" (meaning "fill the remaining
> device"). MTD already has code that rejects such a parser string
> outright.

The next '-' partition be ignore at function "cmdline_parts_set", and the client will get a right result.
Is there any other worry about '-' I don't know ?

> 
> So, I'd recommend one of the following:
> (1) We (MTD users) make more time to properly test this patch and post
> relevant results (i.e., tested partition strings) or
> (2) Somebody (Cai?) spend time to actually make block/cmdline-parser.c
> fully equivalent to the parser in drivers/mtd/cmdlinepart.c. That
> means it should be trivial to compare the two and say "yes, these are
> equivalent". That is currently not the case, AFAICT.

I understand your worry about, we use cmdlinepart many years.
I will spend time to make block/cmdline-parser.c fully equivalent to the
parser in drivers/mtd/cmdlinepart.c.

> 
> Without one of those two, I will give my NAK.
> 
> Brian

Re: [PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib

[Ezequiel Garcia]

On Fri, Nov 08, 2013 at 06:53:29AM +0000, Caizhiyong wrote:
> >> For further information, see "https://lkml.org/lkml/2013/8/6/550"
> > 
> > Thanks for doing this.  Could we please get some acked-by's or,
> > preferably, tested-by's from the MTD people?
> 
> Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

I don't remember acking this patch! Instead, I do remember asking
for the test results, prooving the this change has _no_ change of
behavior compared to the MTD parsing code:

https://lkml.org/lkml/2013/10/25/164

Such results was never posted and unless we see those, I think
I'd rather NACK this patch instead. I like the cleanup, but only
if it's guaranteed to _not_ brake things, specially when dealing
with a kernel parameter.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

RE: [PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib

[Caizhiyong]

> Such results was never posted and unless we see those, I think
> I'd rather NACK this patch instead. I like the cleanup, but only
> if it's guaranteed to _not_ brake things, specially when dealing
> with a kernel parameter.

Do you have some test case or test standard for me perform.

> --
> Ezequiel García, Free Electrons
> Embedded Linux, Kernel and Android Engineering
> http://free-electrons.com