lkml.org 
[lkml]   [2016]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms
    On Mon, 18 Apr 2016 16:48:26 +0300
    Roger Quadros <rogerq@ti.com> 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@ti.com> 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@atomide.com> 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?

    >
    > >>
    > >> 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).


    --
    Boris Brezillon, Free Electrons
    Embedded Linux and Kernel engineering
    http://free-electrons.com

    \
     
     \ /
      Last update: 2016-04-18 16:41    [W:5.456 / U:0.268 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site