Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1380512 > unrolled thread

Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC  NAND on non-OMAP platforms Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-16 11:00 +0200
    Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Roger Quadros <rogerq@ti.com> - 2016-04-18 14:40 +0200
      Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Roger Quadros <rogerq@ti.com> - 2016-04-18 15:00 +0200
        Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC  NAND on non-OMAP platforms Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-18 15:20 +0200
          Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Roger Quadros <rogerq@ti.com> - 2016-04-18 15:50 +0200
            Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC  NAND on non-OMAP platforms Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-18 16:20 +0200
              Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Roger Quadros <rogerq@ti.com> - 2016-04-18 16:50 +0200
                Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC  NAND on non-OMAP platforms Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-18 17:00 +0200
                  Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Roger Quadros <rogerq@ti.com> - 2016-04-19 14:50 +0200
                    Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC  NAND on non-OMAP platforms Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-19 15:00 +0200
                    Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC  NAND on non-OMAP platforms Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-19 22:20 +0200
                      Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Roger Quadros <rogerq@ti.com> - 2016-04-20 11:00 +0200
                      Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Tony Lindgren <tony@atomide.com> - 2016-04-20 16:50 +0200
                Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC  NAND on non-OMAP platforms Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-19 15:30 +0200
                  Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Roger Quadros <rogerq@ti.com> - 2016-04-19 16:30 +0200
                    Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND  on non-OMAP platforms Roger Quadros <rogerq@ti.com> - 2016-04-19 17:00 +0200

#1380512 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

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

Thanks,

Boris

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

[toc] | [next] | [standalone]

#1381666 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

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.

--cheers,
-roger

== attaching ubi to mtd9
[   27.173973] ubi0: attaching mtd9
[   27.178311] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.184828] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.191324] ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 0:0, read only 64 bytes, retry
[   27.203378] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.209860] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.216388] ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 0:0, read only 64 bytes, retry
[   27.228468] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.234976] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.241471] ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 0:0, read only 64 bytes, retry
[   27.253802] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.260278] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.266812] ubi0 error: ubi_io_read: error -74 (ECC error) while reading 64 bytes from PEB 0:0, read 64 bytes
[   27.277254] CPU: 0 PID: 2032 Comm: ubiattach Not tainted 4.6.0-rc1-00053-ga662ef4 #625
[   27.285549] Hardware name: Generic DRA74X (Flattened Device Tree)
[   27.291949] [<c010feec>] (unwind_backtrace) from [<c010c110>] (show_stack+0x10/0x14)
[   27.300083] [<c010c110>] (show_stack) from [<c0470f24>] (dump_stack+0xac/0xe0)
[   27.307664] [<c0470f24>] (dump_stack) from [<c05a5f8c>] (ubi_io_read+0x11c/0x2fc)
[   27.315511] [<c05a5f8c>] (ubi_io_read) from [<c05a6388>] (ubi_io_read_ec_hdr+0x44/0x228)
[   27.323989] [<c05a6388>] (ubi_io_read_ec_hdr) from [<c05aaef8>] (ubi_attach+0x138/0x149c)
[   27.332579] [<c05aaef8>] (ubi_attach) from [<c059fc78>] (ubi_attach_mtd_dev+0x3d0/0xbe4)
[   27.341063] [<c059fc78>] (ubi_attach_mtd_dev) from [<c05a15d4>] (ctrl_cdev_ioctl+0xe4/0x224)
[   27.349928] [<c05a15d4>] (ctrl_cdev_ioctl) from [<c029e380>] (do_vfs_ioctl+0x90/0xa2c)
[   27.358242] [<c029e380>] (do_vfs_ioctl) from [<c029ed88>] (SyS_ioctl+0x6c/0x7c)
[   27.365910] [<c029ed88>] (SyS_ioctl) from [<c0107840>] (ret_fast_syscall+0x0/0x1c)
[   27.374541] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.381025] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.387551] ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 512 bytes from PEB 0:512, read only 512 bytes, retry
[   27.399953] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.406465] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.412950] ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 512 bytes from PEB 0:512, read only 512 bytes, retry
[   27.425349] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.431837] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.438356] ubi0 warning: ubi_io_read: error -74 (ECC error) while reading 512 bytes from PEB 0:512, read only 512 bytes, retry
[   27.450735] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.457243] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.463739] ubi0 error: ubi_io_read: error -74 (ECC error) while reading 512 bytes from PEB 0:512, read 512 bytes
[   27.474526] CPU: 0 PID: 2032 Comm: ubiattach Not tainted 4.6.0-rc1-00053-ga662ef4 #625
[   27.482824] Hardware name: Generic DRA74X (Flattened Device Tree)
[   27.489218] [<c010feec>] (unwind_backtrace) from [<c010c110>] (show_stack+0x10/0x14)
[   27.497348] [<c010c110>] (show_stack) from [<c0470f24>] (dump_stack+0xac/0xe0)
[   27.504923] [<c0470f24>] (dump_stack) from [<c05a5f8c>] (ubi_io_read+0x11c/0x2fc)
[   27.512772] [<c05a5f8c>] (ubi_io_read) from [<c05a65b8>] (ubi_io_read_vid_hdr+0x4c/0x230)
[   27.521355] [<c05a65b8>] (ubi_io_read_vid_hdr) from [<c05ab04c>] (ubi_attach+0x28c/0x149c)
[   27.530024] [<c05ab04c>] (ubi_attach) from [<c059fc78>] (ubi_attach_mtd_dev+0x3d0/0xbe4)
[   27.538517] [<c059fc78>] (ubi_attach_mtd_dev) from [<c05a15d4>] (ctrl_cdev_ioctl+0xe4/0x224)
[   27.547409] [<c05a15d4>] (ctrl_cdev_ioctl) from [<c029e380>] (do_vfs_ioctl+0x90/0xa2c)
[   27.555736] [<c029e380>] (do_vfs_ioctl) from [<c029ed88>] (SyS_ioctl+0x6c/0x7c)
[   27.563415] [<c029ed88>] (SyS_ioctl) from [<c0107840>] (ret_fast_syscall+0x0/0x1c)
[   27.572208] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.579160] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.586560] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.593311] omap2-nand omap2-nand.0: uncorrectable bit-flips found
[   27.600038] omap2-nand omap2-nand.0: uncorrectable bit-flips found

[toc] | [prev] | [next] | [standalone]

#1381671 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

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.

Looks like we need to read exactly the ECC bytes through the ECC engine and not
the entire 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);
+	chip->read_buf(mtd, chip->oob_poi, chip->ecc.total);
 
 	/* Calculate ecc bytes */
 	chip->ecc.calculate(mtd, buf, ecc_calc);

[toc] | [prev] | [next] | [standalone]

#1381684

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.

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

[toc] | [prev] | [next] | [standalone]

#1381721 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

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.

>>
>> 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?

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

eccpos[0]: 2, oobsize 64, ecctotal 56, writesize 2048

Nand part is MT29F2G16ABAEAWP
This has page size 2048 bytes and OOB size 64 bytes.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1381740

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

[toc] | [prev] | [next] | [standalone]

#1381778 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

On 18/04/16 17:10, Boris Brezillon wrote:
> 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?

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.

--
cheers,
-roger

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index e622a1b..eb85d6b 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 + BADBLOCK_MARKER_LENGTH, -1);
+	chip->read_buf(mtd, chip->oob_poi, chip->ecc.total);
 
 	/* Calculate ecc bytes */
 	chip->ecc.calculate(mtd, buf, ecc_calc);

[toc] | [prev] | [next] | [standalone]

#1381788

On Mon, 18 Apr 2016 17:39:01 +0300
Roger Quadros <rogerq@ti.com> wrote:

> On 18/04/16 17:10, Boris Brezillon wrote:
> > 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?
> 
> 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

> 
> --
> cheers,
> -roger
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index e622a1b..eb85d6b 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 + BADBLOCK_MARKER_LENGTH, -1);
> +	chip->read_buf(mtd, chip->oob_poi, chip->ecc.total);
>  
>  	/* Calculate ecc bytes */
>  	chip->ecc.calculate(mtd, buf, ecc_calc);



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

[toc] | [prev] | [next] | [standalone]

#1382478 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

Boris,

On 18/04/16 17:57, Boris Brezillon wrote:
> On Mon, 18 Apr 2016 17:39:01 +0300
> Roger Quadros <rogerq@ti.com> wrote:
> 
>> On 18/04/16 17:10, Boris Brezillon wrote:
>>> 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?
>>
>> 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.

--
cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1382483

On Tue, 19 Apr 2016 15:46:19 +0300
Roger Quadros <rogerq@ti.com> wrote:

> Boris,
> 
> On 18/04/16 17:57, Boris Brezillon wrote:
> > On Mon, 18 Apr 2016 17:39:01 +0300
> > Roger Quadros <rogerq@ti.com> wrote:
> > 
> >> On 18/04/16 17:10, Boris Brezillon wrote:
> >>> 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?
> >>
> >> 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

[toc] | [prev] | [next] | [standalone]

#1382822

On Tue, 19 Apr 2016 15:46:19 +0300
Roger Quadros <rogerq@ti.com> wrote:

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

Just pushed what's supposed to be my new nand/next into
nand/next-with-gpmc-rework, can you test it? I'll push it to nand/next
once I have your confirmation that everything is good.

Thanks,

Boris

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

[toc] | [prev] | [next] | [standalone]

#1383176 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

On 19/04/16 23:11, Boris Brezillon wrote:
> On Tue, 19 Apr 2016 15:46:19 +0300
> Roger Quadros <rogerq@ti.com> wrote:
> 
>>
>> 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.
> 
> Just pushed what's supposed to be my new nand/next into
> nand/next-with-gpmc-rework, can you test it? I'll push it to nand/next
> once I have your confirmation that everything is good.
> 

Tested this on dra7-evm, am437x-gp-evm and omap3-beagle and all seems good. Thanks :).

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1383457 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

* Boris Brezillon <boris.brezillon@free-electrons.com> [160419 13:13]:
> On Tue, 19 Apr 2016 15:46:19 +0300
> Roger Quadros <rogerq@ti.com> wrote:
> 
> > 
> > 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.
> 
> Just pushed what's supposed to be my new nand/next into
> nand/next-with-gpmc-rework, can you test it? I'll push it to nand/next
> once I have your confirmation that everything is good.

Looks good to me too, merges fine with my for-next and still
works for me.

Thanks,

Tony

[toc] | [prev] | [next] | [standalone]

#1382513

On Mon, 18 Apr 2016 17:39:01 +0300
Roger Quadros <rogerq@ti.com> wrote:

> On 18/04/16 17:10, Boris Brezillon wrote:
> > 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?
> 
> 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.
> 
> --
> cheers,
> -roger
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index e622a1b..eb85d6b 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 + BADBLOCK_MARKER_LENGTH, -1);
> +	chip->read_buf(mtd, chip->oob_poi, chip->ecc.total);

Are you sure this patch works? Cause it seems to me that it should be

	chip->read_buf(mtd, chip->oob_poi + BADBLOCK_MARKER_LENGTH,
		       chip->ecc.total);

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

[toc] | [prev] | [next] | [standalone]

#1382561 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

On 19/04/16 16:22, Boris Brezillon wrote:
> On Mon, 18 Apr 2016 17:39:01 +0300
> Roger Quadros <rogerq@ti.com> wrote:
> 
>> On 18/04/16 17:10, Boris Brezillon wrote:
>>> 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?
>>
>> 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.
>>
>> --
>> cheers,
>> -roger
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>> index e622a1b..eb85d6b 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 + BADBLOCK_MARKER_LENGTH, -1);
>> +	chip->read_buf(mtd, chip->oob_poi, chip->ecc.total);
> 
> Are you sure this patch works? Cause it seems to me that it should be

For my limited test case yes. My test case involves reading an existing ubifs partition
and creating a new one and then reading it back using an older kernel.

> 
> 	chip->read_buf(mtd, chip->oob_poi + BADBLOCK_MARKER_LENGTH,
> 		       chip->ecc.total);
> 

You are right. Else we'd have wrong OOB data during reads.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1382591 — Re: [PATCH v6 00/17] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms

On 19/04/16 17:26, Roger Quadros wrote:
> On 19/04/16 16:22, Boris Brezillon wrote:
>> On Mon, 18 Apr 2016 17:39:01 +0300
>> Roger Quadros <rogerq@ti.com> wrote:
>>
>>> On 18/04/16 17:10, Boris Brezillon wrote:
>>>> 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?
>>>
>>> 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.
>>>
>>> --
>>> cheers,
>>> -roger
>>>
>>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>>> index e622a1b..eb85d6b 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 + BADBLOCK_MARKER_LENGTH, -1);
>>> +	chip->read_buf(mtd, chip->oob_poi, chip->ecc.total);
>>
>> Are you sure this patch works? Cause it seems to me that it should be
> 
> For my limited test case yes. My test case involves reading an existing ubifs partition
> and creating a new one and then reading it back using an older kernel.
> 
>>
>> 	chip->read_buf(mtd, chip->oob_poi + BADBLOCK_MARKER_LENGTH,
>> 		       chip->ecc.total);
>>
> 
> You are right. Else we'd have wrong OOB data during reads.

Strange, but nanddump is showing the correct OOB data with or without this
change. I'm puzzled.

--
cheers,
-roger

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web