[PATCH] block: support embedded device command line partition

[PATCH] block: support embedded device command line partition

[Caizhiyong]

From: Cai Zhiyong <caizhiyong@huawei.com>

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.

This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
About the partition verbose reference "Documentation/block/cmdline-partition.txt"

Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
---
 Documentation/block/cmdline-partition.txt |  40 ++++
 block/partitions/Kconfig                  |   6 +
 block/partitions/Makefile                 |   1 +
 block/partitions/check.c                  |   4 +
 block/partitions/cmdline.c                | 326 ++++++++++++++++++++++++++++++
 block/partitions/cmdline.h                |   2 +
 6 files changed, 379 insertions(+)
 create mode 100644 Documentation/block/cmdline-partition.txt
 create mode 100644 block/partitions/cmdline.c
 create mode 100644 block/partitions/cmdline.h

diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt
new file mode 100644
index 0000000..651863d
--- /dev/null
+++ b/Documentation/block/cmdline-partition.txt
@@ -0,0 +1,40 @@
+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>]
+  <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:
+    '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/partitions/Kconfig b/block/partitions/Kconfig
index 4cebb2f..2ebf251 100644
--- a/block/partitions/Kconfig
+++ b/block/partitions/Kconfig
@@ -260,3 +260,9 @@ config SYSV68_PARTITION
 	  partition table format used by Motorola Delta machines (using
 	  sysv68).
 	  Otherwise, say N.
+
+config CMDLINE_PARTITION
+	bool "Command line partition support" if PARTITION_ADVANCED
+	help
+	  Say Y here if you would read the partitions table from bootargs.
+	  The format for the command line is just like mtdparts.
diff --git a/block/partitions/Makefile b/block/partitions/Makefile
index 2be4d7b..4e9d584 100644
--- a/block/partitions/Makefile
+++ b/block/partitions/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_IBM_PARTITION) += ibm.o
 obj-$(CONFIG_EFI_PARTITION) += efi.o
 obj-$(CONFIG_KARMA_PARTITION) += karma.o
 obj-$(CONFIG_SYSV68_PARTITION) += sysv68.o
+obj-$(CONFIG_CMDLINE_PARTITION) += cmdline.o
diff --git a/block/partitions/check.c b/block/partitions/check.c
index 19ba207..9ac1df7 100644
--- a/block/partitions/check.c
+++ b/block/partitions/check.c
@@ -34,6 +34,7 @@
 #include "efi.h"
 #include "karma.h"
 #include "sysv68.h"
+#include "cmdline.h"
 
 int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/
 
@@ -65,6 +66,9 @@ static int (*check_part[])(struct parsed_partitions *) = {
 	adfspart_check_ADFS,
 #endif
 
+#ifdef CONFIG_CMDLINE_PARTITION
+	cmdline_partition,
+#endif
 #ifdef CONFIG_EFI_PARTITION
 	efi_partition,		/* this must come before msdos */
 #endif
diff --git a/block/partitions/cmdline.c b/block/partitions/cmdline.c
new file mode 100644
index 0000000..dc69483
--- /dev/null
+++ b/block/partitions/cmdline.c
@@ -0,0 +1,326 @@
+/*
+ * Copyright (C) 2013 HUAWEI
+ * Author: Cai Zhiyong <caizhiyong@huawei.com>
+ *
+ * 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.
+ *
+ * This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
+ * The format for the command line is just like mtdparts.
+ *
+ * Verbose config please reference "Documentation/block/cmdline-partition.txt"
+ *
+ */
+
+#include <linux/buffer_head.h>
+#include <linux/module.h>
+#include <linux/ctype.h>
+
+#include "check.h"
+#include "cmdline.h"
+
+struct cmdline_subpart {
+	char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */
+	sector_t from;
+	sector_t size;
+	struct cmdline_subpart *next_subpart;
+};
+
+struct cmdline_parts {
+	char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
+	struct cmdline_subpart *subpart;
+	struct cmdline_parts *next_parts;
+};
+
+static char *cmdline_string;
+static struct cmdline_parts *cmdline_parts;
+
+static int parse_subpart(struct cmdline_subpart **subpart, char *cmdline)
+{
+	int ret = 0;
+	struct cmdline_subpart *new_subpart;
+
+	*subpart = NULL;
+
+	new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
+	if (!new_subpart)
+		return -ENOMEM;
+
+	if (*cmdline == '-') {
+		new_subpart->size = (sector_t)(~0ULL);
+		cmdline++;
+	} else {
+		new_subpart->size = (sector_t)memparse(cmdline, &cmdline);
+		if (new_subpart->size < (sector_t)PAGE_SIZE) {
+			pr_warn("cmdline partition size is invalid.");
+			ret = -EINVAL;
+			goto fail;
+		}
+	}
+
+	if (*cmdline == '@') {
+		cmdline++;
+		new_subpart->from = (sector_t)memparse(cmdline, &cmdline);
+	} else {
+		new_subpart->from = (sector_t)(~0ULL);
+	}
+
+	if (*cmdline == '(') {
+		int length;
+		char *next = strchr(++cmdline, ')');
+
+		if (!next) {
+			pr_warn("cmdline partition format is invalid.");
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		length = min_t(int, next - cmdline,
+			       sizeof(new_subpart->name) - 1);
+		strncpy(new_subpart->name, cmdline, length);
+		new_subpart->name[length] = '\0';
+
+		cmdline = ++next;
+	} else
+		new_subpart->name[0] = '\0';
+
+	*subpart = new_subpart;
+	return 0;
+fail:
+	kfree(new_subpart);
+	return ret;
+}
+
+static void free_subpart(struct cmdline_parts *parts)
+{
+	struct cmdline_subpart *subpart;
+
+	while (parts->subpart) {
+		subpart = parts->subpart;
+		parts->subpart = subpart->next_subpart;
+		kfree(subpart);
+	}
+}
+
+static void free_parts(struct cmdline_parts **parts)
+{
+	struct cmdline_parts *next_parts;
+
+	while (*parts) {
+		next_parts = (*parts)->next_parts;
+		free_subpart(*parts);
+		kfree(*parts);
+		*parts = next_parts;
+	}
+}
+
+static int parse_parts(struct cmdline_parts **parts, const char *cmdline)
+{
+	int ret = -EINVAL;
+	char *next;
+	int length;
+	struct cmdline_subpart **next_subpart;
+	struct cmdline_parts *newparts;
+	char buf[BDEVNAME_SIZE + 32 + 4];
+
+	*parts = NULL;
+
+	newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
+	if (!newparts)
+		return -ENOMEM;
+
+	next = strchr(cmdline, ':');
+	if (!next) {
+		pr_warn("cmdline partition has not block device.");
+		goto fail;
+	}
+
+	length = min_t(int, next - cmdline, sizeof(newparts->name) - 1);
+	strncpy(newparts->name, cmdline, length);
+	newparts->name[length] = '\0';
+
+	next_subpart = &newparts->subpart;
+
+	while (next && *(++next)) {
+		cmdline = next;
+		next = strchr(cmdline, ',');
+
+		length = (!next) ? (sizeof(buf) - 1) :
+			min_t(int, next - cmdline, sizeof(buf) - 1);
+
+		strncpy(buf, cmdline, length);
+		buf[length] = '\0';
+
+		ret = parse_subpart(next_subpart, buf);
+		if (ret)
+			goto fail;
+
+		next_subpart = &(*next_subpart)->next_subpart;
+	}
+
+	if (!newparts->subpart) {
+		pr_warn("cmdline partition has not valid partition.");
+		goto fail;
+	}
+
+	*parts = newparts;
+
+	return 0;
+fail:
+	free_subpart(newparts);
+	kfree(newparts);
+	return ret;
+}
+
+static int parse_cmdline(struct cmdline_parts **parts, const char *cmdline)
+{
+	int ret;
+	char *buf;
+	char *pbuf;
+	char *next;
+	struct cmdline_parts **next_parts;
+
+	*parts = NULL;
+
+	next = pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	next_parts = parts;
+
+	while (next && *pbuf) {
+		next = strchr(pbuf, ';');
+		if (next)
+			*next = '\0';
+
+		ret = parse_parts(next_parts, pbuf);
+		if (ret)
+			goto fail;
+
+		if (next)
+			pbuf = ++next;
+
+		next_parts = &(*next_parts)->next_parts;
+	}
+
+	if (!*parts) {
+		pr_warn("cmdline partition has not valid partition.");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	ret = 0;
+done:
+	kfree(buf);
+	return ret;
+
+fail:
+	free_parts(parts);
+	goto done;
+}
+
+/*
+ * Purpose: allocate cmdline partitions.
+ * Returns:
+ * -1 if unable to read the partition table
+ *  0 if this isn't our partition table
+ *  1 if successful
+ */
+static int parse_partitions(struct parsed_partitions *state,
+			    struct cmdline_parts *parts)
+{
+	int slot;
+	sector_t from = 0;
+	sector_t disk_size;
+	char buf[BDEVNAME_SIZE];
+	struct cmdline_subpart *subpart;
+
+	bdevname(state->bdev, buf);
+
+	while (parts && strncmp(buf, parts->name, BDEVNAME_SIZE))
+		parts = parts->next_parts;
+
+	if (!parts)
+		return 0;
+
+	disk_size = get_capacity(state->bdev->bd_disk) << 9;
+
+	for (slot = 1, subpart = parts->subpart;
+	     subpart && slot < state->limit;
+	     subpart = subpart->next_subpart, slot++) {
+		int label_min;
+		struct partition_meta_info *info;
+		char tmp[sizeof(info->volname) + 4];
+
+		if (subpart->from == (sector_t)(~0ULL))
+			subpart->from = from;
+		else
+			from = subpart->from;
+
+		if (from >= disk_size)
+			break;
+
+		if (subpart->size > (disk_size - from))
+			subpart->size = disk_size - from;
+
+		from += subpart->size;
+
+		put_partition(state, slot, subpart->from >> 9,
+			      subpart->size >> 9);
+
+		info = &state->parts[slot].info;
+
+		label_min = min_t(int, sizeof(info->volname) - 1,
+				  sizeof(subpart->name));
+		strncpy(info->volname, subpart->name, label_min);
+		info->volname[label_min] = '\0';
+
+		snprintf(tmp, sizeof(tmp), "(%s)", info->volname);
+		strlcat(state->pp_buf, tmp, PAGE_SIZE);
+
+		state->parts[slot].has_info = true;
+	}
+
+	strlcat(state->pp_buf, "\n", PAGE_SIZE);
+
+	return 1;
+}
+
+static int __init cmdline_parts_setup(char *s)
+{
+	cmdline_string = s;
+	return 1;
+}
+__setup("blkdevparts=", cmdline_parts_setup);
+
+/*
+ * Purpose: allocate cmdline partitions.
+ * Returns:
+ * -1 if unable to read the partition table
+ *  0 if this isn't our partition table
+ *  1 if successful
+ */
+int cmdline_partition(struct parsed_partitions *state)
+{
+	if (cmdline_string) {
+		if (cmdline_parts)
+			free_parts(&cmdline_parts);
+
+		if (parse_cmdline(&cmdline_parts, cmdline_string))
+			goto fail;
+
+		cmdline_string = NULL;
+	}
+
+	if (!cmdline_parts)
+		return 0;
+
+	return parse_partitions(state, cmdline_parts);
+
+fail:
+	cmdline_string = NULL;
+	return -1;
+}
diff --git a/block/partitions/cmdline.h b/block/partitions/cmdline.h
new file mode 100644
index 0000000..26e0f8d
--- /dev/null
+++ b/block/partitions/cmdline.h
@@ -0,0 +1,2 @@
+
+int cmdline_partition(struct parsed_partitions *state);
-- 
1.8.1.5

Re: [PATCH] block: support embedded device command line partition

[Andrew Morton]

On Sat, 3 Aug 2013 09:57:04 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:

> From: Cai Zhiyong <caizhiyong@huawei.com>
> 
> 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.
> 
> This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
> About the partition verbose reference "Documentation/block/cmdline-partition.txt"
> 

Seems OK to me.

The obvious question is: can we consolidate this with
drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have
two parsers which do the same thing?

RE: [PATCH] block: support embedded device command line partition

[Caizhiyong]

> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org]
> Sent: Tuesday, August 06, 2013 6:22 AM
> To: Caizhiyong
> Cc: Karel Zak; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin; Jens Axboe;
> David Woodhouse; Marius Groeger
> Subject: Re: [PATCH] block: support embedded device command line partition
> 
> On Sat, 3 Aug 2013 09:57:04 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> 
> > From: Cai Zhiyong <caizhiyong@huawei.com>
> >
> > 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.
> >
> > This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
> > About the partition verbose reference
> "Documentation/block/cmdline-partition.txt"
> >
> 
> Seems OK to me.
> 
> The obvious question is: can we consolidate this with
> drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have
> two parsers which do the same thing?

I will move the parser to "lib/cmdline.c".

Looking forward to your feedback.
Best regards,
Cai Zhiyong.

http://www.huawei.com

Re: [PATCH] block: support embedded device command line partition

[Andrew Morton]

On Tue, 6 Aug 2013 10:53:01 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:

> > -----Original Message-----
> > From: Andrew Morton [mailto:akpm@linux-foundation.org]
> > Sent: Tuesday, August 06, 2013 6:22 AM
> > To: Caizhiyong
> > Cc: Karel Zak; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin; Jens Axboe;
> > David Woodhouse; Marius Groeger
> > Subject: Re: [PATCH] block: support embedded device command line partition
> > 
> > On Sat, 3 Aug 2013 09:57:04 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> > 
> > > From: Cai Zhiyong <caizhiyong@huawei.com>
> > >
> > > 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.
> > >
> > > This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
> > > About the partition verbose reference
> > "Documentation/block/cmdline-partition.txt"
> > >
> > 
> > Seems OK to me.
> > 
> > The obvious question is: can we consolidate this with
> > drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have
> > two parsers which do the same thing?
> 
> I will move the parser to "lib/cmdline.c".
> 

block/cmdline.c is an appropriate place for block-subsystem library
code.

But simply moving the file doesn't achieve anything.  To prove that the
code is indeed library-style code, we should actually *use* it in some
client code.  Such as MTD!

[PATCH] block: add command line partition parser

[Caizhiyong]

From: Cai Zhiyong <caizhiyong@huawei.com>

move the command line parser to a separate module, and change it into
library-style code.

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

Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
---
 block/Kconfig                  |   6 +
 block/Makefile                 |   1 +
 block/cmdline-parser.c         | 249 +++++++++++++++++++++++++++++++++
 block/partitions/Kconfig       |   1 +
 block/partitions/cmdline.c     | 311 ++++++-----------------------------------
 include/linux/cmdline-parser.h |  43 ++++++
 6 files changed, 342 insertions(+), 269 deletions(-)
 create mode 100644 block/cmdline-parser.c
 create mode 100644 include/linux/cmdline-parser.h

diff --git a/block/Kconfig b/block/Kconfig
index a7e40a7..7f38e40 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -99,6 +99,12 @@ config BLK_DEV_THROTTLING
 
 	See Documentation/cgroups/blkio-controller.txt for more information.
 
+config CMDLINE_PARSER
+	bool "Block device command line partition parser"
+	default n
+	---help---
+	Parsing command line, get the partitions information.
+
 menu "Partition Types"
 
 source "block/partitions/Kconfig"
diff --git a/block/Makefile b/block/Makefile
index 39b76ba..4fa4be5 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_IOSCHED_CFQ)	+= cfq-iosched.o
 
 obj-$(CONFIG_BLOCK_COMPAT)	+= compat_ioctl.o
 obj-$(CONFIG_BLK_DEV_INTEGRITY)	+= blk-integrity.o
+obj-$(CONFIG_CMDLINE_PARSER)	+= cmdline-parser.o
diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c
new file mode 100644
index 0000000..18fb435
--- /dev/null
+++ b/block/cmdline-parser.c
@@ -0,0 +1,249 @@
+/*
+ * Parse command line, get partition information
+ *
+ * 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)
+{
+	int ret = 0;
+	struct cmdline_subpart *new_subpart;
+
+	*subpart = NULL;
+
+	new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
+	if (!new_subpart)
+		return -ENOMEM;
+
+	if (*partdef == '-') {
+		new_subpart->size = (sector_t)(~0ULL);
+		partdef++;
+	} else {
+		new_subpart->size = (sector_t)memparse(partdef, &partdef);
+		if (new_subpart->size < (sector_t)PAGE_SIZE) {
+			pr_warn("cmdline partition size is invalid.");
+			ret = -EINVAL;
+			goto fail;
+		}
+	}
+
+	if (*partdef == '@') {
+		partdef++;
+		new_subpart->from = (sector_t)memparse(partdef, &partdef);
+	} else {
+		new_subpart->from = (sector_t)(~0ULL);
+	}
+
+	if (*partdef == '(') {
+		int length;
+		char *next = strchr(++partdef, ')');
+
+		if (!next) {
+			pr_warn("cmdline partition format is invalid.");
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		length = min_t(int, next - partdef,
+			       sizeof(new_subpart->name) - 1);
+		strncpy(new_subpart->name, partdef, length);
+		new_subpart->name[length] = '\0';
+
+		partdef = ++next;
+	} else
+		new_subpart->name[0] = '\0';
+
+	new_subpart->flags = 0;
+
+	if (!strncmp(partdef, "ro", 2)) {
+		new_subpart->flags |= PF_RDONLY;
+		partdef += 2;
+	}
+
+	if (!strncmp(partdef, "lk", 2)) {
+		new_subpart->flags |= PF_POWERUP_LOCK;
+		partdef += 2;
+	}
+
+	*subpart = new_subpart;
+	return 0;
+fail:
+	kfree(new_subpart);
+	return ret;
+}
+
+static void free_subpart(struct cmdline_parts *parts)
+{
+	struct cmdline_subpart *subpart;
+
+	while (parts->subpart) {
+		subpart = parts->subpart;
+		parts->subpart = subpart->next_subpart;
+		kfree(subpart);
+	}
+}
+
+static int parse_parts(struct cmdline_parts **parts, const char *bdevdef)
+{
+	int ret = -EINVAL;
+	char *next;
+	int length;
+	struct cmdline_subpart **next_subpart;
+	struct cmdline_parts *newparts;
+	char buf[BDEVNAME_SIZE + 32 + 4];
+
+	*parts = NULL;
+
+	newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
+	if (!newparts)
+		return -ENOMEM;
+
+	next = strchr(bdevdef, ':');
+	if (!next) {
+		pr_warn("cmdline partition has no block device.");
+		goto fail;
+	}
+
+	length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1);
+	strncpy(newparts->name, bdevdef, length);
+	newparts->name[length] = '\0';
+	newparts->nr_subparts = 0;
+
+	next_subpart = &newparts->subpart;
+
+	while (next && *(++next)) {
+		bdevdef = next;
+		next = strchr(bdevdef, ',');
+
+		length = (!next) ? (sizeof(buf) - 1) :
+			min_t(int, next - bdevdef, sizeof(buf) - 1);
+
+		strncpy(buf, bdevdef, length);
+		buf[length] = '\0';
+
+		ret = parse_subpart(next_subpart, buf);
+		if (ret)
+			goto fail;
+
+		newparts->nr_subparts++;
+		next_subpart = &(*next_subpart)->next_subpart;
+	}
+
+	if (!newparts->subpart) {
+		pr_warn("cmdline partition has no valid partition.");
+		goto fail;
+	}
+
+	*parts = newparts;
+
+	return 0;
+fail:
+	free_subpart(newparts);
+	kfree(newparts);
+	return ret;
+}
+
+void cmdline_parts_free(struct cmdline_parts **parts)
+{
+	struct cmdline_parts *next_parts;
+
+	while (*parts) {
+		next_parts = (*parts)->next_parts;
+		free_subpart(*parts);
+		kfree(*parts);
+		*parts = next_parts;
+	}
+}
+
+int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline)
+{
+	int ret;
+	char *buf;
+	char *pbuf;
+	char *next;
+	struct cmdline_parts **next_parts;
+
+	*parts = NULL;
+
+	next = pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	next_parts = parts;
+
+	while (next && *pbuf) {
+		next = strchr(pbuf, ';');
+		if (next)
+			*next = '\0';
+
+		ret = parse_parts(next_parts, pbuf);
+		if (ret)
+			goto fail;
+
+		if (next)
+			pbuf = ++next;
+
+		next_parts = &(*next_parts)->next_parts;
+	}
+
+	if (!*parts) {
+		pr_warn("cmdline partition has no valid partition.");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	ret = 0;
+done:
+	kfree(buf);
+	return ret;
+
+fail:
+	cmdline_parts_free(parts);
+	goto done;
+}
+
+struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
+					 const char *bdev)
+{
+	while (parts && strncmp(bdev, parts->name, sizeof(parts->name)))
+		parts = parts->next_parts;
+	return parts;
+}
+
+/*
+ *  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)
+
+{
+	sector_t from = 0;
+	struct cmdline_subpart *subpart;
+
+	for (subpart = parts->subpart; subpart;
+	     subpart = subpart->next_subpart, slot++) {
+		if (subpart->from == (sector_t)(~0ULL))
+			subpart->from = from;
+		else
+			from = subpart->from;
+
+		if (from >= disk_size)
+			break;
+
+		if (subpart->size > (disk_size - from))
+			subpart->size = disk_size - from;
+
+		from += subpart->size;
+
+		if (add_part(slot, subpart, param))
+			break;
+	}
+}
diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig
index 2ebf251..87a3208 100644
--- a/block/partitions/Kconfig
+++ b/block/partitions/Kconfig
@@ -263,6 +263,7 @@ config SYSV68_PARTITION
 
 config CMDLINE_PARTITION
 	bool "Command line partition support" if PARTITION_ADVANCED
+	select CMDLINE_PARSER
 	help
 	  Say Y here if you would read the partitions table from bootargs.
 	  The format for the command line is just like mtdparts.
diff --git a/block/partitions/cmdline.c b/block/partitions/cmdline.c
index 1a1eac7..56cf4ff 100644
--- a/block/partitions/cmdline.c
+++ b/block/partitions/cmdline.c
@@ -8,219 +8,54 @@
  * by absolute address of data on the block device.
  * Users can easily change the partition.
  *
- * This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
  * The format for the command line is just like mtdparts.
  *
  * Verbose config please reference "Documentation/block/cmdline-partition.txt"
  *
  */
 
-#include <linux/buffer_head.h>
-#include <linux/module.h>
-#include <linux/ctype.h>
+#include <linux/cmdline-parser.h>
 
 #include "check.h"
 #include "cmdline.h"
 
-struct cmdline_subpart {
-	char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */
-	sector_t from;
-	sector_t size;
-	struct cmdline_subpart *next_subpart;
-};
+static char *cmdline;
+static struct cmdline_parts *bdev_parts;
 
-struct cmdline_parts {
-	char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
-	struct cmdline_subpart *subpart;
-	struct cmdline_parts *next_parts;
-};
-
-static char *cmdline_string;
-static struct cmdline_parts *cmdline_parts;
-
-static int parse_subpart(struct cmdline_subpart **subpart, char *cmdline)
-{
-	int ret = 0;
-	struct cmdline_subpart *new_subpart;
-
-	*subpart = NULL;
-
-	new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
-	if (!new_subpart)
-		return -ENOMEM;
-
-	if (*cmdline == '-') {
-		new_subpart->size = (sector_t)(~0ULL);
-		cmdline++;
-	} else {
-		new_subpart->size = (sector_t)memparse(cmdline, &cmdline);
-		if (new_subpart->size < (sector_t)PAGE_SIZE) {
-			pr_warn("cmdline partition size is invalid.");
-			ret = -EINVAL;
-			goto fail;
-		}
-	}
-
-	if (*cmdline == '@') {
-		cmdline++;
-		new_subpart->from = (sector_t)memparse(cmdline, &cmdline);
-	} else {
-		new_subpart->from = (sector_t)(~0ULL);
-	}
-
-	if (*cmdline == '(') {
-		int length;
-		char *next = strchr(++cmdline, ')');
-
-		if (!next) {
-			pr_warn("cmdline partition format is invalid.");
-			ret = -EINVAL;
-			goto fail;
-		}
-
-		length = min_t(int, next - cmdline,
-			       sizeof(new_subpart->name) - 1);
-		strncpy(new_subpart->name, cmdline, length);
-		new_subpart->name[length] = '\0';
-
-		cmdline = ++next;
-	} else
-		new_subpart->name[0] = '\0';
-
-	*subpart = new_subpart;
-	return 0;
-fail:
-	kfree(new_subpart);
-	return ret;
-}
-
-static void free_subpart(struct cmdline_parts *parts)
-{
-	struct cmdline_subpart *subpart;
-
-	while (parts->subpart) {
-		subpart = parts->subpart;
-		parts->subpart = subpart->next_subpart;
-		kfree(subpart);
-	}
-}
-
-static void free_parts(struct cmdline_parts **parts)
-{
-	struct cmdline_parts *next_parts;
-
-	while (*parts) {
-		next_parts = (*parts)->next_parts;
-		free_subpart(*parts);
-		kfree(*parts);
-		*parts = next_parts;
-	}
-}
-
-static int parse_parts(struct cmdline_parts **parts, const char *cmdline)
+static int add_part(int slot, struct cmdline_subpart *subpart, void *param)
 {
-	int ret = -EINVAL;
-	char *next;
-	int length;
-	struct cmdline_subpart **next_subpart;
-	struct cmdline_parts *newparts;
-	char buf[BDEVNAME_SIZE + 32 + 4];
+	int label_min;
+	struct partition_meta_info *info;
+	char tmp[sizeof(info->volname) + 4];
+	struct parsed_partitions *state = (struct parsed_partitions *)param;
 
-	*parts = NULL;
+	if (slot >= state->limit)
+		return 1;
 
-	newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
-	if (!newparts)
-		return -ENOMEM;
+	put_partition(state, slot, subpart->from >> 9,
+		      subpart->size >> 9);
 
-	next = strchr(cmdline, ':');
-	if (!next) {
-		pr_warn("cmdline partition has no block device.");
-		goto fail;
-	}
-
-	length = min_t(int, next - cmdline, sizeof(newparts->name) - 1);
-	strncpy(newparts->name, cmdline, length);
-	newparts->name[length] = '\0';
-
-	next_subpart = &newparts->subpart;
+	info = &state->parts[slot].info;
 
-	while (next && *(++next)) {
-		cmdline = next;
-		next = strchr(cmdline, ',');
+	label_min = min_t(int, sizeof(info->volname) - 1,
+			  sizeof(subpart->name));
+	strncpy(info->volname, subpart->name, label_min);
+	info->volname[label_min] = '\0';
 
-		length = (!next) ? (sizeof(buf) - 1) :
-			min_t(int, next - cmdline, sizeof(buf) - 1);
+	snprintf(tmp, sizeof(tmp), "(%s)", info->volname);
+	strlcat(state->pp_buf, tmp, PAGE_SIZE);
 
-		strncpy(buf, cmdline, length);
-		buf[length] = '\0';
-
-		ret = parse_subpart(next_subpart, buf);
-		if (ret)
-			goto fail;
-
-		next_subpart = &(*next_subpart)->next_subpart;
-	}
-
-	if (!newparts->subpart) {
-		pr_warn("cmdline partition has no valid partition.");
-		goto fail;
-	}
-
-	*parts = newparts;
+	state->parts[slot].has_info = true;
 
 	return 0;
-fail:
-	free_subpart(newparts);
-	kfree(newparts);
-	return ret;
 }
 
-static int parse_cmdline(struct cmdline_parts **parts, const char *cmdline)
+static int __init cmdline_parts_setup(char *s)
 {
-	int ret;
-	char *buf;
-	char *pbuf;
-	char *next;
-	struct cmdline_parts **next_parts;
-
-	*parts = NULL;
-
-	next = pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	next_parts = parts;
-
-	while (next && *pbuf) {
-		next = strchr(pbuf, ';');
-		if (next)
-			*next = '\0';
-
-		ret = parse_parts(next_parts, pbuf);
-		if (ret)
-			goto fail;
-
-		if (next)
-			pbuf = ++next;
-
-		next_parts = &(*next_parts)->next_parts;
-	}
-
-	if (!*parts) {
-		pr_warn("cmdline partition has no valid partition.");
-		ret = -EINVAL;
-		goto fail;
-	}
-
-	ret = 0;
-done:
-	kfree(buf);
-	return ret;
-
-fail:
-	free_parts(parts);
-	goto done;
+	cmdline = s;
+	return 1;
 }
+__setup("blkdevparts=", cmdline_parts_setup);
 
 /*
  * Purpose: allocate cmdline partitions.
@@ -229,98 +64,36 @@ fail:
  *  0 if this isn't our partition table
  *  1 if successful
  */
-static int parse_partitions(struct parsed_partitions *state,
-			    struct cmdline_parts *parts)
+int cmdline_partition(struct parsed_partitions *state)
 {
-	int slot;
-	sector_t from = 0;
 	sector_t disk_size;
-	char buf[BDEVNAME_SIZE];
-	struct cmdline_subpart *subpart;
+	char bdev[BDEVNAME_SIZE];
+	struct cmdline_parts *parts;
+
+	if (cmdline) {
+		if (bdev_parts)
+			cmdline_parts_free(&bdev_parts);
 
-	bdevname(state->bdev, buf);
+		if (cmdline_parts_parse(&bdev_parts, cmdline)) {
+			cmdline = NULL;
+			return -1;
+		}
+		cmdline = NULL;
+	}
 
-	while (parts && strncmp(buf, parts->name, BDEVNAME_SIZE))
-		parts = parts->next_parts;
+	if (!bdev_parts)
+		return 0;
 
+	bdevname(state->bdev, bdev);
+	parts = cmdline_parts_find(bdev_parts, bdev);
 	if (!parts)
 		return 0;
 
 	disk_size = get_capacity(state->bdev->bd_disk) << 9;
 
-	for (slot = 1, subpart = parts->subpart;
-	     subpart && slot < state->limit;
-	     subpart = subpart->next_subpart, slot++) {
-		int label_min;
-		struct partition_meta_info *info;
-		char tmp[sizeof(info->volname) + 4];
-
-		if (subpart->from == (sector_t)(~0ULL))
-			subpart->from = from;
-		else
-			from = subpart->from;
-
-		if (from >= disk_size)
-			break;
-
-		if (subpart->size > (disk_size - from))
-			subpart->size = disk_size - from;
-
-		from += subpart->size;
-
-		put_partition(state, slot, subpart->from >> 9,
-			      subpart->size >> 9);
-
-		info = &state->parts[slot].info;
-
-		label_min = min_t(int, sizeof(info->volname) - 1,
-				  sizeof(subpart->name));
-		strncpy(info->volname, subpart->name, label_min);
-		info->volname[label_min] = '\0';
-
-		snprintf(tmp, sizeof(tmp), "(%s)", info->volname);
-		strlcat(state->pp_buf, tmp, PAGE_SIZE);
-
-		state->parts[slot].has_info = true;
-	}
+	cmdline_parts_set(parts, disk_size, 1, add_part, (void *)state);
 
 	strlcat(state->pp_buf, "\n", PAGE_SIZE);
 
 	return 1;
 }
-
-static int __init cmdline_parts_setup(char *s)
-{
-	cmdline_string = s;
-	return 1;
-}
-__setup("blkdevparts=", cmdline_parts_setup);
-
-/*
- * Purpose: allocate cmdline partitions.
- * Returns:
- * -1 if unable to read the partition table
- *  0 if this isn't our partition table
- *  1 if successful
- */
-int cmdline_partition(struct parsed_partitions *state)
-{
-	if (cmdline_string) {
-		if (cmdline_parts)
-			free_parts(&cmdline_parts);
-
-		if (parse_cmdline(&cmdline_parts, cmdline_string))
-			goto fail;
-
-		cmdline_string = NULL;
-	}
-
-	if (!cmdline_parts)
-		return 0;
-
-	return parse_partitions(state, cmdline_parts);
-
-fail:
-	cmdline_string = NULL;
-	return -1;
-}
diff --git a/include/linux/cmdline-parser.h b/include/linux/cmdline-parser.h
new file mode 100644
index 0000000..98e892e
--- /dev/null
+++ b/include/linux/cmdline-parser.h
@@ -0,0 +1,43 @@
+/*
+ * Parsing command line, get the partitions information.
+ *
+ * Written by Cai Zhiyong <caizhiyong@huawei.com>
+ *
+ */
+#ifndef CMDLINEPARSEH
+#define CMDLINEPARSEH
+
+#include <linux/blkdev.h>
+
+/* partition flags */
+#define PF_RDONLY                   0x01 /* Device is read only */
+#define PF_POWERUP_LOCK             0x02 /* Always locked after reset */
+
+struct cmdline_subpart {
+	char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */
+	sector_t from;
+	sector_t size;
+	int flags;
+	struct cmdline_subpart *next_subpart;
+};
+
+struct cmdline_parts {
+	char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
+	unsigned int nr_subparts;
+	struct cmdline_subpart *subpart;
+	struct cmdline_parts *next_parts;
+};
+
+void cmdline_parts_free(struct cmdline_parts **parts);
+
+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);
+
+#endif /* CMDLINEPARSEH */
-- 
1.8.1.5

Re: [PATCH] block: add command line partition parser

[Andrew Morton]

On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:

> move the command line parser to a separate module, and change it into
> library-style code.
> 
> reference: https://lkml.org/lkml/2013/8/6/550

Well OK.  But to prove the library's usefulness and to generally clean
up the kernel, someone needs to sign up to the task of converting
drivers/mtd/cmdlinepart.c to use this code.

I've been hopefully cc'ing various MTD people but am not being
overwhelmed with waves of enthusiasm ;)

Re: [PATCH] block: add command line partition parser

[Andrew Morton]

On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:

> move the command line parser to a separate module, and change it into
> library-style code.
> 
> reference: https://lkml.org/lkml/2013/8/6/550

Well OK.  But to prove the library's usefulness and to generally clean
up the kernel, someone needs to sign up to the task of converting
drivers/mtd/cmdlinepart.c to use this code.

I've been hopefully cc'ing various MTD people but am not being
overwhelmed with waves of enthusiasm ;)

Re: [PATCH] block: add command line partition parser

[Brian Norris]

On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
>
>> move the command line parser to a separate module, and change it into
>> library-style code.
>>
>> reference: https://lkml.org/lkml/2013/8/6/550

The most recent patch is an addendum to this linked patch then?

> Well OK.  But to prove the library's usefulness and to generally clean
> up the kernel, someone needs to sign up to the task of converting
> drivers/mtd/cmdlinepart.c to use this code.
>
> I've been hopefully cc'ing various MTD people but am not being
> overwhelmed with waves of enthusiasm ;)

"I've been" implies that you have done so prior to this email. And
"people" implies more than one person. I see that you CC'd David
Woodhouse over a week ago, but he's fairly silent these days on MTD
things. It's Artem or me who handle most of the day-to-day of MTD. And
this is the first time I've seen this! (BTW, please include
linux-mtd@lists.infradead.org for anything involving MTD.)

This seems reasonable, and I'd be willing to work with this proposal.

Caizhiyong, can you submit a clear single patch (or series of
patches), CC'd to linux-mtd at least? Then we can see about supporting
it in MTD. It doesn't look too difficult, but I need to check that it
faithfully mimics the capability we currently rely on. There have been
previous discussions on changing it, but this was rejected in favor of
allowing more flexibility. Here's part of one such conversation:

http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html

So I would recommend:
(1) consider carefully the implications of your command-line format
now, rather than later
(2) if you want MTD to use it, it needs to support the features we use now

Some particular cases to consider: overlapping partitions (how do
block devices handle overlapping partitions?), out-of-order
specification, zero sized partitions, mixed syntax (some specified
with an offset, some not), multiple '-' partitions.

Anyway, if you resend, we can review.

Thanks,
Brian

Re: [PATCH] block: add command line partition parser

[Brian Norris]

On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
>
>> move the command line parser to a separate module, and change it into
>> library-style code.
>>
>> reference: https://lkml.org/lkml/2013/8/6/550

The most recent patch is an addendum to this linked patch then?

> Well OK.  But to prove the library's usefulness and to generally clean
> up the kernel, someone needs to sign up to the task of converting
> drivers/mtd/cmdlinepart.c to use this code.
>
> I've been hopefully cc'ing various MTD people but am not being
> overwhelmed with waves of enthusiasm ;)

"I've been" implies that you have done so prior to this email. And
"people" implies more than one person. I see that you CC'd David
Woodhouse over a week ago, but he's fairly silent these days on MTD
things. It's Artem or me who handle most of the day-to-day of MTD. And
this is the first time I've seen this! (BTW, please include
linux-mtd@lists.infradead.org for anything involving MTD.)

This seems reasonable, and I'd be willing to work with this proposal.

Caizhiyong, can you submit a clear single patch (or series of
patches), CC'd to linux-mtd at least? Then we can see about supporting
it in MTD. It doesn't look too difficult, but I need to check that it
faithfully mimics the capability we currently rely on. There have been
previous discussions on changing it, but this was rejected in favor of
allowing more flexibility. Here's part of one such conversation:

http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html

So I would recommend:
(1) consider carefully the implications of your command-line format
now, rather than later
(2) if you want MTD to use it, it needs to support the features we use now

Some particular cases to consider: overlapping partitions (how do
block devices handle overlapping partitions?), out-of-order
specification, zero sized partitions, mixed syntax (some specified
with an offset, some not), multiple '-' partitions.

Anyway, if you resend, we can review.

Thanks,
Brian

Re: [PATCH] block: add command line partition parser

[Andrew Morton]

On Wed, 14 Aug 2013 17:11:51 -0700 Brian Norris <computersforpeace@gmail.com> wrote:

> On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> >
> >> move the command line parser to a separate module, and change it into
> >> library-style code.
> >>
> >> reference: https://lkml.org/lkml/2013/8/6/550
> 
> The most recent patch is an addendum to this linked patch then?

The unifed latest version is appended.

> > Well OK.  But to prove the library's usefulness and to generally clean
> > up the kernel, someone needs to sign up to the task of converting
> > drivers/mtd/cmdlinepart.c to use this code.
> >
> > I've been hopefully cc'ing various MTD people but am not being
> > overwhelmed with waves of enthusiasm ;)
> 
> "I've been" implies that you have done so prior to this email. And
> "people" implies more than one person. I see that you CC'd David
> Woodhouse over a week ago, but he's fairly silent these days on MTD
> things. It's Artem or me who handle most of the day-to-day of MTD. And
> this is the first time I've seen this! (BTW, please include
> linux-mtd@lists.infradead.org for anything involving MTD.)
> 
> This seems reasonable, and I'd be willing to work with this proposal.

Thanks.

> Caizhiyong, can you submit a clear single patch (or series of
> patches), CC'd to linux-mtd at least? Then we can see about supporting
> it in MTD. It doesn't look too difficult, but I need to check that it
> faithfully mimics the capability we currently rely on. There have been
> previous discussions on changing it, but this was rejected in favor of
> allowing more flexibility. Here's part of one such conversation:
> 
> http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> 
> So I would recommend:
> (1) consider carefully the implications of your command-line format
> now, rather than later
> (2) if you want MTD to use it, it needs to support the features we use now
> 
> Some particular cases to consider: overlapping partitions (how do
> block devices handle overlapping partitions?), out-of-order
> specification, zero sized partitions, mixed syntax (some specified
> with an offset, some not), multiple '-' partitions.

This patch is still at the review-and-under-test stage so now is a fine
time to make changes.

> Anyway, if you resend, we can review.

From: Cai Zhiyong <caizhiyong@huawei.com>
Subject: block: support 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.

This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
About the partition verbose reference
"Documentation/block/cmdline-partition.txt"

[akpm@linux-foundation.org: fix printk text]
Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
Cc: Karel Zak <kzak@redhat.com>
Cc: "Wanglin (Albert)" <albert.wanglin@huawei.com>
Cc: Marius Groeger <mag@sysgo.de>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Artem Bityutskiy <dedekind@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 Documentation/block/cmdline-partition.txt |   40 +++
 block/Kconfig                             |    6 
 block/Makefile                            |    1 
 block/cmdline-parser.c                    |  249 ++++++++++++++++++++
 block/partitions/Kconfig                  |    7 
 block/partitions/Makefile                 |    1 
 block/partitions/check.c                  |    4 
 block/partitions/cmdline.c                |   99 +++++++
 block/partitions/cmdline.h                |    2 
 include/linux/cmdline-parser.h            |   43 +++
 10 files changed, 452 insertions(+)

diff -puN /dev/null Documentation/block/cmdline-partition.txt
--- /dev/null
+++ a/Documentation/block/cmdline-partition.txt
@@ -0,0 +1,40 @@
+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>]
+  <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:
+    'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'
+
+  dmesg:
+    mmcblk0: p1(data0) p2(data1) p3()
+    mmcblk0boot0: p1(boot) p2(kernel)
+
diff -puN block/partitions/Kconfig~block-support-embedded-device-command-line-partition block/partitions/Kconfig
--- a/block/partitions/Kconfig~block-support-embedded-device-command-line-partition
+++ a/block/partitions/Kconfig
@@ -260,3 +260,10 @@ config SYSV68_PARTITION
 	  partition table format used by Motorola Delta machines (using
 	  sysv68).
 	  Otherwise, say N.
+
+config CMDLINE_PARTITION
+	bool "Command line partition support" if PARTITION_ADVANCED
+	select CMDLINE_PARSER
+	help
+	  Say Y here if you would read the partitions table from bootargs.
+	  The format for the command line is just like mtdparts.
diff -puN block/partitions/Makefile~block-support-embedded-device-command-line-partition block/partitions/Makefile
--- a/block/partitions/Makefile~block-support-embedded-device-command-line-partition
+++ a/block/partitions/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_ACORN_PARTITION) += acorn.o
 obj-$(CONFIG_AMIGA_PARTITION) += amiga.o
 obj-$(CONFIG_ATARI_PARTITION) += atari.o
 obj-$(CONFIG_AIX_PARTITION) += aix.o
+obj-$(CONFIG_CMDLINE_PARTITION) += cmdline.o
 obj-$(CONFIG_MAC_PARTITION) += mac.o
 obj-$(CONFIG_LDM_PARTITION) += ldm.o
 obj-$(CONFIG_MSDOS_PARTITION) += msdos.o
diff -puN block/partitions/check.c~block-support-embedded-device-command-line-partition block/partitions/check.c
--- a/block/partitions/check.c~block-support-embedded-device-command-line-partition
+++ a/block/partitions/check.c
@@ -34,6 +34,7 @@
 #include "efi.h"
 #include "karma.h"
 #include "sysv68.h"
+#include "cmdline.h"
 
 int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/
 
@@ -65,6 +66,9 @@ static int (*check_part[])(struct parsed
 	adfspart_check_ADFS,
 #endif
 
+#ifdef CONFIG_CMDLINE_PARTITION
+	cmdline_partition,
+#endif
 #ifdef CONFIG_EFI_PARTITION
 	efi_partition,		/* this must come before msdos */
 #endif
diff -puN /dev/null block/partitions/cmdline.c
--- /dev/null
+++ a/block/partitions/cmdline.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2013 HUAWEI
+ * Author: Cai Zhiyong <caizhiyong@huawei.com>
+ *
+ * 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.
+ *
+ * Verbose config please reference "Documentation/block/cmdline-partition.txt"
+ *
+ */
+
+#include <linux/cmdline-parser.h>
+
+#include "check.h"
+#include "cmdline.h"
+
+static char *cmdline;
+static struct cmdline_parts *bdev_parts;
+
+static int add_part(int slot, struct cmdline_subpart *subpart, void *param)
+{
+	int label_min;
+	struct partition_meta_info *info;
+	char tmp[sizeof(info->volname) + 4];
+	struct parsed_partitions *state = (struct parsed_partitions *)param;
+
+	if (slot >= state->limit)
+		return 1;
+
+	put_partition(state, slot, subpart->from >> 9,
+		      subpart->size >> 9);
+
+	info = &state->parts[slot].info;
+
+	label_min = min_t(int, sizeof(info->volname) - 1,
+			  sizeof(subpart->name));
+	strncpy(info->volname, subpart->name, label_min);
+	info->volname[label_min] = '\0';
+
+	snprintf(tmp, sizeof(tmp), "(%s)", info->volname);
+	strlcat(state->pp_buf, tmp, PAGE_SIZE);
+
+	state->parts[slot].has_info = true;
+
+	return 0;
+}
+
+static int __init cmdline_parts_setup(char *s)
+{
+	cmdline = s;
+	return 1;
+}
+__setup("blkdevparts=", cmdline_parts_setup);
+
+/*
+ * Purpose: allocate cmdline partitions.
+ * Returns:
+ * -1 if unable to read the partition table
+ *  0 if this isn't our partition table
+ *  1 if successful
+ */
+int cmdline_partition(struct parsed_partitions *state)
+{
+	sector_t disk_size;
+	char bdev[BDEVNAME_SIZE];
+	struct cmdline_parts *parts;
+
+	if (cmdline) {
+		if (bdev_parts)
+			cmdline_parts_free(&bdev_parts);
+
+		if (cmdline_parts_parse(&bdev_parts, cmdline)) {
+			cmdline = NULL;
+			return -1;
+		}
+		cmdline = NULL;
+	}
+
+	if (!bdev_parts)
+		return 0;
+
+	bdevname(state->bdev, bdev);
+	parts = cmdline_parts_find(bdev_parts, bdev);
+	if (!parts)
+		return 0;
+
+	disk_size = get_capacity(state->bdev->bd_disk) << 9;
+
+	cmdline_parts_set(parts, disk_size, 1, add_part, (void *)state);
+
+	strlcat(state->pp_buf, "\n", PAGE_SIZE);
+
+	return 1;
+}
diff -puN /dev/null block/partitions/cmdline.h
--- /dev/null
+++ a/block/partitions/cmdline.h
@@ -0,0 +1,2 @@
+
+int cmdline_partition(struct parsed_partitions *state);
diff -puN block/Kconfig~block-support-embedded-device-command-line-partition block/Kconfig
--- a/block/Kconfig~block-support-embedded-device-command-line-partition
+++ a/block/Kconfig
@@ -99,6 +99,12 @@ config BLK_DEV_THROTTLING
 
 	See Documentation/cgroups/blkio-controller.txt for more information.
 
+config CMDLINE_PARSER
+	bool "Block device command line partition parser"
+	default n
+	---help---
+	Parsing command line, get the partitions information.
+
 menu "Partition Types"
 
 source "block/partitions/Kconfig"
diff -puN block/Makefile~block-support-embedded-device-command-line-partition block/Makefile
--- a/block/Makefile~block-support-embedded-device-command-line-partition
+++ a/block/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_IOSCHED_CFQ)	+= cfq-iosched
 
 obj-$(CONFIG_BLOCK_COMPAT)	+= compat_ioctl.o
 obj-$(CONFIG_BLK_DEV_INTEGRITY)	+= blk-integrity.o
+obj-$(CONFIG_CMDLINE_PARSER)	+= cmdline-parser.o
diff -puN /dev/null block/cmdline-parser.c
--- /dev/null
+++ a/block/cmdline-parser.c
@@ -0,0 +1,249 @@
+/*
+ * Parse command line, get partition information
+ *
+ * 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)
+{
+	int ret = 0;
+	struct cmdline_subpart *new_subpart;
+
+	*subpart = NULL;
+
+	new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
+	if (!new_subpart)
+		return -ENOMEM;
+
+	if (*partdef == '-') {
+		new_subpart->size = (sector_t)(~0ULL);
+		partdef++;
+	} else {
+		new_subpart->size = (sector_t)memparse(partdef, &partdef);
+		if (new_subpart->size < (sector_t)PAGE_SIZE) {
+			pr_warn("cmdline partition size is invalid.");
+			ret = -EINVAL;
+			goto fail;
+		}
+	}
+
+	if (*partdef == '@') {
+		partdef++;
+		new_subpart->from = (sector_t)memparse(partdef, &partdef);
+	} else {
+		new_subpart->from = (sector_t)(~0ULL);
+	}
+
+	if (*partdef == '(') {
+		int length;
+		char *next = strchr(++partdef, ')');
+
+		if (!next) {
+			pr_warn("cmdline partition format is invalid.");
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		length = min_t(int, next - partdef,
+			       sizeof(new_subpart->name) - 1);
+		strncpy(new_subpart->name, partdef, length);
+		new_subpart->name[length] = '\0';
+
+		partdef = ++next;
+	} else
+		new_subpart->name[0] = '\0';
+
+	new_subpart->flags = 0;
+
+	if (!strncmp(partdef, "ro", 2)) {
+		new_subpart->flags |= PF_RDONLY;
+		partdef += 2;
+	}
+
+	if (!strncmp(partdef, "lk", 2)) {
+		new_subpart->flags |= PF_POWERUP_LOCK;
+		partdef += 2;
+	}
+
+	*subpart = new_subpart;
+	return 0;
+fail:
+	kfree(new_subpart);
+	return ret;
+}
+
+static void free_subpart(struct cmdline_parts *parts)
+{
+	struct cmdline_subpart *subpart;
+
+	while (parts->subpart) {
+		subpart = parts->subpart;
+		parts->subpart = subpart->next_subpart;
+		kfree(subpart);
+	}
+}
+
+static int parse_parts(struct cmdline_parts **parts, const char *bdevdef)
+{
+	int ret = -EINVAL;
+	char *next;
+	int length;
+	struct cmdline_subpart **next_subpart;
+	struct cmdline_parts *newparts;
+	char buf[BDEVNAME_SIZE + 32 + 4];
+
+	*parts = NULL;
+
+	newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
+	if (!newparts)
+		return -ENOMEM;
+
+	next = strchr(bdevdef, ':');
+	if (!next) {
+		pr_warn("cmdline partition has no block device.");
+		goto fail;
+	}
+
+	length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1);
+	strncpy(newparts->name, bdevdef, length);
+	newparts->name[length] = '\0';
+	newparts->nr_subparts = 0;
+
+	next_subpart = &newparts->subpart;
+
+	while (next && *(++next)) {
+		bdevdef = next;
+		next = strchr(bdevdef, ',');
+
+		length = (!next) ? (sizeof(buf) - 1) :
+			min_t(int, next - bdevdef, sizeof(buf) - 1);
+
+		strncpy(buf, bdevdef, length);
+		buf[length] = '\0';
+
+		ret = parse_subpart(next_subpart, buf);
+		if (ret)
+			goto fail;
+
+		newparts->nr_subparts++;
+		next_subpart = &(*next_subpart)->next_subpart;
+	}
+
+	if (!newparts->subpart) {
+		pr_warn("cmdline partition has no valid partition.");
+		goto fail;
+	}
+
+	*parts = newparts;
+
+	return 0;
+fail:
+	free_subpart(newparts);
+	kfree(newparts);
+	return ret;
+}
+
+void cmdline_parts_free(struct cmdline_parts **parts)
+{
+	struct cmdline_parts *next_parts;
+
+	while (*parts) {
+		next_parts = (*parts)->next_parts;
+		free_subpart(*parts);
+		kfree(*parts);
+		*parts = next_parts;
+	}
+}
+
+int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline)
+{
+	int ret;
+	char *buf;
+	char *pbuf;
+	char *next;
+	struct cmdline_parts **next_parts;
+
+	*parts = NULL;
+
+	next = pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	next_parts = parts;
+
+	while (next && *pbuf) {
+		next = strchr(pbuf, ';');
+		if (next)
+			*next = '\0';
+
+		ret = parse_parts(next_parts, pbuf);
+		if (ret)
+			goto fail;
+
+		if (next)
+			pbuf = ++next;
+
+		next_parts = &(*next_parts)->next_parts;
+	}
+
+	if (!*parts) {
+		pr_warn("cmdline partition has no valid partition.");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	ret = 0;
+done:
+	kfree(buf);
+	return ret;
+
+fail:
+	cmdline_parts_free(parts);
+	goto done;
+}
+
+struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
+					 const char *bdev)
+{
+	while (parts && strncmp(bdev, parts->name, sizeof(parts->name)))
+		parts = parts->next_parts;
+	return parts;
+}
+
+/*
+ *  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)
+
+{
+	sector_t from = 0;
+	struct cmdline_subpart *subpart;
+
+	for (subpart = parts->subpart; subpart;
+	     subpart = subpart->next_subpart, slot++) {
+		if (subpart->from == (sector_t)(~0ULL))
+			subpart->from = from;
+		else
+			from = subpart->from;
+
+		if (from >= disk_size)
+			break;
+
+		if (subpart->size > (disk_size - from))
+			subpart->size = disk_size - from;
+
+		from += subpart->size;
+
+		if (add_part(slot, subpart, param))
+			break;
+	}
+}
diff -puN /dev/null include/linux/cmdline-parser.h
--- /dev/null
+++ a/include/linux/cmdline-parser.h
@@ -0,0 +1,43 @@
+/*
+ * Parsing command line, get the partitions information.
+ *
+ * Written by Cai Zhiyong <caizhiyong@huawei.com>
+ *
+ */
+#ifndef CMDLINEPARSEH
+#define CMDLINEPARSEH
+
+#include <linux/blkdev.h>
+
+/* partition flags */
+#define PF_RDONLY                   0x01 /* Device is read only */
+#define PF_POWERUP_LOCK             0x02 /* Always locked after reset */
+
+struct cmdline_subpart {
+	char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */
+	sector_t from;
+	sector_t size;
+	int flags;
+	struct cmdline_subpart *next_subpart;
+};
+
+struct cmdline_parts {
+	char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
+	unsigned int nr_subparts;
+	struct cmdline_subpart *subpart;
+	struct cmdline_parts *next_parts;
+};
+
+void cmdline_parts_free(struct cmdline_parts **parts);
+
+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);
+
+#endif /* CMDLINEPARSEH */
_

Re: [PATCH] block: add command line partition parser

[Andrew Morton]

On Wed, 14 Aug 2013 17:11:51 -0700 Brian Norris <computersforpeace@gmail.com> wrote:

> On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> >
> >> move the command line parser to a separate module, and change it into
> >> library-style code.
> >>
> >> reference: https://lkml.org/lkml/2013/8/6/550
> 
> The most recent patch is an addendum to this linked patch then?

The unifed latest version is appended.

> > Well OK.  But to prove the library's usefulness and to generally clean
> > up the kernel, someone needs to sign up to the task of converting
> > drivers/mtd/cmdlinepart.c to use this code.
> >
> > I've been hopefully cc'ing various MTD people but am not being
> > overwhelmed with waves of enthusiasm ;)
> 
> "I've been" implies that you have done so prior to this email. And
> "people" implies more than one person. I see that you CC'd David
> Woodhouse over a week ago, but he's fairly silent these days on MTD
> things. It's Artem or me who handle most of the day-to-day of MTD. And
> this is the first time I've seen this! (BTW, please include
> linux-mtd@lists.infradead.org for anything involving MTD.)
> 
> This seems reasonable, and I'd be willing to work with this proposal.

Thanks.

> Caizhiyong, can you submit a clear single patch (or series of
> patches), CC'd to linux-mtd at least? Then we can see about supporting
> it in MTD. It doesn't look too difficult, but I need to check that it
> faithfully mimics the capability we currently rely on. There have been
> previous discussions on changing it, but this was rejected in favor of
> allowing more flexibility. Here's part of one such conversation:
> 
> http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> 
> So I would recommend:
> (1) consider carefully the implications of your command-line format
> now, rather than later
> (2) if you want MTD to use it, it needs to support the features we use now
> 
> Some particular cases to consider: overlapping partitions (how do
> block devices handle overlapping partitions?), out-of-order
> specification, zero sized partitions, mixed syntax (some specified
> with an offset, some not), multiple '-' partitions.

This patch is still at the review-and-under-test stage so now is a fine
time to make changes.

> Anyway, if you resend, we can review.

From: Cai Zhiyong <caizhiyong@huawei.com>
Subject: block: support 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.

This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
About the partition verbose reference
"Documentation/block/cmdline-partition.txt"

[akpm@linux-foundation.org: fix printk text]
Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
Cc: Karel Zak <kzak@redhat.com>
Cc: "Wanglin (Albert)" <albert.wanglin@huawei.com>
Cc: Marius Groeger <mag@sysgo.de>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Artem Bityutskiy <dedekind@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 Documentation/block/cmdline-partition.txt |   40 +++
 block/Kconfig                             |    6 
 block/Makefile                            |    1 
 block/cmdline-parser.c                    |  249 ++++++++++++++++++++
 block/partitions/Kconfig                  |    7 
 block/partitions/Makefile                 |    1 
 block/partitions/check.c                  |    4 
 block/partitions/cmdline.c                |   99 +++++++
 block/partitions/cmdline.h                |    2 
 include/linux/cmdline-parser.h            |   43 +++
 10 files changed, 452 insertions(+)

diff -puN /dev/null Documentation/block/cmdline-partition.txt
--- /dev/null
+++ a/Documentation/block/cmdline-partition.txt
@@ -0,0 +1,40 @@
+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>]
+  <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:
+    'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'
+
+  dmesg:
+    mmcblk0: p1(data0) p2(data1) p3()
+    mmcblk0boot0: p1(boot) p2(kernel)
+
diff -puN block/partitions/Kconfig~block-support-embedded-device-command-line-partition block/partitions/Kconfig
--- a/block/partitions/Kconfig~block-support-embedded-device-command-line-partition
+++ a/block/partitions/Kconfig
@@ -260,3 +260,10 @@ config SYSV68_PARTITION
 	  partition table format used by Motorola Delta machines (using
 	  sysv68).
 	  Otherwise, say N.
+
+config CMDLINE_PARTITION
+	bool "Command line partition support" if PARTITION_ADVANCED
+	select CMDLINE_PARSER
+	help
+	  Say Y here if you would read the partitions table from bootargs.
+	  The format for the command line is just like mtdparts.
diff -puN block/partitions/Makefile~block-support-embedded-device-command-line-partition block/partitions/Makefile
--- a/block/partitions/Makefile~block-support-embedded-device-command-line-partition
+++ a/block/partitions/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_ACORN_PARTITION) += acorn.o
 obj-$(CONFIG_AMIGA_PARTITION) += amiga.o
 obj-$(CONFIG_ATARI_PARTITION) += atari.o
 obj-$(CONFIG_AIX_PARTITION) += aix.o
+obj-$(CONFIG_CMDLINE_PARTITION) += cmdline.o
 obj-$(CONFIG_MAC_PARTITION) += mac.o
 obj-$(CONFIG_LDM_PARTITION) += ldm.o
 obj-$(CONFIG_MSDOS_PARTITION) += msdos.o
diff -puN block/partitions/check.c~block-support-embedded-device-command-line-partition block/partitions/check.c
--- a/block/partitions/check.c~block-support-embedded-device-command-line-partition
+++ a/block/partitions/check.c
@@ -34,6 +34,7 @@
 #include "efi.h"
 #include "karma.h"
 #include "sysv68.h"
+#include "cmdline.h"
 
 int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/
 
@@ -65,6 +66,9 @@ static int (*check_part[])(struct parsed
 	adfspart_check_ADFS,
 #endif
 
+#ifdef CONFIG_CMDLINE_PARTITION
+	cmdline_partition,
+#endif
 #ifdef CONFIG_EFI_PARTITION
 	efi_partition,		/* this must come before msdos */
 #endif
diff -puN /dev/null block/partitions/cmdline.c
--- /dev/null
+++ a/block/partitions/cmdline.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2013 HUAWEI
+ * Author: Cai Zhiyong <caizhiyong@huawei.com>
+ *
+ * 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.
+ *
+ * Verbose config please reference "Documentation/block/cmdline-partition.txt"
+ *
+ */
+
+#include <linux/cmdline-parser.h>
+
+#include "check.h"
+#include "cmdline.h"
+
+static char *cmdline;
+static struct cmdline_parts *bdev_parts;
+
+static int add_part(int slot, struct cmdline_subpart *subpart, void *param)
+{
+	int label_min;
+	struct partition_meta_info *info;
+	char tmp[sizeof(info->volname) + 4];
+	struct parsed_partitions *state = (struct parsed_partitions *)param;
+
+	if (slot >= state->limit)
+		return 1;
+
+	put_partition(state, slot, subpart->from >> 9,
+		      subpart->size >> 9);
+
+	info = &state->parts[slot].info;
+
+	label_min = min_t(int, sizeof(info->volname) - 1,
+			  sizeof(subpart->name));
+	strncpy(info->volname, subpart->name, label_min);
+	info->volname[label_min] = '\0';
+
+	snprintf(tmp, sizeof(tmp), "(%s)", info->volname);
+	strlcat(state->pp_buf, tmp, PAGE_SIZE);
+
+	state->parts[slot].has_info = true;
+
+	return 0;
+}
+
+static int __init cmdline_parts_setup(char *s)
+{
+	cmdline = s;
+	return 1;
+}
+__setup("blkdevparts=", cmdline_parts_setup);
+
+/*
+ * Purpose: allocate cmdline partitions.
+ * Returns:
+ * -1 if unable to read the partition table
+ *  0 if this isn't our partition table
+ *  1 if successful
+ */
+int cmdline_partition(struct parsed_partitions *state)
+{
+	sector_t disk_size;
+	char bdev[BDEVNAME_SIZE];
+	struct cmdline_parts *parts;
+
+	if (cmdline) {
+		if (bdev_parts)
+			cmdline_parts_free(&bdev_parts);
+
+		if (cmdline_parts_parse(&bdev_parts, cmdline)) {
+			cmdline = NULL;
+			return -1;
+		}
+		cmdline = NULL;
+	}
+
+	if (!bdev_parts)
+		return 0;
+
+	bdevname(state->bdev, bdev);
+	parts = cmdline_parts_find(bdev_parts, bdev);
+	if (!parts)
+		return 0;
+
+	disk_size = get_capacity(state->bdev->bd_disk) << 9;
+
+	cmdline_parts_set(parts, disk_size, 1, add_part, (void *)state);
+
+	strlcat(state->pp_buf, "\n", PAGE_SIZE);
+
+	return 1;
+}
diff -puN /dev/null block/partitions/cmdline.h
--- /dev/null
+++ a/block/partitions/cmdline.h
@@ -0,0 +1,2 @@
+
+int cmdline_partition(struct parsed_partitions *state);
diff -puN block/Kconfig~block-support-embedded-device-command-line-partition block/Kconfig
--- a/block/Kconfig~block-support-embedded-device-command-line-partition
+++ a/block/Kconfig
@@ -99,6 +99,12 @@ config BLK_DEV_THROTTLING
 
 	See Documentation/cgroups/blkio-controller.txt for more information.
 
+config CMDLINE_PARSER
+	bool "Block device command line partition parser"
+	default n
+	---help---
+	Parsing command line, get the partitions information.
+
 menu "Partition Types"
 
 source "block/partitions/Kconfig"
diff -puN block/Makefile~block-support-embedded-device-command-line-partition block/Makefile
--- a/block/Makefile~block-support-embedded-device-command-line-partition
+++ a/block/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_IOSCHED_CFQ)	+= cfq-iosched
 
 obj-$(CONFIG_BLOCK_COMPAT)	+= compat_ioctl.o
 obj-$(CONFIG_BLK_DEV_INTEGRITY)	+= blk-integrity.o
+obj-$(CONFIG_CMDLINE_PARSER)	+= cmdline-parser.o
diff -puN /dev/null block/cmdline-parser.c
--- /dev/null
+++ a/block/cmdline-parser.c
@@ -0,0 +1,249 @@
+/*
+ * Parse command line, get partition information
+ *
+ * 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)
+{
+	int ret = 0;
+	struct cmdline_subpart *new_subpart;
+
+	*subpart = NULL;
+
+	new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
+	if (!new_subpart)
+		return -ENOMEM;
+
+	if (*partdef == '-') {
+		new_subpart->size = (sector_t)(~0ULL);
+		partdef++;
+	} else {
+		new_subpart->size = (sector_t)memparse(partdef, &partdef);
+		if (new_subpart->size < (sector_t)PAGE_SIZE) {
+			pr_warn("cmdline partition size is invalid.");
+			ret = -EINVAL;
+			goto fail;
+		}
+	}
+
+	if (*partdef == '@') {
+		partdef++;
+		new_subpart->from = (sector_t)memparse(partdef, &partdef);
+	} else {
+		new_subpart->from = (sector_t)(~0ULL);
+	}
+
+	if (*partdef == '(') {
+		int length;
+		char *next = strchr(++partdef, ')');
+
+		if (!next) {
+			pr_warn("cmdline partition format is invalid.");
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		length = min_t(int, next - partdef,
+			       sizeof(new_subpart->name) - 1);
+		strncpy(new_subpart->name, partdef, length);
+		new_subpart->name[length] = '\0';
+
+		partdef = ++next;
+	} else
+		new_subpart->name[0] = '\0';
+
+	new_subpart->flags = 0;
+
+	if (!strncmp(partdef, "ro", 2)) {
+		new_subpart->flags |= PF_RDONLY;
+		partdef += 2;
+	}
+
+	if (!strncmp(partdef, "lk", 2)) {
+		new_subpart->flags |= PF_POWERUP_LOCK;
+		partdef += 2;
+	}
+
+	*subpart = new_subpart;
+	return 0;
+fail:
+	kfree(new_subpart);
+	return ret;
+}
+
+static void free_subpart(struct cmdline_parts *parts)
+{
+	struct cmdline_subpart *subpart;
+
+	while (parts->subpart) {
+		subpart = parts->subpart;
+		parts->subpart = subpart->next_subpart;
+		kfree(subpart);
+	}
+}
+
+static int parse_parts(struct cmdline_parts **parts, const char *bdevdef)
+{
+	int ret = -EINVAL;
+	char *next;
+	int length;
+	struct cmdline_subpart **next_subpart;
+	struct cmdline_parts *newparts;
+	char buf[BDEVNAME_SIZE + 32 + 4];
+
+	*parts = NULL;
+
+	newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
+	if (!newparts)
+		return -ENOMEM;
+
+	next = strchr(bdevdef, ':');
+	if (!next) {
+		pr_warn("cmdline partition has no block device.");
+		goto fail;
+	}
+
+	length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1);
+	strncpy(newparts->name, bdevdef, length);
+	newparts->name[length] = '\0';
+	newparts->nr_subparts = 0;
+
+	next_subpart = &newparts->subpart;
+
+	while (next && *(++next)) {
+		bdevdef = next;
+		next = strchr(bdevdef, ',');
+
+		length = (!next) ? (sizeof(buf) - 1) :
+			min_t(int, next - bdevdef, sizeof(buf) - 1);
+
+		strncpy(buf, bdevdef, length);
+		buf[length] = '\0';
+
+		ret = parse_subpart(next_subpart, buf);
+		if (ret)
+			goto fail;
+
+		newparts->nr_subparts++;
+		next_subpart = &(*next_subpart)->next_subpart;
+	}
+
+	if (!newparts->subpart) {
+		pr_warn("cmdline partition has no valid partition.");
+		goto fail;
+	}
+
+	*parts = newparts;
+
+	return 0;
+fail:
+	free_subpart(newparts);
+	kfree(newparts);
+	return ret;
+}
+
+void cmdline_parts_free(struct cmdline_parts **parts)
+{
+	struct cmdline_parts *next_parts;
+
+	while (*parts) {
+		next_parts = (*parts)->next_parts;
+		free_subpart(*parts);
+		kfree(*parts);
+		*parts = next_parts;
+	}
+}
+
+int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline)
+{
+	int ret;
+	char *buf;
+	char *pbuf;
+	char *next;
+	struct cmdline_parts **next_parts;
+
+	*parts = NULL;
+
+	next = pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	next_parts = parts;
+
+	while (next && *pbuf) {
+		next = strchr(pbuf, ';');
+		if (next)
+			*next = '\0';
+
+		ret = parse_parts(next_parts, pbuf);
+		if (ret)
+			goto fail;
+
+		if (next)
+			pbuf = ++next;
+
+		next_parts = &(*next_parts)->next_parts;
+	}
+
+	if (!*parts) {
+		pr_warn("cmdline partition has no valid partition.");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	ret = 0;
+done:
+	kfree(buf);
+	return ret;
+
+fail:
+	cmdline_parts_free(parts);
+	goto done;
+}
+
+struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
+					 const char *bdev)
+{
+	while (parts && strncmp(bdev, parts->name, sizeof(parts->name)))
+		parts = parts->next_parts;
+	return parts;
+}
+
+/*
+ *  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)
+
+{
+	sector_t from = 0;
+	struct cmdline_subpart *subpart;
+
+	for (subpart = parts->subpart; subpart;
+	     subpart = subpart->next_subpart, slot++) {
+		if (subpart->from == (sector_t)(~0ULL))
+			subpart->from = from;
+		else
+			from = subpart->from;
+
+		if (from >= disk_size)
+			break;
+
+		if (subpart->size > (disk_size - from))
+			subpart->size = disk_size - from;
+
+		from += subpart->size;
+
+		if (add_part(slot, subpart, param))
+			break;
+	}
+}
diff -puN /dev/null include/linux/cmdline-parser.h
--- /dev/null
+++ a/include/linux/cmdline-parser.h
@@ -0,0 +1,43 @@
+/*
+ * Parsing command line, get the partitions information.
+ *
+ * Written by Cai Zhiyong <caizhiyong@huawei.com>
+ *
+ */
+#ifndef CMDLINEPARSEH
+#define CMDLINEPARSEH
+
+#include <linux/blkdev.h>
+
+/* partition flags */
+#define PF_RDONLY                   0x01 /* Device is read only */
+#define PF_POWERUP_LOCK             0x02 /* Always locked after reset */
+
+struct cmdline_subpart {
+	char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */
+	sector_t from;
+	sector_t size;
+	int flags;
+	struct cmdline_subpart *next_subpart;
+};
+
+struct cmdline_parts {
+	char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
+	unsigned int nr_subparts;
+	struct cmdline_subpart *subpart;
+	struct cmdline_parts *next_parts;
+};
+
+void cmdline_parts_free(struct cmdline_parts **parts);
+
+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);
+
+#endif /* CMDLINEPARSEH */
_

RE: [PATCH] block: add command line partition parser

[Caizhiyong]

> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Thursday, August 15, 2013 8:12 AM
> To: Andrew Morton
> Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> Huang Shijie
> Subject: Re: [PATCH] block: add command line partition parser
> 
> On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> >
> >> move the command line parser to a separate module, and change it into
> >> library-style code.
> >>
> >> reference: https://lkml.org/lkml/2013/8/6/550
> 
> The most recent patch is an addendum to this linked patch then?
> 
> > Well OK.  But to prove the library's usefulness and to generally clean
> > up the kernel, someone needs to sign up to the task of converting
> > drivers/mtd/cmdlinepart.c to use this code.
> >
> > I've been hopefully cc'ing various MTD people but am not being
> > overwhelmed with waves of enthusiasm ;)
> 
> "I've been" implies that you have done so prior to this email. And
> "people" implies more than one person. I see that you CC'd David
> Woodhouse over a week ago, but he's fairly silent these days on MTD
> things. It's Artem or me who handle most of the day-to-day of MTD. And
> this is the first time I've seen this! (BTW, please include
> linux-mtd@lists.infradead.org for anything involving MTD.)
> 
> This seems reasonable, and I'd be willing to work with this proposal.
> 
> Caizhiyong, can you submit a clear single patch (or series of
> patches), CC'd to linux-mtd at least? Then we can see about supporting
> it in MTD. It doesn't look too difficult, but I need to check that it
> faithfully mimics the capability we currently rely on. There have been
> previous discussions on changing it, but this was rejected in favor of
> allowing more flexibility. Here's part of one such conversation:
> 
> http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> 
> So I would recommend:
> (1) consider carefully the implications of your command-line format
> now, rather than later
> (2) if you want MTD to use it, it needs to support the features we use now

It is fully functional reference MTD, :-).

> 
> Some particular cases to consider: overlapping partitions (how do
> block devices handle overlapping partitions?), out-of-order
> specification, zero sized partitions, mixed syntax (some specified
> with an offset, some not), multiple '-' partitions.

I think the 'offset' just is used to hide some MTD space.
There are two way:
1) redefine the 'offset' as a gap between forward partition and next partition.
2) add code forbid command line partitions overlapping and out-of-order.

I recommend 1), it seems to solve those problem(overlapping and out-of-order), but it will affect habit.

> 
> Anyway, if you resend, we can review.
> 
> Thanks,
> Brian

RE: [PATCH] block: add command line partition parser

[Caizhiyong]

> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Thursday, August 15, 2013 8:12 AM
> To: Andrew Morton
> Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> Huang Shijie
> Subject: Re: [PATCH] block: add command line partition parser
> 
> On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> >
> >> move the command line parser to a separate module, and change it into
> >> library-style code.
> >>
> >> reference: https://lkml.org/lkml/2013/8/6/550
> 
> The most recent patch is an addendum to this linked patch then?
> 
> > Well OK.  But to prove the library's usefulness and to generally clean
> > up the kernel, someone needs to sign up to the task of converting
> > drivers/mtd/cmdlinepart.c to use this code.
> >
> > I've been hopefully cc'ing various MTD people but am not being
> > overwhelmed with waves of enthusiasm ;)
> 
> "I've been" implies that you have done so prior to this email. And
> "people" implies more than one person. I see that you CC'd David
> Woodhouse over a week ago, but he's fairly silent these days on MTD
> things. It's Artem or me who handle most of the day-to-day of MTD. And
> this is the first time I've seen this! (BTW, please include
> linux-mtd@lists.infradead.org for anything involving MTD.)
> 
> This seems reasonable, and I'd be willing to work with this proposal.
> 
> Caizhiyong, can you submit a clear single patch (or series of
> patches), CC'd to linux-mtd at least? Then we can see about supporting
> it in MTD. It doesn't look too difficult, but I need to check that it
> faithfully mimics the capability we currently rely on. There have been
> previous discussions on changing it, but this was rejected in favor of
> allowing more flexibility. Here's part of one such conversation:
> 
> http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> 
> So I would recommend:
> (1) consider carefully the implications of your command-line format
> now, rather than later
> (2) if you want MTD to use it, it needs to support the features we use now

It is fully functional reference MTD, :-).

> 
> Some particular cases to consider: overlapping partitions (how do
> block devices handle overlapping partitions?), out-of-order
> specification, zero sized partitions, mixed syntax (some specified
> with an offset, some not), multiple '-' partitions.

I think the 'offset' just is used to hide some MTD space.
There are two way:
1) redefine the 'offset' as a gap between forward partition and next partition.
2) add code forbid command line partitions overlapping and out-of-order.

I recommend 1), it seems to solve those problem(overlapping and out-of-order), but it will affect habit.

> 
> Anyway, if you resend, we can review.
> 
> Thanks,
> Brian

Re: [PATCH] block: add command line partition parser

[Brian Norris]

On Thu, Aug 15, 2013 at 03:38:47AM +0000, Caizhiyong wrote:
> > -----Original Message-----
> > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > Sent: Thursday, August 15, 2013 8:12 AM
> > To: Andrew Morton
> > Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> > Huang Shijie
> > Subject: Re: [PATCH] block: add command line partition parser
> > 
> > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> > <akpm@linux-foundation.org> wrote:
> > > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> > >
> > >> move the command line parser to a separate module, and change it into
> > >> library-style code.
> > >>
> > >> reference: https://lkml.org/lkml/2013/8/6/550
> > 
> > The most recent patch is an addendum to this linked patch then?
> > 
> > > Well OK.  But to prove the library's usefulness and to generally clean
> > > up the kernel, someone needs to sign up to the task of converting
> > > drivers/mtd/cmdlinepart.c to use this code.
> > >
> > > I've been hopefully cc'ing various MTD people but am not being
> > > overwhelmed with waves of enthusiasm ;)
> > 
> > "I've been" implies that you have done so prior to this email. And
> > "people" implies more than one person. I see that you CC'd David
> > Woodhouse over a week ago, but he's fairly silent these days on MTD
> > things. It's Artem or me who handle most of the day-to-day of MTD. And
> > this is the first time I've seen this! (BTW, please include
> > linux-mtd@lists.infradead.org for anything involving MTD.)
> > 
> > This seems reasonable, and I'd be willing to work with this proposal.
> > 
> > Caizhiyong, can you submit a clear single patch (or series of
> > patches), CC'd to linux-mtd at least? Then we can see about supporting
> > it in MTD. It doesn't look too difficult, but I need to check that it
> > faithfully mimics the capability we currently rely on. There have been
> > previous discussions on changing it, but this was rejected in favor of
> > allowing more flexibility. Here's part of one such conversation:
> > 
> > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> > 
> > So I would recommend:
> > (1) consider carefully the implications of your command-line format
> > now, rather than later
> > (2) if you want MTD to use it, it needs to support the features we use now
> 
> It is fully functional reference MTD, :-).

I realize that. I just want to be clear that we have to reconcile (1)
and (2). IOW, if block device requirements stray too far from MTD
requirements, then we might as well drop the idea of integration now.
But if they agree, then we can move forward.

> > Some particular cases to consider: overlapping partitions (how do
> > block devices handle overlapping partitions?), out-of-order
> > specification, zero sized partitions, mixed syntax (some specified
> > with an offset, some not), multiple '-' partitions.
> 
> I think the 'offset' just is used to hide some MTD space.

No, it specifies offset as a distance from the beginning of the flash,
so partitions can be numbered out of order. This is intentionally
utilized by some users, for example, to ensure that a particular
partition is always /dev/mtd0, even if it is not the first partition
physically.

> There are two way:
> 1) redefine the 'offset' as a gap between forward partition and next partition.
> 2) add code forbid command line partitions overlapping and out-of-order.
> 
> I recommend 1), it seems to solve those problem(overlapping and out-of-order), but it will affect habit.

The linked discussion is where MTD settled on retaining old practice. I
brought it up not so that we change it here, but so that you would
understand what you are agreeing to if you adopt a common MTD and block
device parsing infrastructure.

[Note that I am much less familiar with block device mechanics than with
MTD.] Are any of the problem areas I mentioned actually forbidden on
block devices? I know, for instance, that an MBR partition table can
specify partitions out of order. And I've googled around and seen some
posts about people (unintentionally) ending up with overlapping hard
disk partitions.

So from my primitive knowledge, it sounds like a block devices parser
could agree with the same principle put forward by Shmulik in that
second URL:

 "So far, mtdparts commandline parsing has been very lenient and liberal.
  I think we should keep this approach; give the user the flexibility,
  he'll be responsible to provide meaningful cmdline parts for his
  system."

Brian

Re: [PATCH] block: add command line partition parser

[Brian Norris]

On Thu, Aug 15, 2013 at 03:38:47AM +0000, Caizhiyong wrote:
> > -----Original Message-----
> > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > Sent: Thursday, August 15, 2013 8:12 AM
> > To: Andrew Morton
> > Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> > Huang Shijie
> > Subject: Re: [PATCH] block: add command line partition parser
> > 
> > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> > <akpm@linux-foundation.org> wrote:
> > > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> > >
> > >> move the command line parser to a separate module, and change it into
> > >> library-style code.
> > >>
> > >> reference: https://lkml.org/lkml/2013/8/6/550
> > 
> > The most recent patch is an addendum to this linked patch then?
> > 
> > > Well OK.  But to prove the library's usefulness and to generally clean
> > > up the kernel, someone needs to sign up to the task of converting
> > > drivers/mtd/cmdlinepart.c to use this code.
> > >
> > > I've been hopefully cc'ing various MTD people but am not being
> > > overwhelmed with waves of enthusiasm ;)
> > 
> > "I've been" implies that you have done so prior to this email. And
> > "people" implies more than one person. I see that you CC'd David
> > Woodhouse over a week ago, but he's fairly silent these days on MTD
> > things. It's Artem or me who handle most of the day-to-day of MTD. And
> > this is the first time I've seen this! (BTW, please include
> > linux-mtd@lists.infradead.org for anything involving MTD.)
> > 
> > This seems reasonable, and I'd be willing to work with this proposal.
> > 
> > Caizhiyong, can you submit a clear single patch (or series of
> > patches), CC'd to linux-mtd at least? Then we can see about supporting
> > it in MTD. It doesn't look too difficult, but I need to check that it
> > faithfully mimics the capability we currently rely on. There have been
> > previous discussions on changing it, but this was rejected in favor of
> > allowing more flexibility. Here's part of one such conversation:
> > 
> > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> > 
> > So I would recommend:
> > (1) consider carefully the implications of your command-line format
> > now, rather than later
> > (2) if you want MTD to use it, it needs to support the features we use now
> 
> It is fully functional reference MTD, :-).

I realize that. I just want to be clear that we have to reconcile (1)
and (2). IOW, if block device requirements stray too far from MTD
requirements, then we might as well drop the idea of integration now.
But if they agree, then we can move forward.

> > Some particular cases to consider: overlapping partitions (how do
> > block devices handle overlapping partitions?), out-of-order
> > specification, zero sized partitions, mixed syntax (some specified
> > with an offset, some not), multiple '-' partitions.
> 
> I think the 'offset' just is used to hide some MTD space.

No, it specifies offset as a distance from the beginning of the flash,
so partitions can be numbered out of order. This is intentionally
utilized by some users, for example, to ensure that a particular
partition is always /dev/mtd0, even if it is not the first partition
physically.

> There are two way:
> 1) redefine the 'offset' as a gap between forward partition and next partition.
> 2) add code forbid command line partitions overlapping and out-of-order.
> 
> I recommend 1), it seems to solve those problem(overlapping and out-of-order), but it will affect habit.

The linked discussion is where MTD settled on retaining old practice. I
brought it up not so that we change it here, but so that you would
understand what you are agreeing to if you adopt a common MTD and block
device parsing infrastructure.

[Note that I am much less familiar with block device mechanics than with
MTD.] Are any of the problem areas I mentioned actually forbidden on
block devices? I know, for instance, that an MBR partition table can
specify partitions out of order. And I've googled around and seen some
posts about people (unintentionally) ending up with overlapping hard
disk partitions.

So from my primitive knowledge, it sounds like a block devices parser
could agree with the same principle put forward by Shmulik in that
second URL:

 "So far, mtdparts commandline parsing has been very lenient and liberal.
  I think we should keep this approach; give the user the flexibility,
  he'll be responsible to provide meaningful cmdline parts for his
  system."

Brian

RE: [PATCH] block: add command line partition parser

[Caizhiyong]

> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Thursday, August 15, 2013 1:00 PM
> To: Caizhiyong
> Cc: Andrew Morton; Karel Zak; linux-mtd@lists.infradead.org;
> linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> Huang Shijie
> Subject: Re: [PATCH] block: add command line partition parser
> 
> On Thu, Aug 15, 2013 at 03:38:47AM +0000, Caizhiyong wrote:
> > > -----Original Message-----
> > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > > Sent: Thursday, August 15, 2013 8:12 AM
> > > To: Andrew Morton
> > > Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik
> Ladkani;
> > > Huang Shijie
> > > Subject: Re: [PATCH] block: add command line partition parser
> > >
> > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> > > <akpm@linux-foundation.org> wrote:
> > > > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> > > >
> > > >> move the command line parser to a separate module, and change it into
> > > >> library-style code.
> > > >>
> > > >> reference: https://lkml.org/lkml/2013/8/6/550
> > >
> > > The most recent patch is an addendum to this linked patch then?
> > >
> > > > Well OK.  But to prove the library's usefulness and to generally clean
> > > > up the kernel, someone needs to sign up to the task of converting
> > > > drivers/mtd/cmdlinepart.c to use this code.
> > > >
> > > > I've been hopefully cc'ing various MTD people but am not being
> > > > overwhelmed with waves of enthusiasm ;)
> > >
> > > "I've been" implies that you have done so prior to this email. And
> > > "people" implies more than one person. I see that you CC'd David
> > > Woodhouse over a week ago, but he's fairly silent these days on MTD
> > > things. It's Artem or me who handle most of the day-to-day of MTD. And
> > > this is the first time I've seen this! (BTW, please include
> > > linux-mtd@lists.infradead.org for anything involving MTD.)
> > >
> > > This seems reasonable, and I'd be willing to work with this proposal.
> > >
> > > Caizhiyong, can you submit a clear single patch (or series of
> > > patches), CC'd to linux-mtd at least? Then we can see about supporting
> > > it in MTD. It doesn't look too difficult, but I need to check that it
> > > faithfully mimics the capability we currently rely on. There have been
> > > previous discussions on changing it, but this was rejected in favor of
> > > allowing more flexibility. Here's part of one such conversation:
> > >
> > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> > > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> > > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> > >
> > > So I would recommend:
> > > (1) consider carefully the implications of your command-line format
> > > now, rather than later
> > > (2) if you want MTD to use it, it needs to support the features we use now
> >
> > It is fully functional reference MTD, :-).
> 
> I realize that. I just want to be clear that we have to reconcile (1)
> and (2). IOW, if block device requirements stray too far from MTD
> requirements, then we might as well drop the idea of integration now.
> But if they agree, then we can move forward.
> 
> > > Some particular cases to consider: overlapping partitions (how do
> > > block devices handle overlapping partitions?), out-of-order
> > > specification, zero sized partitions, mixed syntax (some specified
> > > with an offset, some not), multiple '-' partitions.
> >
> > I think the 'offset' just is used to hide some MTD space.
> 
> No, it specifies offset as a distance from the beginning of the flash,
> so partitions can be numbered out of order. This is intentionally
> utilized by some users, for example, to ensure that a particular
> partition is always /dev/mtd0, even if it is not the first partition
> physically.
> 
> > There are two way:
> > 1) redefine the 'offset' as a gap between forward partition and next partition.
> > 2) add code forbid command line partitions overlapping and out-of-order.
> >
> > I recommend 1), it seems to solve those problem(overlapping and out-of-order),
> but it will affect habit.
> 
> The linked discussion is where MTD settled on retaining old practice. I
> brought it up not so that we change it here, but so that you would
> understand what you are agreeing to if you adopt a common MTD and block
> device parsing infrastructure.
> 
> [Note that I am much less familiar with block device mechanics than with
> MTD.] Are any of the problem areas I mentioned actually forbidden on
> block devices? I know, for instance, that an MBR partition table can
> specify partitions out of order. And I've googled around and seen some
> posts about people (unintentionally) ending up with overlapping hard
> disk partitions.
> 
> So from my primitive knowledge, it sounds like a block devices parser
> could agree with the same principle put forward by Shmulik in that
> second URL:
> 
>  "So far, mtdparts commandline parsing has been very lenient and liberal.
>   I think we should keep this approach; give the user the flexibility,
>   he'll be responsible to provide meaningful cmdline parts for his
>   system."
> 
> Brian

I want to use the MTD command line partition method on block devices (eMMC).
It is very suitable for embedded systems. I think, in embedded system partition method,
if somebody need some feature on MTD device, he may be also need it on block device.
so I fully functional reference MTD command line partition.

I tested the out-of-order and overlapping on my system, used command line partition, It is work ok.
The block device code is not make any restrictions on partition out-of-order and overlapping.

I hope extend the flexibility to block device.

RE: [PATCH] block: add command line partition parser

[Caizhiyong]

> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Thursday, August 15, 2013 1:00 PM
> To: Caizhiyong
> Cc: Andrew Morton; Karel Zak; linux-mtd@lists.infradead.org;
> linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> Huang Shijie
> Subject: Re: [PATCH] block: add command line partition parser
> 
> On Thu, Aug 15, 2013 at 03:38:47AM +0000, Caizhiyong wrote:
> > > -----Original Message-----
> > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > > Sent: Thursday, August 15, 2013 8:12 AM
> > > To: Andrew Morton
> > > Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik
> Ladkani;
> > > Huang Shijie
> > > Subject: Re: [PATCH] block: add command line partition parser
> > >
> > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> > > <akpm@linux-foundation.org> wrote:
> > > > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> > > >
> > > >> move the command line parser to a separate module, and change it into
> > > >> library-style code.
> > > >>
> > > >> reference: https://lkml.org/lkml/2013/8/6/550
> > >
> > > The most recent patch is an addendum to this linked patch then?
> > >
> > > > Well OK.  But to prove the library's usefulness and to generally clean
> > > > up the kernel, someone needs to sign up to the task of converting
> > > > drivers/mtd/cmdlinepart.c to use this code.
> > > >
> > > > I've been hopefully cc'ing various MTD people but am not being
> > > > overwhelmed with waves of enthusiasm ;)
> > >
> > > "I've been" implies that you have done so prior to this email. And
> > > "people" implies more than one person. I see that you CC'd David
> > > Woodhouse over a week ago, but he's fairly silent these days on MTD
> > > things. It's Artem or me who handle most of the day-to-day of MTD. And
> > > this is the first time I've seen this! (BTW, please include
> > > linux-mtd@lists.infradead.org for anything involving MTD.)
> > >
> > > This seems reasonable, and I'd be willing to work with this proposal.
> > >
> > > Caizhiyong, can you submit a clear single patch (or series of
> > > patches), CC'd to linux-mtd at least? Then we can see about supporting
> > > it in MTD. It doesn't look too difficult, but I need to check that it
> > > faithfully mimics the capability we currently rely on. There have been
> > > previous discussions on changing it, but this was rejected in favor of
> > > allowing more flexibility. Here's part of one such conversation:
> > >
> > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> > > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> > > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> > >
> > > So I would recommend:
> > > (1) consider carefully the implications of your command-line format
> > > now, rather than later
> > > (2) if you want MTD to use it, it needs to support the features we use now
> >
> > It is fully functional reference MTD, :-).
> 
> I realize that. I just want to be clear that we have to reconcile (1)
> and (2). IOW, if block device requirements stray too far from MTD
> requirements, then we might as well drop the idea of integration now.
> But if they agree, then we can move forward.
> 
> > > Some particular cases to consider: overlapping partitions (how do
> > > block devices handle overlapping partitions?), out-of-order
> > > specification, zero sized partitions, mixed syntax (some specified
> > > with an offset, some not), multiple '-' partitions.
> >
> > I think the 'offset' just is used to hide some MTD space.
> 
> No, it specifies offset as a distance from the beginning of the flash,
> so partitions can be numbered out of order. This is intentionally
> utilized by some users, for example, to ensure that a particular
> partition is always /dev/mtd0, even if it is not the first partition
> physically.
> 
> > There are two way:
> > 1) redefine the 'offset' as a gap between forward partition and next partition.
> > 2) add code forbid command line partitions overlapping and out-of-order.
> >
> > I recommend 1), it seems to solve those problem(overlapping and out-of-order),
> but it will affect habit.
> 
> The linked discussion is where MTD settled on retaining old practice. I
> brought it up not so that we change it here, but so that you would
> understand what you are agreeing to if you adopt a common MTD and block
> device parsing infrastructure.
> 
> [Note that I am much less familiar with block device mechanics than with
> MTD.] Are any of the problem areas I mentioned actually forbidden on
> block devices? I know, for instance, that an MBR partition table can
> specify partitions out of order. And I've googled around and seen some
> posts about people (unintentionally) ending up with overlapping hard
> disk partitions.
> 
> So from my primitive knowledge, it sounds like a block devices parser
> could agree with the same principle put forward by Shmulik in that
> second URL:
> 
>  "So far, mtdparts commandline parsing has been very lenient and liberal.
>   I think we should keep this approach; give the user the flexibility,
>   he'll be responsible to provide meaningful cmdline parts for his
>   system."
> 
> Brian

I want to use the MTD command line partition method on block devices (eMMC).
It is very suitable for embedded systems. I think, in embedded system partition method,
if somebody need some feature on MTD device, he may be also need it on block device.
so I fully functional reference MTD command line partition.

I tested the out-of-order and overlapping on my system, used command line partition, It is work ok.
The block device code is not make any restrictions on partition out-of-order and overlapping.

I hope extend the flexibility to block device.

Re: [PATCH] block: add command line partition parser

[Brian Norris]

On Thu, Aug 15, 2013 at 06:16:04AM +0000, Caizhiyong wrote:
> > -----Original Message-----
> > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > Sent: Thursday, August 15, 2013 1:00 PM
> > To: Caizhiyong
> > Cc: Andrew Morton; Karel Zak; linux-mtd@lists.infradead.org;
> > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> > Huang Shijie
> > Subject: Re: [PATCH] block: add command line partition parser
> > 
> > On Thu, Aug 15, 2013 at 03:38:47AM +0000, Caizhiyong wrote:
> > > > -----Original Message-----
> > > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > > > Sent: Thursday, August 15, 2013 8:12 AM
> > > > To: Andrew Morton
> > > > Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik
> > Ladkani;
> > > > Huang Shijie
> > > > Subject: Re: [PATCH] block: add command line partition parser
> > > >
> > > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> > > > <akpm@linux-foundation.org> wrote:
> > > > > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> > > > >
> > > > >> move the command line parser to a separate module, and change it into
> > > > >> library-style code.
> > > > >>
> > > > >> reference: https://lkml.org/lkml/2013/8/6/550
> > > >
> > > > The most recent patch is an addendum to this linked patch then?
> > > >
> > > > > Well OK.  But to prove the library's usefulness and to generally clean
> > > > > up the kernel, someone needs to sign up to the task of converting
> > > > > drivers/mtd/cmdlinepart.c to use this code.
> > > > >
> > > > > I've been hopefully cc'ing various MTD people but am not being
> > > > > overwhelmed with waves of enthusiasm ;)
> > > >
> > > > "I've been" implies that you have done so prior to this email. And
> > > > "people" implies more than one person. I see that you CC'd David
> > > > Woodhouse over a week ago, but he's fairly silent these days on MTD
> > > > things. It's Artem or me who handle most of the day-to-day of MTD. And
> > > > this is the first time I've seen this! (BTW, please include
> > > > linux-mtd@lists.infradead.org for anything involving MTD.)
> > > >
> > > > This seems reasonable, and I'd be willing to work with this proposal.
> > > >
> > > > Caizhiyong, can you submit a clear single patch (or series of
> > > > patches), CC'd to linux-mtd at least? Then we can see about supporting
> > > > it in MTD. It doesn't look too difficult, but I need to check that it
> > > > faithfully mimics the capability we currently rely on. There have been
> > > > previous discussions on changing it, but this was rejected in favor of
> > > > allowing more flexibility. Here's part of one such conversation:
> > > >
> > > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> > > > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> > > > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> > > >
> > > > So I would recommend:
> > > > (1) consider carefully the implications of your command-line format
> > > > now, rather than later
> > > > (2) if you want MTD to use it, it needs to support the features we use now
> > >
> > > It is fully functional reference MTD, :-).
> > 
> > I realize that. I just want to be clear that we have to reconcile (1)
> > and (2). IOW, if block device requirements stray too far from MTD
> > requirements, then we might as well drop the idea of integration now.
> > But if they agree, then we can move forward.
> > 
> > > > Some particular cases to consider: overlapping partitions (how do
> > > > block devices handle overlapping partitions?), out-of-order
> > > > specification, zero sized partitions, mixed syntax (some specified
> > > > with an offset, some not), multiple '-' partitions.
> > >
> > > I think the 'offset' just is used to hide some MTD space.
> > 
> > No, it specifies offset as a distance from the beginning of the flash,
> > so partitions can be numbered out of order. This is intentionally
> > utilized by some users, for example, to ensure that a particular
> > partition is always /dev/mtd0, even if it is not the first partition
> > physically.
> > 
> > > There are two way:
> > > 1) redefine the 'offset' as a gap between forward partition and next partition.
> > > 2) add code forbid command line partitions overlapping and out-of-order.
> > >
> > > I recommend 1), it seems to solve those problem(overlapping and out-of-order),
> > but it will affect habit.
> > 
> > The linked discussion is where MTD settled on retaining old practice. I
> > brought it up not so that we change it here, but so that you would
> > understand what you are agreeing to if you adopt a common MTD and block
> > device parsing infrastructure.
> > 
> > [Note that I am much less familiar with block device mechanics than with
> > MTD.] Are any of the problem areas I mentioned actually forbidden on
> > block devices? I know, for instance, that an MBR partition table can
> > specify partitions out of order. And I've googled around and seen some
> > posts about people (unintentionally) ending up with overlapping hard
> > disk partitions.
> > 
> > So from my primitive knowledge, it sounds like a block devices parser
> > could agree with the same principle put forward by Shmulik in that
> > second URL:
> > 
> >  "So far, mtdparts commandline parsing has been very lenient and liberal.
> >   I think we should keep this approach; give the user the flexibility,
> >   he'll be responsible to provide meaningful cmdline parts for his
> >   system."
> > 
> > Brian
> 
> I want to use the MTD command line partition method on block devices (eMMC).
> It is very suitable for embedded systems. I think, in embedded system partition method,
> if somebody need some feature on MTD device, he may be also need it on block device.
> so I fully functional reference MTD command line partition.

I agree.

I'm curious: have you seen any need for a similar arrangement via
device-tree? See, for example, drivers/mtd/ofpart.c.

> I tested the out-of-order and overlapping on my system, used command line partition, It is work ok.
> The block device code is not make any restrictions on partition out-of-order and overlapping.

OK, good. Thanks for checking.

> I hope extend the flexibility to block device.

Sure. I'll try to review the full patch soon and test out integrating
it with MTD.

Thanks,
Brian

Re: [PATCH] block: add command line partition parser

[Brian Norris]

On Thu, Aug 15, 2013 at 06:16:04AM +0000, Caizhiyong wrote:
> > -----Original Message-----
> > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > Sent: Thursday, August 15, 2013 1:00 PM
> > To: Caizhiyong
> > Cc: Andrew Morton; Karel Zak; linux-mtd@lists.infradead.org;
> > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> > Huang Shijie
> > Subject: Re: [PATCH] block: add command line partition parser
> > 
> > On Thu, Aug 15, 2013 at 03:38:47AM +0000, Caizhiyong wrote:
> > > > -----Original Message-----
> > > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > > > Sent: Thursday, August 15, 2013 8:12 AM
> > > > To: Andrew Morton
> > > > Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik
> > Ladkani;
> > > > Huang Shijie
> > > > Subject: Re: [PATCH] block: add command line partition parser
> > > >
> > > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> > > > <akpm@linux-foundation.org> wrote:
> > > > > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:
> > > > >
> > > > >> move the command line parser to a separate module, and change it into
> > > > >> library-style code.
> > > > >>
> > > > >> reference: https://lkml.org/lkml/2013/8/6/550
> > > >
> > > > The most recent patch is an addendum to this linked patch then?
> > > >
> > > > > Well OK.  But to prove the library's usefulness and to generally clean
> > > > > up the kernel, someone needs to sign up to the task of converting
> > > > > drivers/mtd/cmdlinepart.c to use this code.
> > > > >
> > > > > I've been hopefully cc'ing various MTD people but am not being
> > > > > overwhelmed with waves of enthusiasm ;)
> > > >
> > > > "I've been" implies that you have done so prior to this email. And
> > > > "people" implies more than one person. I see that you CC'd David
> > > > Woodhouse over a week ago, but he's fairly silent these days on MTD
> > > > things. It's Artem or me who handle most of the day-to-day of MTD. And
> > > > this is the first time I've seen this! (BTW, please include
> > > > linux-mtd@lists.infradead.org for anything involving MTD.)
> > > >
> > > > This seems reasonable, and I'd be willing to work with this proposal.
> > > >
> > > > Caizhiyong, can you submit a clear single patch (or series of
> > > > patches), CC'd to linux-mtd at least? Then we can see about supporting
> > > > it in MTD. It doesn't look too difficult, but I need to check that it
> > > > faithfully mimics the capability we currently rely on. There have been
> > > > previous discussions on changing it, but this was rejected in favor of
> > > > allowing more flexibility. Here's part of one such conversation:
> > > >
> > > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> > > > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> > > > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> > > >
> > > > So I would recommend:
> > > > (1) consider carefully the implications of your command-line format
> > > > now, rather than later
> > > > (2) if you want MTD to use it, it needs to support the features we use now
> > >
> > > It is fully functional reference MTD, :-).
> > 
> > I realize that. I just want to be clear that we have to reconcile (1)
> > and (2). IOW, if block device requirements stray too far from MTD
> > requirements, then we might as well drop the idea of integration now.
> > But if they agree, then we can move forward.
> > 
> > > > Some particular cases to consider: overlapping partitions (how do
> > > > block devices handle overlapping partitions?), out-of-order
> > > > specification, zero sized partitions, mixed syntax (some specified
> > > > with an offset, some not), multiple '-' partitions.
> > >
> > > I think the 'offset' just is used to hide some MTD space.
> > 
> > No, it specifies offset as a distance from the beginning of the flash,
> > so partitions can be numbered out of order. This is intentionally
> > utilized by some users, for example, to ensure that a particular
> > partition is always /dev/mtd0, even if it is not the first partition
> > physically.
> > 
> > > There are two way:
> > > 1) redefine the 'offset' as a gap between forward partition and next partition.
> > > 2) add code forbid command line partitions overlapping and out-of-order.
> > >
> > > I recommend 1), it seems to solve those problem(overlapping and out-of-order),
> > but it will affect habit.
> > 
> > The linked discussion is where MTD settled on retaining old practice. I
> > brought it up not so that we change it here, but so that you would
> > understand what you are agreeing to if you adopt a common MTD and block
> > device parsing infrastructure.
> > 
> > [Note that I am much less familiar with block device mechanics than with
> > MTD.] Are any of the problem areas I mentioned actually forbidden on
> > block devices? I know, for instance, that an MBR partition table can
> > specify partitions out of order. And I've googled around and seen some
> > posts about people (unintentionally) ending up with overlapping hard
> > disk partitions.
> > 
> > So from my primitive knowledge, it sounds like a block devices parser
> > could agree with the same principle put forward by Shmulik in that
> > second URL:
> > 
> >  "So far, mtdparts commandline parsing has been very lenient and liberal.
> >   I think we should keep this approach; give the user the flexibility,
> >   he'll be responsible to provide meaningful cmdline parts for his
> >   system."
> > 
> > Brian
> 
> I want to use the MTD command line partition method on block devices (eMMC).
> It is very suitable for embedded systems. I think, in embedded system partition method,
> if somebody need some feature on MTD device, he may be also need it on block device.
> so I fully functional reference MTD command line partition.

I agree.

I'm curious: have you seen any need for a similar arrangement via
device-tree? See, for example, drivers/mtd/ofpart.c.

> I tested the out-of-order and overlapping on my system, used command line partition, It is work ok.
> The block device code is not make any restrictions on partition out-of-order and overlapping.

OK, good. Thanks for checking.

> I hope extend the flexibility to block device.

Sure. I'll try to review the full patch soon and test out integrating
it with MTD.

Thanks,
Brian

RE: [PATCH] block: add command line partition parser

[Caizhiyong]

> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Thursday, August 15, 2013 3:10 PM
> To: Caizhiyong
> Cc: Andrew Morton; Karel Zak; linux-mtd@lists.infradead.org;
> linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> Huang Shijie
> Subject: Re: [PATCH] block: add command line partition parser
> 
> On Thu, Aug 15, 2013 at 06:16:04AM +0000, Caizhiyong wrote:
> > > -----Original Message-----
> > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > > Sent: Thursday, August 15, 2013 1:00 PM
> > > To: Caizhiyong
> > > Cc: Andrew Morton; Karel Zak; linux-mtd@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik
> Ladkani;
> > > Huang Shijie
> > > Subject: Re: [PATCH] block: add command line partition parser
> > >
> > > On Thu, Aug 15, 2013 at 03:38:47AM +0000, Caizhiyong wrote:
> > > > > -----Original Message-----
> > > > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > > > > Sent: Thursday, August 15, 2013 8:12 AM
> > > > > To: Andrew Morton
> > > > > Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> > > > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik
> > > Ladkani;
> > > > > Huang Shijie
> > > > > Subject: Re: [PATCH] block: add command line partition parser
> > > > >
> > > > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> > > > > <akpm@linux-foundation.org> wrote:
> > > > > > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com>
> wrote:
> > > > > >
> > > > > >> move the command line parser to a separate module, and change it into
> > > > > >> library-style code.
> > > > > >>
> > > > > >> reference: https://lkml.org/lkml/2013/8/6/550
> > > > >
> > > > > The most recent patch is an addendum to this linked patch then?
> > > > >
> > > > > > Well OK.  But to prove the library's usefulness and to generally clean
> > > > > > up the kernel, someone needs to sign up to the task of converting
> > > > > > drivers/mtd/cmdlinepart.c to use this code.
> > > > > >
> > > > > > I've been hopefully cc'ing various MTD people but am not being
> > > > > > overwhelmed with waves of enthusiasm ;)
> > > > >
> > > > > "I've been" implies that you have done so prior to this email. And
> > > > > "people" implies more than one person. I see that you CC'd David
> > > > > Woodhouse over a week ago, but he's fairly silent these days on MTD
> > > > > things. It's Artem or me who handle most of the day-to-day of MTD. And
> > > > > this is the first time I've seen this! (BTW, please include
> > > > > linux-mtd@lists.infradead.org for anything involving MTD.)
> > > > >
> > > > > This seems reasonable, and I'd be willing to work with this proposal.
> > > > >
> > > > > Caizhiyong, can you submit a clear single patch (or series of
> > > > > patches), CC'd to linux-mtd at least? Then we can see about supporting
> > > > > it in MTD. It doesn't look too difficult, but I need to check that it
> > > > > faithfully mimics the capability we currently rely on. There have been
> > > > > previous discussions on changing it, but this was rejected in favor of
> > > > > allowing more flexibility. Here's part of one such conversation:
> > > > >
> > > > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> > > > >
> http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> > > > > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> > > > >
> > > > > So I would recommend:
> > > > > (1) consider carefully the implications of your command-line format
> > > > > now, rather than later
> > > > > (2) if you want MTD to use it, it needs to support the features we use now
> > > >
> > > > It is fully functional reference MTD, :-).
> > >
> > > I realize that. I just want to be clear that we have to reconcile (1)
> > > and (2). IOW, if block device requirements stray too far from MTD
> > > requirements, then we might as well drop the idea of integration now.
> > > But if they agree, then we can move forward.
> > >
> > > > > Some particular cases to consider: overlapping partitions (how do
> > > > > block devices handle overlapping partitions?), out-of-order
> > > > > specification, zero sized partitions, mixed syntax (some specified
> > > > > with an offset, some not), multiple '-' partitions.
> > > >
> > > > I think the 'offset' just is used to hide some MTD space.
> > >
> > > No, it specifies offset as a distance from the beginning of the flash,
> > > so partitions can be numbered out of order. This is intentionally
> > > utilized by some users, for example, to ensure that a particular
> > > partition is always /dev/mtd0, even if it is not the first partition
> > > physically.
> > >
> > > > There are two way:
> > > > 1) redefine the 'offset' as a gap between forward partition and next partition.
> > > > 2) add code forbid command line partitions overlapping and out-of-order.
> > > >
> > > > I recommend 1), it seems to solve those problem(overlapping and out-of-order),
> > > but it will affect habit.
> > >
> > > The linked discussion is where MTD settled on retaining old practice. I
> > > brought it up not so that we change it here, but so that you would
> > > understand what you are agreeing to if you adopt a common MTD and block
> > > device parsing infrastructure.
> > >
> > > [Note that I am much less familiar with block device mechanics than with
> > > MTD.] Are any of the problem areas I mentioned actually forbidden on
> > > block devices? I know, for instance, that an MBR partition table can
> > > specify partitions out of order. And I've googled around and seen some
> > > posts about people (unintentionally) ending up with overlapping hard
> > > disk partitions.
> > >
> > > So from my primitive knowledge, it sounds like a block devices parser
> > > could agree with the same principle put forward by Shmulik in that
> > > second URL:
> > >
> > >  "So far, mtdparts commandline parsing has been very lenient and liberal.
> > >   I think we should keep this approach; give the user the flexibility,
> > >   he'll be responsible to provide meaningful cmdline parts for his
> > >   system."
> > >
> > > Brian
> >
> > I want to use the MTD command line partition method on block devices (eMMC).
> > It is very suitable for embedded systems. I think, in embedded system partition
> method,
> > if somebody need some feature on MTD device, he may be also need it on block device.
> > so I fully functional reference MTD command line partition.
> 
> I agree.
> 
> I'm curious: have you seen any need for a similar arrangement via
> device-tree? See, for example, drivers/mtd/ofpart.c.

So far, I have no seen. We mainly use ARM. For ARM, device-tree is a new thing.

> 
> > I tested the out-of-order and overlapping on my system, used command line partition,
> It is work ok.
> > The block device code is not make any restrictions on partition out-of-order and
> overlapping.
> 
> OK, good. Thanks for checking.
> 
> > I hope extend the flexibility to block device.
> 
> Sure. I'll try to review the full patch soon and test out integrating
> it with MTD.

If there is no problem, I will send my next patch, mtd cmdline parts use cmdline-parser lib.

> 
> Thanks,
> Brian

RE: [PATCH] block: add command line partition parser

[Caizhiyong]

> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Thursday, August 15, 2013 3:10 PM
> To: Caizhiyong
> Cc: Andrew Morton; Karel Zak; linux-mtd@lists.infradead.org;
> linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik Ladkani;
> Huang Shijie
> Subject: Re: [PATCH] block: add command line partition parser
> 
> On Thu, Aug 15, 2013 at 06:16:04AM +0000, Caizhiyong wrote:
> > > -----Original Message-----
> > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > > Sent: Thursday, August 15, 2013 1:00 PM
> > > To: Caizhiyong
> > > Cc: Andrew Morton; Karel Zak; linux-mtd@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik
> Ladkani;
> > > Huang Shijie
> > > Subject: Re: [PATCH] block: add command line partition parser
> > >
> > > On Thu, Aug 15, 2013 at 03:38:47AM +0000, Caizhiyong wrote:
> > > > > -----Original Message-----
> > > > > From: Brian Norris [mailto:computersforpeace@gmail.com]
> > > > > Sent: Thursday, August 15, 2013 8:12 AM
> > > > > To: Andrew Morton
> > > > > Cc: Caizhiyong; Karel Zak; linux-mtd@lists.infradead.org;
> > > > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik
> > > Ladkani;
> > > > > Huang Shijie
> > > > > Subject: Re: [PATCH] block: add command line partition parser
> > > > >
> > > > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton
> > > > > <akpm@linux-foundation.org> wrote:
> > > > > > On Tue, 13 Aug 2013 06:02:17 +0000 Caizhiyong <caizhiyong@huawei.com>
> wrote:
> > > > > >
> > > > > >> move the command line parser to a separate module, and change it into
> > > > > >> library-style code.
> > > > > >>
> > > > > >> reference: https://lkml.org/lkml/2013/8/6/550
> > > > >
> > > > > The most recent patch is an addendum to this linked patch then?
> > > > >
> > > > > > Well OK.  But to prove the library's usefulness and to generally clean
> > > > > > up the kernel, someone needs to sign up to the task of converting
> > > > > > drivers/mtd/cmdlinepart.c to use this code.
> > > > > >
> > > > > > I've been hopefully cc'ing various MTD people but am not being
> > > > > > overwhelmed with waves of enthusiasm ;)
> > > > >
> > > > > "I've been" implies that you have done so prior to this email. And
> > > > > "people" implies more than one person. I see that you CC'd David
> > > > > Woodhouse over a week ago, but he's fairly silent these days on MTD
> > > > > things. It's Artem or me who handle most of the day-to-day of MTD. And
> > > > > this is the first time I've seen this! (BTW, please include
> > > > > linux-mtd@lists.infradead.org for anything involving MTD.)
> > > > >
> > > > > This seems reasonable, and I'd be willing to work with this proposal.
> > > > >
> > > > > Caizhiyong, can you submit a clear single patch (or series of
> > > > > patches), CC'd to linux-mtd at least? Then we can see about supporting
> > > > > it in MTD. It doesn't look too difficult, but I need to check that it
> > > > > faithfully mimics the capability we currently rely on. There have been
> > > > > previous discussions on changing it, but this was rejected in favor of
> > > > > allowing more flexibility. Here's part of one such conversation:
> > > > >
> > > > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html
> > > > >
> http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html
> > > > > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html
> > > > >
> > > > > So I would recommend:
> > > > > (1) consider carefully the implications of your command-line format
> > > > > now, rather than later
> > > > > (2) if you want MTD to use it, it needs to support the features we use now
> > > >
> > > > It is fully functional reference MTD, :-).
> > >
> > > I realize that. I just want to be clear that we have to reconcile (1)
> > > and (2). IOW, if block device requirements stray too far from MTD
> > > requirements, then we might as well drop the idea of integration now.
> > > But if they agree, then we can move forward.
> > >
> > > > > Some particular cases to consider: overlapping partitions (how do
> > > > > block devices handle overlapping partitions?), out-of-order
> > > > > specification, zero sized partitions, mixed syntax (some specified
> > > > > with an offset, some not), multiple '-' partitions.
> > > >
> > > > I think the 'offset' just is used to hide some MTD space.
> > >
> > > No, it specifies offset as a distance from the beginning of the flash,
> > > so partitions can be numbered out of order. This is intentionally
> > > utilized by some users, for example, to ensure that a particular
> > > partition is always /dev/mtd0, even if it is not the first partition
> > > physically.
> > >
> > > > There are two way:
> > > > 1) redefine the 'offset' as a gap between forward partition and next partition.
> > > > 2) add code forbid command line partitions overlapping and out-of-order.
> > > >
> > > > I recommend 1), it seems to solve those problem(overlapping and out-of-order),
> > > but it will affect habit.
> > >
> > > The linked discussion is where MTD settled on retaining old practice. I
> > > brought it up not so that we change it here, but so that you would
> > > understand what you are agreeing to if you adopt a common MTD and block
> > > device parsing infrastructure.
> > >
> > > [Note that I am much less familiar with block device mechanics than with
> > > MTD.] Are any of the problem areas I mentioned actually forbidden on
> > > block devices? I know, for instance, that an MBR partition table can
> > > specify partitions out of order. And I've googled around and seen some
> > > posts about people (unintentionally) ending up with overlapping hard
> > > disk partitions.
> > >
> > > So from my primitive knowledge, it sounds like a block devices parser
> > > could agree with the same principle put forward by Shmulik in that
> > > second URL:
> > >
> > >  "So far, mtdparts commandline parsing has been very lenient and liberal.
> > >   I think we should keep this approach; give the user the flexibility,
> > >   he'll be responsible to provide meaningful cmdline parts for his
> > >   system."
> > >
> > > Brian
> >
> > I want to use the MTD command line partition method on block devices (eMMC).
> > It is very suitable for embedded systems. I think, in embedded system partition
> method,
> > if somebody need some feature on MTD device, he may be also need it on block device.
> > so I fully functional reference MTD command line partition.
> 
> I agree.
> 
> I'm curious: have you seen any need for a similar arrangement via
> device-tree? See, for example, drivers/mtd/ofpart.c.

So far, I have no seen. We mainly use ARM. For ARM, device-tree is a new thing.

> 
> > I tested the out-of-order and overlapping on my system, used command line partition,
> It is work ok.
> > The block device code is not make any restrictions on partition out-of-order and
> overlapping.
> 
> OK, good. Thanks for checking.
> 
> > I hope extend the flexibility to block device.
> 
> Sure. I'll try to review the full patch soon and test out integrating
> it with MTD.

If there is no problem, I will send my next patch, mtd cmdline parts use cmdline-parser lib.

> 
> Thanks,
> Brian

Re: [PATCH] block: add command line partition parser

[Brian Norris]

On 08/15/2013 12:45 AM, Caizhiyong wrote:
>> -----Original Message-----
>> From: Brian Norris [mailto:computersforpeace@gmail.com]
...
>> On Thu, Aug 15, 2013 at 06:16:04AM +0000, Caizhiyong wrote:
>>> I want to use the MTD command line partition method on block devices (eMMC).
>>> It is very suitable for embedded systems. I think, in embedded system partition
>> method,
>>> if somebody need some feature on MTD device, he may be also need it on block device.
>>> so I fully functional reference MTD command line partition.
>>
>> I agree.
>>
>> I'm curious: have you seen any need for a similar arrangement via
>> device-tree? See, for example, drivers/mtd/ofpart.c.
>
> So far, I have no seen. We mainly use ARM. For ARM, device-tree is a new thing.

But device-tree is *the* thing for ARM going forward, and the kernel 
command line can get unwieldy (and beyond its limits) pretty quickly 
like this.

>>> I hope extend the flexibility to block device.
>>
>> Sure. I'll try to review the full patch soon and test out integrating
>> it with MTD.
>
> If there is no problem, I will send my next patch, mtd cmdline parts use cmdline-parser lib.

We will get some reviews on this soon (no time for me tonight). Do you 
already have a mtd patch? If so, you might as well post it for review as 
well.

Thanks,
Brian

Re: [PATCH] block: add command line partition parser

[Brian Norris]

On 08/15/2013 12:45 AM, Caizhiyong wrote:
>> -----Original Message-----
>> From: Brian Norris [mailto:computersforpeace@gmail.com]
...
>> On Thu, Aug 15, 2013 at 06:16:04AM +0000, Caizhiyong wrote:
>>> I want to use the MTD command line partition method on block devices (eMMC).
>>> It is very suitable for embedded systems. I think, in embedded system partition
>> method,
>>> if somebody need some feature on MTD device, he may be also need it on block device.
>>> so I fully functional reference MTD command line partition.
>>
>> I agree.
>>
>> I'm curious: have you seen any need for a similar arrangement via
>> device-tree? See, for example, drivers/mtd/ofpart.c.
>
> So far, I have no seen. We mainly use ARM. For ARM, device-tree is a new thing.

But device-tree is *the* thing for ARM going forward, and the kernel 
command line can get unwieldy (and beyond its limits) pretty quickly 
like this.

>>> I hope extend the flexibility to block device.
>>
>> Sure. I'll try to review the full patch soon and test out integrating
>> it with MTD.
>
> If there is no problem, I will send my next patch, mtd cmdline parts use cmdline-parser lib.

We will get some reviews on this soon (no time for me tonight). Do you 
already have a mtd patch? If so, you might as well post it for review as 
well.

Thanks,
Brian

Re: [PATCH] block: support embedded device command line partition

[Stephen Warren]

On 08/03/2013 03:57 AM, Caizhiyong wrote:
> From: Cai Zhiyong <caizhiyong@huawei.com>
> 
> 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.
> 
> This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
> About the partition verbose reference "Documentation/block/cmdline-partition.txt"

Interesting. NVIDIA Tegra devices running our downstream kernels have a
"tegrapt" (for "Tegra Partition Table") command-line option for
basically the same purpose. While I'd personally prefer we move our eMMC
layout towards standardized GPT, the feature implemented by this patch
may well provide an easy migration/co-existence path instead, so could
be quite useful.

> diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt

> +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>]
> +  <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.

The device-name isn't always fixed.

For example, what if there are multiple SDHCI controllers, one hosting a
fixed eMMC device and the other an SD card? It's quite typical for the
eMMC's device name (which is likely what should be affected by this
feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when
an SD card is present. Is there a more precise/stable way to define
which device the command-line option applies to?

For the root= command-line option, we use UUID= or PARTUUID= to work
around this issue, but that won't work for naming the whole device.

What about using the MMIO address of the controller hosting the device,
or something like that?

> +<size>
> +    partition size, in bytes, such as: 512, 1m, 1G.

s/m/M/?

I assume M/G are MiB/GiB?

Does it make sense to allow specifying the sizes in units of
sectors/blocks? If so, which specific block/sector size in the case of
things like NAND flash which IIRC have multiple types/levels of sectors;
perhaps this is a bad idea. I guess we could easily add this feature
later by introducing a new suffix, so no need to solve the issue now.

> +
> +<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.

Do partitions usually have a PARTNAME attribute when probed through
normal mechanisms like MBR? If so, I guess this is fine.

Perhaps we can just use , as the delimiter for all fields, rather than
special-casing part-name to use (), so:

size,offset,partname,size,offset,partname,...

The partname field could easily be empty if not needed.

> +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)

RE: [PATCH] block: support embedded device command line partition

[Caizhiyong]

> > +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.
> 
> The device-name isn't always fixed.
> 
> For example, what if there are multiple SDHCI controllers, one hosting a
> fixed eMMC device and the other an SD card? It's quite typical for the
> eMMC's device name (which is likely what should be affected by this
> feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when
> an SD card is present. Is there a more precise/stable way to define
> which device the command-line option applies to?
  
Yes. Fixed is for single controller.
For multiple controllers, currently, there is not a simple way.
I tend to do something in the eMMC driver, such as initialize order,
but I have not tried.

> 
> For the root= command-line option, we use UUID= or PARTUUID= to work
> around this issue, but that won't work for naming the whole device.
> 
> What about using the MMIO address of the controller hosting the device,
> or something like that?
> 

  MMIO may be not a good idea, it is not intuitive. 

> > +<size>
> > +    partition size, in bytes, such as: 512, 1m, 1G.
> 
> s/m/M/?
> 
> I assume M/G are MiB/GiB?
> 
> Does it make sense to allow specifying the sizes in units of
> sectors/blocks? If so, which specific block/sector size in the case of
> things like NAND flash which IIRC have multiple types/levels of sectors;
> perhaps this is a bad idea. I guess we could easily add this feature
> later by introducing a new suffix, so no need to solve the issue now.
> 
> > +
> > +<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.
> 
> Do partitions usually have a PARTNAME attribute when probed through
> normal mechanisms like MBR? If so, I guess this is fine.
> 
> Perhaps we can just use , as the delimiter for all fields, rather than
> special-casing part-name to use (), so:
> 
> size,offset,partname,size,offset,partname,...
> 
> The partname field could easily be empty if not needed.
> 

If no need partname, your bootargs are mmcblk0:1G,1G,1G,...
 
> > +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)

Re: [PATCH] block: support embedded device command line partition

[Stephen Warren]

On 08/15/2013 08:54 PM, Caizhiyong wrote:
>>> +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.
>>
>> The device-name isn't always fixed.
>>
>> For example, what if there are multiple SDHCI controllers, one hosting a
>> fixed eMMC device and the other an SD card? It's quite typical for the
>> eMMC's device name (which is likely what should be affected by this
>> feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when
>> an SD card is present. Is there a more precise/stable way to define
>> which device the command-line option applies to?
>   
> Yes. Fixed is for single controller.
> For multiple controllers, currently, there is not a simple way.
> I tend to do something in the eMMC driver, such as initialize order,
> but I have not tried.

There have been proposals before to try and create a fixed naming for
the controllers (or rather the block devices they generate...) but
they've been rejected. I don't think we should rely on being able to do
that.

>>> +
>>> +<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.
>>
>> Do partitions usually have a PARTNAME attribute when probed through
>> normal mechanisms like MBR? If so, I guess this is fine.
>>
>> Perhaps we can just use , as the delimiter for all fields, rather than
>> special-casing part-name to use (), so:
>>
>> size,offset,partname,size,offset,partname,...
>>
>> The partname field could easily be empty if not needed.
> 
> If no need partname, your bootargs are mmcblk0:1G,1G,1G,...

Well, you always need the offset too. I don't think there's any harm in
forcing all fields to be specified in all cases; it makes the whole
system much more regular and less error-prone.

Alternatively, use a different separator between fields for a given
partition, and between partitions, e.g.:

size,offset,partname;size,offset,partname

That way, you know that if you see a ; you're looking at a new
partition, and hence the partname field need not always be specified.
Although, if you want to specify a partname but not an offset you'd
still need empty fields, so just requiring all fields to always be
present still seems safest to me.

RE: [PATCH] block: support embedded device command line partition

[Caizhiyong]

> On 08/15/2013 08:54 PM, Caizhiyong wrote:
> >>> +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.
> >>
> >> The device-name isn't always fixed.
> >>
> >> For example, what if there are multiple SDHCI controllers, one hosting a
> >> fixed eMMC device and the other an SD card? It's quite typical for the
> >> eMMC's device name (which is likely what should be affected by this
> >> feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when
> >> an SD card is present. Is there a more precise/stable way to define
> >> which device the command-line option applies to?
> >
> > Yes. Fixed is for single controller.
> > For multiple controllers, currently, there is not a simple way.
> > I tend to do something in the eMMC driver, such as initialize order,
> > but I have not tried.
> 
> There have been proposals before to try and create a fixed naming for
> the controllers (or rather the block devices they generate...) but
> they've been rejected. I don't think we should rely on being able to do
> that.
> 
> >>> +
> >>> +<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.
> >>
> >> Do partitions usually have a PARTNAME attribute when probed through
> >> normal mechanisms like MBR? If so, I guess this is fine.
> >>
> >> Perhaps we can just use , as the delimiter for all fields, rather than
> >> special-casing part-name to use (), so:
> >>
> >> size,offset,partname,size,offset,partname,...
> >>
> >> The partname field could easily be empty if not needed.
> >
> > If no need partname, your bootargs are mmcblk0:1G,1G,1G,...
> 
> Well, you always need the offset too. I don't think there's any harm in
> forcing all fields to be specified in all cases; it makes the whole
> system much more regular and less error-prone.
> 
> Alternatively, use a different separator between fields for a given
> partition, and between partitions, e.g.:
> 
> size,offset,partname;size,offset,partname
> 
> That way, you know that if you see a ; you're looking at a new
> partition, and hence the partname field need not always be specified.
> Although, if you want to specify a partname but not an offset you'd
> still need empty fields, so just requiring all fields to always be
> present still seems safest to me.

I just follow MTD cmdline partition format.(reference drivers/mtd/cmdlinepart.c)
There are many pitfalls in using this partition format, the designer is more
consideration its ease of use, rather than safe.
There is an other conversation: https://lkml.org/lkml/2013/8/3/16

Re: [PATCH] block: support embedded device command line partition

[Stephen Warren]

On 08/19/2013 02:36 AM, Caizhiyong wrote:
>> On 08/15/2013 08:54 PM, Caizhiyong wrote:
>>>>> +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.
>>>>
>>>> The device-name isn't always fixed.
>>>>
>>>> For example, what if there are multiple SDHCI controllers, one hosting a
>>>> fixed eMMC device and the other an SD card? It's quite typical for the
>>>> eMMC's device name (which is likely what should be affected by this
>>>> feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when
>>>> an SD card is present. Is there a more precise/stable way to define
>>>> which device the command-line option applies to?
>>>
>>> Yes. Fixed is for single controller.
>>> For multiple controllers, currently, there is not a simple way.
>>> I tend to do something in the eMMC driver, such as initialize order,
>>> but I have not tried.
>>
>> There have been proposals before to try and create a fixed naming for
>> the controllers (or rather the block devices they generate...) but
>> they've been rejected. I don't think we should rely on being able to do
>> that.
>>
>>>>> +
>>>>> +<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.
>>>>
>>>> Do partitions usually have a PARTNAME attribute when probed through
>>>> normal mechanisms like MBR? If so, I guess this is fine.
>>>>
>>>> Perhaps we can just use , as the delimiter for all fields, rather than
>>>> special-casing part-name to use (), so:
>>>>
>>>> size,offset,partname,size,offset,partname,...
>>>>
>>>> The partname field could easily be empty if not needed.
>>>
>>> If no need partname, your bootargs are mmcblk0:1G,1G,1G,...
>>
>> Well, you always need the offset too. I don't think there's any harm in
>> forcing all fields to be specified in all cases; it makes the whole
>> system much more regular and less error-prone.
>>
>> Alternatively, use a different separator between fields for a given
>> partition, and between partitions, e.g.:
>>
>> size,offset,partname;size,offset,partname
>>
>> That way, you know that if you see a ; you're looking at a new
>> partition, and hence the partname field need not always be specified.
>> Although, if you want to specify a partname but not an offset you'd
>> still need empty fields, so just requiring all fields to always be
>> present still seems safest to me.
> 
> I just follow MTD cmdline partition format.(reference drivers/mtd/cmdlinepart.c)

Ah OK, consistency with an existing format used for similar purposes
probably does override any other concerns.

> There are many pitfalls in using this partition format, the designer is more
> consideration its ease of use, rather than safe.
> There is an other conversation: https://lkml.org/lkml/2013/8/3/16

Re: [PATCH] block: support embedded device command line partition

[Linus Walleij]

On Sat, Aug 3, 2013 at 11:57 AM, Caizhiyong <caizhiyong@huawei.com> wrote:

> From: Cai Zhiyong <caizhiyong@huawei.com>
>
> 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.
>
> This code reference MTD partition, source "drivers/mtd/cmdlinepart.c"
> About the partition verbose reference "Documentation/block/cmdline-partition.txt"
>
> Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>

I saw this patch appear in kernel v3.12-rc1, and it's nice since it's exactly
what we need. So needed that we proposed it in 2010:
http://marc.info/?l=linux-kernel&m=127425650923757&w=2
http://marc.info/?l=linux-kernel&m=127599718024364&w=2

Is this patch inspired by Ulfs patch? (I can see the code is different,
but the cmdline argument is the same for example.) Some credit
could have been proper in that case.

Just asking: sometimes wheels do get reinvented.

Yours,
Linus Walleij

RE: [PATCH] block: support embedded device command line partition

[Caizhiyong]

 
> I saw this patch appear in kernel v3.12-rc1, and it's nice since it's exactly
> what we need. So needed that we proposed it in 2010:
> http://marc.info/?l=linux-kernel&m=127425650923757&w=2
> http://marc.info/?l=linux-kernel&m=127599718024364&w=2
> 
> Is this patch inspired by Ulfs patch? (I can see the code is different,
> but the cmdline argument is the same for example.) Some credit
> could have been proper in that case.

I have not searched other patch, English reading/writing is not an easy thing for me.

Thank Ulf Hansson. He was the first one who thought of this method.

> 
> Just asking: sometimes wheels do get reinvented.
> 
> Yours,
> Linus Walleij

RE: [PATCH] block: support embedded device command line partition

[Caizhiyong]

> From: Karel Zak [mailto:kzak@redhat.com]
> Sent: Wednesday, July 31, 2013 9:25 PM
> To: Caizhiyong
> Cc: Andrew Morton; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin
> Subject: Re: [PATCH] block: support embedded device command line partition
> 
> On Sat, Jul 27, 2013 at 01:56:24PM +0000, Caizhiyong wrote:
> > +static int parse_partitions(struct parsed_partitions *state,
> > +			    struct cmdline_parts *parts)
> > +{
> > +	int slot;
> > +	uint64_t from = 0;
> > +	uint64_t disk_size;
> > +	char buf[BDEVNAME_SIZE];
> > +	struct cmdline_subpart *subpart;
> > +
> > +	bdevname(state->bdev, buf);
> > +
> > +	while (parts && strncmp(buf, parts->name, BDEVNAME_SIZE))
> > +		parts = parts->next_parts;
> > +
> > +	if (!parts)
> > +		return 0;
> > +
> > +	disk_size = (uint64_t) get_capacity(state->bdev->bd_disk) << 9;
> > +
> > +	for (slot = 1, subpart = parts->subpart;
> > +	     subpart && slot < state->limit;
> > +	     subpart = subpart->next_subpart, slot++) {
> > +
> > +		unsigned label_max;
> > +		struct partition_meta_info *info;
> > +
> > +		if (subpart->from == OFFSET_CONTINUOUS)
> > +			subpart->from = from;
> > +		else
> > +			from = subpart->from;
> > +
> > +		if (from >= disk_size)
> > +			break;
> > +
> > +		if (subpart->size > (disk_size - from))
> > +			subpart->size = disk_size - from;
> > +
> > +		from += subpart->size;
> > +
> > +		put_partition(state, slot, le64_to_cpu(subpart->from >> 9),
> > +			      le64_to_cpu(subpart->size >> 9));
> 
>  Why le64_to_cpu()?
> 
>  I guess that memparse() does not return explicitly LE and it also
>  seems that your code  on another places uses ->size and ->from
>  without any extra care.

Right. I will remove.

> 
>     Karel
> 
> --
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com

Re: [PATCH] block: support embedded device command line partition

[Karel Zak]

On Sat, Jul 27, 2013 at 01:56:24PM +0000, Caizhiyong wrote:
> +static int parse_partitions(struct parsed_partitions *state,
> +			    struct cmdline_parts *parts)
> +{
> +	int slot;
> +	uint64_t from = 0;
> +	uint64_t disk_size;
> +	char buf[BDEVNAME_SIZE];
> +	struct cmdline_subpart *subpart;
> +
> +	bdevname(state->bdev, buf);
> +
> +	while (parts && strncmp(buf, parts->name, BDEVNAME_SIZE))
> +		parts = parts->next_parts;
> +
> +	if (!parts)
> +		return 0;
> +
> +	disk_size = (uint64_t) get_capacity(state->bdev->bd_disk) << 9;
> +
> +	for (slot = 1, subpart = parts->subpart;
> +	     subpart && slot < state->limit;
> +	     subpart = subpart->next_subpart, slot++) {
> +
> +		unsigned label_max;
> +		struct partition_meta_info *info;
> +
> +		if (subpart->from == OFFSET_CONTINUOUS)
> +			subpart->from = from;
> +		else
> +			from = subpart->from;
> +
> +		if (from >= disk_size)
> +			break;
> +
> +		if (subpart->size > (disk_size - from))
> +			subpart->size = disk_size - from;
> +
> +		from += subpart->size;
> +
> +		put_partition(state, slot, le64_to_cpu(subpart->from >> 9),
> +			      le64_to_cpu(subpart->size >> 9));

 Why le64_to_cpu()?
 
 I guess that memparse() does not return explicitly LE and it also
 seems that your code  on another places uses ->size and ->from
 without any extra care.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] block: support embedded device command line partition

[Andrew Morton]

On Sat, 27 Jul 2013 13:56:24 +0000 Caizhiyong <caizhiyong@huawei.com> wrote:

> From: Cai Zhiyong <caizhiyong@huawei.com>
> 
> Read block device partition table from command line. This partition used for fixed block device (eMMC) embedded device.
> It no MBR, can save storage space. Bootloader can be easily accessed by absolute address of data on the block device. It support partition name, provides partition interface.

That seems a reasonable thing to be able to do.

> This code reference MTD partition, source "./drivers/mtd/cmdlinepart.c"
> The format for the command line is just like mtdparts.
> 
> Examples: eMMC disk name is "mmcblk0"
> bootargs 'cmdlineparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'

We should document this user interface properly.  Is there
documentation under Documentation/ which can be referenced?  If not,
something new should be created.

>
> ...
>
> +struct cmdline_parts {
> +	char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
> +	struct cmdline_subpart *subpart;
> +	struct cmdline_parts *next_parts;
> +};
> +
> +static DEFINE_MUTEX(cmdline_string_mutex);
> +
> +static char cmdline_string[512] = { 0 }; static struct cmdline_parts 
> +*cmdline_parts;

That's messed up.

> +static int parse_subpart(struct cmdline_subpart **subpart, char
> +*cmdline) {

Please convert all the function definitions to standard kernel style:

static int parse_subpart(struct cmdline_subpart **subpart, char *cmdline)
{

> +	int ret = 0;
> +	struct cmdline_subpart *new_subpart;
> +
> +	(*subpart) = NULL;
> +
> +	new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
> +	if (!new_subpart)
> +		return -ENOMEM;
> +
> +	if ((*cmdline) == '-') {
> +		new_subpart->size = SIZE_REMAINING;
> +		cmdline++;
> +	} else {
> +		new_subpart->size = memparse(cmdline, &cmdline);
> +		if (new_subpart->size < PAGE_SIZE) {
> +			pr_warn("cmdline partition size is invalid.");
> +			ret = -EFAULT;

EFAULT is inappropriate here.  EINVAL would be suitable.

> +			goto fail;
> +		}
> +	}
> +
> +	if ((*cmdline) == '@') {
> +		cmdline++;
> +		new_subpart->from = memparse(cmdline, &cmdline);
> +	} else {
> +		new_subpart->from = OFFSET_CONTINUOUS;
> +	}
> +
> +	if ((*cmdline) == '(') {
> +

Remove this newline.

> +		int length;
> +		char *next = strchr(++cmdline, ')');
> +
> +		if (!next) {
> +			pr_warn("cmdline partition format is invalid.");
> +			ret = -EFAULT;

EINVAL

> +			goto fail;
> +		}
> +
> +		length = min((int)(next - cmdline),
> +			     (int)(sizeof(new_subpart->name) - 1));

OK, that's pretty ghastly.

Ideally, the types of the various variable should be compatible, so no
casting is needed.

If that is really truly not practical then use min_t rather than
open-coding the casts.

> +		strncpy(new_subpart->name, cmdline, length);
> +		new_subpart->name[length] = '\0';
> +
> +		cmdline = ++next;
> +	} else
> +		new_subpart->name[0] = '\0';
> +
> +	(*subpart) = new_subpart;
> +	return 0;
> +fail:
> +	kfree(new_subpart);
> +	return ret;
> +}
> +
> +static void free_subpart(struct cmdline_parts *parts) {
> +	struct cmdline_subpart *subpart;
> +
> +	while (parts->subpart) {
> +		subpart = parts->subpart;
> +		parts->subpart = subpart->next_subpart;
> +		kfree(subpart);
> +	}
> +}
> +
> +static void free_parts(struct cmdline_parts **parts) {
> +	struct cmdline_parts *next_parts;
> +
> +	while ((*parts)) {
> +		next_parts = (*parts)->next_parts;
> +		free_subpart((*parts));
> +		kfree((*parts));
> +		(*parts) = next_parts;
> +	}
> +}
> +
> +static int parse_parts(struct cmdline_parts **parts, const char
> +*cmdline) {
> +	int ret = -EFAULT;

EINVAL?

> +	char *next;
> +	int length;
> +	struct cmdline_subpart **next_subpart;
> +	struct cmdline_parts *newparts;
> +	char buf[BDEVNAME_SIZE + 32 + 4];
> +
> +	(*parts) = NULL;
> +
> +	newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
> +	if (!newparts)
> +		return -ENOMEM;
> +
> +	next = strchr(cmdline, ':');
> +	if (!next) {
> +		pr_warn("cmdline partition has not block device.");
> +		goto fail;
> +	}
> +
> +	length = min((int)(next - cmdline), (int)(sizeof(newparts->name) - 1));

Ditto.

> +	strncpy(newparts->name, cmdline, length);
> +	newparts->name[length] = '\0';
> +
> +	next_subpart = &newparts->subpart;
> +
> +	while (next && *(++next)) {
> +

Remove newline.

> +		cmdline = next;
> +		next = strchr(cmdline, ',');
> +
> +		length = (!next) ? (sizeof(buf) - 1) :
> +			min((int)(next - cmdline), (int)(sizeof(buf) - 1));

Sort the types out.

> +		strncpy(buf, cmdline, length);
> +		buf[length] = '\0';
> +
> +		ret = parse_subpart(next_subpart, buf);
> +		if (ret)
> +			goto fail;
> +
> +		next_subpart = &(*next_subpart)->next_subpart;
> +	}
> +
> +	if (!newparts->subpart) {
> +		pr_warn("cmdline partition has not valid partition.");
> +		goto fail;
> +	}
> +
> +	(*parts) = newparts;

The code adds these unneeded parentheses in several places.  They are
unneeded ;)

> +	return 0;
> +fail:
> +	free_subpart(newparts);
> +	kfree(newparts);
> +	return ret;
> +}
> +
> +static int parse_cmdline(struct cmdline_parts **parts, const char
> +*cmdline) {
> +	int ret;
> +	char *buf;
> +	char *pbuf;
> +	char *next;
> +	struct cmdline_parts **next_parts;
> +
> +	(*parts) = NULL;
> +
> +	next = pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	next_parts = parts;
> +
> +	while (next && *pbuf) {
> +

Remove newline.

> +		next = strchr(pbuf, ';');
> +		if (next)
> +			(*next) = '\0';
> +
> +		ret = parse_parts(next_parts, pbuf);
> +		if (ret)
> +			goto fail;
> +
> +		if (next)
> +			pbuf = ++next;
> +
> +		next_parts = &(*next_parts)->next_parts;
> +	}
> +
> +	if (!(*parts)) {
> +		pr_warn("cmdline partition has not valid partition.");
> +		ret = -EFAULT;
> +		goto fail;
> +	}
> +
> +	ret = 0;
> +done:
> +	kfree(buf);
> +	return ret;
> +
> +fail:
> +	free_parts(parts);
> +	goto done;
> +}
> +
> +/*
> + * Purpose: allocate cmdline partitions.
> + * Returns:
> + * -1 if unable to read the partition table
> + *  0 if this isn't our partition table
> + *  1 if successful
> + */
> +static int parse_partitions(struct parsed_partitions *state,
> +			    struct cmdline_parts *parts)
> +{
> +	int slot;
> +	uint64_t from = 0;
> +	uint64_t disk_size;
> +	char buf[BDEVNAME_SIZE];
> +	struct cmdline_subpart *subpart;
> +
> +	bdevname(state->bdev, buf);
> +
> +	while (parts && strncmp(buf, parts->name, BDEVNAME_SIZE))
> +		parts = parts->next_parts;
> +
> +	if (!parts)
> +		return 0;
> +
> +	disk_size = (uint64_t) get_capacity(state->bdev->bd_disk) << 9;

Remove the space after the cast.

get_capacity() returns sector_t.  That is appropriate.  It would be
saner if all this code were to use sector_t as well.

Also, u64 is preferred over uint64_t in kernel code.

> +	for (slot = 1, subpart = parts->subpart;
> +	     subpart && slot < state->limit;
> +	     subpart = subpart->next_subpart, slot++) {
> +

Remove newline.

> +		unsigned label_max;
> +		struct partition_meta_info *info;
> +
> +		if (subpart->from == OFFSET_CONTINUOUS)
> +			subpart->from = from;
> +		else
> +			from = subpart->from;
> +
> +		if (from >= disk_size)
> +			break;
> +
> +		if (subpart->size > (disk_size - from))
> +			subpart->size = disk_size - from;
> +
> +		from += subpart->size;
> +
> +		put_partition(state, slot, le64_to_cpu(subpart->from >> 9),
> +			      le64_to_cpu(subpart->size >> 9));
> +
> +		info = &state->parts[slot].info;
> +
> +		label_max = min(sizeof(info->volname) - 1,
> +			sizeof(subpart->name));
> +		strncpy(info->volname, subpart->name, label_max);
> +		info->volname[label_max] = '\0';
> +		state->parts[slot].has_info = true;
> +	}
> +
> +	strlcat(state->pp_buf, "\n", PAGE_SIZE);
> +
> +	return 1;
> +}
> +
> +static int set_cmdline_parts(char *str) {
> +	strncpy(cmdline_string, str, sizeof(cmdline_string) - 1);
> +	cmdline_string[sizeof(cmdline_string) - 1] = '\0';
> +	return 1;
> +}
> +__setup("cmdlineparts=", set_cmdline_parts);
> +
> +void cmdline_parts_setup(char *str)
> +{
> +	mutex_lock(&cmdline_string_mutex);
> +	set_cmdline_parts(str);
> +	mutex_unlock(&cmdline_string_mutex);
> +}
> +EXPORT_SYMBOL(cmdline_parts_setup);

This export is unneed, I expect.

cmdline_parts_setup has no references and can simply be removed?

> +/*
> + * Purpose: allocate cmdline partitions.
> + * Returns:
> + * -1 if unable to read the partition table
> + *  0 if this isn't our partition table
> + *  1 if successful
> + */
> +int cmdline_partition(struct parsed_partitions *state) {
> +	int ret;
> +
> +	mutex_lock(&cmdline_string_mutex);
> +	if (cmdline_string[0]) {
> +
> +		if (cmdline_parts)
> +			free_parts(&cmdline_parts);
> +
> +		if (parse_cmdline(&cmdline_parts, cmdline_string)) {
> +			ret = 0;
> +			goto fail;
> +		}
> +		cmdline_string[0] = '\0';
> +	}
> +	mutex_unlock(&cmdline_string_mutex);
> +
> +	if (!cmdline_parts)
> +		return 0;
> +
> +	return parse_partitions(state, cmdline_parts);

But we dropped the mutex.  Nothing protects cmdline_parts during the
execution of parse_partitions().

> +fail:
> +	cmdline_string[0] = '\0';
> +	mutex_unlock(&cmdline_string_mutex);
> +	return ret;
> +}
> diff --git a/block/partitions/cmdline.h b/block/partitions/cmdline.h new file mode 100644 index 0000000..26e0f8d
> --- /dev/null
> +++ b/block/partitions/cmdline.h
> @@ -0,0 +1,2 @@
> +
> +int cmdline_partition(struct parsed_partitions *state);
> --
> 1.8.1.5

[PATCH] block: support embedded device command line partition

[Caizhiyong]

From: Cai Zhiyong <caizhiyong@huawei.com>

Read block device partition table from command line. This partition used for fixed block device (eMMC) embedded device.
It no MBR, can save storage space. Bootloader can be easily accessed by absolute address of data on the block device. It support partition name, provides partition interface.

This code reference MTD partition, source "./drivers/mtd/cmdlinepart.c"
The format for the command line is just like mtdparts.

Examples: eMMC disk name is "mmcblk0"
bootargs 'cmdlineparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'

Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
---
 block/partitions/Kconfig   |   6 +
 block/partitions/Makefile  |   1 +
 block/partitions/check.c   |   4 +
 block/partitions/cmdline.c | 346 +++++++++++++++++++++++++++++++++++++++++++++
 block/partitions/cmdline.h |   2 +
 5 files changed, 359 insertions(+)
 create mode 100644 block/partitions/cmdline.c  create mode 100644 block/partitions/cmdline.h

diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig index 4cebb2f..2ebf251 100644
--- a/block/partitions/Kconfig
+++ b/block/partitions/Kconfig
@@ -260,3 +260,9 @@ config SYSV68_PARTITION
 	  partition table format used by Motorola Delta machines (using
 	  sysv68).
 	  Otherwise, say N.
+
+config CMDLINE_PARTITION
+	bool "Command line partition support" if PARTITION_ADVANCED
+	help
+	  Say Y here if you would read the partitions table from bootargs.
+	  The format for the command line is just like mtdparts.
diff --git a/block/partitions/Makefile b/block/partitions/Makefile index 2be4d7b..4e9d584 100644
--- a/block/partitions/Makefile
+++ b/block/partitions/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_IBM_PARTITION) += ibm.o
 obj-$(CONFIG_EFI_PARTITION) += efi.o
 obj-$(CONFIG_KARMA_PARTITION) += karma.o
 obj-$(CONFIG_SYSV68_PARTITION) += sysv68.o
+obj-$(CONFIG_CMDLINE_PARTITION) += cmdline.o
diff --git a/block/partitions/check.c b/block/partitions/check.c index 19ba207..9ac1df7 100644
--- a/block/partitions/check.c
+++ b/block/partitions/check.c
@@ -34,6 +34,7 @@
 #include "efi.h"
 #include "karma.h"
 #include "sysv68.h"
+#include "cmdline.h"
 
 int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/
 
@@ -65,6 +66,9 @@ static int (*check_part[])(struct parsed_partitions *) = {
 	adfspart_check_ADFS,
 #endif
 
+#ifdef CONFIG_CMDLINE_PARTITION
+	cmdline_partition,
+#endif
 #ifdef CONFIG_EFI_PARTITION
 	efi_partition,		/* this must come before msdos */
 #endif
diff --git a/block/partitions/cmdline.c b/block/partitions/cmdline.c new file mode 100644 index 0000000..05c7e17
--- /dev/null
+++ b/block/partitions/cmdline.c
@@ -0,0 +1,346 @@
+/*
+ * Copyright (C) 2013 HUAWEI
+ * Author: Cai Zhiyong <caizhiyong@huawei.com>
+ *
+ * Read block device partition table from command line.
+ * Design the partition for eMMC device.
+ * This code reference MTD partition, source "./drivers/mtd/cmdlinepart.c"
+ * The format for the command line is just like mtdparts.
+ *
+ * Examples:
+ *
+ * eMMC disk name is "mmcblk0"
+ * bootargs:
+ * 'cmdlineparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'
+ *
+ */
+
+#include <linux/buffer_head.h>
+#include <linux/module.h>
+#include <linux/ctype.h>
+
+#include "check.h"
+#include "cmdline.h"
+
+#define SIZE_REMAINING              ULLONG_MAX
+#define OFFSET_CONTINUOUS           ULLONG_MAX
+
+struct cmdline_subpart {
+	char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */
+	uint64_t from;
+	uint64_t size;
+	struct cmdline_subpart *next_subpart;
+};
+
+struct cmdline_parts {
+	char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
+	struct cmdline_subpart *subpart;
+	struct cmdline_parts *next_parts;
+};
+
+static DEFINE_MUTEX(cmdline_string_mutex);
+
+static char cmdline_string[512] = { 0 }; static struct cmdline_parts 
+*cmdline_parts;
+
+static int parse_subpart(struct cmdline_subpart **subpart, char
+*cmdline) {
+	int ret = 0;
+	struct cmdline_subpart *new_subpart;
+
+	(*subpart) = NULL;
+
+	new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
+	if (!new_subpart)
+		return -ENOMEM;
+
+	if ((*cmdline) == '-') {
+		new_subpart->size = SIZE_REMAINING;
+		cmdline++;
+	} else {
+		new_subpart->size = memparse(cmdline, &cmdline);
+		if (new_subpart->size < PAGE_SIZE) {
+			pr_warn("cmdline partition size is invalid.");
+			ret = -EFAULT;
+			goto fail;
+		}
+	}
+
+	if ((*cmdline) == '@') {
+		cmdline++;
+		new_subpart->from = memparse(cmdline, &cmdline);
+	} else {
+		new_subpart->from = OFFSET_CONTINUOUS;
+	}
+
+	if ((*cmdline) == '(') {
+
+		int length;
+		char *next = strchr(++cmdline, ')');
+
+		if (!next) {
+			pr_warn("cmdline partition format is invalid.");
+			ret = -EFAULT;
+			goto fail;
+		}
+
+		length = min((int)(next - cmdline),
+			     (int)(sizeof(new_subpart->name) - 1));
+		strncpy(new_subpart->name, cmdline, length);
+		new_subpart->name[length] = '\0';
+
+		cmdline = ++next;
+	} else
+		new_subpart->name[0] = '\0';
+
+	(*subpart) = new_subpart;
+	return 0;
+fail:
+	kfree(new_subpart);
+	return ret;
+}
+
+static void free_subpart(struct cmdline_parts *parts) {
+	struct cmdline_subpart *subpart;
+
+	while (parts->subpart) {
+		subpart = parts->subpart;
+		parts->subpart = subpart->next_subpart;
+		kfree(subpart);
+	}
+}
+
+static void free_parts(struct cmdline_parts **parts) {
+	struct cmdline_parts *next_parts;
+
+	while ((*parts)) {
+		next_parts = (*parts)->next_parts;
+		free_subpart((*parts));
+		kfree((*parts));
+		(*parts) = next_parts;
+	}
+}
+
+static int parse_parts(struct cmdline_parts **parts, const char
+*cmdline) {
+	int ret = -EFAULT;
+	char *next;
+	int length;
+	struct cmdline_subpart **next_subpart;
+	struct cmdline_parts *newparts;
+	char buf[BDEVNAME_SIZE + 32 + 4];
+
+	(*parts) = NULL;
+
+	newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
+	if (!newparts)
+		return -ENOMEM;
+
+	next = strchr(cmdline, ':');
+	if (!next) {
+		pr_warn("cmdline partition has not block device.");
+		goto fail;
+	}
+
+	length = min((int)(next - cmdline), (int)(sizeof(newparts->name) - 1));
+	strncpy(newparts->name, cmdline, length);
+	newparts->name[length] = '\0';
+
+	next_subpart = &newparts->subpart;
+
+	while (next && *(++next)) {
+
+		cmdline = next;
+		next = strchr(cmdline, ',');
+
+		length = (!next) ? (sizeof(buf) - 1) :
+			min((int)(next - cmdline), (int)(sizeof(buf) - 1));
+
+		strncpy(buf, cmdline, length);
+		buf[length] = '\0';
+
+		ret = parse_subpart(next_subpart, buf);
+		if (ret)
+			goto fail;
+
+		next_subpart = &(*next_subpart)->next_subpart;
+	}
+
+	if (!newparts->subpart) {
+		pr_warn("cmdline partition has not valid partition.");
+		goto fail;
+	}
+
+	(*parts) = newparts;
+
+	return 0;
+fail:
+	free_subpart(newparts);
+	kfree(newparts);
+	return ret;
+}
+
+static int parse_cmdline(struct cmdline_parts **parts, const char
+*cmdline) {
+	int ret;
+	char *buf;
+	char *pbuf;
+	char *next;
+	struct cmdline_parts **next_parts;
+
+	(*parts) = NULL;
+
+	next = pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	next_parts = parts;
+
+	while (next && *pbuf) {
+
+		next = strchr(pbuf, ';');
+		if (next)
+			(*next) = '\0';
+
+		ret = parse_parts(next_parts, pbuf);
+		if (ret)
+			goto fail;
+
+		if (next)
+			pbuf = ++next;
+
+		next_parts = &(*next_parts)->next_parts;
+	}
+
+	if (!(*parts)) {
+		pr_warn("cmdline partition has not valid partition.");
+		ret = -EFAULT;
+		goto fail;
+	}
+
+	ret = 0;
+done:
+	kfree(buf);
+	return ret;
+
+fail:
+	free_parts(parts);
+	goto done;
+}
+
+/*
+ * Purpose: allocate cmdline partitions.
+ * Returns:
+ * -1 if unable to read the partition table
+ *  0 if this isn't our partition table
+ *  1 if successful
+ */
+static int parse_partitions(struct parsed_partitions *state,
+			    struct cmdline_parts *parts)
+{
+	int slot;
+	uint64_t from = 0;
+	uint64_t disk_size;
+	char buf[BDEVNAME_SIZE];
+	struct cmdline_subpart *subpart;
+
+	bdevname(state->bdev, buf);
+
+	while (parts && strncmp(buf, parts->name, BDEVNAME_SIZE))
+		parts = parts->next_parts;
+
+	if (!parts)
+		return 0;
+
+	disk_size = (uint64_t) get_capacity(state->bdev->bd_disk) << 9;
+
+	for (slot = 1, subpart = parts->subpart;
+	     subpart && slot < state->limit;
+	     subpart = subpart->next_subpart, slot++) {
+
+		unsigned label_max;
+		struct partition_meta_info *info;
+
+		if (subpart->from == OFFSET_CONTINUOUS)
+			subpart->from = from;
+		else
+			from = subpart->from;
+
+		if (from >= disk_size)
+			break;
+
+		if (subpart->size > (disk_size - from))
+			subpart->size = disk_size - from;
+
+		from += subpart->size;
+
+		put_partition(state, slot, le64_to_cpu(subpart->from >> 9),
+			      le64_to_cpu(subpart->size >> 9));
+
+		info = &state->parts[slot].info;
+
+		label_max = min(sizeof(info->volname) - 1,
+			sizeof(subpart->name));
+		strncpy(info->volname, subpart->name, label_max);
+		info->volname[label_max] = '\0';
+		state->parts[slot].has_info = true;
+	}
+
+	strlcat(state->pp_buf, "\n", PAGE_SIZE);
+
+	return 1;
+}
+
+static int set_cmdline_parts(char *str) {
+	strncpy(cmdline_string, str, sizeof(cmdline_string) - 1);
+	cmdline_string[sizeof(cmdline_string) - 1] = '\0';
+	return 1;
+}
+__setup("cmdlineparts=", set_cmdline_parts);
+
+void cmdline_parts_setup(char *str)
+{
+	mutex_lock(&cmdline_string_mutex);
+	set_cmdline_parts(str);
+	mutex_unlock(&cmdline_string_mutex);
+}
+EXPORT_SYMBOL(cmdline_parts_setup);
+
+/*
+ * Purpose: allocate cmdline partitions.
+ * Returns:
+ * -1 if unable to read the partition table
+ *  0 if this isn't our partition table
+ *  1 if successful
+ */
+int cmdline_partition(struct parsed_partitions *state) {
+	int ret;
+
+	mutex_lock(&cmdline_string_mutex);
+	if (cmdline_string[0]) {
+
+		if (cmdline_parts)
+			free_parts(&cmdline_parts);
+
+		if (parse_cmdline(&cmdline_parts, cmdline_string)) {
+			ret = 0;
+			goto fail;
+		}
+		cmdline_string[0] = '\0';
+	}
+	mutex_unlock(&cmdline_string_mutex);
+
+	if (!cmdline_parts)
+		return 0;
+
+	return parse_partitions(state, cmdline_parts);
+
+fail:
+	cmdline_string[0] = '\0';
+	mutex_unlock(&cmdline_string_mutex);
+	return ret;
+}
diff --git a/block/partitions/cmdline.h b/block/partitions/cmdline.h new file mode 100644 index 0000000..26e0f8d
--- /dev/null
+++ b/block/partitions/cmdline.h
@@ -0,0 +1,2 @@
+
+int cmdline_partition(struct parsed_partitions *state);
--
1.8.1.5