Hi Boris,

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

> On Wed, 11 Jul 2018 15:51:39 +0200
> Miquel Raynal <miquel.ray...@bootlin.com> wrote:
> 
> > > > +
> > > > +       argc -= 3;
> > > > +       argv += 3;
> > > > +
> > > > +       /* Do the parsing */
> > > > +       if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "write", 5)) {
> > > > +               bool read, raw, woob;
> > > > +               uint *waddr = NULL;
> > > > +               u64 off, len;
> > > > +
> > > > +               read = !strncmp(cmd, "read", 4);
> > > > +               raw = strstr(cmd, ".raw");
> > > > +               woob = strstr(cmd, ".oob");
> > > > +
> > > > +               if (!read) {
> > > > +                       if (argc < 1)
> > > > +                               return CMD_RET_USAGE;
> > > > +
> > > > +                       waddr = map_sysmem(simple_strtoul(argv[0], 
> > > > NULL, 10),
> > > > +                                          0);
> > > > +                       argc--;
> > > > +                       argv++;
> > > > +               }
> > > > +
> > > > +               off = argc > 0 ? simple_strtoul(argv[0], NULL, 10) : 0;
> > > > +               len = 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 != (mtd->writesize + mtd->oobsize))) {
> > > > +                       printf("OOB operations are limited to single 
> > > > pages\n");
> > > > +                       return -EINVAL;
> > > > +               }      
> > > 
> > > Is this a uboot limitation? I don't think you have such a limitation in
> > > Linux.    
> > 
> > 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.  
> 
> 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.

> 
> >   
> > >     
> > > > +
> > > > +               if ((off + len) >= mtd->size) {      
> > > 
> > > That doesn't work when reading the last page of the MTD device with
> > > woob = 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)?    
> > 
> > Because the relevant patch (and its fix :) ) has not been backported
> > yet.
> > 
> > And now I understand your voice in my ears "do it".  
> 
> Hehe. So I guess I'll see this patch in your v2 :-).

Indeed ;)

Cheers,
Miquèl

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to