There are typically two types to set the mtd partitions:
<1> set with the `size`, such as
gpmi-nand:100m(boot),100m(kernel),1g(rootfs)
<2> set with the `offset`, such as
gpmi-nand:100m@0(boot),100m@100m(kernel),1g@200m(rootfs)
gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
If we mix these two types, such as:
gpmi-nand:100m@0(boot),100m(kernel),1g@200m(rootfs)
gpmi-nand:1g@200m(rootfs),100m@0(boot),100m(kernel)
It's hard to understand the cmdline. And also it is hard to sort the
partitions in this mixed type. So we explicitly forbid the mixed type.
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/cmdlinepart.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index fe7e3a5..0b7b2ad 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -35,6 +35,15 @@
*
* 1 NOR Flash with 2 partitions, 1 NAND with one
* edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home)
+ *
+ * Note:
+ * If you choose to set the @offset for the <partdef>, please set all
+ * the partitions with the same syntax, such as:
+ * gpmi-nand:100m@0(boot),100m@100m(kernel),1g@200m(rootfs)
+ *
+ * Please do _NOT_ set the partitions like this:
+ * gpmi-nand:100m@0(boot),100m(kernel),1g@200m(rootfs)
+ * The `kernel` partition does not set with the @offset, this is not permitted.
*/
#include <linux/kernel.h>
--
1.7.4.4
This patch is based on the assumption that all the partitions are
in the right offset order.
Assume we have a 1GB(8Gb) nand chip, and we set the partitions
in the command line like this:
#gpmi-nand:100m(boot),100m(kernel),1g(rootfs)
In this case, the partition truncating occurs. The current code will
get the following result:
----------------------------------
root@freescale ~$ cat /proc/mtd
dev: size erasesize name
mtd0: 06400000 00040000 "boot"
mtd1: 06400000 00040000 "kernel"
----------------------------------
It is obvious that we lost the truncated partition `rootfs` which should
be 824M in this case.
Why? The old code sets the wrong partitions number when the truncating
occurs. This patch fixes it. Alao add a `break` to shortcut the code in this
case.
After apply this patch, the result becomes:
----------------------------------
root@freescale ~$ cat /proc/mtd
dev: size erasesize name
mtd0: 06400000 00040000 "boot"
mtd1: 06400000 00040000 "kernel"
mtd2: 33800000 00040000 "rootfs"
----------------------------------
We get the right result.
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/cmdlinepart.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index f40d390..9f0afe5 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -382,7 +382,8 @@ static int parse_cmdline_partitions(struct mtd_info *master,
"%s: partitioning exceeds flash size, truncating\n",
part->mtd_id);
part->parts[i].size = master->size - offset;
- part->num_parts = i;
+ part->num_parts = i + 1;
+ break;
}
offset += part->parts[i].size;
}
--
1.7.4.4
Assume we have a 1GB(8Gb) nand chip.
It is legit if we set the partitions as the following:
gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
But the current code can not parse out any partition with this
cmdline.
This patch sorts the unsorted partitions by the @offset.
For there are maybe only several partitions, i use the simple
Bubble sort algorithm.
Signed-off-by: Huang Shijie <[email protected]>
---
drivers/mtd/cmdlinepart.c | 29 +++++++++++++++++++++++++++++
1 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index 0b7b2ad..f40d390 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -234,6 +234,32 @@ static struct mtd_partition * newpart(char *s,
return parts;
}
+/* There are only several partitions, so the Bubble sort is enough. */
+static inline void sort_partitons(struct mtd_partition *parts, int num_parts)
+{
+ int i, j;
+
+ if (num_parts < 2)
+ return;
+
+ if (parts[0].offset == OFFSET_CONTINUOUS)
+ return;
+
+ /* sort by the offset */
+ for (i = 0; i < num_parts - 1; i++) {
+ for (j = 1; j < num_parts - i; j++) {
+ if (parts[j - 1].offset > parts[j].offset) {
+ struct mtd_partition tmp;
+
+ tmp = parts[j - 1];
+ parts[j - 1] = parts[j];
+ parts[j] = tmp;
+ }
+ }
+ }
+ return;
+}
+
/*
* Parse the command line.
*/
@@ -292,6 +318,9 @@ static int mtdpart_setup_real(char *s)
this_mtd->mtd_id = (char*)(this_mtd + 1);
strlcpy(this_mtd->mtd_id, mtd_id, mtd_id_len + 1);
+ /* sort the partitions */
+ sort_partitons(parts, num_parts);
+
/* link into chain */
this_mtd->next = partitions;
partitions = this_mtd;
--
1.7.4.4
On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
> This patch is based on the assumption that all the partitions are
> in the right offset order.
>
> Assume we have a 1GB(8Gb) nand chip, and we set the partitions
> in the command line like this:
> #gpmi-nand:100m(boot),100m(kernel),1g(rootfs)
>
> In this case, the partition truncating occurs. The current code will
> get the following result:
>
> ----------------------------------
> root@freescale ~$ cat /proc/mtd
> dev: size erasesize name
> mtd0: 06400000 00040000 "boot"
> mtd1: 06400000 00040000 "kernel"
> ----------------------------------
>
> It is obvious that we lost the truncated partition `rootfs` which should
> be 824M in this case.
>
> Why? The old code sets the wrong partitions number when the truncating
> occurs. This patch fixes it. Alao add a `break` to shortcut the code in this
> case.
>
> After apply this patch, the result becomes:
> ----------------------------------
> root@freescale ~$ cat /proc/mtd
> dev: size erasesize name
> mtd0: 06400000 00040000 "boot"
> mtd1: 06400000 00040000 "kernel"
> mtd2: 33800000 00040000 "rootfs"
> ----------------------------------
>
> We get the right result.
>
> Signed-off-by: Huang Shijie <[email protected]>
Should this have CC to -stable?
--
Best Regards,
Artem Bityutskiy
On Thu, Aug 30, 2012 at 2:43 PM, Artem Bityutskiy <[email protected]> wrote:
> On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
>> This patch is based on the assumption that all the partitions are
>> in the right offset order.
>>
>> Assume we have a 1GB(8Gb) nand chip, and we set the partitions
>> in the command line like this:
>> #gpmi-nand:100m(boot),100m(kernel),1g(rootfs)
>>
>> In this case, the partition truncating occurs. The current code will
>> get the following result:
>>
>> ----------------------------------
>> root@freescale ~$ cat /proc/mtd
>> dev: size erasesize name
>> mtd0: 06400000 00040000 "boot"
>> mtd1: 06400000 00040000 "kernel"
>> ----------------------------------
>>
>> It is obvious that we lost the truncated partition `rootfs` which should
>> be 824M in this case.
>>
>> Why? The old code sets the wrong partitions number when the truncating
>> occurs. This patch fixes it. Alao add a `break` to shortcut the code in this
>> case.
>>
>> After apply this patch, the result becomes:
>> ----------------------------------
>> root@freescale ~$ cat /proc/mtd
>> dev: size erasesize name
>> mtd0: 06400000 00040000 "boot"
>> mtd1: 06400000 00040000 "kernel"
>> mtd2: 33800000 00040000 "rootfs"
>> ----------------------------------
>>
>> We get the right result.
>>
>> Signed-off-by: Huang Shijie <[email protected]>
>
> Should this have CC to -stable?
It's ok to CC to stable.
thanks
Huang Shijie
>
> --
> Best Regards,
> Artem Bityutskiy
On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
> There are typically two types to set the mtd partitions:
>
> <1> set with the `size`, such as
> gpmi-nand:100m(boot),100m(kernel),1g(rootfs)
>
> <2> set with the `offset`, such as
> gpmi-nand:100m@0(boot),100m@100m(kernel),1g@200m(rootfs)
> gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
>
> If we mix these two types, such as:
> gpmi-nand:100m@0(boot),100m(kernel),1g@200m(rootfs)
> gpmi-nand:1g@200m(rootfs),100m@0(boot),100m(kernel)
>
> It's hard to understand the cmdline. And also it is hard to sort the
> partitions in this mixed type. So we explicitly forbid the mixed type.
So "explicitly forbid" is just to add a "do not do this" comment?
--
Best Regards,
Artem Bityutskiy
On Fri, Aug 31, 2012 at 7:45 AM, Artem Bityutskiy <[email protected]> wrote:
> On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
>> There are typically two types to set the mtd partitions:
>>
>> <1> set with the `size`, such as
>> gpmi-nand:100m(boot),100m(kernel),1g(rootfs)
>>
>> <2> set with the `offset`, such as
>> gpmi-nand:100m@0(boot),100m@100m(kernel),1g@200m(rootfs)
>> gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
>>
>> If we mix these two types, such as:
>> gpmi-nand:100m@0(boot),100m(kernel),1g@200m(rootfs)
>> gpmi-nand:1g@200m(rootfs),100m@0(boot),100m(kernel)
>>
>> It's hard to understand the cmdline. And also it is hard to sort the
>> partitions in this mixed type. So we explicitly forbid the mixed type.
>
> So "explicitly forbid" is just to add a "do not do this" comment?
>
This is the simplest way. ;)
It's make code complicated if we change the code to forbid this mixed type.
Best Regards
Huang Shijie
On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
> Assume we have a 1GB(8Gb) nand chip.
> It is legit if we set the partitions as the following:
> gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
>
> But the current code can not parse out any partition with this
> cmdline.
>
> This patch sorts the unsorted partitions by the @offset.
> For there are maybe only several partitions, i use the simple
> Bubble sort algorithm.
>
> Signed-off-by: Huang Shijie <[email protected]>
I still cannot find time to actually think about this carefully, but the
commit message does not sound convincing, it does not explain why
sorting is the right way to fix the issue, and what would be the
alternatives. It actually also does not explain why exactly we currently
cannot parse the example string.
--
Best Regards,
Artem Bityutskiy
On Fri, Aug 31, 2012 at 9:59 AM, Artem Bityutskiy <[email protected]> wrote:
> On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
>> Assume we have a 1GB(8Gb) nand chip.
>> It is legit if we set the partitions as the following:
>> gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
>>
>> But the current code can not parse out any partition with this
>> cmdline.
>>
>> This patch sorts the unsorted partitions by the @offset.
>> For there are maybe only several partitions, i use the simple
>> Bubble sort algorithm.
>>
>> Signed-off-by: Huang Shijie <[email protected]>
>
> I still cannot find time to actually think about this carefully, but the
> commit message does not sound convincing, it does not explain why
> sorting is the right way to fix the issue, and what would be the
> alternatives. It actually also does not explain why exactly we currently
> cannot parse the example string.
>
thanks .
I will add more comment in the next version.
Best Regards
Huang Shijie
On Fri, Aug 31, 2012 at 7:45 AM, Artem Bityutskiy <[email protected]> wrote:
> On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
>> There are typically two types to set the mtd partitions:
>>
>> <1> set with the `size`, such as
>> gpmi-nand:100m(boot),100m(kernel),1g(rootfs)
>>
>> <2> set with the `offset`, such as
>> gpmi-nand:100m@0(boot),100m@100m(kernel),1g@200m(rootfs)
>> gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
>>
>> If we mix these two types, such as:
>> gpmi-nand:100m@0(boot),100m(kernel),1g@200m(rootfs)
>> gpmi-nand:1g@200m(rootfs),100m@0(boot),100m(kernel)
>>
>> It's hard to understand the cmdline. And also it is hard to sort the
>> partitions in this mixed type. So we explicitly forbid the mixed type.
>
> So "explicitly forbid" is just to add a "do not do this" comment?
>
Do you think we should change the code to forbid this mixed type?
Huang Shijie