From mboxrd@z Thu Jan  1 00:00:00 1970
From: Miquel Raynal <miquel.raynal@bootlin.com>
Date: Mon, 3 Sep 2018 09:13:31 +0200
Subject: [U-Boot] [PATCH v7 05/13] cmd: mtdparts: remove mandatory
 'mtdparts=' prefix
In-Reply-To: <1a5afaf5-3860-e9f9-3345-2a7f0ff7d4c0@denx.de>
References: <20180831145741.17350-1-miquel.raynal@bootlin.com>
 <20180831145741.17350-6-miquel.raynal@bootlin.com>
 <1a5afaf5-3860-e9f9-3345-2a7f0ff7d4c0@denx.de>
Message-ID: <20180903091331.24927011@xps13>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
To: u-boot@lists.denx.de

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=3D... =20
> > > While this may ease the partition declaration job to be passed to =20
> > 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. =20
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> =20
> > Acked-by: Jagan Teki <jagan@openedev.com>
> > ---
> >   cmd/mtdparts.c | 17 ++++++-----------
> >   1 file changed, 6 insertions(+), 11 deletions(-) =20
> > > diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c =20
> > index 2e547894c6..f7ed1a0779 100644
> > --- a/cmd/mtdparts.c
> > +++ b/cmd/mtdparts.c
> > @@ -44,7 +44,7 @@
> >    *
> >    * 'mtdparts' - partition list
> >    *
> > - * mtdparts=3Dmtdparts=3D<mtd-def>[;<mtd-def>...]
> > + * mtdparts=3D[mtdparts=3D]<mtd-def>[;<mtd-def>...]
> >    *
> >    * <mtd-def>  :=3D <mtd-id>:<part-def>[,<part-def>...]
> >    * <mtd-id>   :=3D 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=3Dnor0=3Dedb7312-nor
> > - * mtdparts=3Dmtdparts=3Dedb7312-nor:-
> > + * mtdparts=3D[mtdparts=3D]edb7312-nor:-
> >    *
> >    * 1 NOR Flash with 2 partitions, 1 NAND with one
> >    * mtdids=3Dnor0=3Dedb7312-nor,nand0=3Dedb7312-nand
> > - * mtdparts=3Dmtdparts=3Dedb7312-nor:256k(ARMboot)ro,-(root);edb7312-n=
and:-(home)
> > + * mtdparts=3D[mtdparts=3D]edb7312-nor:256k(ARMboot)ro,-(root);edb7312=
-nand:-(home)
> >    *
> >    */ =20
> >   > @@ -1099,9 +1099,6 @@ static int generate_mtdparts(char *buf, u32 b=
uflen) =20
> >   		return 0;
> >   	} =20
> >   > -	strcpy(p, "mtdparts=3D"); =20
> > -	p +=3D 9;
> > -
> >   	list_for_each(dentry, &devices) {
> >   		dev =3D list_entry(dentry, struct mtd_device, link); =20
> >   > @@ -1572,11 +1569,9 @@ static int parse_mtdparts(const char *const =
mtdparts) =20
> >   	if (!p)
> >   		p =3D mtdparts; =20
> >   > -	if (strncmp(p, "mtdparts=3D", 9) !=3D 0) { =20
> > -		printf("mtdparts variable doesn't start with 'mtdparts=3D'\n");
> > -		return err;
> > -	}
> > -	p +=3D 9;
> > +	/* Skip the useless prefix, if any */
> > +	if (strncmp(p, "mtdparts=3D", 9) =3D=3D 0)
> > +		p +=3D 9; =20
> >   >   	while (*p !=3D '\0') { =20
> >   		err =3D 1;
> >  =20
> 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=3Dmtddev0:-(all)"
setenv bootargs "console=3D/dev/tty0 [...] ${mtdparts}"

instead of this way:

setenv mtdparts "mtddev0:-(all)"
setenv bootargs "console=3D/dev/tty0 [...] mtdparts=3D${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=C3=A8l