[patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

[patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

[akpm]

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.

Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
Signed-off-by: Sandeep Paulraj <s-paulraj@ti.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/mtd/nand/davinci_nand.c |   45 ++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 6 deletions(-)

diff -puN drivers/mtd/nand/davinci_nand.c~mtd-nand-davinci-add-4-bit-ecc-support-for-large-page-nand-chips drivers/mtd/nand/davinci_nand.c
--- a/drivers/mtd/nand/davinci_nand.c~mtd-nand-davinci-add-4-bit-ecc-support-for-large-page-nand-chips
+++ a/drivers/mtd/nand/davinci_nand.c
@@ -348,6 +348,12 @@ compare:
 	if (!(syndrome[0] | syndrome[1] | syndrome[2] | syndrome[3]))
 		return 0;
 
+	/*
+	 * Clear any previous address calculation by doing a dummy read of an
+	 * error address register.
+	 */
+	davinci_nand_readl(info, NAND_ERR_ADD1_OFFSET);
+
 	/* Start address calculation, and wait for it to complete.
 	 * We _could_ start reading more data while this is working,
 	 * to speed up the overall page read.
@@ -359,8 +365,10 @@ compare:
 
 		switch ((fsr >> 8) & 0x0f) {
 		case 0:		/* no error, should not happen */
+			davinci_nand_readl(info, NAND_ERR_ERRVAL1_OFFSET);
 			return 0;
 		case 1:		/* five or more errors detected */
+			davinci_nand_readl(info, NAND_ERR_ERRVAL1_OFFSET);
 			return -EIO;
 		case 2:		/* error addresses computed */
 		case 3:
@@ -500,6 +508,26 @@ static struct nand_ecclayout hwecc4_smal
 	},
 };
 
+/* 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 = 40,
+	.eccpos = {
+		/* at the end of spare sector */
+		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, 49, 50, 51, 52, 53,
+		54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
+		},
+	.oobfree = {
+		/* 1 byte at offset 0 holds manufacturer badblock marker */
+		{.offset = 1, .length = 23, },
+		/* 5 bytes at offset 8 hold BBT markers */
+		/* 8 bytes at offset 16 hold JFFS2 clean markers */
+	},
+};
 
 static int __init nand_davinci_probe(struct platform_device *pdev)
 {
@@ -690,15 +718,20 @@ static int __init nand_davinci_probe(str
 				info->mtd.oobsize - 16;
 			goto syndrome_done;
 		}
+		if (chunks == 4) {
+			info->ecclayout = hwecc4_2048;
+			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
-		 * before reading the body of the page, to avoid
-		 * the "infix OOB" model of NAND_ECC_HW_SYNDROME
-		 * (and preserve manufacturer badblock markings).
+		/* 4K page chips are not yet supported. The eccpos from
+		 * nand_ecclayout cannot hold 80 bytes and change to eccpos[]
+		 * breaks userspace ioctl interface with mtd-utils. Once we
+		 * resolve this issue, NAND_ECC_HW_OOB_FIRST mode can be used
+		 * for the 4K page chips.
 		 */
 		dev_warn(&pdev->dev, "no 4-bit ECC support yet "
-				"for large page NAND\n");
+				"for 4K page NAND\n");
 		ret = -EIO;
 		goto err_scan;
 
_

Re: [patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

[Troy Kisky]

akpm@linux-foundation.org wrote:
> 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.
> 
> Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
> Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
> Signed-off-by: Sandeep Paulraj <s-paulraj@ti.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  drivers/mtd/nand/davinci_nand.c |   45 ++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff -puN drivers/mtd/nand/davinci_nand.c~mtd-nand-davinci-add-4-bit-ecc-support-for-large-page-nand-chips drivers/mtd/nand/davinci_nand.c
> --- a/drivers/mtd/nand/davinci_nand.c~mtd-nand-davinci-add-4-bit-ecc-support-for-large-page-nand-chips
> +++ a/drivers/mtd/nand/davinci_nand.c
> @@ -348,6 +348,12 @@ compare:
>  	if (!(syndrome[0] | syndrome[1] | syndrome[2] | syndrome[3]))
>  		return 0;
>  
> +	/*
> +	 * Clear any previous address calculation by doing a dummy read of an
> +	 * error address register.
> +	 */
> +	davinci_nand_readl(info, NAND_ERR_ADD1_OFFSET);
> +
>  	/* Start address calculation, and wait for it to complete.
>  	 * We _could_ start reading more data while this is working,
>  	 * to speed up the overall page read.
> @@ -359,8 +365,10 @@ compare:
>  
>  		switch ((fsr >> 8) & 0x0f) {
>  		case 0:		/* no error, should not happen */
> +			davinci_nand_readl(info, NAND_ERR_ERRVAL1_OFFSET);
>  			return 0;
>  		case 1:		/* five or more errors detected */
> +			davinci_nand_readl(info, NAND_ERR_ERRVAL1_OFFSET);
>  			return -EIO;
>  		case 2:		/* error addresses computed */
>  		case 3:
> @@ -500,6 +508,26 @@ static struct nand_ecclayout hwecc4_smal
>  	},
>  };
>  
> +/* 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 = 40,
> +	.eccpos = {
> +		/* at the end of spare sector */
> +		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, 49, 50, 51, 52, 53,
> +		54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
> +		},
> +	.oobfree = {
> +		/* 1 byte at offset 0 holds manufacturer badblock marker */
> +		{.offset = 1, .length = 23, },

I thought Sneha Narnakaje was going to change offset = 2, length = 22 ?

RE: [patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

[Narnakaje, Snehaprabha]

Troy,

> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky@boundarydevices.com]
> Sent: Friday, August 07, 2009 3:54 PM
> To: akpm@linux-foundation.org
> Cc: dwmw2@infradead.org; linux-mtd@lists.infradead.org; Narnakaje,
> Snehaprabha; dbrownell@users.sourceforge.net; Paulraj, Sandeep;
> tglx@linutronix.de
> Subject: Re: [patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for
> large page NAND chips
> 
> akpm@linux-foundation.org wrote:
> > 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.
> >
> > Reviewed-by: David Brownell <dbrownell@users.sourceforge.net>
> > Signed-off-by: Sneha Narnakaje <nsnehaprabha@ti.com>
> > Signed-off-by: Sandeep Paulraj <s-paulraj@ti.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >
> >  drivers/mtd/nand/davinci_nand.c |   45 ++++++++++++++++++++++++++----
> >  1 file changed, 39 insertions(+), 6 deletions(-)
> >
> > diff -puN drivers/mtd/nand/davinci_nand.c~mtd-nand-davinci-add-4-bit-
> ecc-support-for-large-page-nand-chips drivers/mtd/nand/davinci_nand.c
> > --- a/drivers/mtd/nand/davinci_nand.c~mtd-nand-davinci-add-4-bit-ecc-
> support-for-large-page-nand-chips
> > +++ a/drivers/mtd/nand/davinci_nand.c
> > @@ -348,6 +348,12 @@ compare:
> >  	if (!(syndrome[0] | syndrome[1] | syndrome[2] | syndrome[3]))
> >  		return 0;
> >
> > +	/*
> > +	 * Clear any previous address calculation by doing a dummy read of
> an
> > +	 * error address register.
> > +	 */
> > +	davinci_nand_readl(info, NAND_ERR_ADD1_OFFSET);
> > +
> >  	/* Start address calculation, and wait for it to complete.
> >  	 * We _could_ start reading more data while this is working,
> >  	 * to speed up the overall page read.
> > @@ -359,8 +365,10 @@ compare:
> >
> >  		switch ((fsr >> 8) & 0x0f) {
> >  		case 0:		/* no error, should not happen */
> > +			davinci_nand_readl(info, NAND_ERR_ERRVAL1_OFFSET);
> >  			return 0;
> >  		case 1:		/* five or more errors detected */
> > +			davinci_nand_readl(info, NAND_ERR_ERRVAL1_OFFSET);
> >  			return -EIO;
> >  		case 2:		/* error addresses computed */
> >  		case 3:
> > @@ -500,6 +508,26 @@ static struct nand_ecclayout hwecc4_smal
> >  	},
> >  };
> >
> > +/* 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 = 40,
> > +	.eccpos = {
> > +		/* at the end of spare sector */
> > +		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, 49, 50, 51, 52, 53,
> > +		54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
> > +		},
> > +	.oobfree = {
> > +		/* 1 byte at offset 0 holds manufacturer badblock marker */
> > +		{.offset = 1, .length = 23, },
> 
> I thought Sneha Narnakaje was going to change offset = 2, length = 22 ?

Troy,

Sorry, may be I didn't understand. I thought the suggestion was to change the offset in nand_bbt.c.

I will resubmit this patch, with the suggested change.

Thanks
Sneha

> 
> 

Re: [patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

[David Brownell]

On Friday 07 August 2009, Troy Kisky wrote:
> > +static struct nand_ecclayout hwecc4_2048 __initconst = {
> > +     ...
> > +     .oobfree = {
> > +             /* 1 byte at offset 0 holds manufacturer badblock marker */
> > +             {.offset = 1, .length = 23, },
> 
> I thought Sneha Narnakaje was going to change offset = 2, length = 22 ?

Not clear why; this is only for 8-bit NAND, which (right?)
only uses single byte manufacturer badblock markers ...

Will a TI person be updating U-Boot?   Please?  :)
During the U-Boot v2009.10 window, which opens up 
in a couple days...

- Dave

RE: [patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

[Paulraj, Sandeep]

Dave,

I've already sent the patch to U-Boot and it has been accepted. Its in the nand next branch. For things to become fully functional i'll need a syncs between the various branches of U-Boot. I'll have to enable stuff in the DM355 and Dm365 configs

since you ask about the U-Boot NAND driver, its based on old TI releases. And i'll tell you the reason why i decided that. The current DaVinci NAND driver in the kernel seems to have issues. 4 BIT ECC correction does not take place in DM365. But random delays in the from of printks seem to help. We are debugging and once that is done i can send an updated U-Boot NAND driver based on your NAND kernel driver

Thanks,
Sandeep
________________________________________
From: David Brownell [david-b@pacbell.net]
Sent: Thursday, August 27, 2009 10:08 PM
To: Troy Kisky
Cc: akpm@linux-foundation.org; dwmw2@infradead.org; linux-mtd@lists.infradead.org; Narnakaje, Snehaprabha; Paulraj, Sandeep; tglx@linutronix.de
Subject: Re: [patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

On Friday 07 August 2009, Troy Kisky wrote:
> > +static struct nand_ecclayout hwecc4_2048 __initconst = {
> > +     ...
> > +     .oobfree = {
> > +             /* 1 byte at offset 0 holds manufacturer badblock marker */
> > +             {.offset = 1, .length = 23, },
>
> I thought Sneha Narnakaje was going to change offset = 2, length = 22 ?

Not clear why; this is only for 8-bit NAND, which (right?)
only uses single byte manufacturer badblock markers ...

Will a TI person be updating U-Boot?   Please?  :)
During the U-Boot v2009.10 window, which opens up
in a couple days...

- Dave

Re: [patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

[David Brownell]

On Friday 28 August 2009, Paulraj, Sandeep wrote:
> I've already sent the patch to U-Boot and it has been accepted.
> Its in the nand next branch.

Actually at that time it was also in the u-boot-next branch;
and since then (after U-Boot 2009.08 got tagged) it's been
merged into the current U-Boot GIT tree.  :)


> For things to become fully functional i'll need a syncs
> between the various branches of U-Boot. I'll have to enable
> stuff in the DM355 and Dm365 configs    
 
I've got a DM355 config patch; why don't I send that along,
after I test with the latest u-boot and some Linux version
which includes the relevant NAND apatches.  It's been
waiting on that merge, which has now happened.

- Dave

RE: [patch 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for large page NAND chips

[Paulraj, Sandeep]

> On Friday 28 August 2009, Paulraj, Sandeep wrote:
> > I've already sent the patch to U-Boot and it has been accepted.
> > Its in the nand next branch.
> 
> Actually at that time it was also in the u-boot-next branch;
> and since then (after U-Boot 2009.08 got tagged) it's been
> merged into the current U-Boot GIT tree.  :)
[Sandeep] that's very correct
> 
> 
> > For things to become fully functional i'll need a syncs
> > between the various branches of U-Boot. I'll have to enable
> > stuff in the DM355 and Dm365 configs
> 
> I've got a DM355 config patch; why don't I send that along,
> after I test with the latest u-boot and some Linux version
> which includes the relevant NAND apatches.  It's been
> waiting on that merge, which has now happened.
[Sandeep] No probs. My config update patch(to enable NAND) got accepted but Jean-Christophe forgot to merge it. So I was about to send it again since he insisted that I send again.
> 
> - Dave
> 
>