Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[vimal singh]

On Thu, May 7, 2009 at 12:46 PM, David Brownell <david-b@pacbell.net> wrote:
> I'm glad to see this patch is so small ... basically, just
> adding a special case for 2K pages, and keeping the core of
> this NAND driver the same.  Not needing to change the 4-bit
> ECC support from the patch I sent earlier seems a good sign.
>
> Two comments though:  (a) the board-dm355-evm.c file isn't
> yet in mainline, so the MTD folk can't take this patch as-is;
> (b) as noted elsewhere, there are still issues with 4K pages
> and the NAND core infrastructure.
>
> This patch is sufficient to support development boards for
> the dm355, dm357, and dm365 ... right?  They all have 2 GByte
> NAND chips, ISTR with 2K pages, and haven't yet had to switch
> to more current parts with 4K pages.
>
>
> On Wednesday 06 May 2009, nsnehaprabha@ti.com wrote:
>> --- a/drivers/mtd/nand/davinci_nand.c
>> +++ b/drivers/mtd/nand/davinci_nand.c
>> @@ -500,6 +500,21 @@ static struct nand_ecclayout hwecc4_small __initconst
>> = { },
>>  };
>>
>> +/* An ECC layout for using 4-bit ECC with large-page (2048bytes) flash,
>> + * storing ten ECC bytes plus the manufacturer's bad block marker byte,
>> + * and not overlapping the default BBT markers.
>> + */
>> +static struct nand_ecclayout hwecc4_2048 __initconst = {
>> +       .eccbytes = 10,
>
> Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...
>
>
>> +       .eccpos = { 0, 1, 2, 3, 4, 6, 7, 8, 9, 10,
>> +               11, 12, 13, 14, 15, 24, 25, 26, 27, 28,
>> +               29, 30, 31, 32, 33, 34, 35, 36, 37, 38,
>> +               39, 40, 41, 42, 43, 44, 45, 46, 47, 48, },
>> +       .oobfree = {
>> +               {.offset = 16, .length = 8, },
>> +               {.offset = 49, },
>
> Comments would be good, highlighting (a) byte 5 is reserved,
> it's the manufacturer bad block marker, (b) 8 bytes @16 are
> expected by JFFS2.  Not everyone will "just know" those.
How about leaving bytes '4' and '5' for bad block marker, to support 16-bit
NAND parts too.

---
Regards,
\/ | |\/| /-\ |_

Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[David Brownell]

On Thursday 07 May 2009, vimal singh wrote:
> > Comments would be good, highlighting (a) byte 5 is reserved,
> > it's the manufacturer bad block marker, (b) 8 bytes @16 are
> > expected by JFFS2.  Not everyone will "just know" those.
>
> How about leaving bytes '4' and '5' for bad block marker, to support 16-bit
> NAND parts too.

This 4-bit ECC engine only works for 8-bit wide parts ...
or are you suggesting that in case TI re-engineers that
engine in the future?

Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[vimal singh]

On Thu, May 7, 2009 at 10:24 PM, David Brownell <david-b@pacbell.net> wrote:
> On Thursday 07 May 2009, vimal singh wrote:
>> >> How about leaving bytes '4' and '5' for bad block marker, to support 16-bit
>> >> NAND parts too.
>> >
>> > This 4-bit ECC engine only works for 8-bit wide parts ...
>> > or are you suggesting that in case TI re-engineers that
>> > engine in the future?
>>
>> I am omap guy and was not aware of that. In omap HW BCH
>> ECC (4- or 8- bit correction)can work both kind of memories.
>
> I'm somewhat more of an OMAP guy too -- just got sucked in to
> trying to get a dm355 board to run off NAND in mainline-bound
> code, and you can see where that landed me!
>
> Last I looked at the OMAP2/OMAP3 NAND controller driver, it
> didn't yet support most fancy hardware features, like ECC
> using more than one bit or the prefetch/postwrite logic.
I planned to push simple driver first, as it is present in L-O tree,
and then pushing other features later.
'prefetch-read/post-write' logic has been already implemented
in omapkernel git (For which sometime back I submitted patch on L-O list).
I'll be pushing those patches separately.

ECC using more than one bit (i.e. 4- or -8 bit) can be implemented
by configuring HW ECC engine accordingly in BCH ECC mode. But this
feature is still not implemented, may be in future I'll be
looking into this.

---
Regards,
\/ | |\/| /-\ |_

Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[vimal singh]

On Thu, May 7, 2009 at 2:41 PM, David Brownell <david-b@pacbell.net> wrote:
> On Thursday 07 May 2009, vimal singh wrote:
>> > Comments would be good, highlighting (a) byte 5 is reserved,
>> > it's the manufacturer bad block marker, (b) 8 bytes @16 are
>> > expected by JFFS2.  Not everyone will "just know" those.
>>
>> How about leaving bytes '4' and '5' for bad block marker, to support 16-bit
>> NAND parts too.
>
> This 4-bit ECC engine only works for 8-bit wide parts ...
> or are you suggesting that in case TI re-engineers that
> engine in the future?
I am omap guy and was not aware of that. In omap HW BCH ECC (4- or 8- bit
correction)can work both kind of memories.

Regards,
vimal

Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[David Brownell]

On Thursday 07 May 2009, vimal singh wrote:
> >> How about leaving bytes '4' and '5' for bad block marker, to support 16-bit
> >> NAND parts too.
> >
> > This 4-bit ECC engine only works for 8-bit wide parts ...
> > or are you suggesting that in case TI re-engineers that
> > engine in the future?
>
> I am omap guy and was not aware of that. In omap HW BCH
> ECC (4- or 8- bit correction)can work both kind of memories.

I'm somewhat more of an OMAP guy too -- just got sucked in to
trying to get a dm355 board to run off NAND in mainline-bound
code, and you can see where that landed me!

Last I looked at the OMAP2/OMAP3 NAND controller driver, it
didn't yet support most fancy hardware features, like ECC
using more than one bit or the prefetch/postwrite logic. 

One nice feature of that OMAP2/OMAP3 controller, beyond the
support for 8 bit ECC, is that it buffers the ECC syndrome
data so that it doesn't need NAND_ECC_HW_OOB_FIRST logic to
prevent trashing the manufacturer OOB markers.  The limit
of max 4K pages seems like not a big worry for now.

- Dave

[PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[nsnehaprabha]

From: Sneha Narnakaje <nsnehaprabha@ti.com>

This patch adds 4-bit ECC support for large page NAND chips using the new ECC
mode NAND_ECC_HW_OOB_FIRST. The platform data from board-dm355-evm has been
adjusted to use this mode.

The patches have been verified on DM355 device with 2K Micron devices using
mtd-tests and JFFS2. Error correction upto 4-bits has also been verified using
nandwrite/nanddump utilities.

Note: This patch is dependent on '[patch 2.6.30-rc1] NAND: minor davinci
nand cleanup' and '[patch 2.6.30-rc1] NAND: davinci nand, 4-bit ECC for
smallpage' from David Brownell:
http://lists.infradead.org/pipermail/linux-mtd/2009-April/025206.html
http://lists.infradead.org/pipermail/linux-mtd/2009-April/025207.html

Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
---
 arch/arm/mach-davinci/board-dm355-evm.c |    3 ++-
 drivers/mtd/nand/davinci_nand.c         |   22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/board-dm355-evm.c b/arch/arm/mach-davinci/board-dm355-evm.c
index f32e3d8..39c5a4e 100644
--- a/arch/arm/mach-davinci/board-dm355-evm.c
+++ b/arch/arm/mach-davinci/board-dm355-evm.c
@@ -85,8 +85,9 @@ static struct davinci_nand_pdata davinci_nand_data = {
 	.mask_chipsel		= BIT(14),
 	.parts			= davinci_nand_partitions,
 	.nr_parts		= ARRAY_SIZE(davinci_nand_partitions),
-	.ecc_mode		= NAND_ECC_HW_SYNDROME,
+	.ecc_mode		= NAND_ECC_HW,
 	.options		= NAND_USE_FLASH_BBT,
+	.ecc_bits		= 4,
 };
 
 static struct resource davinci_nand_resources[] = {
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 006843a..f84bc16 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -500,6 +500,21 @@ static struct nand_ecclayout hwecc4_small __initconst = {
 	},
 };
 
+/* An ECC layout for using 4-bit ECC with large-page (2048bytes) flash,
+ * storing ten ECC bytes plus the manufacturer's bad block marker byte,
+ * and not overlapping the default BBT markers.
+ */
+static struct nand_ecclayout hwecc4_2048 __initconst = {
+	.eccbytes = 10,
+	.eccpos = { 0, 1, 2, 3, 4, 6, 7, 8, 9, 10,
+		11, 12, 13, 14, 15, 24, 25, 26, 27, 28,
+		29, 30, 31, 32, 33, 34, 35, 36, 37, 38,
+		39, 40, 41, 42, 43, 44, 45, 46, 47, 48, },
+	.oobfree = {
+		{.offset = 16, .length = 8, },
+		{.offset = 49, },
+	},
+};
 
 static int __init nand_davinci_probe(struct platform_device *pdev)
 {
@@ -690,6 +705,13 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 				info->mtd.oobsize - 16;
 			goto syndrome_done;
 		}
+		if (chunks == 4) {
+			info->ecclayout = hwecc4_2048;
+			info->ecclayout.oobfree[1].length =
+				info->mtd.oobsize - 49;
+			info->chip.ecc.mode = NAND_ECC_HW_OOB_FIRST;
+			goto syndrome_done;
+		}
 
 		/* For large page chips we'll be wanting to use a
 		 * not-yet-implemented mode that reads OOB data
-- 
1.6.0.4

Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[David Brownell]

I'm glad to see this patch is so small ... basically, just
adding a special case for 2K pages, and keeping the core of
this NAND driver the same.  Not needing to change the 4-bit
ECC support from the patch I sent earlier seems a good sign.

Two comments though:  (a) the board-dm355-evm.c file isn't
yet in mainline, so the MTD folk can't take this patch as-is;
(b) as noted elsewhere, there are still issues with 4K pages
and the NAND core infrastructure.

This patch is sufficient to support development boards for
the dm355, dm357, and dm365 ... right?  They all have 2 GByte
NAND chips, ISTR with 2K pages, and haven't yet had to switch
to more current parts with 4K pages.


On Wednesday 06 May 2009, nsnehaprabha@ti.com wrote:
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -500,6 +500,21 @@ static struct nand_ecclayout hwecc4_small __initconst
> = { },
>  };
>  
> +/* An ECC layout for using 4-bit ECC with large-page (2048bytes) flash,
> + * storing ten ECC bytes plus the manufacturer's bad block marker byte,
> + * and not overlapping the default BBT markers.
> + */
> +static struct nand_ecclayout hwecc4_2048 __initconst = {
> +       .eccbytes = 10,

Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...


> +       .eccpos = { 0, 1, 2, 3, 4, 6, 7, 8, 9, 10,
> +               11, 12, 13, 14, 15, 24, 25, 26, 27, 28,
> +               29, 30, 31, 32, 33, 34, 35, 36, 37, 38,
> +               39, 40, 41, 42, 43, 44, 45, 46, 47, 48, },
> +       .oobfree = {
> +               {.offset = 16, .length = 8, },
> +               {.offset = 49, },

Comments would be good, highlighting (a) byte 5 is reserved,
it's the manufacturer bad block marker, (b) 8 bytes @16 are
expected by JFFS2.  Not everyone will "just know" those.


> +       },
> +};
>  
>  static int __init nand_davinci_probe(struct platform_device *pdev)
>  {

> @@ -690,6 +705,13 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
>                                 info->mtd.oobsize - 16;
>                         goto syndrome_done;
>                 }
> +               if (chunks == 4) {
> +                       info->ecclayout = hwecc4_2048;
> +                       info->ecclayout.oobfree[1].length =
> +                               info->mtd.oobsize - 49;
> +                       info->chip.ecc.mode = NAND_ECC_HW_OOB_FIRST;
> +                       goto syndrome_done;
> +               }
>  
>                 /* For large page chips we'll be wanting to use a
>                  * not-yet-implemented mode that reads OOB data

You should update that comment ... that not-yet-implemented mode
is now called "NAND_ECC_HW_OOB_FIRST", from patch 1/2 ... ;)

RE: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

Dave,

> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net]
> Sent: Thursday, May 07, 2009 3:17 AM
> To: Narnakaje, Snehaprabha
> Cc: davinci-linux-open-source@linux.davincidsp.com; linux-
> mtd@lists.infradead.org; dwmw2@infradead.org; tglx@linutronix.de;
> akpm@linux-foundation.org
> Subject: Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large
> page NAND chips
> 
> I'm glad to see this patch is so small ... basically, just
> adding a special case for 2K pages, and keeping the core of
> this NAND driver the same.  Not needing to change the 4-bit
> ECC support from the patch I sent earlier seems a good sign.

Yes, I had to use the .calculate in the new read_page handler to stop the ECC though.

> 
> Two comments though:  (a) the board-dm355-evm.c file isn't
> yet in mainline, so the MTD folk can't take this patch as-is;

Sorry, I didn't realize this. I will separate the board-dm355-evm.c from this patch and submit the patch #3 for board-dm355-evm.c to go to davinci-linux-open-source.

> (b) as noted elsewhere, there are still issues with 4K pages
> and the NAND core infrastructure.

Yes, this is still an issue, if we make the read_page handler use .eccpos[] for positioning the ECC bytes in the OOB area. If we had fixed prepad+ecc nand_ecclayout we would avoid using eccpos[] (This is what my initial set of 4-bit ECC patches did to support 4K). But this is not a generic solution.

The main problem is with nand_ooblayout structure that is not extendable for large pages 4K or more, without breaking the userspace IOCTLs that is dependent on the size of this structure. 

Question to linux-mtd list: Are there plans in the linux-mtd to address this generic issue, now that NAND manufacturers are coming up NAND chips > 4K page size?

> 
> This patch is sufficient to support development boards for
> the dm355, dm357, and dm365 ... right?  They all have 2 GByte
> NAND chips, ISTR with 2K pages, and haven't yet had to switch
> to more current parts with 4K pages.
> 
> 
> On Wednesday 06 May 2009, nsnehaprabha@ti.com wrote:
> > --- a/drivers/mtd/nand/davinci_nand.c
> > +++ b/drivers/mtd/nand/davinci_nand.c
> > @@ -500,6 +500,21 @@ static struct nand_ecclayout hwecc4_small
> __initconst
> > = { },
> >  };
> >
> > +/* An ECC layout for using 4-bit ECC with large-page (2048bytes) flash,
> > + * storing ten ECC bytes plus the manufacturer's bad block marker byte,
> > + * and not overlapping the default BBT markers.
> > + */
> > +static struct nand_ecclayout hwecc4_2048 __initconst = {
> > +       .eccbytes = 10,
> 
> Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...

No, .eccbytes is for each chips->ecc.steps. And all nand read/write APIs, we handle ecc.steps (for loop). There is chips->ecc.total that is initialized as (chips->ecc.steps * chips->ecc.bytes).

It is strange that .eccbytes is for each chunk, while eccpos[] and .oobfree[] have to handle/cover all chunks.

> 
> 
> > +       .eccpos = { 0, 1, 2, 3, 4, 6, 7, 8, 9, 10,
> > +               11, 12, 13, 14, 15, 24, 25, 26, 27, 28,
> > +               29, 30, 31, 32, 33, 34, 35, 36, 37, 38,
> > +               39, 40, 41, 42, 43, 44, 45, 46, 47, 48, },
> > +       .oobfree = {
> > +               {.offset = 16, .length = 8, },
> > +               {.offset = 49, },
> 
> Comments would be good, highlighting (a) byte 5 is reserved,
> it's the manufacturer bad block marker, (b) 8 bytes @16 are
> expected by JFFS2.  Not everyone will "just know" those.

OK, will add in the next patch.

> 
> 
> > +       },
> > +};
> >
> >  static int __init nand_davinci_probe(struct platform_device *pdev)
> >  {
> 
> > @@ -690,6 +705,13 @@ static int __init nand_davinci_probe(struct
> platform_device *pdev)
> >                                 info->mtd.oobsize - 16;
> >                         goto syndrome_done;
> >                 }
> > +               if (chunks == 4) {
> > +                       info->ecclayout = hwecc4_2048;
> > +                       info->ecclayout.oobfree[1].length =
> > +                               info->mtd.oobsize - 49;
> > +                       info->chip.ecc.mode = NAND_ECC_HW_OOB_FIRST;
> > +                       goto syndrome_done;
> > +               }
> >
> >                 /* For large page chips we'll be wanting to use a
> >                  * not-yet-implemented mode that reads OOB data
> 
> You should update that comment ... that not-yet-implemented mode
> is now called "NAND_ECC_HW_OOB_FIRST", from patch 1/2 ... ;)

Yes, but will have a comment about 4K though.

Thanks
Sneha

> 

Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[David Brownell]

On Thursday 07 May 2009, Narnakaje, Snehaprabha wrote:

> > I'm glad to see this patch is so small ... basically, just
> > adding a special case for 2K pages, and keeping the core of
> > this NAND driver the same.  Not needing to change the 4-bit
> > ECC support from the patch I sent earlier seems a good sign.
> 
> Yes, I had to use the .calculate in the new read_page handler
> to stop the ECC though. 

Oh, now I see what you were talking about off-list.  That
doesn't seem quite right.  I'll take a look.


> > Two comments though:  (a) the board-dm355-evm.c file isn't
> > yet in mainline, so the MTD folk can't take this patch as-is;
> 
> Sorry, I didn't realize this. I will separate the board-dm355-evm.c
> from this patch and submit the patch #3 for board-dm355-evm.c to go
> to davinci-linux-open-source.  

Let's wait until we see the previous davinci_nand.c patches get
queued up somewhere "official" though ... in fact, I don't see
why they shouldn't merge for 2.6.30-soon.


> > (b) as noted elsewhere, there are still issues with 4K pages
> > and the NAND core infrastructure.
> 
> Yes, this is still an issue, if we make the read_page handler
> use .eccpos[] for positioning the ECC bytes in the OOB area.
> If we had fixed prepad+ecc nand_ecclayout we would avoid using
> eccpos[] (This is what my initial set of 4-bit ECC patches did
> to support 4K). But this is not a generic solution.    
> 
> The main problem is with nand_ooblayout structure that is not
> extendable for large pages 4K or more, without breaking the
> userspace IOCTLs that is dependent on the size of this structure.   
> 
> Question to linux-mtd list: Are there plans in the linux-mtd to 
> address this generic issue, now that NAND manufacturers are
> coming up NAND chips > 4K page size?

They've been doing it for a while now, actually.  :)

And it wouldn't be an issue here ... *IF* this 4-bit ECC didn't
need to use 80 bytes (8 chunks of 512 bytes, 10 bytes ECC per
chunk) from a 64 byte field.  Doesn't fit.

Maybe the ecclayout stuff should migrate to sysfs, and just
drop the old ioctl stuff for parts that can't use it ... as
is being done to support chips that are 4 GByte and bigger.


> > > +static struct nand_ecclayout hwecc4_2048 __initconst = {
> > > +       .eccbytes = 10,
> > 
> > Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...
> 
> No, .eccbytes is for each chips->ecc.steps. 

So it's the same as ecc.bytes, which is not exported to userspace.


> And all nand read/write 
> APIs, we handle ecc.steps (for loop). There is chips->ecc.total that
> is initialized as (chips->ecc.steps * chips->ecc.bytes).  
> 
> It is strange that .eccbytes is for each chunk, while eccpos[] and
> .oobfree[] have to handle/cover all chunks. 

Also strange:  when ECC_HW_SYNDROME is in use, nothing even
looks at the nand_ecclayout to make sure it matches the actual
layout being used ... which derives entirely from the prepad,
postpad, and bytes fields of "ecc".  (Except that some of the
OOB bytes not used for ECC may be hidden, e.g. manufacturer
bad block markers are usually not listed.)

- Dave

RE: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net]
> Sent: Thursday, May 07, 2009 1:11 PM
> To: Narnakaje, Snehaprabha
> Cc: davinci-linux-open-source@linux.davincidsp.com; linux-
> mtd@lists.infradead.org; dwmw2@infradead.org; tglx@linutronix.de;
> akpm@linux-foundation.org
> Subject: Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large
> page NAND chips
> 
> On Thursday 07 May 2009, Narnakaje, Snehaprabha wrote:
> 
> > > I'm glad to see this patch is so small ... basically, just
> > > adding a special case for 2K pages, and keeping the core of
> > > this NAND driver the same.  Not needing to change the 4-bit
> > > ECC support from the patch I sent earlier seems a good sign.
> >
> > Yes, I had to use the .calculate in the new read_page handler
> > to stop the ECC though.
> 
> Oh, now I see what you were talking about off-list.  That
> doesn't seem quite right.  I'll take a look.

Looks like you had to implement is_readmode option in the .calculate function, to use the NAND_ECC_HW mode for small pages. The read_page handler for NAND_ECC_HW calls the .calculate function, and in our case we shouldn't be doing anything here for read. We use the .calculate function from write_page.

> 
> 
> > > Two comments though:  (a) the board-dm355-evm.c file isn't
> > > yet in mainline, so the MTD folk can't take this patch as-is;
> >
> > Sorry, I didn't realize this. I will separate the board-dm355-evm.c
> > from this patch and submit the patch #3 for board-dm355-evm.c to go
> > to davinci-linux-open-source.
> 
> Let's wait until we see the previous davinci_nand.c patches get
> queued up somewhere "official" though ... in fact, I don't see
> why they shouldn't merge for 2.6.30-soon.

I was hoping my patch (dependent on your patch set) would at least trigger the review/acceptance :-)

> 
> 
> > > (b) as noted elsewhere, there are still issues with 4K pages
> > > and the NAND core infrastructure.
> >
> > Yes, this is still an issue, if we make the read_page handler
> > use .eccpos[] for positioning the ECC bytes in the OOB area.
> > If we had fixed prepad+ecc nand_ecclayout we would avoid using
> > eccpos[] (This is what my initial set of 4-bit ECC patches did
> > to support 4K). But this is not a generic solution.
> >
> > The main problem is with nand_ooblayout structure that is not
> > extendable for large pages 4K or more, without breaking the
> > userspace IOCTLs that is dependent on the size of this structure.
> >
> > Question to linux-mtd list: Are there plans in the linux-mtd to
> > address this generic issue, now that NAND manufacturers are
> > coming up NAND chips > 4K page size?
> 
> They've been doing it for a while now, actually.  :)
> 
> And it wouldn't be an issue here ... *IF* this 4-bit ECC didn't
> need to use 80 bytes (8 chunks of 512 bytes, 10 bytes ECC per
> chunk) from a 64 byte field.  Doesn't fit.
> 
> Maybe the ecclayout stuff should migrate to sysfs, and just
> drop the old ioctl stuff for parts that can't use it ... as
> is being done to support chips that are 4 GByte and bigger.

I will explore more on this option.

> 
> 
> > > > +static struct nand_ecclayout hwecc4_2048 __initconst = {
> > > > +       .eccbytes = 10,
> > >
> > > Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...
> >
> > No, .eccbytes is for each chips->ecc.steps.
> 
> So it's the same as ecc.bytes, which is not exported to userspace.
> 
> 
> > And all nand read/write
> > APIs, we handle ecc.steps (for loop). There is chips->ecc.total that
> > is initialized as (chips->ecc.steps * chips->ecc.bytes).
> >
> > It is strange that .eccbytes is for each chunk, while eccpos[] and
> > .oobfree[] have to handle/cover all chunks.
> 
> Also strange:  when ECC_HW_SYNDROME is in use, nothing even
> looks at the nand_ecclayout to make sure it matches the actual
> layout being used ... which derives entirely from the prepad,
> postpad, and bytes fields of "ecc".  (Except that some of the
> OOB bytes not used for ECC may be hidden, e.g. manufacturer
> bad block markers are usually not listed.)

If you leave enough prepad to cover manufacturer's BB and JFFS2 markers, this mechanism was OK, I guess.

Thanks
Sneha

> 
> - Dave
> 
> 
> 

Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[Troy Kisky]

>>>> +static struct nand_ecclayout hwecc4_2048 __initconst = {
>>>> +       .eccbytes = 10,
>>> Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...
>> No, .eccbytes is for each chips->ecc.steps. 
> 
> So it's the same as ecc.bytes, which is not exported to userspace.
> 
> 
>> And all nand read/write 
>> APIs, we handle ecc.steps (for loop). There is chips->ecc.total that
>> is initialized as (chips->ecc.steps * chips->ecc.bytes).  
>>
>> It is strange that .eccbytes is for each chunk, while eccpos[] and
>> .oobfree[] have to handle/cover all chunks. 

No, eccbytes should equal ecc.total


i.e. from nand_base.c

static struct nand_ecclayout nand_oob_64 = {
        .eccbytes = 24,
        .eccpos = {
                   40, 41, 42, 43, 44, 45, 46, 47,
                   48, 49, 50, 51, 52, 53, 54, 55,
                   56, 57, 58, 59, 60, 61, 62, 63},
        .oobfree = {
                {.offset = 2,
                 .length = 38}}
};
Troy

RE: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> Sent: Thursday, May 07, 2009 6:37 PM
> To: David Brownell
> Cc: Narnakaje, Snehaprabha; dwmw2@infradead.org; davinci-linux-open-
> source@linux.davincidsp.com; linux-mtd@lists.infradead.org;
> tglx@linutronix.de; akpm@linux-foundation.org
> Subject: Re: [PATCH 2/2] NAND on DM355: Add 4-bit ECC support for large
> page NAND chips
> 
> >>>> +static struct nand_ecclayout hwecc4_2048 __initconst = {
> >>>> +       .eccbytes = 10,
> >>> Not ".eccbytes = 40"?  This is 4 chunks, 10 ecc bytes each...
> >> No, .eccbytes is for each chips->ecc.steps.
> >
> > So it's the same as ecc.bytes, which is not exported to userspace.
> >
> >
> >> And all nand read/write
> >> APIs, we handle ecc.steps (for loop). There is chips->ecc.total that
> >> is initialized as (chips->ecc.steps * chips->ecc.bytes).
> >>
> >> It is strange that .eccbytes is for each chunk, while eccpos[] and
> >> .oobfree[] have to handle/cover all chunks.
> 
> No, eccbytes should equal ecc.total

Thanks Troy and Dave.

Yes, it should indeed be 40 --> total size. eccbytes in the nand_ecclayout is different from chip->ecc.bytes. We do set the chip->ecc.bytes to 10 which is for each trunk.

I will change this and test it. 

> 
> 
> i.e. from nand_base.c
> 
> static struct nand_ecclayout nand_oob_64 = {
>         .eccbytes = 24,
>         .eccpos = {
>                    40, 41, 42, 43, 44, 45, 46, 47,
>                    48, 49, 50, 51, 52, 53, 54, 55,
>                    56, 57, 58, 59, 60, 61, 62, 63},
>         .oobfree = {
>                 {.offset = 2,
>                  .length = 38}}
> };
> Troy
> 
>