From mboxrd@z Thu Jan  1 00:00:00 1970
From: Miquel Raynal <miquel.raynal@bootlin.com>
Date: Wed, 11 Jul 2018 16:17:38 +0200
Subject: [U-Boot] [RFC PATCH 17/20] cmd: mtd: add 'mtd' command
In-Reply-To: <20180711160109.727fd22f@bbrezillon>
References: <20180606153040.21397-1-miquel.raynal@bootlin.com>
 <20180606153040.21397-18-miquel.raynal@bootlin.com>
 <20180606214524.550442e2@bbrezillon>
 <20180711155139.69f381e2@xps13>
 <20180711160109.727fd22f@bbrezillon>
Message-ID: <20180711161738.1af8d84b@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 Boris,

Boris Brezillon <boris.brezillon@bootlin.com> wrote on Wed, 11 Jul 2018
16:01:09 +0200:

> On Wed, 11 Jul 2018 15:51:39 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>=20
> > > > +
> > > > +	argc -=3D 3;
> > > > +	argv +=3D 3;
> > > > +
> > > > +	/* Do the parsing */
> > > > +	if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "write", 5)) {
> > > > +		bool read, raw, woob;
> > > > +		uint *waddr =3D NULL;
> > > > +		u64 off, len;
> > > > +
> > > > +		read =3D !strncmp(cmd, "read", 4);
> > > > +		raw =3D strstr(cmd, ".raw");
> > > > +		woob =3D strstr(cmd, ".oob");
> > > > +
> > > > +		if (!read) {
> > > > +			if (argc < 1)
> > > > +				return CMD_RET_USAGE;
> > > > +
> > > > +			waddr =3D map_sysmem(simple_strtoul(argv[0], NULL, 10),
> > > > +					   0);
> > > > +			argc--;
> > > > +			argv++;
> > > > +		}
> > > > +
> > > > +		off =3D argc > 0 ? simple_strtoul(argv[0], NULL, 10) : 0;
> > > > +		len =3D argc > 1 ? simple_strtoul(argv[1], NULL, 10) :
> > > > +				 mtd->writesize + (woob ? mtd->oobsize : 0);
> > > > +
> > > > +		if ((u32)off % mtd->writesize) {
> > > > +			printf("Section not page-aligned (0x%x)\n",
> > > > +			       mtd->writesize);
> > > > +			return -EINVAL;
> > > > +		}
> > > > +
> > > > +		if (woob && (len !=3D (mtd->writesize + mtd->oobsize))) {
> > > > +			printf("OOB operations are limited to single pages\n");
> > > > +			return -EINVAL;
> > > > +		}     =20
> > >=20
> > > Is this a uboot limitation? I don't think you have such a limitation =
in
> > > Linux.   =20
> >=20
> > Kind of, only one single page write with OOB at a time is possible
> > says a comment on mtd_oob_ops in mtd.h in Linux. Reads are actually not
> > limited. But I really prefer to keep this limitation that simplifies _a
> > lot_ the logic and is not really useful to a u-boot user I suppose. =20
>=20
> It is useful when you write things in raw mode and the image you write
> contains OOB (which in turn contains pre-generated ECC bytes). I know we
> do such things on sunxi when writing the SPL.

:'(

Ok.

>=20
> >  =20
> > >    =20
> > > > +
> > > > +		if ((off + len) >=3D mtd->size) {     =20
> > >=20
> > > That doesn't work when reading the last page of the MTD device with
> > > woob =3D true. See how Linux handle that here [1]. BTW, why don't you=
 let
> > > mtdcore.c do these checks for you (that's also true for unaligned
> > > accesses)?   =20
> >=20
> > Because the relevant patch (and its fix :) ) has not been backported
> > yet.
> >=20
> > And now I understand your voice in my ears "do it". =20
>=20
> Hehe. So I guess I'll see this patch in your v2 :-).

Indeed ;)

Cheers,
Miqu=C3=A8l