On Tue, 19 Apr 2016 15:46:19 +0300 Roger Quadros <rogerq@xxxxxx> wrote: > Boris, > > On 18/04/16 17:57, Boris Brezillon wrote: > > On Mon, 18 Apr 2016 17:39:01 +0300 > > Roger Quadros <rogerq@xxxxxx> wrote: > > > >> On 18/04/16 17:10, Boris Brezillon wrote: > >>> On Mon, 18 Apr 2016 16:48:26 +0300 > >>> Roger Quadros <rogerq@xxxxxx> wrote: > >>> > >>>> Boris, > >>>> > >>>> On 18/04/16 16:13, Boris Brezillon wrote: > >>>>> 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. > >>>> > >>>> The controller can read the full OOB region but we only want it to read just > >>>> the ECC bytes because that is the way the ELM ECC engine works. > >>> > >>> Ok, I think I got it: the ECC correction is pipelined with data read, > >>> and the controller expect to have ECC bytes right after the in-band > >>> data, is that correct? > >> > >> That is correct. > >> > >>> > >>>> > >>>>>> > >>>>>> 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]. > >>>> > >>>> We just need the position of the first ECC byte offset. > >>>> Is that the most optimal way to get it? > >>> > >>> For the BCH case, it seems that ECC bytes always start at offset > >>> BADBLOCK_MARKER_LENGTH, so you can just pass > >>> mtd->writesize + BADBLOCK_MARKER_LENGTH. > >>> > >>> Let me know if this works, and I'll squash those changes into the > >>> faulty commit (I know this implies a rebase + push -f, but IMO that's > >>> better than breaking bisectability). > >>> > >>> > >> > >> So, the below patch works as well. Please feel free to fold it with your patch. > > > > Will do. > > > > Thanks, > > > > Boris > > After all the changes we discussed in [1] I was able to test nand/next-with-gpmc-rework > and it worked fine. > > [1] - http://thread.gmane.org/gmane.comp.hardware.netbook.arm.sunxi/22596/focus=22936 > > I'd be happy to test the branch again after you've incorporated all changes. > > Since you are gong to do a push -f anyways, I was wondering if you want to pull in my > gpmc branch first to avoid the merge conflict. But it is totally up to you. Sure, I can do that. -- 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
