Message ID | 20180831145741.17350-6-miquel.raynal@bootlin.com |
---|---|
State | Changes Requested |
Delegated to: | Jagannadha Sutradharudu Teki |
Headers | show |
Series | Cleaner MTD devices management | expand |
On 31.08.2018 16:57, Miquel Raynal wrote: > All U-Boot users must define the mtdparts environment variable with: > setenv mtdparts mtdparts=... > > While this may ease the partition declaration job to be passed to > Linux, this is a pure software limitation and forcing this prefix is a > complete non-sense. Let the user to declare manually the mtdparts > variable without the prefix. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > Acked-by: Jagan Teki <jagan@openedev.com> > --- > cmd/mtdparts.c | 17 ++++++----------- > 1 file changed, 6 insertions(+), 11 deletions(-) > > diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c > index 2e547894c6..f7ed1a0779 100644 > --- a/cmd/mtdparts.c > +++ b/cmd/mtdparts.c > @@ -44,7 +44,7 @@ > * > * 'mtdparts' - partition list > * > - * mtdparts=mtdparts=<mtd-def>[;<mtd-def>...] > + * mtdparts=[mtdparts=]<mtd-def>[;<mtd-def>...] > * > * <mtd-def> := <mtd-id>:<part-def>[,<part-def>...] > * <mtd-id> := unique device tag used by linux kernel to find mtd device (mtd->name) > @@ -62,11 +62,11 @@ > * > * 1 NOR Flash, with 1 single writable partition: > * mtdids=nor0=edb7312-nor > - * mtdparts=mtdparts=edb7312-nor:- > + * mtdparts=[mtdparts=]edb7312-nor:- > * > * 1 NOR Flash with 2 partitions, 1 NAND with one > * mtdids=nor0=edb7312-nor,nand0=edb7312-nand > - * mtdparts=mtdparts=edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) > + * mtdparts=[mtdparts=]edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) > * > */ > > @@ -1099,9 +1099,6 @@ static int generate_mtdparts(char *buf, u32 buflen) > return 0; > } > > - strcpy(p, "mtdparts="); > - p += 9; > - > list_for_each(dentry, &devices) { > dev = list_entry(dentry, struct mtd_device, link); > > @@ -1572,11 +1569,9 @@ static int parse_mtdparts(const char *const mtdparts) > if (!p) > p = mtdparts; > > - if (strncmp(p, "mtdparts=", 9) != 0) { > - printf("mtdparts variable doesn't start with 'mtdparts='\n"); > - return err; > - } > - p += 9; > + /* Skip the useless prefix, if any */ > + if (strncmp(p, "mtdparts=", 9) == 0) > + p += 9; > > while (*p != '\0') { > err = 1; > I always wondered about this double mtdparts thing but was never brave enough to touch it. ;) Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan
Hi Stefan, Stefan Roese <sr@denx.de> wrote on Sat, 1 Sep 2018 10:50:54 +0200: > On 31.08.2018 16:57, Miquel Raynal wrote: > > All U-Boot users must define the mtdparts environment variable with: > > setenv mtdparts mtdparts=... > > > While this may ease the partition declaration job to be passed to > > Linux, this is a pure software limitation and forcing this prefix is a > > complete non-sense. Let the user to declare manually the mtdparts > > variable without the prefix. > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > Acked-by: Jagan Teki <jagan@openedev.com> > > --- > > cmd/mtdparts.c | 17 ++++++----------- > > 1 file changed, 6 insertions(+), 11 deletions(-) > > > diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c > > index 2e547894c6..f7ed1a0779 100644 > > --- a/cmd/mtdparts.c > > +++ b/cmd/mtdparts.c > > @@ -44,7 +44,7 @@ > > * > > * 'mtdparts' - partition list > > * > > - * mtdparts=mtdparts=<mtd-def>[;<mtd-def>...] > > + * mtdparts=[mtdparts=]<mtd-def>[;<mtd-def>...] > > * > > * <mtd-def> := <mtd-id>:<part-def>[,<part-def>...] > > * <mtd-id> := unique device tag used by linux kernel to find mtd device (mtd->name) > > @@ -62,11 +62,11 @@ > > * > > * 1 NOR Flash, with 1 single writable partition: > > * mtdids=nor0=edb7312-nor > > - * mtdparts=mtdparts=edb7312-nor:- > > + * mtdparts=[mtdparts=]edb7312-nor:- > > * > > * 1 NOR Flash with 2 partitions, 1 NAND with one > > * mtdids=nor0=edb7312-nor,nand0=edb7312-nand > > - * mtdparts=mtdparts=edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) > > + * mtdparts=[mtdparts=]edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) > > * > > */ > > > @@ -1099,9 +1099,6 @@ static int generate_mtdparts(char *buf, u32 buflen) > > return 0; > > } > > > - strcpy(p, "mtdparts="); > > - p += 9; > > - > > list_for_each(dentry, &devices) { > > dev = list_entry(dentry, struct mtd_device, link); > > > @@ -1572,11 +1569,9 @@ static int parse_mtdparts(const char *const mtdparts) > > if (!p) > > p = mtdparts; > > > - if (strncmp(p, "mtdparts=", 9) != 0) { > > - printf("mtdparts variable doesn't start with 'mtdparts='\n"); > > - return err; > > - } > > - p += 9; > > + /* Skip the useless prefix, if any */ > > + if (strncmp(p, "mtdparts=", 9) == 0) > > + p += 9; > > > while (*p != '\0') { > > err = 1; > > > I always wondered about this double mtdparts thing but was never brave > enough to touch it. ;) If I understand correctly, this double mtdparts thing comes from lazy people who decided to build the bootargs this way: setenv mtdparts "mtdparts=mtddev0:-(all)" setenv bootargs "console=/dev/tty0 [...] ${mtdparts}" instead of this way: setenv mtdparts "mtddev0:-(all)" setenv bootargs "console=/dev/tty0 [...] mtdparts=${mtdparts}" Which, personally, I find much clearer. I'm still puzzled by the real use of mtdids, I will check this week if I didn't broke anything in the way U-Boot and Linux share the MTD devices and their partitions. Thanks, Miquèl
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c index 2e547894c6..f7ed1a0779 100644 --- a/cmd/mtdparts.c +++ b/cmd/mtdparts.c @@ -44,7 +44,7 @@ * * 'mtdparts' - partition list * - * mtdparts=mtdparts=<mtd-def>[;<mtd-def>...] + * mtdparts=[mtdparts=]<mtd-def>[;<mtd-def>...] * * <mtd-def> := <mtd-id>:<part-def>[,<part-def>...] * <mtd-id> := unique device tag used by linux kernel to find mtd device (mtd->name) @@ -62,11 +62,11 @@ * * 1 NOR Flash, with 1 single writable partition: * mtdids=nor0=edb7312-nor - * mtdparts=mtdparts=edb7312-nor:- + * mtdparts=[mtdparts=]edb7312-nor:- * * 1 NOR Flash with 2 partitions, 1 NAND with one * mtdids=nor0=edb7312-nor,nand0=edb7312-nand - * mtdparts=mtdparts=edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) + * mtdparts=[mtdparts=]edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) * */ @@ -1099,9 +1099,6 @@ static int generate_mtdparts(char *buf, u32 buflen) return 0; } - strcpy(p, "mtdparts="); - p += 9; - list_for_each(dentry, &devices) { dev = list_entry(dentry, struct mtd_device, link); @@ -1572,11 +1569,9 @@ static int parse_mtdparts(const char *const mtdparts) if (!p) p = mtdparts; - if (strncmp(p, "mtdparts=", 9) != 0) { - printf("mtdparts variable doesn't start with 'mtdparts='\n"); - return err; - } - p += 9; + /* Skip the useless prefix, if any */ + if (strncmp(p, "mtdparts=", 9) == 0) + p += 9; while (*p != '\0') { err = 1;