[PATCH v2 2/3] NAND: Add new ECC mode - ECC_HW_OOB_FIRST

[PATCH v2 2/3] NAND: Add new ECC mode - ECC_HW_OOB_FIRST

[s-paulraj]

From: Sandeep Paulraj <s-paulraj@ti.com>

The patch applies to linux-mtd GIT tree

This patch adds the new mode NAND_ECC_HW_OOB_FIRST in the nand code to
support 4-bit ECC on TI DaVinci devices with large page (up to 4K) NAND
chips. This ECC mode is similar to NAND_ECC_HW, with the exception of
read_page API that first reads the OOB area, reads the data in chunks, feeds
the ECC from OOB area to the ECC hw engine and perform any correction on the
data as per the ECC status reported by the engine.

"ECC_HW_OOB_FIRST" name suggested by Thomas Gleixner

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
---
 drivers/mtd/nand/nand_base.c |   59 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index a738333..93bb8f3 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -980,6 +980,54 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /**
+ * nand_read_page_hwecc_oob_first - [REPLACABLE] hw ecc, read oob first
+ * @mtd:	mtd info structure
+ * @chip:	nand chip info structure
+ * @buf:	buffer to store read data
+ *
+ * Hardware ECC for large page chips, require OOB to be read first.
+ * For this ECC mode, the write_page method is re-used from ECC_HW.
+ * These methods read/write ECC from the OOB area, unlike the
+ * ECC_HW_SYNDROME support with multiple ECC steps, follows the
+ * "infix ECC" scheme and reads/writes ECC from the data area, by
+ * overwriting the NAND manufacturer bad block markings.
+ */
+static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
+	struct nand_chip *chip, uint8_t *buf, int page)
+{
+	int i, eccsize = chip->ecc.size;
+	int eccbytes = chip->ecc.bytes;
+	int eccsteps = chip->ecc.steps;
+	uint8_t *p = buf;
+	uint8_t *ecc_code = chip->buffers->ecccode;
+	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	uint8_t *ecc_calc = chip->buffers->ecccalc;
+
+	/* Read the OOB area first */
+	chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
+	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+
+	for (i = 0; i < chip->ecc.total; i++)
+		ecc_code[i] = chip->oob_poi[eccpos[i]];
+
+	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		int stat;
+
+		chip->ecc.hwctl(mtd, NAND_ECC_READ);
+		chip->read_buf(mtd, p, eccsize);
+		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
+
+		stat = chip->ecc.correct(mtd, p, &ecc_code[i], NULL);
+		if (stat < 0)
+			mtd->ecc_stats.failed++;
+		else
+			mtd->ecc_stats.corrected += stat;
+	}
+	return 0;
+}
+
+/**
  * nand_read_page_syndrome - [REPLACABLE] hardware ecc syndrom based page read
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
@@ -2673,6 +2721,17 @@ int nand_scan_tail(struct mtd_info *mtd)
 	 */
 
 	switch (chip->ecc.mode) {
+	case NAND_ECC_HW_OOB_FIRST:
+		/* Similar to NAND_ECC_HW, but a separate read_page handle */
+		if (!chip->ecc.calculate || !chip->ecc.correct ||
+		     !chip->ecc.hwctl) {
+			printk(KERN_WARNING "No ECC functions supplied, "
+			       "Hardware ECC not possible\n");
+			BUG();
+		}
+		if (!chip->ecc.read_page)
+			chip->ecc.read_page = nand_read_page_hwecc_oob_first;
+
 	case NAND_ECC_HW:
 		/* Use standard hwecc read page function ? */
 		if (!chip->ecc.read_page)
-- 
1.6.0.4

Re: [PATCH v2 2/3] NAND: Add new ECC mode - ECC_HW_OOB_FIRST

[David Brownell]

On Tuesday 16 June 2009, s-paulraj@ti.com wrote:
> From: Sandeep Paulraj <s-paulraj@ti.com>
> 
> The patch applies to linux-mtd GIT tree
> 
> This patch adds the new mode NAND_ECC_HW_OOB_FIRST in the nand code to
> support 4-bit ECC on TI DaVinci devices with large page (up to 4K) NAND
> chips. This ECC mode is similar to NAND_ECC_HW, with the exception of
> read_page API that first reads the OOB area, reads the data in chunks, feeds
> the ECC from OOB area to the ECC hw engine and perform any correction on the
> data as per the ECC status reported by the engine.
> 
> "ECC_HW_OOB_FIRST" name suggested by Thomas Gleixner
> 
> 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

MTD folk ... any responses?

I don't think patches #1 and #2 changed much since the previous post.

The only issue was with patch #3, where details of the OOB layout had
needed tweaking.



> ---
>  drivers/mtd/nand/nand_base.c |   59 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 59 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index a738333..93bb8f3 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -980,6 +980,54 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>  }
>  
>  /**
> + * nand_read_page_hwecc_oob_first - [REPLACABLE] hw ecc, read oob first
> + * @mtd:	mtd info structure
> + * @chip:	nand chip info structure
> + * @buf:	buffer to store read data
> + *
> + * Hardware ECC for large page chips, require OOB to be read first.
> + * For this ECC mode, the write_page method is re-used from ECC_HW.
> + * These methods read/write ECC from the OOB area, unlike the
> + * ECC_HW_SYNDROME support with multiple ECC steps, follows the
> + * "infix ECC" scheme and reads/writes ECC from the data area, by
> + * overwriting the NAND manufacturer bad block markings.
> + */
> +static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
> +	struct nand_chip *chip, uint8_t *buf, int page)
> +{
> +	int i, eccsize = chip->ecc.size;
> +	int eccbytes = chip->ecc.bytes;
> +	int eccsteps = chip->ecc.steps;
> +	uint8_t *p = buf;
> +	uint8_t *ecc_code = chip->buffers->ecccode;
> +	uint32_t *eccpos = chip->ecc.layout->eccpos;
> +	uint8_t *ecc_calc = chip->buffers->ecccalc;
> +
> +	/* Read the OOB area first */
> +	chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
> +	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> +	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
> +
> +	for (i = 0; i < chip->ecc.total; i++)
> +		ecc_code[i] = chip->oob_poi[eccpos[i]];
> +
> +	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> +		int stat;
> +
> +		chip->ecc.hwctl(mtd, NAND_ECC_READ);
> +		chip->read_buf(mtd, p, eccsize);
> +		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
> +
> +		stat = chip->ecc.correct(mtd, p, &ecc_code[i], NULL);
> +		if (stat < 0)
> +			mtd->ecc_stats.failed++;
> +		else
> +			mtd->ecc_stats.corrected += stat;
> +	}
> +	return 0;
> +}
> +
> +/**
>   * nand_read_page_syndrome - [REPLACABLE] hardware ecc syndrom based page read
>   * @mtd:	mtd info structure
>   * @chip:	nand chip info structure
> @@ -2673,6 +2721,17 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	 */
>  
>  	switch (chip->ecc.mode) {
> +	case NAND_ECC_HW_OOB_FIRST:
> +		/* Similar to NAND_ECC_HW, but a separate read_page handle */
> +		if (!chip->ecc.calculate || !chip->ecc.correct ||
> +		     !chip->ecc.hwctl) {
> +			printk(KERN_WARNING "No ECC functions supplied, "
> +			       "Hardware ECC not possible\n");
> +			BUG();
> +		}
> +		if (!chip->ecc.read_page)
> +			chip->ecc.read_page = nand_read_page_hwecc_oob_first;
> +
>  	case NAND_ECC_HW:
>  		/* Use standard hwecc read page function ? */
>  		if (!chip->ecc.read_page)
> 

Re: [PATCH v2 2/3] NAND: Add new ECC mode - ECC_HW_OOB_FIRST

[Artem Bityutskiy]

On Tue, 2009-06-16 at 16:23 -0400, s-paulraj@ti.com wrote:
> From: Sandeep Paulraj <s-paulraj@ti.com>
> 
> The patch applies to linux-mtd GIT tree
> 
> This patch adds the new mode NAND_ECC_HW_OOB_FIRST in the nand code to
> support 4-bit ECC on TI DaVinci devices with large page (up to 4K) NAND
> chips. This ECC mode is similar to NAND_ECC_HW, with the exception of
> read_page API that first reads the OOB area, reads the data in chunks, feeds
> the ECC from OOB area to the ECC hw engine and perform any correction on the
> data as per the ECC status reported by the engine.
> 
> "ECC_HW_OOB_FIRST" name suggested by Thomas Gleixner
> 
> 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

I've applied your 3 patches and they do not compile:

  CC [M]  drivers/mtd/nand/nand_ecc.o          
drivers/mtd/nand/nand_base.c: In function ‘nand_scan_tail’:
drivers/mtd/nand/nand_base.c:2724: error: ‘NAND_ECC_HW_OOB_FIRST’ undeclared (first use in this function)
drivers/mtd/nand/nand_base.c:2724: error: (Each undeclared identifier is reported only once              
drivers/mtd/nand/nand_base.c:2724: error: for each function it appears in.)                              
make[3]: *** [drivers/mtd/nand/nand_base.o] Error 1                                                      
make[3]: *** Waiting for unfinished jobs....                                                             
  CC [M]  drivers/mtd/devices/docprobe.o                                                                 

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

RE: [PATCH v2 2/3] NAND: Add new ECC mode - ECC_HW_OOB_FIRST

[Paulraj, Sandeep]

I resubmitted this patch after fixing the compilation issue

> -----Original Message-----
> From: Artem Bityutskiy [mailto:dedekind@infradead.org]
> Sent: Monday, June 22, 2009 4:22 AM
> To: Paulraj, Sandeep
> Cc: davinci-linux-open-source@linux.davincidsp.com; linux-
> mtd@lists.infradead.org; dwmw2@infradead.org; tglx@linutronix.de;
> akpm@linux-foundation.org; Narnakaje, Snehaprabha
> Subject: Re: [PATCH v2 2/3] NAND: Add new ECC mode - ECC_HW_OOB_FIRST
> 
> On Tue, 2009-06-16 at 16:23 -0400, s-paulraj@ti.com wrote:
> > From: Sandeep Paulraj <s-paulraj@ti.com>
> >
> > The patch applies to linux-mtd GIT tree
> >
> > This patch adds the new mode NAND_ECC_HW_OOB_FIRST in the nand code to
> > support 4-bit ECC on TI DaVinci devices with large page (up to 4K) NAND
> > chips. This ECC mode is similar to NAND_ECC_HW, with the exception of
> > read_page API that first reads the OOB area, reads the data in chunks,
> feeds
> > the ECC from OOB area to the ECC hw engine and perform any correction on
> the
> > data as per the ECC status reported by the engine.
> >
> > "ECC_HW_OOB_FIRST" name suggested by Thomas Gleixner
> >
> > 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
> 
> I've applied your 3 patches and they do not compile:
> 
>   CC [M]  drivers/mtd/nand/nand_ecc.o
> drivers/mtd/nand/nand_base.c: In function ‘nand_scan_tail’:
> drivers/mtd/nand/nand_base.c:2724: error: ‘NAND_ECC_HW_OOB_FIRST’
> undeclared (first use in this function)
> drivers/mtd/nand/nand_base.c:2724: error: (Each undeclared identifier is
> reported only once
> drivers/mtd/nand/nand_base.c:2724: error: for each function it appears
> in.)
> make[3]: *** [drivers/mtd/nand/nand_base.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
>   CC [M]  drivers/mtd/devices/docprobe.o
> 
> --
> Best regards,
> Artem Bityutskiy (Битюцкий Артём)
> 

Re: [PATCH v2 2/3] NAND: Add new ECC mode - ECC_HW_OOB_FIRST

[Vitaly Wool]

On Wed, Jun 17, 2009 at 12:23 AM, <s-paulraj@ti.com> wrote:
> From: Sandeep Paulraj <s-paulraj@ti.com>
>
> The patch applies to linux-mtd GIT tree
>
> This patch adds the new mode NAND_ECC_HW_OOB_FIRST in the nand code to
> support 4-bit ECC on TI DaVinci devices with large page (up to 4K) NAND
> chips. This ECC mode is similar to NAND_ECC_HW, with the exception of
> read_page API that first reads the OOB area, reads the data in chunks, feeds
> the ECC from OOB area to the ECC hw engine and perform any correction on the
> data as per the ECC status reported by the engine.
>
> "ECC_HW_OOB_FIRST" name suggested by Thomas Gleixner

What I'd like to state is that we're starting to have so many
HW_OOB_XXX variants that it gets ridiculous. I'm sure we've come to a
point where we need to reconsider the base method for handing data and
ECC. Especially given the DMA capabilities of the most modern NAND
controllers.

Thanks,
   Vitaly