Patchwork cmdline partition parsing behavior has changed: regression?

login
register
mail settings
Submitter Javier Martin
Date Jan. 30, 2013, 11:23 a.m.
Message ID <CACKLOr3XoQfTuy4XpORz4QwqPhfEa8mQNzdKdrbJ2UCA-X+MhQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/216843/
State New
Headers show

Comments

Javier Martin - Jan. 30, 2013, 11:23 a.m.
Hi,
I've been using the following cmdline string to boot my kernel for
several releases until 3.6:

"console=ttymxc0,115200 mem=54MB init=/sbin/preinit
root=/dev/mtdblock2
mtdparts=physmap-flash.0:384k(U-Boot),2560k(kernel0),27904k(rootfs0),2048k(config0),2560k(kernel1),27904k(rootfs1),2048k(config1),32k(env1),32k(env2);"

Partition parsing always has worked properly and the "physmap-flash.0"
NOR flash was properly detected and partitioned.

However, in 3.8-rc3 I've found that this string does not work any
longer. In fact I had to add the following dirty hack to make it work:

-------
    mtd: dirty hack to fix cmdline partition parsing.

    Behavior of cmdline partition parsing seems to have changed
    recently. Currently, it doesn't like that we end our partition
    description string with a ';' character.

    A more suitable solution is yet to be found.

    Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>


Is this a regression? Should I just change my cmdline string?

Regards.
Christopher Cordahi - Jan. 31, 2013, 12:46 a.m.
Hi Javier,

On Wed, Jan 30, 2013 at 6:23 AM, javier Martin
<javier.martin@vista-silicon.com> wrote:
> I've been using the following cmdline string to boot my kernel for
> several releases until 3.6:
>
> "console=ttymxc0,115200 mem=54MB init=/sbin/preinit
> root=/dev/mtdblock2
> mtdparts=physmap-flash.0:384k(U-Boot),2560k(kernel0),27904k(rootfs0),2048k(config0),2560k(kernel1),27904k(rootfs1),2048k(config1),32k(env1),32k(env2);"

The semicolon is meant to be a separator, not a terminator.  See
comment at top of file that predates 2.6.12
The format for the command line is as follows:
mtdparts=<mtddef>[;<mtddef]

> Partition parsing always has worked properly and the "physmap-flash.0"
> NOR flash was properly detected and partitioned.

I suspect you were always getting a warning on boot prior to the first
mtd partition listing
mtd: no mtd-id

> However, in 3.8-rc3 I've found that this string does not work any
> longer.

This technically invalid mtdparts string is now treated as an error
with revised error handling
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=9e0606fc4ea27fb275f6987751224c60ee055ef1

> In fact I had to add the following dirty hack to make it work:
>
> diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
> index c533f27..4ef0e7a 100644
> --- a/drivers/mtd/cmdlinepart.c
> +++ b/drivers/mtd/cmdlinepart.c
> @@ -271,6 +271,10 @@ static int mtdpart_setup_real(char *s)
>                      this_mtd->mtd_id, this_mtd->num_parts));
>
>
> +               /* FIXME: dirty hack */
> +               if (*s == ';')
> +                       return 0;
> +
>                 /* EOS - we're done */
>                 if (*s == 0)
>                         break;

I believe this "dirty hack" will break any implementations (including
mine) with more than one MTD part definition where the semicolon is
required as a separator.  A less intrusive and forgiving "hack" is to
add an identical "EOS - we're done" test and break immediately before
the error is returned where "mtd: no mtd-id" is output.

--
Chris

Patch

diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index c533f27..4ef0e7a 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -271,6 +271,10 @@  static int mtdpart_setup_real(char *s)
                     this_mtd->mtd_id, this_mtd->num_parts));


+               /* FIXME: dirty hack */
+               if (*s == ';')
+                       return 0;
+
                /* EOS - we're done */
                if (*s == 0)
                        break;