Hi Roger, On Mon, 18 Apr 2016 15:52:58 +0300 Roger Quadros <rogerq@xxxxxx> wrote: > On 18/04/16 15:31, Roger Quadros wrote: > > On 16/04/16 11:57, Boris Brezillon wrote: > >> On Fri, 15 Apr 2016 09:19:51 -0700 > >> Tony Lindgren <tony@xxxxxxxxxxx> wrote: > >> > >>> > >>>> Or should I just pull this immutable branch in my current nand/next and > >>>> let you pull the same immutable branch in omap-soc. I mean, would this > >>>> prevent conflicts when our branches are merged into linux-next, no > >>>> matter the order. > >>> > >>> Ideally just one or more branches with just minimal changes in > >>> them against -rc1. But you may have other dependencies in > >>> your NAND tree so that may no longer be doable :) Usually if > >>> I merge something that may need to get merged into other > >>> branches, I just apply them into a separate branch against -rc1 > >>> to start with, then merge that branch in. > >> > >> Okay, in this case, that's pretty much what I did from the beginning, > >> except the immutable branch was provided by Roger (based on 4.6-rc1). > >> Thanks for this detailed explanation, I'll try to remember that when > >> I'll need to provide an immutable branch for another subsystem. > >> > >> Roger, my request remains, could you check/test my conflict resolution > >> (branch nand/next-with-gpmc-rework)? > > > > I couldn't test that branch yet as nand/next is broken on omap platforms > > (at least on dra7-evm). > > > > The commit where it breaks is: > > a662ef4 mtd: nand: omap2: use mtd_ooblayout_xxx() helpers where appropriate > > > > I'm trying to figure out what went wrong there. Failure log below. > > OK. I was able to fix it when at commit a662ef4 with the below patch. Thanks for debugging that. > > Looks like we need to read exactly the ECC bytes through the ECC engine and not > the entire OOB region. Hm, it looks like there's a bug somewhere else, because I don't see any reason why the controller wouldn't be able to read the full OOB region. > > --cheers, > -oger > > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c > index e622a1b..46b61d2 100644 > --- a/drivers/mtd/nand/omap2.c > +++ b/drivers/mtd/nand/omap2.c > @@ -1547,8 +1547,8 @@ static int omap_read_page_bch(struct mtd_info *mtd, struct nand_chip *chip, > chip->read_buf(mtd, buf, mtd->writesize); > > /* Read oob bytes */ > - chip->cmdfunc(mtd, NAND_CMD_RNDOUT, mtd->writesize, -1); > - chip->read_buf(mtd, chip->oob_poi, mtd->oobsize); > + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, mtd->writesize + chip->ecc.layout->eccpos[0], -1); The whole point of this series is to get rid of chip->ecc.layout, so we'd rather use the mtd_ooblayout_find_eccregion() instead of chip->ecc.layout->eccpos[0]. > + chip->read_buf(mtd, chip->oob_poi, chip->ecc.total); Can you print the ->oobsize, ->writesize, chip->ecc.layout->eccpos[0] and chip->ecc.total values here. I'll also need your NAND page layout (page size and OOB size provided in the datasheet). Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
