All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 0/3] MTD: at91: Add PMECC support for at91 nand flash driver
@ 2012-05-26 13:24 ` Josh Wu
  0 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-26 13:24 UTC (permalink / raw)
  To: linux-mtd, linux-arm-kernel, dedekind1
  Cc: hongxu.cn, nicolas.ferre, ivan.djelic, plagnioj, Josh Wu

Code is based on 3.4-rc2

Changes since v8,
	use _relaxed read/write in most place. use writel in operations of Control Register since it needs memory barrier.
	allocate the data for PMECC computation.
	add pmecc prefix for related variable/functions.
	modify code according to J.C's suggestion. except:
		>> +	for (i = 2; i <= 2 * host->cap; i += 2) {
		> manage the j in the for loop
		since that will change to: 
		+	for (i = 2, j = 1; i <= 2 * host->cap; i += 2, j = i / 2) {
		it is not as simple as original one.

Changes since v7,
	add time out for PMECC status reading.
	modify the oobfree[0].offset to 2.
	fix coding style.

Changes since v6,
	split into 3 patches.
	remove of_flat_dt_is_compatible() function. use additional dt parameter "has-pmecc".
	refine the error handling code.
	refine original atmel_nand_init_params() function.

Changes since v5,
	add has_pmecc field to replace cpu_has_pmecc() function. Use compatible check in when proble.
	simplify the pmecc_get_ecc_bytes() function.

Changes since v4,
	fix typo and checkpatch warnings.
	fix according to JC's suggestion. replace cpu_is_xxx() with DT
	modify dt binding atmel nand document to add pmecc support.
	tested in sam9263 without break hw ecc.
	add ecc.strength.

Josh Wu (3):
  extract the hw ecc function.
  add DT variables.
  add pmecc support

 .../devicetree/bindings/mtd/atmel-nand.txt         |    6 +
 drivers/mtd/nand/atmel_nand.c                      |  939 ++++++++++++++++++--
 drivers/mtd/nand/atmel_nand_ecc.h                  |  127 ++-
 3 files changed, 997 insertions(+), 75 deletions(-)

-- 
1.7.10

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 0/3] MTD: at91: Add PMECC support for at91 nand flash driver
@ 2012-05-26 13:24 ` Josh Wu
  0 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-26 13:24 UTC (permalink / raw)
  To: linux-arm-kernel

Code is based on 3.4-rc2

Changes since v8,
	use _relaxed read/write in most place. use writel in operations of Control Register since it needs memory barrier.
	allocate the data for PMECC computation.
	add pmecc prefix for related variable/functions.
	modify code according to J.C's suggestion. except:
		>> +	for (i = 2; i <= 2 * host->cap; i += 2) {
		> manage the j in the for loop
		since that will change to: 
		+	for (i = 2, j = 1; i <= 2 * host->cap; i += 2, j = i / 2) {
		it is not as simple as original one.

Changes since v7,
	add time out for PMECC status reading.
	modify the oobfree[0].offset to 2.
	fix coding style.

Changes since v6,
	split into 3 patches.
	remove of_flat_dt_is_compatible() function. use additional dt parameter "has-pmecc".
	refine the error handling code.
	refine original atmel_nand_init_params() function.

Changes since v5,
	add has_pmecc field to replace cpu_has_pmecc() function. Use compatible check in when proble.
	simplify the pmecc_get_ecc_bytes() function.

Changes since v4,
	fix typo and checkpatch warnings.
	fix according to JC's suggestion. replace cpu_is_xxx() with DT
	modify dt binding atmel nand document to add pmecc support.
	tested in sam9263 without break hw ecc.
	add ecc.strength.

Josh Wu (3):
  extract the hw ecc function.
  add DT variables.
  add pmecc support

 .../devicetree/bindings/mtd/atmel-nand.txt         |    6 +
 drivers/mtd/nand/atmel_nand.c                      |  939 ++++++++++++++++++--
 drivers/mtd/nand/atmel_nand_ecc.h                  |  127 ++-
 3 files changed, 997 insertions(+), 75 deletions(-)

-- 
1.7.10

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 1/3] MTD: at91: extract hw ecc initialization to one function and use relaxed read/write
  2012-05-26 13:24 ` Josh Wu
@ 2012-05-26 13:24   ` Josh Wu
  -1 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-26 13:24 UTC (permalink / raw)
  To: linux-mtd, linux-arm-kernel, dedekind1
  Cc: hongxu.cn, nicolas.ferre, ivan.djelic, plagnioj, Josh Wu

use _relaxed read/write in most place. And use writel in operations of Control Register since it needs memory barrier.

Signed-off-by: Hong Xu <hong.xu@atmel.com>
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/mtd/nand/atmel_nand.c     |  158 ++++++++++++++++++++-----------------
 drivers/mtd/nand/atmel_nand_ecc.h |   11 ++-
 2 files changed, 94 insertions(+), 75 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 2165576..ba61153 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -42,20 +42,15 @@
 
 #include <mach/cpu.h>
 
+/* Hardware ECC registers */
+#include "atmel_nand_ecc.h"
+
 static int use_dma = 1;
 module_param(use_dma, int, 0);
 
 static int on_flash_bbt = 0;
 module_param(on_flash_bbt, int, 0);
 
-/* Register access macros */
-#define ecc_readl(add, reg)				\
-	__raw_readl(add + ATMEL_ECC_##reg)
-#define ecc_writel(add, reg, value)			\
-	__raw_writel((value), add + ATMEL_ECC_##reg)
-
-#include "atmel_nand_ecc.h"	/* Hardware ECC registers */
-
 /* oob layout for large page size
  * bad block info is on bytes 0 and 1
  * the bytes have to be consecutives to avoid
@@ -304,13 +299,13 @@ static int atmel_nand_calculate(struct mtd_info *mtd,
 	unsigned int ecc_value;
 
 	/* get the first 2 ECC bytes */
-	ecc_value = ecc_readl(host->ecc, PR);
+	ecc_value = ecc_readl_relaxed(host->ecc, PR);
 
 	ecc_code[0] = ecc_value & 0xFF;
 	ecc_code[1] = (ecc_value >> 8) & 0xFF;
 
 	/* get the last 2 ECC bytes */
-	ecc_value = ecc_readl(host->ecc, NPR) & ATMEL_ECC_NPARITY;
+	ecc_value = ecc_readl_relaxed(host->ecc, NPR) & ATMEL_ECC_NPARITY;
 
 	ecc_code[2] = ecc_value & 0xFF;
 	ecc_code[3] = (ecc_value >> 8) & 0xFF;
@@ -406,16 +401,16 @@ static int atmel_nand_correct(struct mtd_info *mtd, u_char *dat,
 	unsigned int ecc_word, ecc_bit;
 
 	/* get the status from the Status Register */
-	ecc_status = ecc_readl(host->ecc, SR);
+	ecc_status = ecc_readl_relaxed(host->ecc, SR);
 
 	/* if there's no error */
 	if (likely(!(ecc_status & ATMEL_ECC_RECERR)))
 		return 0;
 
 	/* get error bit offset (4 bits) */
-	ecc_bit = ecc_readl(host->ecc, PR) & ATMEL_ECC_BITADDR;
+	ecc_bit = ecc_readl_relaxed(host->ecc, PR) & ATMEL_ECC_BITADDR;
 	/* get word address (12 bits) */
-	ecc_word = ecc_readl(host->ecc, PR) & ATMEL_ECC_WORDADDR;
+	ecc_word = ecc_readl_relaxed(host->ecc, PR) & ATMEL_ECC_WORDADDR;
 	ecc_word >>= 4;
 
 	/* if there are multiple errors */
@@ -523,6 +518,76 @@ static int __devinit atmel_of_init_port(struct atmel_nand_host *host,
 }
 #endif
 
+static int __init atmel_hw_nand_init_params(struct platform_device *pdev,
+					 struct atmel_nand_host *host)
+{
+	struct resource		*regs;
+	struct mtd_info		*mtd;
+	struct nand_chip	*nand_chip;
+
+	nand_chip = &host->nand_chip;
+	mtd = &host->mtd;
+
+	nand_chip->ecc.mode = NAND_ECC_SOFT;
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!regs) {
+		dev_err(host->dev,
+			"Can't get I/O resource regs, use software ECC\n");
+		return 0;
+	}
+
+	host->ecc = ioremap(regs->start, resource_size(regs));
+	if (host->ecc == NULL) {
+		dev_err(host->dev, "ioremap failed\n");
+		return -EIO;
+	}
+
+	nand_chip->ecc.mode = NAND_ECC_HW;
+	nand_chip->ecc.calculate = atmel_nand_calculate;
+	nand_chip->ecc.correct = atmel_nand_correct;
+	nand_chip->ecc.hwctl = atmel_nand_hwctl;
+	nand_chip->ecc.read_page = atmel_nand_read_page;
+	nand_chip->ecc.bytes = 4;
+	nand_chip->ecc.strength = 1;
+
+	/* ECC is calculated for the whole page (1 step) */
+	nand_chip->ecc.size = mtd->writesize;
+
+	/* set ECC page size and oob layout */
+	switch (mtd->writesize) {
+	case 512:
+		nand_chip->ecc.layout = &atmel_oobinfo_small;
+		ecc_writel_relaxed(host->ecc, MR, ATMEL_ECC_PAGESIZE_528);
+		break;
+	case 1024:
+		nand_chip->ecc.layout = &atmel_oobinfo_large;
+		ecc_writel_relaxed(host->ecc, MR, ATMEL_ECC_PAGESIZE_1056);
+		break;
+	case 2048:
+		nand_chip->ecc.layout = &atmel_oobinfo_large;
+		ecc_writel_relaxed(host->ecc, MR, ATMEL_ECC_PAGESIZE_2112);
+		break;
+	case 4096:
+		nand_chip->ecc.layout = &atmel_oobinfo_large;
+		ecc_writel_relaxed(host->ecc, MR, ATMEL_ECC_PAGESIZE_4224);
+		break;
+	default:
+		/* page size not handled by HW ECC */
+		/* switching back to soft ECC */
+		nand_chip->ecc.mode = NAND_ECC_SOFT;
+		nand_chip->ecc.calculate = NULL;
+		nand_chip->ecc.correct = NULL;
+		nand_chip->ecc.hwctl = NULL;
+		nand_chip->ecc.read_page = NULL;
+		nand_chip->ecc.postpad = 0;
+		nand_chip->ecc.prepad = 0;
+		nand_chip->ecc.bytes = 0;
+		break;
+	}
+
+	return 0;
+}
+
 /*
  * Probe for the NAND device.
  */
@@ -531,7 +596,6 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	struct atmel_nand_host *host;
 	struct mtd_info *mtd;
 	struct nand_chip *nand_chip;
-	struct resource *regs;
 	struct resource *mem;
 	struct mtd_part_parser_data ppdata = {};
 	int res;
@@ -583,29 +647,6 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 		nand_chip->dev_ready = atmel_nand_device_ready;
 
 	nand_chip->ecc.mode = host->board.ecc_mode;
-
-	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!regs && nand_chip->ecc.mode == NAND_ECC_HW) {
-		printk(KERN_ERR "atmel_nand: can't get I/O resource "
-				"regs\nFalling back on software ECC\n");
-		nand_chip->ecc.mode = NAND_ECC_SOFT;
-	}
-
-	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		host->ecc = ioremap(regs->start, resource_size(regs));
-		if (host->ecc == NULL) {
-			printk(KERN_ERR "atmel_nand: ioremap failed\n");
-			res = -EIO;
-			goto err_ecc_ioremap;
-		}
-		nand_chip->ecc.calculate = atmel_nand_calculate;
-		nand_chip->ecc.correct = atmel_nand_correct;
-		nand_chip->ecc.hwctl = atmel_nand_hwctl;
-		nand_chip->ecc.read_page = atmel_nand_read_page;
-		nand_chip->ecc.bytes = 4;
-		nand_chip->ecc.strength = 1;
-	}
-
 	nand_chip->chip_delay = 20;		/* 20us command delay time */
 
 	if (host->board.bus_width_16)	/* 16-bit bus width */
@@ -657,40 +698,9 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	}
 
 	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		/* ECC is calculated for the whole page (1 step) */
-		nand_chip->ecc.size = mtd->writesize;
-
-		/* set ECC page size and oob layout */
-		switch (mtd->writesize) {
-		case 512:
-			nand_chip->ecc.layout = &atmel_oobinfo_small;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_528);
-			break;
-		case 1024:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_1056);
-			break;
-		case 2048:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_2112);
-			break;
-		case 4096:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_4224);
-			break;
-		default:
-			/* page size not handled by HW ECC */
-			/* switching back to soft ECC */
-			nand_chip->ecc.mode = NAND_ECC_SOFT;
-			nand_chip->ecc.calculate = NULL;
-			nand_chip->ecc.correct = NULL;
-			nand_chip->ecc.hwctl = NULL;
-			nand_chip->ecc.read_page = NULL;
-			nand_chip->ecc.postpad = 0;
-			nand_chip->ecc.prepad = 0;
-			nand_chip->ecc.bytes = 0;
-			break;
-		}
+		res = atmel_hw_nand_init_params(pdev, host);
+		if (res != 0)
+			goto err_hw_ecc;
 	}
 
 	/* second phase scan */
@@ -707,15 +717,15 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 		return res;
 
 err_scan_tail:
+	if (host->ecc)
+		iounmap(host->ecc);
+err_hw_ecc:
 err_scan_ident:
 err_no_card:
 	atmel_nand_disable(host);
 	platform_set_drvdata(pdev, NULL);
 	if (host->dma_chan)
 		dma_release_channel(host->dma_chan);
-	if (host->ecc)
-		iounmap(host->ecc);
-err_ecc_ioremap:
 	iounmap(host->io_base);
 err_nand_ioremap:
 	kfree(host);
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
index 578c776..5cbff11 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -3,7 +3,7 @@
  * Based on AT91SAM9260 datasheet revision B.
  *
  * Copyright (C) 2007 Andrew Victor
- * Copyright (C) 2007 Atmel Corporation.
+ * Copyright (C) 2007 - 2012 Atmel Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -36,4 +36,13 @@
 #define ATMEL_ECC_NPR		0x10			/* NParity register */
 #define		ATMEL_ECC_NPARITY	(0xffff << 0)		/* NParity */
 
+/* Relaxed version of Register Access Macros */
+#define ecc_readl_relaxed(add, reg)				\
+	readl_relaxed(add + ATMEL_ECC_##reg)
+#define ecc_writel_relaxed(add, reg, value)			\
+	writel_relaxed((value), add + ATMEL_ECC_##reg)
+/* Register Write Macros with memory barries, use to write control regiser */
+#define ecc_writel(add, reg, value)				\
+	writel((value), add + ATMEL_ECC_##reg)
+
 #endif
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v9 1/3] MTD: at91: extract hw ecc initialization to one function and use relaxed read/write
@ 2012-05-26 13:24   ` Josh Wu
  0 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-26 13:24 UTC (permalink / raw)
  To: linux-arm-kernel

use _relaxed read/write in most place. And use writel in operations of Control Register since it needs memory barrier.

Signed-off-by: Hong Xu <hong.xu@atmel.com>
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/mtd/nand/atmel_nand.c     |  158 ++++++++++++++++++++-----------------
 drivers/mtd/nand/atmel_nand_ecc.h |   11 ++-
 2 files changed, 94 insertions(+), 75 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 2165576..ba61153 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -42,20 +42,15 @@
 
 #include <mach/cpu.h>
 
+/* Hardware ECC registers */
+#include "atmel_nand_ecc.h"
+
 static int use_dma = 1;
 module_param(use_dma, int, 0);
 
 static int on_flash_bbt = 0;
 module_param(on_flash_bbt, int, 0);
 
-/* Register access macros */
-#define ecc_readl(add, reg)				\
-	__raw_readl(add + ATMEL_ECC_##reg)
-#define ecc_writel(add, reg, value)			\
-	__raw_writel((value), add + ATMEL_ECC_##reg)
-
-#include "atmel_nand_ecc.h"	/* Hardware ECC registers */
-
 /* oob layout for large page size
  * bad block info is on bytes 0 and 1
  * the bytes have to be consecutives to avoid
@@ -304,13 +299,13 @@ static int atmel_nand_calculate(struct mtd_info *mtd,
 	unsigned int ecc_value;
 
 	/* get the first 2 ECC bytes */
-	ecc_value = ecc_readl(host->ecc, PR);
+	ecc_value = ecc_readl_relaxed(host->ecc, PR);
 
 	ecc_code[0] = ecc_value & 0xFF;
 	ecc_code[1] = (ecc_value >> 8) & 0xFF;
 
 	/* get the last 2 ECC bytes */
-	ecc_value = ecc_readl(host->ecc, NPR) & ATMEL_ECC_NPARITY;
+	ecc_value = ecc_readl_relaxed(host->ecc, NPR) & ATMEL_ECC_NPARITY;
 
 	ecc_code[2] = ecc_value & 0xFF;
 	ecc_code[3] = (ecc_value >> 8) & 0xFF;
@@ -406,16 +401,16 @@ static int atmel_nand_correct(struct mtd_info *mtd, u_char *dat,
 	unsigned int ecc_word, ecc_bit;
 
 	/* get the status from the Status Register */
-	ecc_status = ecc_readl(host->ecc, SR);
+	ecc_status = ecc_readl_relaxed(host->ecc, SR);
 
 	/* if there's no error */
 	if (likely(!(ecc_status & ATMEL_ECC_RECERR)))
 		return 0;
 
 	/* get error bit offset (4 bits) */
-	ecc_bit = ecc_readl(host->ecc, PR) & ATMEL_ECC_BITADDR;
+	ecc_bit = ecc_readl_relaxed(host->ecc, PR) & ATMEL_ECC_BITADDR;
 	/* get word address (12 bits) */
-	ecc_word = ecc_readl(host->ecc, PR) & ATMEL_ECC_WORDADDR;
+	ecc_word = ecc_readl_relaxed(host->ecc, PR) & ATMEL_ECC_WORDADDR;
 	ecc_word >>= 4;
 
 	/* if there are multiple errors */
@@ -523,6 +518,76 @@ static int __devinit atmel_of_init_port(struct atmel_nand_host *host,
 }
 #endif
 
+static int __init atmel_hw_nand_init_params(struct platform_device *pdev,
+					 struct atmel_nand_host *host)
+{
+	struct resource		*regs;
+	struct mtd_info		*mtd;
+	struct nand_chip	*nand_chip;
+
+	nand_chip = &host->nand_chip;
+	mtd = &host->mtd;
+
+	nand_chip->ecc.mode = NAND_ECC_SOFT;
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!regs) {
+		dev_err(host->dev,
+			"Can't get I/O resource regs, use software ECC\n");
+		return 0;
+	}
+
+	host->ecc = ioremap(regs->start, resource_size(regs));
+	if (host->ecc == NULL) {
+		dev_err(host->dev, "ioremap failed\n");
+		return -EIO;
+	}
+
+	nand_chip->ecc.mode = NAND_ECC_HW;
+	nand_chip->ecc.calculate = atmel_nand_calculate;
+	nand_chip->ecc.correct = atmel_nand_correct;
+	nand_chip->ecc.hwctl = atmel_nand_hwctl;
+	nand_chip->ecc.read_page = atmel_nand_read_page;
+	nand_chip->ecc.bytes = 4;
+	nand_chip->ecc.strength = 1;
+
+	/* ECC is calculated for the whole page (1 step) */
+	nand_chip->ecc.size = mtd->writesize;
+
+	/* set ECC page size and oob layout */
+	switch (mtd->writesize) {
+	case 512:
+		nand_chip->ecc.layout = &atmel_oobinfo_small;
+		ecc_writel_relaxed(host->ecc, MR, ATMEL_ECC_PAGESIZE_528);
+		break;
+	case 1024:
+		nand_chip->ecc.layout = &atmel_oobinfo_large;
+		ecc_writel_relaxed(host->ecc, MR, ATMEL_ECC_PAGESIZE_1056);
+		break;
+	case 2048:
+		nand_chip->ecc.layout = &atmel_oobinfo_large;
+		ecc_writel_relaxed(host->ecc, MR, ATMEL_ECC_PAGESIZE_2112);
+		break;
+	case 4096:
+		nand_chip->ecc.layout = &atmel_oobinfo_large;
+		ecc_writel_relaxed(host->ecc, MR, ATMEL_ECC_PAGESIZE_4224);
+		break;
+	default:
+		/* page size not handled by HW ECC */
+		/* switching back to soft ECC */
+		nand_chip->ecc.mode = NAND_ECC_SOFT;
+		nand_chip->ecc.calculate = NULL;
+		nand_chip->ecc.correct = NULL;
+		nand_chip->ecc.hwctl = NULL;
+		nand_chip->ecc.read_page = NULL;
+		nand_chip->ecc.postpad = 0;
+		nand_chip->ecc.prepad = 0;
+		nand_chip->ecc.bytes = 0;
+		break;
+	}
+
+	return 0;
+}
+
 /*
  * Probe for the NAND device.
  */
@@ -531,7 +596,6 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	struct atmel_nand_host *host;
 	struct mtd_info *mtd;
 	struct nand_chip *nand_chip;
-	struct resource *regs;
 	struct resource *mem;
 	struct mtd_part_parser_data ppdata = {};
 	int res;
@@ -583,29 +647,6 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 		nand_chip->dev_ready = atmel_nand_device_ready;
 
 	nand_chip->ecc.mode = host->board.ecc_mode;
-
-	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!regs && nand_chip->ecc.mode == NAND_ECC_HW) {
-		printk(KERN_ERR "atmel_nand: can't get I/O resource "
-				"regs\nFalling back on software ECC\n");
-		nand_chip->ecc.mode = NAND_ECC_SOFT;
-	}
-
-	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		host->ecc = ioremap(regs->start, resource_size(regs));
-		if (host->ecc == NULL) {
-			printk(KERN_ERR "atmel_nand: ioremap failed\n");
-			res = -EIO;
-			goto err_ecc_ioremap;
-		}
-		nand_chip->ecc.calculate = atmel_nand_calculate;
-		nand_chip->ecc.correct = atmel_nand_correct;
-		nand_chip->ecc.hwctl = atmel_nand_hwctl;
-		nand_chip->ecc.read_page = atmel_nand_read_page;
-		nand_chip->ecc.bytes = 4;
-		nand_chip->ecc.strength = 1;
-	}
-
 	nand_chip->chip_delay = 20;		/* 20us command delay time */
 
 	if (host->board.bus_width_16)	/* 16-bit bus width */
@@ -657,40 +698,9 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	}
 
 	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		/* ECC is calculated for the whole page (1 step) */
-		nand_chip->ecc.size = mtd->writesize;
-
-		/* set ECC page size and oob layout */
-		switch (mtd->writesize) {
-		case 512:
-			nand_chip->ecc.layout = &atmel_oobinfo_small;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_528);
-			break;
-		case 1024:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_1056);
-			break;
-		case 2048:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_2112);
-			break;
-		case 4096:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_4224);
-			break;
-		default:
-			/* page size not handled by HW ECC */
-			/* switching back to soft ECC */
-			nand_chip->ecc.mode = NAND_ECC_SOFT;
-			nand_chip->ecc.calculate = NULL;
-			nand_chip->ecc.correct = NULL;
-			nand_chip->ecc.hwctl = NULL;
-			nand_chip->ecc.read_page = NULL;
-			nand_chip->ecc.postpad = 0;
-			nand_chip->ecc.prepad = 0;
-			nand_chip->ecc.bytes = 0;
-			break;
-		}
+		res = atmel_hw_nand_init_params(pdev, host);
+		if (res != 0)
+			goto err_hw_ecc;
 	}
 
 	/* second phase scan */
@@ -707,15 +717,15 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 		return res;
 
 err_scan_tail:
+	if (host->ecc)
+		iounmap(host->ecc);
+err_hw_ecc:
 err_scan_ident:
 err_no_card:
 	atmel_nand_disable(host);
 	platform_set_drvdata(pdev, NULL);
 	if (host->dma_chan)
 		dma_release_channel(host->dma_chan);
-	if (host->ecc)
-		iounmap(host->ecc);
-err_ecc_ioremap:
 	iounmap(host->io_base);
 err_nand_ioremap:
 	kfree(host);
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
index 578c776..5cbff11 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -3,7 +3,7 @@
  * Based on AT91SAM9260 datasheet revision B.
  *
  * Copyright (C) 2007 Andrew Victor
- * Copyright (C) 2007 Atmel Corporation.
+ * Copyright (C) 2007 - 2012 Atmel Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -36,4 +36,13 @@
 #define ATMEL_ECC_NPR		0x10			/* NParity register */
 #define		ATMEL_ECC_NPARITY	(0xffff << 0)		/* NParity */
 
+/* Relaxed version of Register Access Macros */
+#define ecc_readl_relaxed(add, reg)				\
+	readl_relaxed(add + ATMEL_ECC_##reg)
+#define ecc_writel_relaxed(add, reg, value)			\
+	writel_relaxed((value), add + ATMEL_ECC_##reg)
+/* Register Write Macros with memory barries, use to write control regiser */
+#define ecc_writel(add, reg, value)				\
+	writel((value), add + ATMEL_ECC_##reg)
+
 #endif
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v9 2/3] MTD: at91: add dt parameters for PMECC
  2012-05-26 13:24 ` Josh Wu
@ 2012-05-26 13:24   ` Josh Wu
  -1 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-26 13:24 UTC (permalink / raw)
  To: linux-mtd, linux-arm-kernel, dedekind1
  Cc: hongxu.cn, nicolas.ferre, ivan.djelic, plagnioj, Josh Wu

Signed-off-by: Hong Xu <hong.xu@atmel.com>
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 .../devicetree/bindings/mtd/atmel-nand.txt         |    6 ++++++
 drivers/mtd/nand/atmel_nand.c                      |   22 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index a200695..e442236 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -16,6 +16,12 @@ Optional properties:
 - nand-ecc-mode : String, operation mode of the NAND ecc mode, soft by default.
   Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
   "soft_bch".
+- atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
+  Only supported by at91sam9x5 or later sam9 product.
+- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
+  Controller. Supported values are: 2, 4, 8, 12, 24.
+- atmel,pmecc-sector-size : sector size for ECC computation. Supported values
+  are: 512, 1024.
 - nand-bus-width : 8 or 16 bus width if not present 8
 - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
 
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index ba61153..9a9bfbf 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -88,6 +88,10 @@ struct atmel_nand_host {
 
 	struct completion	comp;
 	struct dma_chan		*dma_chan;
+
+	bool			has_pmecc;
+	u8			pmecc_corr_cap;
+	u16			pmecc_sector_size;
 };
 
 static int cpu_has_dma(void)
@@ -508,6 +512,24 @@ static int __devinit atmel_of_init_port(struct atmel_nand_host *host,
 	board->enable_pin = of_get_gpio(np, 1);
 	board->det_pin = of_get_gpio(np, 2);
 
+	host->has_pmecc = of_property_read_bool(np, "atmel,has-pmecc");
+
+	if (!(board->ecc_mode == NAND_ECC_HW) || !host->has_pmecc)
+		return 0;
+
+	/* Use PMECC */
+	if (of_property_read_u32(np, "atmel,pmecc-cap", &val) != 0) {
+		dev_err(host->dev, "Cannot decide PMECC Capability\n");
+		return -EINVAL;
+	}
+	host->pmecc_corr_cap = (u8)val;
+
+	if (of_property_read_u32(np, "atmel,pmecc-sector-size", &val) != 0) {
+		dev_err(host->dev, "Cannot decide PMECC Sector Size\n");
+		return -EINVAL;
+	}
+	host->pmecc_sector_size = (u16)val;
+
 	return 0;
 }
 #else
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v9 2/3] MTD: at91: add dt parameters for PMECC
@ 2012-05-26 13:24   ` Josh Wu
  0 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-26 13:24 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Hong Xu <hong.xu@atmel.com>
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 .../devicetree/bindings/mtd/atmel-nand.txt         |    6 ++++++
 drivers/mtd/nand/atmel_nand.c                      |   22 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index a200695..e442236 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -16,6 +16,12 @@ Optional properties:
 - nand-ecc-mode : String, operation mode of the NAND ecc mode, soft by default.
   Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
   "soft_bch".
+- atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
+  Only supported by at91sam9x5 or later sam9 product.
+- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
+  Controller. Supported values are: 2, 4, 8, 12, 24.
+- atmel,pmecc-sector-size : sector size for ECC computation. Supported values
+  are: 512, 1024.
 - nand-bus-width : 8 or 16 bus width if not present 8
 - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
 
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index ba61153..9a9bfbf 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -88,6 +88,10 @@ struct atmel_nand_host {
 
 	struct completion	comp;
 	struct dma_chan		*dma_chan;
+
+	bool			has_pmecc;
+	u8			pmecc_corr_cap;
+	u16			pmecc_sector_size;
 };
 
 static int cpu_has_dma(void)
@@ -508,6 +512,24 @@ static int __devinit atmel_of_init_port(struct atmel_nand_host *host,
 	board->enable_pin = of_get_gpio(np, 1);
 	board->det_pin = of_get_gpio(np, 2);
 
+	host->has_pmecc = of_property_read_bool(np, "atmel,has-pmecc");
+
+	if (!(board->ecc_mode == NAND_ECC_HW) || !host->has_pmecc)
+		return 0;
+
+	/* Use PMECC */
+	if (of_property_read_u32(np, "atmel,pmecc-cap", &val) != 0) {
+		dev_err(host->dev, "Cannot decide PMECC Capability\n");
+		return -EINVAL;
+	}
+	host->pmecc_corr_cap = (u8)val;
+
+	if (of_property_read_u32(np, "atmel,pmecc-sector-size", &val) != 0) {
+		dev_err(host->dev, "Cannot decide PMECC Sector Size\n");
+		return -EINVAL;
+	}
+	host->pmecc_sector_size = (u16)val;
+
 	return 0;
 }
 #else
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
  2012-05-26 13:24 ` Josh Wu
@ 2012-05-26 13:24   ` Josh Wu
  -1 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-26 13:24 UTC (permalink / raw)
  To: linux-mtd, linux-arm-kernel, dedekind1
  Cc: hongxu.cn, nicolas.ferre, ivan.djelic, plagnioj, Josh Wu

The Programmable Multibit ECC (PMECC) controller is a programmable binary
BCH(Bose, Chaudhuri and Hocquenghem) encoder and decoder. This controller
can be used to support both SLC and MLC NAND Flash devices. It supports to
generate ECC to correct 2, 4, 8, 12 or 24 bits of error per sector of data.

To use this driver, the user needs to pass in the correction capability and
the sector size.

This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2,
YAFFS2, UBIFS and mtd-utils.

Signed-off-by: Hong Xu <hong.xu@atmel.com>
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/mtd/nand/atmel_nand.c     |  761 ++++++++++++++++++++++++++++++++++++-
 drivers/mtd/nand/atmel_nand_ecc.h |  116 ++++++
 2 files changed, 876 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 9a9bfbf..ddcf1ed 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -15,6 +15,8 @@
  *     		(u-boot-1.1.5/board/atmel/at91sam9263ek/nand.c)
  *     (C) Copyright 2006 ATMEL Rousset, Lacressonniere Nicolas
  *
+ *  Add Programmable Multibit ECC support for various AT91 SoC
+ *     (C) Copyright 2012 ATMEL, Hong Xu
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -77,6 +79,21 @@ static struct nand_ecclayout atmel_oobinfo_small = {
 	},
 };
 
+/* a structure includes datas for PMECC computation */
+struct atmel_pmecc_data {
+	int16_t			partial_syn[2 * PMECC_MAX_ERROR_NB + 1];
+	int16_t			si[2 * PMECC_MAX_ERROR_NB + 1];
+
+	/* Sigma table */
+	int16_t		smu[PMECC_MAX_ERROR_NB + 2][2 * PMECC_MAX_ERROR_NB + 1];
+	/* polynomal order */
+	int16_t			lmu[PMECC_MAX_ERROR_NB + 1];
+
+	int			mu[PMECC_MAX_ERROR_NB + 1];
+	int			dmu[PMECC_MAX_ERROR_NB + 1];
+	int			delta[PMECC_MAX_ERROR_NB + 1];
+};
+
 struct atmel_nand_host {
 	struct nand_chip	nand_chip;
 	struct mtd_info		mtd;
@@ -92,8 +109,25 @@ struct atmel_nand_host {
 	bool			has_pmecc;
 	u8			pmecc_corr_cap;
 	u16			pmecc_sector_size;
+
+	int			pmecc_bytes_per_sector;
+	int			pmecc_sector_number;
+	int			pmecc_degree;	/* Degree of remainders */
+	int			pmecc_cw_len;	/* Length of codeword */
+
+	void __iomem		*pmerrloc_base;
+	void __iomem		*pmecc_rom_base;
+
+	/* lookup table for alpha_to and index_of */
+	void __iomem		*pmecc_alpha_to;
+	void __iomem		*pmecc_index_of;
+
+	/* data for pmecc computation */
+	struct atmel_pmecc_data	*pmecc_data;
 };
 
+static struct nand_ecclayout atmel_pmecc_oobinfo;
+
 static int cpu_has_dma(void)
 {
 	return cpu_is_at91sam9rl() || cpu_is_at91sam9g45();
@@ -287,6 +321,708 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 }
 
 /*
+ * Return number of ecc bytes per sector according to sector size and
+ * correction capability
+ *
+ * Following table shows what at91 PMECC supported:
+ * Correction Capability	Sector_512_bytes	Sector_1024_bytes
+ * =====================	================	=================
+ *                2-bits                 4-bytes                  4-bytes
+ *                4-bits                 7-bytes                  7-bytes
+ *                8-bits                13-bytes                 14-bytes
+ *               12-bits                20-bytes                 21-bytes
+ *               24-bits                39-bytes                 42-bytes
+ */
+static int pmecc_get_ecc_bytes(int cap, int sector_size)
+{
+	int m = 12 + sector_size / 512;
+	return (m * cap + 7) / 8;
+}
+
+static void pmecc_config_ecc_layout(struct nand_ecclayout *layout, int oobsize,
+	int ecc_len)
+{
+	int i;
+
+	layout->eccbytes = ecc_len;
+
+	/* ECC will occupy the last ecc_len bytes continuously */
+	for (i = 0; i < ecc_len; i++)
+		layout->eccpos[i] = oobsize - ecc_len + i;
+
+	layout->oobfree[0].offset = 2;
+	layout->oobfree[0].length =
+		oobsize - ecc_len - layout->oobfree[0].offset;
+}
+
+static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
+{
+	void __iomem *p;
+
+	switch (host->pmecc_sector_size) {
+	case 512:
+		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512 +
+			PMECC_LOOKUP_TABLE_SIZE_512 * sizeof(int16_t);
+		break;
+	case 1024:
+		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024 +
+			PMECC_LOOKUP_TABLE_SIZE_1024 * sizeof(int16_t);
+		break;
+	default:
+		BUG();
+	}
+
+	return p;
+}
+
+static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
+{
+	void __iomem *p;
+
+	switch (host->pmecc_sector_size) {
+	case 512:
+		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512;
+		break;
+	case 1024:
+		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024;
+		break;
+	default:
+		BUG();
+	}
+
+	return p;
+}
+
+static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
+{
+	int			i;
+	uint32_t		value;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	/* Fill odd syndromes */
+	for (i = 0; i < host->pmecc_corr_cap; i++) {
+		value = pmecc_readl_rem_relaxed(host->ecc, sector, i / 2);
+		value = (i & 1) ? (value & 0xffff0000) >> 16 : value & 0xffff;
+		host->pmecc_data->partial_syn[(2 * i) + 1] = (int16_t)value;
+	}
+}
+
+static void pmecc_substitute(struct mtd_info *mtd)
+{
+	int16_t			*si;
+	int			i, j;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+	int16_t __iomem		*alpha_to = host->pmecc_alpha_to;
+	int16_t __iomem		*index_of = host->pmecc_index_of;
+	int16_t			*partial_syn = host->pmecc_data->partial_syn;
+
+	/* si[] is a table that holds the current syndrome value,
+	 * an element of that table belongs to the field
+	 */
+	si = host->pmecc_data->si;
+
+	for (i = 1; i < 2 * PMECC_MAX_ERROR_NB; i++)
+		si[i] = 0;
+
+	/* Computation 2t syndromes based on S(x) */
+	/* Odd syndromes */
+	for (i = 1; i < 2 * host->pmecc_corr_cap; i += 2) {
+		si[i] = 0;
+		for (j = 0; j < host->pmecc_degree; j++) {
+			if (partial_syn[i] & ((unsigned short)0x1 << j))
+				si[i] = readw_relaxed(alpha_to + i * j) ^ si[i];
+		}
+	}
+	/* Even syndrome = (Odd syndrome) ** 2 */
+	for (i = 2; i <= 2 * host->pmecc_corr_cap; i += 2) {
+		j = i / 2;
+		if (si[j] == 0)
+			si[i] = 0;
+		else {
+			int16_t tmp;
+			tmp = readw_relaxed(index_of + si[j]);
+			tmp = (tmp * 2) % host->pmecc_cw_len;
+			si[i] = readw_relaxed(alpha_to + tmp);
+		}
+	}
+
+	return;
+}
+
+static void pmecc_get_sigma(struct mtd_info *mtd)
+{
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	int		i, j, k;
+	uint32_t	dmu_0_count, tmp;
+	int16_t		(*smu)[2 * PMECC_MAX_ERROR_NB + 1];
+	int16_t		*lmu = host->pmecc_data->lmu;
+	int16_t		*si = host->pmecc_data->si;
+	int		*mu = host->pmecc_data->mu;
+	int		*dmu = host->pmecc_data->dmu;	  /* Discrepancy */
+	int		*delta = host->pmecc_data->delta; /* Delta order */
+	int		cw_len = host->pmecc_cw_len;
+	int16_t		cap = host->pmecc_corr_cap;
+
+	int16_t __iomem	*index_of = host->pmecc_index_of;
+	int16_t __iomem	*alpha_to = host->pmecc_alpha_to;
+
+	/* index of largest delta */
+	int ro;
+	int largest;
+	int diff;
+
+	dmu_0_count = 0;
+	smu = host->pmecc_data->smu;
+
+	/* First Row */
+
+	/* Mu */
+	mu[0] = -1;
+
+	memset(&smu[0][0], 0,
+		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
+	smu[0][0] = 1;
+
+	/* discrepancy set to 1 */
+	dmu[0] = 1;
+	/* polynom order set to 0 */
+	lmu[0] = 0;
+	delta[0]  = (mu[0] * 2 - lmu[0]) >> 1;
+
+	/* Second Row */
+
+	/* Mu */
+	mu[1]  = 0;
+	/* Sigma(x) set to 1 */
+	memset(&smu[1][0], 0,
+		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
+	smu[1][0] = 1;
+
+	/* discrepancy set to S1 */
+	dmu[1] = si[1];
+
+	/* polynom order set to 0 */
+	lmu[1] = 0;
+
+	delta[1]  = (mu[1] * 2 - lmu[1]) >> 1;
+
+	/* Init the Sigma(x) last row */
+	memset(&smu[cap + 1][0], 0,
+		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
+
+	for (i = 1; i <= cap; i++) {
+		mu[i+1] = i << 1;
+		/* Begin Computing Sigma (Mu+1) and L(mu) */
+		/* check if discrepancy is set to 0 */
+		if (dmu[i] == 0) {
+			dmu_0_count++;
+
+			tmp = ((cap - (lmu[i] >> 1) - 1) / 2);
+			if ((cap - (lmu[i] >> 1) - 1) & 0x1)
+				tmp += 2;
+			else
+				tmp += 1;
+
+			if (dmu_0_count == tmp) {
+				for (j = 0; j <= (lmu[i] >> 1) + 1; j++)
+					smu[cap + 1][j] = smu[i][j];
+				lmu[cap + 1] = lmu[i];
+				return;
+			}
+
+			/* copy polynom */
+			for (j = 0; j <= lmu[i] >> 1; j++)
+				smu[i + 1][j] = smu[i][j];
+
+			/* copy previous polynom order to the next */
+			lmu[i + 1] = lmu[i];
+		} else {
+			ro = 0;
+			largest = -1;
+			/* find largest delta with dmu != 0 */
+			for (j = 0; j < i; j++) {
+				if ((dmu[j]) && (delta[j] > largest)) {
+					largest = delta[j];
+					ro = j;
+				}
+			}
+
+			/* compute difference */
+			diff = (mu[i] - mu[ro]);
+
+			/* Compute degree of the new smu polynomial */
+			if ((lmu[i] >> 1) > ((lmu[ro] >> 1) + diff))
+				lmu[i + 1] = lmu[i];
+			else
+				lmu[i + 1] = ((lmu[ro] >> 1) + diff) * 2;
+
+			/* Init smu[i+1] with 0 */
+			for (k = 0; k < (2 * PMECC_MAX_ERROR_NB + 1); k++)
+				smu[i+1][k] = 0;
+
+			/* Compute smu[i+1] */
+			for (k = 0; k <= lmu[ro] >> 1; k++) {
+				int16_t a, b, c;
+
+				if (!(smu[ro][k] && dmu[i]))
+					continue;
+				a = readw_relaxed(index_of + dmu[i]);
+				b = readw_relaxed(index_of + dmu[ro]);
+				c = readw_relaxed(index_of + smu[ro][k]);
+				tmp = a + (cw_len - b) + c;
+				a = readw_relaxed(alpha_to + tmp % cw_len);
+				smu[i + 1][k + diff] = a;
+			}
+
+			for (k = 0; k <= lmu[i] >> 1; k++)
+				smu[i + 1][k] ^= smu[i][k];
+		}
+
+		/* End Computing Sigma (Mu+1) and L(mu) */
+		/* In either case compute delta */
+		delta[i + 1]  = (mu[i + 1] * 2 - lmu[i + 1]) >> 1;
+
+		/* Do not compute discrepancy for the last iteration */
+		if (i >= cap)
+			continue;
+
+		for (k = 0 ; k <= (lmu[i + 1] >> 1); k++) {
+			tmp = 2 * (i - 1);
+			if (k == 0)
+				dmu[i + 1] = si[tmp + 3];
+			else if (smu[i+1][k] && si[tmp + 3 - k]) {
+				int16_t a, b, c;
+				a = readw_relaxed(index_of + smu[i + 1][k]);
+				b = si[2 * (i - 1) + 3 - k];
+				c = readw_relaxed(index_of + b);
+				tmp = a + c;
+				tmp %= cw_len;
+				dmu[i + 1] = readw_relaxed(alpha_to + tmp) ^
+					dmu[i + 1];
+			}
+		}
+	}
+
+	return;
+}
+
+static int pmecc_err_location(struct mtd_info *mtd)
+{
+	int			i;
+	int			err_nbr;	/* number of error */
+	int			roots_nbr;	/* number of roots */
+	int			sector_size;
+	uint32_t		val;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+	int			timeout_count = 0;
+	int			cap = host->pmecc_corr_cap;
+
+	err_nbr = 0;
+	sector_size = host->pmecc_sector_size;
+
+	pmerrloc_writel(host->pmerrloc_base, ELDIS, PMERRLOC_DISABLE);
+
+	for (i = 0; i <= host->pmecc_data->lmu[cap + 1] >> 1; i++) {
+		pmerrloc_writel_sigma_relaxed(host->pmerrloc_base, i,
+				      host->pmecc_data->smu[cap + 1][i]);
+		err_nbr++;
+	}
+
+	val = (err_nbr - 1) << 16;
+	if (sector_size == 1024)
+		val |= 1;
+
+	pmerrloc_writel(host->pmerrloc_base, ELCFG, val);
+	pmerrloc_writel(host->pmerrloc_base, ELEN,
+			sector_size * 8 + host->pmecc_degree * cap);
+
+	while (!(pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
+		 & PMERRLOC_CALC_DONE)) {
+		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT))
+			return -1;	/* Time out */
+		cpu_relax();
+	}
+
+	roots_nbr = (pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
+		& PMERRLOC_ERR_NUM_MASK) >> 8;
+	/* Number of roots == degree of smu hence <= cap */
+	if (roots_nbr == host->pmecc_data->lmu[cap + 1] >> 1)
+		return err_nbr - 1;
+
+	/* Number of roots does not match the degree of smu
+	 * unable to correct error */
+	return -1;
+}
+
+static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf,
+		int extra_bytes, int err_nbr)
+{
+	int			i = 0;
+	int			byte_pos, bit_pos;
+	int			sector_size, ecc_size;
+	uint32_t		tmp;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	sector_size = host->pmecc_sector_size;
+	ecc_size = nand_chip->ecc.bytes;
+
+	while (err_nbr) {
+		tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
+		byte_pos = tmp / 8;
+		bit_pos  = tmp % 8;
+		dev_info(host->dev, "PMECC correction, byte_pos: %d bit_pos: %d\n",
+					byte_pos, bit_pos);
+
+		if (byte_pos < (sector_size + extra_bytes)) {
+			tmp = sector_size +
+				pmecc_readl_relaxed(host->ecc, SADDR);
+
+			if (byte_pos < tmp)
+				*(buf + byte_pos) ^= (1 << bit_pos);
+			else
+				*(buf + byte_pos + ecc_size) ^= (1 << bit_pos);
+		}
+
+		i++;
+		err_nbr--;
+	}
+
+	return;
+}
+
+static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
+	u8 *ecc)
+{
+	int			i, err_nbr;
+	uint8_t			*buf_pos;
+	int			eccbytes;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	eccbytes = nand_chip->ecc.bytes;
+	for (i = 0; i < eccbytes; i++)
+		if (ecc[i] != 0xff)
+			goto normal_check;
+	/* Erased page, return OK */
+	return 0;
+
+normal_check:
+	for (i = 0; i < host->pmecc_sector_number; i++) {
+		err_nbr = 0;
+		if (pmecc_stat & 0x1) {
+			buf_pos = buf + i * host->pmecc_sector_size;
+
+			pmecc_gen_syndrome(mtd, i);
+			pmecc_substitute(mtd);
+			pmecc_get_sigma(mtd);
+
+			err_nbr = pmecc_err_location(mtd);
+			if (err_nbr == -1) {
+				dev_err(host->dev, "PMECC: Too many errors\n");
+				mtd->ecc_stats.failed++;
+				return -EIO;
+			} else {
+				pmecc_correct_data(mtd, buf_pos, 0, err_nbr);
+				mtd->ecc_stats.corrected += err_nbr;
+			}
+		}
+		pmecc_stat >>= 1;
+	}
+
+	return 0;
+}
+
+static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
+		struct nand_chip *chip, uint8_t *buf, int page)
+{
+	uint32_t		stat;
+	int			timeout_count = 0;
+	int			eccsize = chip->ecc.size;
+	uint8_t			*oob = chip->oob_poi;
+	struct atmel_nand_host	*host = chip->priv;
+	uint32_t		*eccpos = chip->ecc.layout->eccpos;
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG)
+		& ~PMECC_CFG_WRITE_OP) | PMECC_CFG_AUTO_ENABLE);
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
+
+	chip->read_buf(mtd, buf, eccsize);
+	chip->read_buf(mtd, oob, mtd->oobsize);
+
+	while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
+		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT))
+			return -EIO;	/* Time out */
+		cpu_relax();
+	}
+
+	stat = pmecc_readl_relaxed(host->ecc, ISR);
+	if (stat != 0) {
+		if (pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]) != 0)
+			return -EIO;
+	}
+
+	return 0;
+}
+
+static void atmel_nand_pmecc_write_page(struct mtd_info *mtd,
+		struct nand_chip *chip, const uint8_t *buf)
+{
+	int			i, j;
+	int			timeout_count = 0;
+	struct atmel_nand_host	*host = chip->priv;
+	uint32_t		*eccpos = chip->ecc.layout->eccpos;
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+
+	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG) |
+		PMECC_CFG_WRITE_OP) & ~PMECC_CFG_AUTO_ENABLE);
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
+
+	chip->write_buf(mtd, (u8 *)buf, mtd->writesize);
+
+	while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
+		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT)) {
+			dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
+			return;	/* Time out */
+		}
+		cpu_relax();
+	}
+
+	for (i = 0; i < host->pmecc_sector_number; i++) {
+		for (j = 0; j < host->pmecc_bytes_per_sector; j++) {
+			int pos;
+
+			pos = i * host->pmecc_bytes_per_sector + j;
+			chip->oob_poi[eccpos[pos]] =
+				pmecc_readb_ecc_relaxed(host->ecc, i, j);
+		}
+	}
+	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	return;
+}
+
+static void atmel_pmecc_core_init(struct mtd_info *mtd)
+{
+	uint32_t		val = 0;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+	struct nand_ecclayout	*ecc_layout;
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+
+	switch (host->pmecc_corr_cap) {
+	case 2:
+		val = PMECC_CFG_BCH_ERR2;
+		break;
+	case 4:
+		val = PMECC_CFG_BCH_ERR4;
+		break;
+	case 8:
+		val = PMECC_CFG_BCH_ERR8;
+		break;
+	case 12:
+		val = PMECC_CFG_BCH_ERR12;
+		break;
+	case 24:
+		val = PMECC_CFG_BCH_ERR24;
+		break;
+	}
+
+	if (host->pmecc_sector_size == 512)
+		val |= PMECC_CFG_SECTOR512;
+	else if (host->pmecc_sector_size == 1024)
+		val |= PMECC_CFG_SECTOR1024;
+
+	switch (host->pmecc_sector_number) {
+	case 1:
+		val |= PMECC_CFG_PAGE_1SECTOR;
+		break;
+	case 2:
+		val |= PMECC_CFG_PAGE_2SECTORS;
+		break;
+	case 4:
+		val |= PMECC_CFG_PAGE_4SECTORS;
+		break;
+	case 8:
+		val |= PMECC_CFG_PAGE_8SECTORS;
+		break;
+	}
+
+	val |= (PMECC_CFG_READ_OP | PMECC_CFG_SPARE_DISABLE
+		| PMECC_CFG_AUTO_DISABLE);
+	pmecc_writel(host->ecc, CFG, val);
+
+	ecc_layout = nand_chip->ecc.layout;
+	pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
+	pmecc_writel(host->ecc, SADDR, ecc_layout->eccpos[0]);
+	pmecc_writel(host->ecc, EADDR,
+			ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
+	/* See datasheet about PMECC Clock Control Register */
+	pmecc_writel(host->ecc, CLK, 2);
+	pmecc_writel(host->ecc, IDR, 0xff);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
+}
+
+static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
+					 struct atmel_nand_host *host)
+{
+	int			cap, sector_size, err_no;
+	struct mtd_info		*mtd;
+	struct nand_chip	*nand_chip;
+	struct resource		*regs;
+	struct resource		*regs_pmerr, *regs_rom;
+
+	cap = host->pmecc_corr_cap;
+	sector_size = host->pmecc_sector_size;
+	dev_info(host->dev, "Initialize PMECC params, cap: %d, sector: %d\n",
+		 cap, sector_size);
+
+	/* Sanity check */
+	if ((sector_size != 512) && (sector_size != 1024)) {
+		dev_err(host->dev,
+			"Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
+			sector_size);
+		return -EINVAL;
+	}
+	if ((cap != 2) && (cap != 4) && (cap != 8) && (cap != 12) &&
+	    (cap != 24)) {
+		dev_err(host->dev,
+			"Unsupported PMECC correction capability, should be 2, 4, 8, 12 or 24\n");
+		return -EINVAL;
+	}
+
+	nand_chip = &host->nand_chip;
+	mtd = &host->mtd;
+
+	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* By default */
+
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!regs) {
+		dev_warn(host->dev,
+			"Can't get I/O resource regs, rolling back on software ECC\n");
+		return 0;
+	}
+
+	host->ecc = ioremap(regs->start, resource_size(regs));
+	if (host->ecc == NULL) {
+		dev_err(host->dev, "ioremap failed\n");
+		err_no = -EIO;
+		goto err_pmecc_ioremap;
+	}
+
+	regs_pmerr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	if (regs_pmerr && regs_rom) {
+		host->pmerrloc_base = ioremap(regs_pmerr->start,
+			resource_size(regs_pmerr));
+		host->pmecc_rom_base = ioremap(regs_rom->start,
+			resource_size(regs_rom));
+
+		if (host->pmerrloc_base && host->pmecc_rom_base) {
+			nand_chip->ecc.mode = NAND_ECC_HW;
+			nand_chip->ecc.read_page =
+				atmel_nand_pmecc_read_page;
+			nand_chip->ecc.write_page =
+				atmel_nand_pmecc_write_page;
+		} else {
+			dev_err(host->dev,
+				"Can not get I/O resource for PMECC controller!\n");
+			err_no = -EIO;
+			goto err_pmloc_ioremap;
+		}
+	}
+
+	/* ECC is calculated for the whole page (1 step) */
+	nand_chip->ecc.size = mtd->writesize;
+
+	/* set ECC page size and oob layout */
+	switch (mtd->writesize) {
+	case 2048:
+		host->pmecc_degree = PMECC_GF_DIMENSION_13;
+		host->pmecc_cw_len = (1 << host->pmecc_degree) - 1;
+		host->pmecc_corr_cap = cap;
+		host->pmecc_sector_number = mtd->writesize / sector_size;
+		host->pmecc_bytes_per_sector = pmecc_get_ecc_bytes(
+			cap, sector_size);
+		host->pmecc_alpha_to = pmecc_get_alpha_to(host);
+		host->pmecc_index_of = pmecc_get_index_of(host);
+
+		nand_chip->ecc.steps = 1;
+		nand_chip->ecc.strength = cap;
+		nand_chip->ecc.bytes = host->pmecc_bytes_per_sector *
+				       host->pmecc_sector_number;
+		if (nand_chip->ecc.bytes > mtd->oobsize - 2) {
+			dev_err(host->dev, "No room for ECC bytes\n");
+			err_no = -EINVAL;
+			goto err;
+		}
+		pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
+					mtd->oobsize,
+					nand_chip->ecc.bytes);
+		nand_chip->ecc.layout = &atmel_pmecc_oobinfo;
+		break;
+	case 512:
+	case 1024:
+	case 4096:
+		/* TODO */
+		dev_warn(host->dev,
+			"Unsupported page size for PMECC, use Software ECC\n");
+	default:
+		/* page size not handled by HW ECC */
+		/* switching back to soft ECC */
+		nand_chip->ecc.mode = NAND_ECC_SOFT;
+		nand_chip->ecc.calculate = NULL;
+		nand_chip->ecc.correct = NULL;
+		nand_chip->ecc.hwctl = NULL;
+		nand_chip->ecc.read_page = NULL;
+		nand_chip->ecc.write_page = NULL;
+		nand_chip->ecc.postpad = 0;
+		nand_chip->ecc.prepad = 0;
+		nand_chip->ecc.bytes = 0;
+		err_no = 0;
+		goto err;
+	}
+
+	/* Allocate data for PMECC computation */
+	host->pmecc_data = kzalloc(sizeof(struct atmel_pmecc_data), GFP_KERNEL);
+	if (!host->pmecc_data) {
+		dev_err(host->dev,
+				"Cannot allocate memory for PMECC computation!\n");
+		err_no = -ENOMEM;
+		goto err;
+	}
+
+	atmel_pmecc_core_init(mtd);
+
+	return 0;
+
+err:
+err_pmloc_ioremap:
+	iounmap(host->ecc);
+	if (host->pmerrloc_base)
+		iounmap(host->pmerrloc_base);
+	if (host->pmecc_rom_base)
+		iounmap(host->pmecc_rom_base);
+err_pmecc_ioremap:
+	return err_no;
+}
+
+/*
  * Calculate HW ECC
  *
  * function called after a write
@@ -720,7 +1456,11 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	}
 
 	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		res = atmel_hw_nand_init_params(pdev, host);
+		if (host->has_pmecc)
+			res = atmel_pmecc_nand_init_params(pdev, host);
+		else
+			res = atmel_hw_nand_init_params(pdev, host);
+
 		if (res != 0)
 			goto err_hw_ecc;
 	}
@@ -741,6 +1481,12 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 err_scan_tail:
 	if (host->ecc)
 		iounmap(host->ecc);
+	if (host->has_pmecc) {
+		if (host->pmerrloc_base)
+			iounmap(host->pmerrloc_base);
+		if (host->pmecc_rom_base)
+			iounmap(host->pmecc_rom_base);
+	}
 err_hw_ecc:
 err_scan_ident:
 err_no_card:
@@ -766,6 +1512,19 @@ static int __exit atmel_nand_remove(struct platform_device *pdev)
 
 	atmel_nand_disable(host);
 
+	if (host->has_pmecc) {
+		pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+		if (host->pmerrloc_base) {
+			pmerrloc_writel(host->pmerrloc_base, ELDIS,
+				PMERRLOC_DISABLE);
+			iounmap(host->pmerrloc_base);
+		}
+		if (host->pmecc_rom_base)
+			iounmap(host->pmecc_rom_base);
+
+		kfree(host->pmecc_data);
+	}
+
 	if (host->ecc)
 		iounmap(host->ecc);
 
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
index 5cbff11..bd404bf 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -45,4 +45,120 @@
 #define ecc_writel(add, reg, value)				\
 	writel((value), add + ATMEL_ECC_##reg)
 
+/* PMECC Register Definitions */
+#define ATMEL_PMECC_CFG			0x000	/* Configuration Register */
+#define		PMECC_CFG_BCH_ERR2		(0 << 0)
+#define		PMECC_CFG_BCH_ERR4		(1 << 0)
+#define		PMECC_CFG_BCH_ERR8		(2 << 0)
+#define		PMECC_CFG_BCH_ERR12		(3 << 0)
+#define		PMECC_CFG_BCH_ERR24		(4 << 0)
+
+#define		PMECC_CFG_SECTOR512		(0 << 4)
+#define		PMECC_CFG_SECTOR1024		(1 << 4)
+
+#define		PMECC_CFG_PAGE_1SECTOR		(0 << 8)
+#define		PMECC_CFG_PAGE_2SECTORS		(1 << 8)
+#define		PMECC_CFG_PAGE_4SECTORS		(2 << 8)
+#define		PMECC_CFG_PAGE_8SECTORS		(3 << 8)
+
+#define		PMECC_CFG_READ_OP		(0 << 12)
+#define		PMECC_CFG_WRITE_OP		(1 << 12)
+
+#define		PMECC_CFG_SPARE_ENABLE		(1 << 16)
+#define		PMECC_CFG_SPARE_DISABLE		(0 << 16)
+
+#define		PMECC_CFG_AUTO_ENABLE		(1 << 20)
+#define		PMECC_CFG_AUTO_DISABLE		(0 << 20)
+
+#define ATMEL_PMECC_SAREA		0x004	/* Spare area size */
+#define ATMEL_PMECC_SADDR		0x008	/* PMECC starting address */
+#define ATMEL_PMECC_EADDR		0x00c	/* PMECC ending address */
+#define ATMEL_PMECC_CLK			0x010	/* PMECC clock control */
+#define		PMECC_CLK_133MHZ		(2 << 0)
+
+#define ATMEL_PMECC_CTRL		0x014	/* PMECC control register */
+#define		PMECC_CTRL_RST			(1 << 0)
+#define		PMECC_CTRL_DATA			(1 << 1)
+#define		PMECC_CTRL_USER			(1 << 2)
+#define		PMECC_CTRL_ENABLE		(1 << 4)
+#define		PMECC_CTRL_DISABLE		(1 << 5)
+
+#define ATMEL_PMECC_SR			0x018	/* PMECC status register */
+#define		PMECC_SR_BUSY			(1 << 0)
+#define		PMECC_SR_ENABLE			(1 << 4)
+
+#define ATMEL_PMECC_IER			0x01c	/* PMECC interrupt enable */
+#define		PMECC_IER_ENABLE		(1 << 0)
+#define ATMEL_PMECC_IDR			0x020	/* PMECC interrupt disable */
+#define		PMECC_IER_DISABLE		(1 << 0)
+#define ATMEL_PMECC_IMR			0x024	/* PMECC interrupt mask */
+#define		PMECC_IER_MASK			(1 << 0)
+#define ATMEL_PMECC_ISR			0x028	/* PMECC interrupt status */
+#define ATMEL_PMECC_ECCx		0x040	/* PMECC ECC x */
+#define ATMEL_PMECC_REMx		0x240	/* PMECC REM x */
+
+/* PMERRLOC Register Definitions */
+#define ATMEL_PMERRLOC_ELCFG		0x000	/* Error location config */
+#define		PMERRLOC_ELCFG_SECTOR_512	(0 << 0)
+#define		PMERRLOC_ELCFG_SECTOR_1024	(1 << 0)
+#define		PMERRLOC_ELCFG_NUM_ERRORS(n)	((n) << 16)
+
+#define ATMEL_PMERRLOC_ELPRIM		0x004	/* Error location primitive */
+#define ATMEL_PMERRLOC_ELEN		0x008	/* Error location enable */
+#define ATMEL_PMERRLOC_ELDIS		0x00c	/* Error location disable */
+#define		PMERRLOC_DISABLE		(1 << 0)
+
+#define ATMEL_PMERRLOC_ELSR		0x010	/* Error location status */
+#define		PMERRLOC_ELSR_BUSY		(1 << 0)
+#define ATMEL_PMERRLOC_ELIER		0x014	/* Error location int enable */
+#define ATMEL_PMERRLOC_ELIDR		0x018	/* Error location int disable */
+#define ATMEL_PMERRLOC_ELIMR		0x01c	/* Error location int mask */
+#define ATMEL_PMERRLOC_ELISR		0x020	/* Error location int status */
+#define		PMERRLOC_ERR_NUM_MASK		(0x1f << 8)
+#define		PMERRLOC_CALC_DONE		(1 << 0)
+#define ATMEL_PMERRLOC_SIGMAx		0x028	/* Error location SIGMA x */
+#define ATMEL_PMERRLOC_ELx		0x08c	/* Error location x */
+
+/* Register access macros for PMECC */
+#define pmecc_readl_relaxed(addr, reg) \
+	readl_relaxed((addr) + ATMEL_PMECC_##reg)
+
+#define pmecc_writel(addr, reg, value) \
+	writel((value), (addr) + ATMEL_PMECC_##reg)
+
+#define pmecc_readb_ecc_relaxed(addr, sector, n) \
+	readb_relaxed((addr) + ATMEL_PMECC_ECCx + ((sector) * 0x40) + (n))
+
+#define pmecc_readl_rem_relaxed(addr, sector, n) \
+	readl_relaxed((addr) + ATMEL_PMECC_REMx + ((sector) * 0x40) + ((n) * 4))
+
+#define pmerrloc_readl_relaxed(addr, reg) \
+	readl_relaxed((addr) + ATMEL_PMERRLOC_##reg)
+
+#define pmerrloc_writel(addr, reg, value) \
+	writel((value), (addr) + ATMEL_PMERRLOC_##reg)
+
+#define pmerrloc_writel_sigma_relaxed(addr, n, value) \
+	writel_relaxed((value), (addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
+
+#define pmerrloc_readl_sigma_relaxed(addr, n) \
+	readl_relaxed((addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
+
+#define pmerrloc_readl_el_relaxed(addr, n) \
+	readl_relaxed((addr) + ATMEL_PMERRLOC_ELx + ((n) * 4))
+
+/* Galois field dimension */
+#define PMECC_GF_DIMENSION_13			13
+#define PMECC_GF_DIMENSION_14			14
+
+#define PMECC_MAX_ERROR_NB			25
+#define PMECC_MAX_ECC_BYTES			42
+#define PMECC_MAX_NB_SECTOR			8
+#define PMECC_MAX_TIMEOUT_COUNT			100
+
+#define PMECC_LOOKUP_TABLE_OFFSET_512		0x8000
+#define PMECC_LOOKUP_TABLE_SIZE_512		0x2000
+#define PMECC_LOOKUP_TABLE_OFFSET_1024		0x10000
+#define PMECC_LOOKUP_TABLE_SIZE_1024		0x4000
+
 #endif
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
@ 2012-05-26 13:24   ` Josh Wu
  0 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-26 13:24 UTC (permalink / raw)
  To: linux-arm-kernel

The Programmable Multibit ECC (PMECC) controller is a programmable binary
BCH(Bose, Chaudhuri and Hocquenghem) encoder and decoder. This controller
can be used to support both SLC and MLC NAND Flash devices. It supports to
generate ECC to correct 2, 4, 8, 12 or 24 bits of error per sector of data.

To use this driver, the user needs to pass in the correction capability and
the sector size.

This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2,
YAFFS2, UBIFS and mtd-utils.

Signed-off-by: Hong Xu <hong.xu@atmel.com>
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/mtd/nand/atmel_nand.c     |  761 ++++++++++++++++++++++++++++++++++++-
 drivers/mtd/nand/atmel_nand_ecc.h |  116 ++++++
 2 files changed, 876 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 9a9bfbf..ddcf1ed 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -15,6 +15,8 @@
  *     		(u-boot-1.1.5/board/atmel/at91sam9263ek/nand.c)
  *     (C) Copyright 2006 ATMEL Rousset, Lacressonniere Nicolas
  *
+ *  Add Programmable Multibit ECC support for various AT91 SoC
+ *     (C) Copyright 2012 ATMEL, Hong Xu
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -77,6 +79,21 @@ static struct nand_ecclayout atmel_oobinfo_small = {
 	},
 };
 
+/* a structure includes datas for PMECC computation */
+struct atmel_pmecc_data {
+	int16_t			partial_syn[2 * PMECC_MAX_ERROR_NB + 1];
+	int16_t			si[2 * PMECC_MAX_ERROR_NB + 1];
+
+	/* Sigma table */
+	int16_t		smu[PMECC_MAX_ERROR_NB + 2][2 * PMECC_MAX_ERROR_NB + 1];
+	/* polynomal order */
+	int16_t			lmu[PMECC_MAX_ERROR_NB + 1];
+
+	int			mu[PMECC_MAX_ERROR_NB + 1];
+	int			dmu[PMECC_MAX_ERROR_NB + 1];
+	int			delta[PMECC_MAX_ERROR_NB + 1];
+};
+
 struct atmel_nand_host {
 	struct nand_chip	nand_chip;
 	struct mtd_info		mtd;
@@ -92,8 +109,25 @@ struct atmel_nand_host {
 	bool			has_pmecc;
 	u8			pmecc_corr_cap;
 	u16			pmecc_sector_size;
+
+	int			pmecc_bytes_per_sector;
+	int			pmecc_sector_number;
+	int			pmecc_degree;	/* Degree of remainders */
+	int			pmecc_cw_len;	/* Length of codeword */
+
+	void __iomem		*pmerrloc_base;
+	void __iomem		*pmecc_rom_base;
+
+	/* lookup table for alpha_to and index_of */
+	void __iomem		*pmecc_alpha_to;
+	void __iomem		*pmecc_index_of;
+
+	/* data for pmecc computation */
+	struct atmel_pmecc_data	*pmecc_data;
 };
 
+static struct nand_ecclayout atmel_pmecc_oobinfo;
+
 static int cpu_has_dma(void)
 {
 	return cpu_is_at91sam9rl() || cpu_is_at91sam9g45();
@@ -287,6 +321,708 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 }
 
 /*
+ * Return number of ecc bytes per sector according to sector size and
+ * correction capability
+ *
+ * Following table shows what at91 PMECC supported:
+ * Correction Capability	Sector_512_bytes	Sector_1024_bytes
+ * =====================	================	=================
+ *                2-bits                 4-bytes                  4-bytes
+ *                4-bits                 7-bytes                  7-bytes
+ *                8-bits                13-bytes                 14-bytes
+ *               12-bits                20-bytes                 21-bytes
+ *               24-bits                39-bytes                 42-bytes
+ */
+static int pmecc_get_ecc_bytes(int cap, int sector_size)
+{
+	int m = 12 + sector_size / 512;
+	return (m * cap + 7) / 8;
+}
+
+static void pmecc_config_ecc_layout(struct nand_ecclayout *layout, int oobsize,
+	int ecc_len)
+{
+	int i;
+
+	layout->eccbytes = ecc_len;
+
+	/* ECC will occupy the last ecc_len bytes continuously */
+	for (i = 0; i < ecc_len; i++)
+		layout->eccpos[i] = oobsize - ecc_len + i;
+
+	layout->oobfree[0].offset = 2;
+	layout->oobfree[0].length =
+		oobsize - ecc_len - layout->oobfree[0].offset;
+}
+
+static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
+{
+	void __iomem *p;
+
+	switch (host->pmecc_sector_size) {
+	case 512:
+		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512 +
+			PMECC_LOOKUP_TABLE_SIZE_512 * sizeof(int16_t);
+		break;
+	case 1024:
+		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024 +
+			PMECC_LOOKUP_TABLE_SIZE_1024 * sizeof(int16_t);
+		break;
+	default:
+		BUG();
+	}
+
+	return p;
+}
+
+static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
+{
+	void __iomem *p;
+
+	switch (host->pmecc_sector_size) {
+	case 512:
+		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512;
+		break;
+	case 1024:
+		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024;
+		break;
+	default:
+		BUG();
+	}
+
+	return p;
+}
+
+static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
+{
+	int			i;
+	uint32_t		value;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	/* Fill odd syndromes */
+	for (i = 0; i < host->pmecc_corr_cap; i++) {
+		value = pmecc_readl_rem_relaxed(host->ecc, sector, i / 2);
+		value = (i & 1) ? (value & 0xffff0000) >> 16 : value & 0xffff;
+		host->pmecc_data->partial_syn[(2 * i) + 1] = (int16_t)value;
+	}
+}
+
+static void pmecc_substitute(struct mtd_info *mtd)
+{
+	int16_t			*si;
+	int			i, j;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+	int16_t __iomem		*alpha_to = host->pmecc_alpha_to;
+	int16_t __iomem		*index_of = host->pmecc_index_of;
+	int16_t			*partial_syn = host->pmecc_data->partial_syn;
+
+	/* si[] is a table that holds the current syndrome value,
+	 * an element of that table belongs to the field
+	 */
+	si = host->pmecc_data->si;
+
+	for (i = 1; i < 2 * PMECC_MAX_ERROR_NB; i++)
+		si[i] = 0;
+
+	/* Computation 2t syndromes based on S(x) */
+	/* Odd syndromes */
+	for (i = 1; i < 2 * host->pmecc_corr_cap; i += 2) {
+		si[i] = 0;
+		for (j = 0; j < host->pmecc_degree; j++) {
+			if (partial_syn[i] & ((unsigned short)0x1 << j))
+				si[i] = readw_relaxed(alpha_to + i * j) ^ si[i];
+		}
+	}
+	/* Even syndrome = (Odd syndrome) ** 2 */
+	for (i = 2; i <= 2 * host->pmecc_corr_cap; i += 2) {
+		j = i / 2;
+		if (si[j] == 0)
+			si[i] = 0;
+		else {
+			int16_t tmp;
+			tmp = readw_relaxed(index_of + si[j]);
+			tmp = (tmp * 2) % host->pmecc_cw_len;
+			si[i] = readw_relaxed(alpha_to + tmp);
+		}
+	}
+
+	return;
+}
+
+static void pmecc_get_sigma(struct mtd_info *mtd)
+{
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	int		i, j, k;
+	uint32_t	dmu_0_count, tmp;
+	int16_t		(*smu)[2 * PMECC_MAX_ERROR_NB + 1];
+	int16_t		*lmu = host->pmecc_data->lmu;
+	int16_t		*si = host->pmecc_data->si;
+	int		*mu = host->pmecc_data->mu;
+	int		*dmu = host->pmecc_data->dmu;	  /* Discrepancy */
+	int		*delta = host->pmecc_data->delta; /* Delta order */
+	int		cw_len = host->pmecc_cw_len;
+	int16_t		cap = host->pmecc_corr_cap;
+
+	int16_t __iomem	*index_of = host->pmecc_index_of;
+	int16_t __iomem	*alpha_to = host->pmecc_alpha_to;
+
+	/* index of largest delta */
+	int ro;
+	int largest;
+	int diff;
+
+	dmu_0_count = 0;
+	smu = host->pmecc_data->smu;
+
+	/* First Row */
+
+	/* Mu */
+	mu[0] = -1;
+
+	memset(&smu[0][0], 0,
+		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
+	smu[0][0] = 1;
+
+	/* discrepancy set to 1 */
+	dmu[0] = 1;
+	/* polynom order set to 0 */
+	lmu[0] = 0;
+	delta[0]  = (mu[0] * 2 - lmu[0]) >> 1;
+
+	/* Second Row */
+
+	/* Mu */
+	mu[1]  = 0;
+	/* Sigma(x) set to 1 */
+	memset(&smu[1][0], 0,
+		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
+	smu[1][0] = 1;
+
+	/* discrepancy set to S1 */
+	dmu[1] = si[1];
+
+	/* polynom order set to 0 */
+	lmu[1] = 0;
+
+	delta[1]  = (mu[1] * 2 - lmu[1]) >> 1;
+
+	/* Init the Sigma(x) last row */
+	memset(&smu[cap + 1][0], 0,
+		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
+
+	for (i = 1; i <= cap; i++) {
+		mu[i+1] = i << 1;
+		/* Begin Computing Sigma (Mu+1) and L(mu) */
+		/* check if discrepancy is set to 0 */
+		if (dmu[i] == 0) {
+			dmu_0_count++;
+
+			tmp = ((cap - (lmu[i] >> 1) - 1) / 2);
+			if ((cap - (lmu[i] >> 1) - 1) & 0x1)
+				tmp += 2;
+			else
+				tmp += 1;
+
+			if (dmu_0_count == tmp) {
+				for (j = 0; j <= (lmu[i] >> 1) + 1; j++)
+					smu[cap + 1][j] = smu[i][j];
+				lmu[cap + 1] = lmu[i];
+				return;
+			}
+
+			/* copy polynom */
+			for (j = 0; j <= lmu[i] >> 1; j++)
+				smu[i + 1][j] = smu[i][j];
+
+			/* copy previous polynom order to the next */
+			lmu[i + 1] = lmu[i];
+		} else {
+			ro = 0;
+			largest = -1;
+			/* find largest delta with dmu != 0 */
+			for (j = 0; j < i; j++) {
+				if ((dmu[j]) && (delta[j] > largest)) {
+					largest = delta[j];
+					ro = j;
+				}
+			}
+
+			/* compute difference */
+			diff = (mu[i] - mu[ro]);
+
+			/* Compute degree of the new smu polynomial */
+			if ((lmu[i] >> 1) > ((lmu[ro] >> 1) + diff))
+				lmu[i + 1] = lmu[i];
+			else
+				lmu[i + 1] = ((lmu[ro] >> 1) + diff) * 2;
+
+			/* Init smu[i+1] with 0 */
+			for (k = 0; k < (2 * PMECC_MAX_ERROR_NB + 1); k++)
+				smu[i+1][k] = 0;
+
+			/* Compute smu[i+1] */
+			for (k = 0; k <= lmu[ro] >> 1; k++) {
+				int16_t a, b, c;
+
+				if (!(smu[ro][k] && dmu[i]))
+					continue;
+				a = readw_relaxed(index_of + dmu[i]);
+				b = readw_relaxed(index_of + dmu[ro]);
+				c = readw_relaxed(index_of + smu[ro][k]);
+				tmp = a + (cw_len - b) + c;
+				a = readw_relaxed(alpha_to + tmp % cw_len);
+				smu[i + 1][k + diff] = a;
+			}
+
+			for (k = 0; k <= lmu[i] >> 1; k++)
+				smu[i + 1][k] ^= smu[i][k];
+		}
+
+		/* End Computing Sigma (Mu+1) and L(mu) */
+		/* In either case compute delta */
+		delta[i + 1]  = (mu[i + 1] * 2 - lmu[i + 1]) >> 1;
+
+		/* Do not compute discrepancy for the last iteration */
+		if (i >= cap)
+			continue;
+
+		for (k = 0 ; k <= (lmu[i + 1] >> 1); k++) {
+			tmp = 2 * (i - 1);
+			if (k == 0)
+				dmu[i + 1] = si[tmp + 3];
+			else if (smu[i+1][k] && si[tmp + 3 - k]) {
+				int16_t a, b, c;
+				a = readw_relaxed(index_of + smu[i + 1][k]);
+				b = si[2 * (i - 1) + 3 - k];
+				c = readw_relaxed(index_of + b);
+				tmp = a + c;
+				tmp %= cw_len;
+				dmu[i + 1] = readw_relaxed(alpha_to + tmp) ^
+					dmu[i + 1];
+			}
+		}
+	}
+
+	return;
+}
+
+static int pmecc_err_location(struct mtd_info *mtd)
+{
+	int			i;
+	int			err_nbr;	/* number of error */
+	int			roots_nbr;	/* number of roots */
+	int			sector_size;
+	uint32_t		val;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+	int			timeout_count = 0;
+	int			cap = host->pmecc_corr_cap;
+
+	err_nbr = 0;
+	sector_size = host->pmecc_sector_size;
+
+	pmerrloc_writel(host->pmerrloc_base, ELDIS, PMERRLOC_DISABLE);
+
+	for (i = 0; i <= host->pmecc_data->lmu[cap + 1] >> 1; i++) {
+		pmerrloc_writel_sigma_relaxed(host->pmerrloc_base, i,
+				      host->pmecc_data->smu[cap + 1][i]);
+		err_nbr++;
+	}
+
+	val = (err_nbr - 1) << 16;
+	if (sector_size == 1024)
+		val |= 1;
+
+	pmerrloc_writel(host->pmerrloc_base, ELCFG, val);
+	pmerrloc_writel(host->pmerrloc_base, ELEN,
+			sector_size * 8 + host->pmecc_degree * cap);
+
+	while (!(pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
+		 & PMERRLOC_CALC_DONE)) {
+		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT))
+			return -1;	/* Time out */
+		cpu_relax();
+	}
+
+	roots_nbr = (pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
+		& PMERRLOC_ERR_NUM_MASK) >> 8;
+	/* Number of roots == degree of smu hence <= cap */
+	if (roots_nbr == host->pmecc_data->lmu[cap + 1] >> 1)
+		return err_nbr - 1;
+
+	/* Number of roots does not match the degree of smu
+	 * unable to correct error */
+	return -1;
+}
+
+static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf,
+		int extra_bytes, int err_nbr)
+{
+	int			i = 0;
+	int			byte_pos, bit_pos;
+	int			sector_size, ecc_size;
+	uint32_t		tmp;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	sector_size = host->pmecc_sector_size;
+	ecc_size = nand_chip->ecc.bytes;
+
+	while (err_nbr) {
+		tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
+		byte_pos = tmp / 8;
+		bit_pos  = tmp % 8;
+		dev_info(host->dev, "PMECC correction, byte_pos: %d bit_pos: %d\n",
+					byte_pos, bit_pos);
+
+		if (byte_pos < (sector_size + extra_bytes)) {
+			tmp = sector_size +
+				pmecc_readl_relaxed(host->ecc, SADDR);
+
+			if (byte_pos < tmp)
+				*(buf + byte_pos) ^= (1 << bit_pos);
+			else
+				*(buf + byte_pos + ecc_size) ^= (1 << bit_pos);
+		}
+
+		i++;
+		err_nbr--;
+	}
+
+	return;
+}
+
+static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
+	u8 *ecc)
+{
+	int			i, err_nbr;
+	uint8_t			*buf_pos;
+	int			eccbytes;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	eccbytes = nand_chip->ecc.bytes;
+	for (i = 0; i < eccbytes; i++)
+		if (ecc[i] != 0xff)
+			goto normal_check;
+	/* Erased page, return OK */
+	return 0;
+
+normal_check:
+	for (i = 0; i < host->pmecc_sector_number; i++) {
+		err_nbr = 0;
+		if (pmecc_stat & 0x1) {
+			buf_pos = buf + i * host->pmecc_sector_size;
+
+			pmecc_gen_syndrome(mtd, i);
+			pmecc_substitute(mtd);
+			pmecc_get_sigma(mtd);
+
+			err_nbr = pmecc_err_location(mtd);
+			if (err_nbr == -1) {
+				dev_err(host->dev, "PMECC: Too many errors\n");
+				mtd->ecc_stats.failed++;
+				return -EIO;
+			} else {
+				pmecc_correct_data(mtd, buf_pos, 0, err_nbr);
+				mtd->ecc_stats.corrected += err_nbr;
+			}
+		}
+		pmecc_stat >>= 1;
+	}
+
+	return 0;
+}
+
+static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
+		struct nand_chip *chip, uint8_t *buf, int page)
+{
+	uint32_t		stat;
+	int			timeout_count = 0;
+	int			eccsize = chip->ecc.size;
+	uint8_t			*oob = chip->oob_poi;
+	struct atmel_nand_host	*host = chip->priv;
+	uint32_t		*eccpos = chip->ecc.layout->eccpos;
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG)
+		& ~PMECC_CFG_WRITE_OP) | PMECC_CFG_AUTO_ENABLE);
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
+
+	chip->read_buf(mtd, buf, eccsize);
+	chip->read_buf(mtd, oob, mtd->oobsize);
+
+	while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
+		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT))
+			return -EIO;	/* Time out */
+		cpu_relax();
+	}
+
+	stat = pmecc_readl_relaxed(host->ecc, ISR);
+	if (stat != 0) {
+		if (pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]) != 0)
+			return -EIO;
+	}
+
+	return 0;
+}
+
+static void atmel_nand_pmecc_write_page(struct mtd_info *mtd,
+		struct nand_chip *chip, const uint8_t *buf)
+{
+	int			i, j;
+	int			timeout_count = 0;
+	struct atmel_nand_host	*host = chip->priv;
+	uint32_t		*eccpos = chip->ecc.layout->eccpos;
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+
+	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG) |
+		PMECC_CFG_WRITE_OP) & ~PMECC_CFG_AUTO_ENABLE);
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
+
+	chip->write_buf(mtd, (u8 *)buf, mtd->writesize);
+
+	while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
+		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT)) {
+			dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
+			return;	/* Time out */
+		}
+		cpu_relax();
+	}
+
+	for (i = 0; i < host->pmecc_sector_number; i++) {
+		for (j = 0; j < host->pmecc_bytes_per_sector; j++) {
+			int pos;
+
+			pos = i * host->pmecc_bytes_per_sector + j;
+			chip->oob_poi[eccpos[pos]] =
+				pmecc_readb_ecc_relaxed(host->ecc, i, j);
+		}
+	}
+	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	return;
+}
+
+static void atmel_pmecc_core_init(struct mtd_info *mtd)
+{
+	uint32_t		val = 0;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+	struct nand_ecclayout	*ecc_layout;
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+
+	switch (host->pmecc_corr_cap) {
+	case 2:
+		val = PMECC_CFG_BCH_ERR2;
+		break;
+	case 4:
+		val = PMECC_CFG_BCH_ERR4;
+		break;
+	case 8:
+		val = PMECC_CFG_BCH_ERR8;
+		break;
+	case 12:
+		val = PMECC_CFG_BCH_ERR12;
+		break;
+	case 24:
+		val = PMECC_CFG_BCH_ERR24;
+		break;
+	}
+
+	if (host->pmecc_sector_size == 512)
+		val |= PMECC_CFG_SECTOR512;
+	else if (host->pmecc_sector_size == 1024)
+		val |= PMECC_CFG_SECTOR1024;
+
+	switch (host->pmecc_sector_number) {
+	case 1:
+		val |= PMECC_CFG_PAGE_1SECTOR;
+		break;
+	case 2:
+		val |= PMECC_CFG_PAGE_2SECTORS;
+		break;
+	case 4:
+		val |= PMECC_CFG_PAGE_4SECTORS;
+		break;
+	case 8:
+		val |= PMECC_CFG_PAGE_8SECTORS;
+		break;
+	}
+
+	val |= (PMECC_CFG_READ_OP | PMECC_CFG_SPARE_DISABLE
+		| PMECC_CFG_AUTO_DISABLE);
+	pmecc_writel(host->ecc, CFG, val);
+
+	ecc_layout = nand_chip->ecc.layout;
+	pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
+	pmecc_writel(host->ecc, SADDR, ecc_layout->eccpos[0]);
+	pmecc_writel(host->ecc, EADDR,
+			ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
+	/* See datasheet about PMECC Clock Control Register */
+	pmecc_writel(host->ecc, CLK, 2);
+	pmecc_writel(host->ecc, IDR, 0xff);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
+}
+
+static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
+					 struct atmel_nand_host *host)
+{
+	int			cap, sector_size, err_no;
+	struct mtd_info		*mtd;
+	struct nand_chip	*nand_chip;
+	struct resource		*regs;
+	struct resource		*regs_pmerr, *regs_rom;
+
+	cap = host->pmecc_corr_cap;
+	sector_size = host->pmecc_sector_size;
+	dev_info(host->dev, "Initialize PMECC params, cap: %d, sector: %d\n",
+		 cap, sector_size);
+
+	/* Sanity check */
+	if ((sector_size != 512) && (sector_size != 1024)) {
+		dev_err(host->dev,
+			"Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
+			sector_size);
+		return -EINVAL;
+	}
+	if ((cap != 2) && (cap != 4) && (cap != 8) && (cap != 12) &&
+	    (cap != 24)) {
+		dev_err(host->dev,
+			"Unsupported PMECC correction capability, should be 2, 4, 8, 12 or 24\n");
+		return -EINVAL;
+	}
+
+	nand_chip = &host->nand_chip;
+	mtd = &host->mtd;
+
+	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* By default */
+
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!regs) {
+		dev_warn(host->dev,
+			"Can't get I/O resource regs, rolling back on software ECC\n");
+		return 0;
+	}
+
+	host->ecc = ioremap(regs->start, resource_size(regs));
+	if (host->ecc == NULL) {
+		dev_err(host->dev, "ioremap failed\n");
+		err_no = -EIO;
+		goto err_pmecc_ioremap;
+	}
+
+	regs_pmerr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	if (regs_pmerr && regs_rom) {
+		host->pmerrloc_base = ioremap(regs_pmerr->start,
+			resource_size(regs_pmerr));
+		host->pmecc_rom_base = ioremap(regs_rom->start,
+			resource_size(regs_rom));
+
+		if (host->pmerrloc_base && host->pmecc_rom_base) {
+			nand_chip->ecc.mode = NAND_ECC_HW;
+			nand_chip->ecc.read_page =
+				atmel_nand_pmecc_read_page;
+			nand_chip->ecc.write_page =
+				atmel_nand_pmecc_write_page;
+		} else {
+			dev_err(host->dev,
+				"Can not get I/O resource for PMECC controller!\n");
+			err_no = -EIO;
+			goto err_pmloc_ioremap;
+		}
+	}
+
+	/* ECC is calculated for the whole page (1 step) */
+	nand_chip->ecc.size = mtd->writesize;
+
+	/* set ECC page size and oob layout */
+	switch (mtd->writesize) {
+	case 2048:
+		host->pmecc_degree = PMECC_GF_DIMENSION_13;
+		host->pmecc_cw_len = (1 << host->pmecc_degree) - 1;
+		host->pmecc_corr_cap = cap;
+		host->pmecc_sector_number = mtd->writesize / sector_size;
+		host->pmecc_bytes_per_sector = pmecc_get_ecc_bytes(
+			cap, sector_size);
+		host->pmecc_alpha_to = pmecc_get_alpha_to(host);
+		host->pmecc_index_of = pmecc_get_index_of(host);
+
+		nand_chip->ecc.steps = 1;
+		nand_chip->ecc.strength = cap;
+		nand_chip->ecc.bytes = host->pmecc_bytes_per_sector *
+				       host->pmecc_sector_number;
+		if (nand_chip->ecc.bytes > mtd->oobsize - 2) {
+			dev_err(host->dev, "No room for ECC bytes\n");
+			err_no = -EINVAL;
+			goto err;
+		}
+		pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
+					mtd->oobsize,
+					nand_chip->ecc.bytes);
+		nand_chip->ecc.layout = &atmel_pmecc_oobinfo;
+		break;
+	case 512:
+	case 1024:
+	case 4096:
+		/* TODO */
+		dev_warn(host->dev,
+			"Unsupported page size for PMECC, use Software ECC\n");
+	default:
+		/* page size not handled by HW ECC */
+		/* switching back to soft ECC */
+		nand_chip->ecc.mode = NAND_ECC_SOFT;
+		nand_chip->ecc.calculate = NULL;
+		nand_chip->ecc.correct = NULL;
+		nand_chip->ecc.hwctl = NULL;
+		nand_chip->ecc.read_page = NULL;
+		nand_chip->ecc.write_page = NULL;
+		nand_chip->ecc.postpad = 0;
+		nand_chip->ecc.prepad = 0;
+		nand_chip->ecc.bytes = 0;
+		err_no = 0;
+		goto err;
+	}
+
+	/* Allocate data for PMECC computation */
+	host->pmecc_data = kzalloc(sizeof(struct atmel_pmecc_data), GFP_KERNEL);
+	if (!host->pmecc_data) {
+		dev_err(host->dev,
+				"Cannot allocate memory for PMECC computation!\n");
+		err_no = -ENOMEM;
+		goto err;
+	}
+
+	atmel_pmecc_core_init(mtd);
+
+	return 0;
+
+err:
+err_pmloc_ioremap:
+	iounmap(host->ecc);
+	if (host->pmerrloc_base)
+		iounmap(host->pmerrloc_base);
+	if (host->pmecc_rom_base)
+		iounmap(host->pmecc_rom_base);
+err_pmecc_ioremap:
+	return err_no;
+}
+
+/*
  * Calculate HW ECC
  *
  * function called after a write
@@ -720,7 +1456,11 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	}
 
 	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		res = atmel_hw_nand_init_params(pdev, host);
+		if (host->has_pmecc)
+			res = atmel_pmecc_nand_init_params(pdev, host);
+		else
+			res = atmel_hw_nand_init_params(pdev, host);
+
 		if (res != 0)
 			goto err_hw_ecc;
 	}
@@ -741,6 +1481,12 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 err_scan_tail:
 	if (host->ecc)
 		iounmap(host->ecc);
+	if (host->has_pmecc) {
+		if (host->pmerrloc_base)
+			iounmap(host->pmerrloc_base);
+		if (host->pmecc_rom_base)
+			iounmap(host->pmecc_rom_base);
+	}
 err_hw_ecc:
 err_scan_ident:
 err_no_card:
@@ -766,6 +1512,19 @@ static int __exit atmel_nand_remove(struct platform_device *pdev)
 
 	atmel_nand_disable(host);
 
+	if (host->has_pmecc) {
+		pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+		if (host->pmerrloc_base) {
+			pmerrloc_writel(host->pmerrloc_base, ELDIS,
+				PMERRLOC_DISABLE);
+			iounmap(host->pmerrloc_base);
+		}
+		if (host->pmecc_rom_base)
+			iounmap(host->pmecc_rom_base);
+
+		kfree(host->pmecc_data);
+	}
+
 	if (host->ecc)
 		iounmap(host->ecc);
 
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h b/drivers/mtd/nand/atmel_nand_ecc.h
index 5cbff11..bd404bf 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -45,4 +45,120 @@
 #define ecc_writel(add, reg, value)				\
 	writel((value), add + ATMEL_ECC_##reg)
 
+/* PMECC Register Definitions */
+#define ATMEL_PMECC_CFG			0x000	/* Configuration Register */
+#define		PMECC_CFG_BCH_ERR2		(0 << 0)
+#define		PMECC_CFG_BCH_ERR4		(1 << 0)
+#define		PMECC_CFG_BCH_ERR8		(2 << 0)
+#define		PMECC_CFG_BCH_ERR12		(3 << 0)
+#define		PMECC_CFG_BCH_ERR24		(4 << 0)
+
+#define		PMECC_CFG_SECTOR512		(0 << 4)
+#define		PMECC_CFG_SECTOR1024		(1 << 4)
+
+#define		PMECC_CFG_PAGE_1SECTOR		(0 << 8)
+#define		PMECC_CFG_PAGE_2SECTORS		(1 << 8)
+#define		PMECC_CFG_PAGE_4SECTORS		(2 << 8)
+#define		PMECC_CFG_PAGE_8SECTORS		(3 << 8)
+
+#define		PMECC_CFG_READ_OP		(0 << 12)
+#define		PMECC_CFG_WRITE_OP		(1 << 12)
+
+#define		PMECC_CFG_SPARE_ENABLE		(1 << 16)
+#define		PMECC_CFG_SPARE_DISABLE		(0 << 16)
+
+#define		PMECC_CFG_AUTO_ENABLE		(1 << 20)
+#define		PMECC_CFG_AUTO_DISABLE		(0 << 20)
+
+#define ATMEL_PMECC_SAREA		0x004	/* Spare area size */
+#define ATMEL_PMECC_SADDR		0x008	/* PMECC starting address */
+#define ATMEL_PMECC_EADDR		0x00c	/* PMECC ending address */
+#define ATMEL_PMECC_CLK			0x010	/* PMECC clock control */
+#define		PMECC_CLK_133MHZ		(2 << 0)
+
+#define ATMEL_PMECC_CTRL		0x014	/* PMECC control register */
+#define		PMECC_CTRL_RST			(1 << 0)
+#define		PMECC_CTRL_DATA			(1 << 1)
+#define		PMECC_CTRL_USER			(1 << 2)
+#define		PMECC_CTRL_ENABLE		(1 << 4)
+#define		PMECC_CTRL_DISABLE		(1 << 5)
+
+#define ATMEL_PMECC_SR			0x018	/* PMECC status register */
+#define		PMECC_SR_BUSY			(1 << 0)
+#define		PMECC_SR_ENABLE			(1 << 4)
+
+#define ATMEL_PMECC_IER			0x01c	/* PMECC interrupt enable */
+#define		PMECC_IER_ENABLE		(1 << 0)
+#define ATMEL_PMECC_IDR			0x020	/* PMECC interrupt disable */
+#define		PMECC_IER_DISABLE		(1 << 0)
+#define ATMEL_PMECC_IMR			0x024	/* PMECC interrupt mask */
+#define		PMECC_IER_MASK			(1 << 0)
+#define ATMEL_PMECC_ISR			0x028	/* PMECC interrupt status */
+#define ATMEL_PMECC_ECCx		0x040	/* PMECC ECC x */
+#define ATMEL_PMECC_REMx		0x240	/* PMECC REM x */
+
+/* PMERRLOC Register Definitions */
+#define ATMEL_PMERRLOC_ELCFG		0x000	/* Error location config */
+#define		PMERRLOC_ELCFG_SECTOR_512	(0 << 0)
+#define		PMERRLOC_ELCFG_SECTOR_1024	(1 << 0)
+#define		PMERRLOC_ELCFG_NUM_ERRORS(n)	((n) << 16)
+
+#define ATMEL_PMERRLOC_ELPRIM		0x004	/* Error location primitive */
+#define ATMEL_PMERRLOC_ELEN		0x008	/* Error location enable */
+#define ATMEL_PMERRLOC_ELDIS		0x00c	/* Error location disable */
+#define		PMERRLOC_DISABLE		(1 << 0)
+
+#define ATMEL_PMERRLOC_ELSR		0x010	/* Error location status */
+#define		PMERRLOC_ELSR_BUSY		(1 << 0)
+#define ATMEL_PMERRLOC_ELIER		0x014	/* Error location int enable */
+#define ATMEL_PMERRLOC_ELIDR		0x018	/* Error location int disable */
+#define ATMEL_PMERRLOC_ELIMR		0x01c	/* Error location int mask */
+#define ATMEL_PMERRLOC_ELISR		0x020	/* Error location int status */
+#define		PMERRLOC_ERR_NUM_MASK		(0x1f << 8)
+#define		PMERRLOC_CALC_DONE		(1 << 0)
+#define ATMEL_PMERRLOC_SIGMAx		0x028	/* Error location SIGMA x */
+#define ATMEL_PMERRLOC_ELx		0x08c	/* Error location x */
+
+/* Register access macros for PMECC */
+#define pmecc_readl_relaxed(addr, reg) \
+	readl_relaxed((addr) + ATMEL_PMECC_##reg)
+
+#define pmecc_writel(addr, reg, value) \
+	writel((value), (addr) + ATMEL_PMECC_##reg)
+
+#define pmecc_readb_ecc_relaxed(addr, sector, n) \
+	readb_relaxed((addr) + ATMEL_PMECC_ECCx + ((sector) * 0x40) + (n))
+
+#define pmecc_readl_rem_relaxed(addr, sector, n) \
+	readl_relaxed((addr) + ATMEL_PMECC_REMx + ((sector) * 0x40) + ((n) * 4))
+
+#define pmerrloc_readl_relaxed(addr, reg) \
+	readl_relaxed((addr) + ATMEL_PMERRLOC_##reg)
+
+#define pmerrloc_writel(addr, reg, value) \
+	writel((value), (addr) + ATMEL_PMERRLOC_##reg)
+
+#define pmerrloc_writel_sigma_relaxed(addr, n, value) \
+	writel_relaxed((value), (addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
+
+#define pmerrloc_readl_sigma_relaxed(addr, n) \
+	readl_relaxed((addr) + ATMEL_PMERRLOC_SIGMAx + ((n) * 4))
+
+#define pmerrloc_readl_el_relaxed(addr, n) \
+	readl_relaxed((addr) + ATMEL_PMERRLOC_ELx + ((n) * 4))
+
+/* Galois field dimension */
+#define PMECC_GF_DIMENSION_13			13
+#define PMECC_GF_DIMENSION_14			14
+
+#define PMECC_MAX_ERROR_NB			25
+#define PMECC_MAX_ECC_BYTES			42
+#define PMECC_MAX_NB_SECTOR			8
+#define PMECC_MAX_TIMEOUT_COUNT			100
+
+#define PMECC_LOOKUP_TABLE_OFFSET_512		0x8000
+#define PMECC_LOOKUP_TABLE_SIZE_512		0x2000
+#define PMECC_LOOKUP_TABLE_OFFSET_1024		0x10000
+#define PMECC_LOOKUP_TABLE_SIZE_1024		0x4000
+
 #endif
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v9 1/3] MTD: at91: extract hw ecc initialization to one function and use relaxed read/write
  2012-05-26 13:24   ` Josh Wu
@ 2012-05-27 12:44     ` Artem Bityutskiy
  -1 siblings, 0 replies; 24+ messages in thread
From: Artem Bityutskiy @ 2012-05-27 12:44 UTC (permalink / raw)
  To: Josh Wu
  Cc: hongxu.cn, nicolas.ferre, linux-mtd, ivan.djelic, plagnioj,
	linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

On Sat, 2012-05-26 at 21:24 +0800, Josh Wu wrote:
> use _relaxed read/write in most place. And use writel in operations of Control Register since it needs memory barrier.
> 
> Signed-off-by: Hong Xu <hong.xu@atmel.com>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>

This should be split on 2 or even 3 parts:

1. You move definitions and things around.
2. You separate out the init stuff into a function
3. You start using _relaxed helpers.

And you should provide better commit messages. You should explain why
you do things - just cleanup? improvement? fix? preparation for
something?

I think this patch is too big, commit message is poor, patch does more
things than documented in the commit message.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 1/3] MTD: at91: extract hw ecc initialization to one function and use relaxed read/write
@ 2012-05-27 12:44     ` Artem Bityutskiy
  0 siblings, 0 replies; 24+ messages in thread
From: Artem Bityutskiy @ 2012-05-27 12:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, 2012-05-26 at 21:24 +0800, Josh Wu wrote:
> use _relaxed read/write in most place. And use writel in operations of Control Register since it needs memory barrier.
> 
> Signed-off-by: Hong Xu <hong.xu@atmel.com>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>

This should be split on 2 or even 3 parts:

1. You move definitions and things around.
2. You separate out the init stuff into a function
3. You start using _relaxed helpers.

And you should provide better commit messages. You should explain why
you do things - just cleanup? improvement? fix? preparation for
something?

I think this patch is too big, commit message is poor, patch does more
things than documented in the commit message.

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120527/5b38f766/attachment.sig>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
  2012-05-26 13:24   ` Josh Wu
@ 2012-05-27 12:50     ` Artem Bityutskiy
  -1 siblings, 0 replies; 24+ messages in thread
From: Artem Bityutskiy @ 2012-05-27 12:50 UTC (permalink / raw)
  To: Josh Wu
  Cc: hongxu.cn, nicolas.ferre, linux-mtd, ivan.djelic, plagnioj,
	linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 892 bytes --]

On Sat, 2012-05-26 at 21:24 +0800, Josh Wu wrote:
> +       while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
> +               if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT)) {
> +                       dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
> +                       return; /* Time out */

How this error is communicated then up the the user?

> +               }
> +               cpu_relax();
> +       }

I see this pattern all over the place - why people consider it reliable?
Is this code guaranteed to run on the same CPU?

Why not to use loops_per_jiffie * msecs_to_jiffies(TIMOUT) instead to
calculate how many iterations to do? Yes, due to HW register reading and
cpu_relax() the real timeout will be larger, but this is about error
anyway, so it does not hurt to iterate longer?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
@ 2012-05-27 12:50     ` Artem Bityutskiy
  0 siblings, 0 replies; 24+ messages in thread
From: Artem Bityutskiy @ 2012-05-27 12:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, 2012-05-26 at 21:24 +0800, Josh Wu wrote:
> +       while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
> +               if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT)) {
> +                       dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
> +                       return; /* Time out */

How this error is communicated then up the the user?

> +               }
> +               cpu_relax();
> +       }

I see this pattern all over the place - why people consider it reliable?
Is this code guaranteed to run on the same CPU?

Why not to use loops_per_jiffie * msecs_to_jiffies(TIMOUT) instead to
calculate how many iterations to do? Yes, due to HW register reading and
cpu_relax() the real timeout will be larger, but this is about error
anyway, so it does not hurt to iterate longer?

-- 
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120527/f05d8118/attachment.sig>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v9 0/3] MTD: at91: Add PMECC support for at91 nand flash driver
  2012-05-26 13:24 ` Josh Wu
@ 2012-05-28  6:39   ` Jean-Christophe PLAGNIOL-VILLARD
  -1 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-28  6:39 UTC (permalink / raw)
  To: Josh Wu
  Cc: hongxu.cn, dedekind1, nicolas.ferre, linux-mtd, ivan.djelic,
	linux-arm-kernel

On 21:24 Sat 26 May     , Josh Wu wrote:
> Code is based on 3.4-rc2
> 
> Changes since v8,
> 	use _relaxed read/write in most place. use writel in operations of Control Register since it needs memory barrier.
> 	allocate the data for PMECC computation.
> 	add pmecc prefix for related variable/functions.
> 	modify code according to J.C's suggestion. except:
> 		>> +	for (i = 2; i <= 2 * host->cap; i += 2) {
> 		> manage the j in the for loop
> 		since that will change to: 
> 		+	for (i = 2, j = 1; i <= 2 * host->cap; i += 2, j = i / 2) {
		why you insist on doing j = i / 2

		you can simplify it by j++

		or loop on j and do i = j << 1;

		it's more simple and faster

Best Regards,
J.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 0/3] MTD: at91: Add PMECC support for at91 nand flash driver
@ 2012-05-28  6:39   ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-28  6:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 21:24 Sat 26 May     , Josh Wu wrote:
> Code is based on 3.4-rc2
> 
> Changes since v8,
> 	use _relaxed read/write in most place. use writel in operations of Control Register since it needs memory barrier.
> 	allocate the data for PMECC computation.
> 	add pmecc prefix for related variable/functions.
> 	modify code according to J.C's suggestion. except:
> 		>> +	for (i = 2; i <= 2 * host->cap; i += 2) {
> 		> manage the j in the for loop
> 		since that will change to: 
> 		+	for (i = 2, j = 1; i <= 2 * host->cap; i += 2, j = i / 2) {
		why you insist on doing j = i / 2

		you can simplify it by j++

		or loop on j and do i = j << 1;

		it's more simple and faster

Best Regards,
J.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
  2012-05-26 13:24   ` Josh Wu
@ 2012-05-28  6:58     ` Jean-Christophe PLAGNIOL-VILLARD
  -1 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-28  6:58 UTC (permalink / raw)
  To: Josh Wu
  Cc: hongxu.cn, dedekind1, nicolas.ferre, linux-mtd, ivan.djelic,
	linux-arm-kernel

On 21:24 Sat 26 May     , Josh Wu wrote:
> The Programmable Multibit ECC (PMECC) controller is a programmable binary
> BCH(Bose, Chaudhuri and Hocquenghem) encoder and decoder. This controller
> can be used to support both SLC and MLC NAND Flash devices. It supports to
> generate ECC to correct 2, 4, 8, 12 or 24 bits of error per sector of data.
> 
> To use this driver, the user needs to pass in the correction capability and
> the sector size.
> 
> This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2,
> YAFFS2, UBIFS and mtd-utils.
> 
> Signed-off-by: Hong Xu <hong.xu@atmel.com>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>  drivers/mtd/nand/atmel_nand.c     |  761 ++++++++++++++++++++++++++++++++++++-
>  drivers/mtd/nand/atmel_nand_ecc.h |  116 ++++++
>  2 files changed, 876 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 9a9bfbf..ddcf1ed 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -15,6 +15,8 @@
>   *     		(u-boot-1.1.5/board/atmel/at91sam9263ek/nand.c)
>   *     (C) Copyright 2006 ATMEL Rousset, Lacressonniere Nicolas
>   *
> + *  Add Programmable Multibit ECC support for various AT91 SoC
> + *     (C) Copyright 2012 ATMEL, Hong Xu
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
> @@ -77,6 +79,21 @@ static struct nand_ecclayout atmel_oobinfo_small = {
>  	},
>  };
>  
> +/* a structure includes datas for PMECC computation */
> +struct atmel_pmecc_data {
> +	int16_t			partial_syn[2 * PMECC_MAX_ERROR_NB + 1];
> +	int16_t			si[2 * PMECC_MAX_ERROR_NB + 1];
> +
> +	/* Sigma table */
> +	int16_t		smu[PMECC_MAX_ERROR_NB + 2][2 * PMECC_MAX_ERROR_NB + 1];
you still hardcode the array in the struct

and if the pmecc evolve we will have to touch again
please allocate them
> +	/* polynomal order */
> +	int16_t			lmu[PMECC_MAX_ERROR_NB + 1];
> +
> +	int			mu[PMECC_MAX_ERROR_NB + 1];
> +	int			dmu[PMECC_MAX_ERROR_NB + 1];
> +	int			delta[PMECC_MAX_ERROR_NB + 1];
> +};
> +
>  struct atmel_nand_host {
>  	struct nand_chip	nand_chip;
>  	struct mtd_info		mtd;
> @@ -92,8 +109,25 @@ struct atmel_nand_host {
>  	bool			has_pmecc;
>  	u8			pmecc_corr_cap;
>  	u16			pmecc_sector_size;
> +
> +	int			pmecc_bytes_per_sector;
> +	int			pmecc_sector_number;
> +	int			pmecc_degree;	/* Degree of remainders */
> +	int			pmecc_cw_len;	/* Length of codeword */
> +
> +	void __iomem		*pmerrloc_base;
> +	void __iomem		*pmecc_rom_base;
> +
> +	/* lookup table for alpha_to and index_of */
> +	void __iomem		*pmecc_alpha_to;
> +	void __iomem		*pmecc_index_of;
> +
> +	/* data for pmecc computation */
> +	struct atmel_pmecc_data	*pmecc_data;
>  };
>  
> +static struct nand_ecclayout atmel_pmecc_oobinfo;
> +
>  static int cpu_has_dma(void)
>  {
>  	return cpu_is_at91sam9rl() || cpu_is_at91sam9g45();
> @@ -287,6 +321,708 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
>  }
>  
>  /*
> + * Return number of ecc bytes per sector according to sector size and
> + * correction capability
> + *
> + * Following table shows what at91 PMECC supported:
> + * Correction Capability	Sector_512_bytes	Sector_1024_bytes
> + * =====================	================	=================
> + *                2-bits                 4-bytes                  4-bytes
> + *                4-bits                 7-bytes                  7-bytes
> + *                8-bits                13-bytes                 14-bytes
> + *               12-bits                20-bytes                 21-bytes
> + *               24-bits                39-bytes                 42-bytes
> + */
> +static int pmecc_get_ecc_bytes(int cap, int sector_size)
> +{
> +	int m = 12 + sector_size / 512;
> +	return (m * cap + 7) / 8;
> +}
> +
> +static void pmecc_config_ecc_layout(struct nand_ecclayout *layout, int oobsize,
> +	int ecc_len)
> +{
> +	int i;
> +
> +	layout->eccbytes = ecc_len;
> +
> +	/* ECC will occupy the last ecc_len bytes continuously */
> +	for (i = 0; i < ecc_len; i++)
> +		layout->eccpos[i] = oobsize - ecc_len + i;
> +
> +	layout->oobfree[0].offset = 2;
> +	layout->oobfree[0].length =
> +		oobsize - ecc_len - layout->oobfree[0].offset;
> +}
> +
> +static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
> +{
> +	void __iomem *p;
> +
> +	switch (host->pmecc_sector_size) {
> +	case 512:
> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512 +
> +			PMECC_LOOKUP_TABLE_SIZE_512 * sizeof(int16_t);
> +		break;
> +	case 1024:
> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024 +
> +			PMECC_LOOKUP_TABLE_SIZE_1024 * sizeof(int16_t);
> +		break;
> +	default:
> +		BUG();
> +	}
> +
> +	return p;
> +}
> +
> +static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
this is a __dev_init function plese check the other too

btw you need to use __dev_init and not __init
> +{
> +	void __iomem *p;
> +
> +	switch (host->pmecc_sector_size) {
> +	case 512:
> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512;
> +		break;
> +	case 1024:
> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024;
> +		break;
> +	default:
> +		BUG();
> +	}
> +
> +	return p;
> +}
> +
> +static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
> +{
> +	int			i;
> +	uint32_t		value;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +
> +	/* Fill odd syndromes */
> +	for (i = 0; i < host->pmecc_corr_cap; i++) {
> +		value = pmecc_readl_rem_relaxed(host->ecc, sector, i / 2);
> +		value = (i & 1) ? (value & 0xffff0000) >> 16 : value & 0xffff;
simplify by 
		if (i & 1)
			val >>= 16;
		value &= 0xffff;

> +		host->pmecc_data->partial_syn[(2 * i) + 1] = (int16_t)value;
> +	}
> +}
> +
> +static void pmecc_substitute(struct mtd_info *mtd)
> +{
> +	int16_t			*si;
> +	int			i, j;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +	int16_t __iomem		*alpha_to = host->pmecc_alpha_to;
> +	int16_t __iomem		*index_of = host->pmecc_index_of;
> +	int16_t			*partial_syn = host->pmecc_data->partial_syn;
> +
> +	/* si[] is a table that holds the current syndrome value,
> +	 * an element of that table belongs to the field
> +	 */
> +	si = host->pmecc_data->si;
> +
> +	for (i = 1; i < 2 * PMECC_MAX_ERROR_NB; i++)
> +		si[i] = 0;
please use memset
> +
> +	/* Computation 2t syndromes based on S(x) */
> +	/* Odd syndromes */
> +	for (i = 1; i < 2 * host->pmecc_corr_cap; i += 2) {
> +		si[i] = 0;
shy this you already init the array at 0 before
> +		for (j = 0; j < host->pmecc_degree; j++) {
> +			if (partial_syn[i] & ((unsigned short)0x1 << j))
> +				si[i] = readw_relaxed(alpha_to + i * j) ^ si[i];
> +		}
> +	}
> +	/* Even syndrome = (Odd syndrome) ** 2 */
> +	for (i = 2; i <= 2 * host->pmecc_corr_cap; i += 2) {
> +		j = i / 2;
> +		if (si[j] == 0)
here if {
} else {
}
> +			si[i] = 0;
> +		else {
> +			int16_t tmp;
missing blank line
> +			tmp = readw_relaxed(index_of + si[j]);
> +			tmp = (tmp * 2) % host->pmecc_cw_len;
> +			si[i] = readw_relaxed(alpha_to + tmp);
> +		}
> +	}
> +
> +	return;
> +}
> +
> +static void pmecc_get_sigma(struct mtd_info *mtd)
> +{
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +
> +	int		i, j, k;
> +	uint32_t	dmu_0_count, tmp;
> +	int16_t		(*smu)[2 * PMECC_MAX_ERROR_NB + 1];
> +	int16_t		*lmu = host->pmecc_data->lmu;
> +	int16_t		*si = host->pmecc_data->si;
> +	int		*mu = host->pmecc_data->mu;
> +	int		*dmu = host->pmecc_data->dmu;	  /* Discrepancy */
> +	int		*delta = host->pmecc_data->delta; /* Delta order */
> +	int		cw_len = host->pmecc_cw_len;
> +	int16_t		cap = host->pmecc_corr_cap;
> +
> +	int16_t __iomem	*index_of = host->pmecc_index_of;
> +	int16_t __iomem	*alpha_to = host->pmecc_alpha_to;
> +
> +	/* index of largest delta */
> +	int ro;
> +	int largest;
> +	int diff;
> +
> +	dmu_0_count = 0;
> +	smu = host->pmecc_data->smu;
> +
> +	/* First Row */
> +
> +	/* Mu */
> +	mu[0] = -1;
> +
> +	memset(&smu[0][0], 0,
> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
> +	smu[0][0] = 1;
> +
> +	/* discrepancy set to 1 */
> +	dmu[0] = 1;
> +	/* polynom order set to 0 */
> +	lmu[0] = 0;
> +	delta[0]  = (mu[0] * 2 - lmu[0]) >> 1;
> +
> +	/* Second Row */
> +
> +	/* Mu */
> +	mu[1]  = 0;
> +	/* Sigma(x) set to 1 */
> +	memset(&smu[1][0], 0,
> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
> +	smu[1][0] = 1;
> +
> +	/* discrepancy set to S1 */
> +	dmu[1] = si[1];
> +
> +	/* polynom order set to 0 */
> +	lmu[1] = 0;
> +
> +	delta[1]  = (mu[1] * 2 - lmu[1]) >> 1;
> +
> +	/* Init the Sigma(x) last row */
> +	memset(&smu[cap + 1][0], 0,
> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
> +
> +	for (i = 1; i <= cap; i++) {
> +		mu[i+1] = i << 1;
> +		/* Begin Computing Sigma (Mu+1) and L(mu) */
> +		/* check if discrepancy is set to 0 */
> +		if (dmu[i] == 0) {
> +			dmu_0_count++;
> +
> +			tmp = ((cap - (lmu[i] >> 1) - 1) / 2);
> +			if ((cap - (lmu[i] >> 1) - 1) & 0x1)
> +				tmp += 2;
> +			else
> +				tmp += 1;
> +
> +			if (dmu_0_count == tmp) {
> +				for (j = 0; j <= (lmu[i] >> 1) + 1; j++)
> +					smu[cap + 1][j] = smu[i][j];
> +				lmu[cap + 1] = lmu[i];
> +				return;
> +			}
> +
> +			/* copy polynom */
> +			for (j = 0; j <= lmu[i] >> 1; j++)
> +				smu[i + 1][j] = smu[i][j];
> +
> +			/* copy previous polynom order to the next */
> +			lmu[i + 1] = lmu[i];
> +		} else {
> +			ro = 0;
> +			largest = -1;
> +			/* find largest delta with dmu != 0 */
> +			for (j = 0; j < i; j++) {
> +				if ((dmu[j]) && (delta[j] > largest)) {
> +					largest = delta[j];
> +					ro = j;
> +				}
> +			}
> +
> +			/* compute difference */
> +			diff = (mu[i] - mu[ro]);
> +
> +			/* Compute degree of the new smu polynomial */
> +			if ((lmu[i] >> 1) > ((lmu[ro] >> 1) + diff))
> +				lmu[i + 1] = lmu[i];
> +			else
> +				lmu[i + 1] = ((lmu[ro] >> 1) + diff) * 2;
> +
> +			/* Init smu[i+1] with 0 */
> +			for (k = 0; k < (2 * PMECC_MAX_ERROR_NB + 1); k++)
> +				smu[i+1][k] = 0;
> +
> +			/* Compute smu[i+1] */
> +			for (k = 0; k <= lmu[ro] >> 1; k++) {
> +				int16_t a, b, c;
> +
> +				if (!(smu[ro][k] && dmu[i]))
> +					continue;
> +				a = readw_relaxed(index_of + dmu[i]);
> +				b = readw_relaxed(index_of + dmu[ro]);
> +				c = readw_relaxed(index_of + smu[ro][k]);
> +				tmp = a + (cw_len - b) + c;
> +				a = readw_relaxed(alpha_to + tmp % cw_len);
> +				smu[i + 1][k + diff] = a;
> +			}
> +
> +			for (k = 0; k <= lmu[i] >> 1; k++)
> +				smu[i + 1][k] ^= smu[i][k];
> +		}
> +
> +		/* End Computing Sigma (Mu+1) and L(mu) */
> +		/* In either case compute delta */
> +		delta[i + 1]  = (mu[i + 1] * 2 - lmu[i + 1]) >> 1;
> +
> +		/* Do not compute discrepancy for the last iteration */
> +		if (i >= cap)
> +			continue;
> +
> +		for (k = 0 ; k <= (lmu[i + 1] >> 1); k++) {
> +			tmp = 2 * (i - 1);
> +			if (k == 0)
> +				dmu[i + 1] = si[tmp + 3];
> +			else if (smu[i+1][k] && si[tmp + 3 - k]) {
> +				int16_t a, b, c;
> +				a = readw_relaxed(index_of + smu[i + 1][k]);
> +				b = si[2 * (i - 1) + 3 - k];
> +				c = readw_relaxed(index_of + b);
> +				tmp = a + c;
> +				tmp %= cw_len;
> +				dmu[i + 1] = readw_relaxed(alpha_to + tmp) ^
> +					dmu[i + 1];
> +			}
> +		}
> +	}
> +
> +	return;
> +}
> +
> +static int pmecc_err_location(struct mtd_info *mtd)
> +{
> +	int			i;
> +	int			err_nbr;	/* number of error */
> +	int			roots_nbr;	/* number of roots */
> +	int			sector_size;
> +	uint32_t		val;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +	int			timeout_count = 0;
> +	int			cap = host->pmecc_corr_cap;
> +
> +	err_nbr = 0;
> +	sector_size = host->pmecc_sector_size;
> +
> +	pmerrloc_writel(host->pmerrloc_base, ELDIS, PMERRLOC_DISABLE);
> +
> +	for (i = 0; i <= host->pmecc_data->lmu[cap + 1] >> 1; i++) {
> +		pmerrloc_writel_sigma_relaxed(host->pmerrloc_base, i,
> +				      host->pmecc_data->smu[cap + 1][i]);
> +		err_nbr++;
> +	}
> +
> +	val = (err_nbr - 1) << 16;
> +	if (sector_size == 1024)
> +		val |= 1;
> +
> +	pmerrloc_writel(host->pmerrloc_base, ELCFG, val);
> +	pmerrloc_writel(host->pmerrloc_base, ELEN,
> +			sector_size * 8 + host->pmecc_degree * cap);
> +
> +	while (!(pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
> +		 & PMERRLOC_CALC_DONE)) {
> +		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT))
> +			return -1;	/* Time out */
> +		cpu_relax();
> +	}
> +
> +	roots_nbr = (pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
> +		& PMERRLOC_ERR_NUM_MASK) >> 8;
> +	/* Number of roots == degree of smu hence <= cap */
> +	if (roots_nbr == host->pmecc_data->lmu[cap + 1] >> 1)
> +		return err_nbr - 1;
> +
> +	/* Number of roots does not match the degree of smu
> +	 * unable to correct error */
> +	return -1;
> +}
> +
> +static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf,
> +		int extra_bytes, int err_nbr)
> +{
> +	int			i = 0;
> +	int			byte_pos, bit_pos;
> +	int			sector_size, ecc_size;
> +	uint32_t		tmp;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +
> +	sector_size = host->pmecc_sector_size;
> +	ecc_size = nand_chip->ecc.bytes;
> +
> +	while (err_nbr) {
> +		tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
> +		byte_pos = tmp / 8;
> +		bit_pos  = tmp % 8;
> +		dev_info(host->dev, "PMECC correction, byte_pos: %d bit_pos: %d\n",
> +					byte_pos, bit_pos);
> +
> +		if (byte_pos < (sector_size + extra_bytes)) {
> +			tmp = sector_size +
> +				pmecc_readl_relaxed(host->ecc, SADDR);
> +
> +			if (byte_pos < tmp)
> +				*(buf + byte_pos) ^= (1 << bit_pos);
> +			else
> +				*(buf + byte_pos + ecc_size) ^= (1 << bit_pos);
> +		}
> +
> +		i++;
> +		err_nbr--;
> +	}
> +
> +	return;
> +}
> +
> +static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
> +	u8 *ecc)
> +{
> +	int			i, err_nbr;
> +	uint8_t			*buf_pos;
> +	int			eccbytes;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +
> +	eccbytes = nand_chip->ecc.bytes;
> +	for (i = 0; i < eccbytes; i++)
> +		if (ecc[i] != 0xff)
> +			goto normal_check;
> +	/* Erased page, return OK */
> +	return 0;
> +
> +normal_check:
> +	for (i = 0; i < host->pmecc_sector_number; i++) {
> +		err_nbr = 0;
> +		if (pmecc_stat & 0x1) {
> +			buf_pos = buf + i * host->pmecc_sector_size;
> +
> +			pmecc_gen_syndrome(mtd, i);
> +			pmecc_substitute(mtd);
> +			pmecc_get_sigma(mtd);
> +
> +			err_nbr = pmecc_err_location(mtd);
> +			if (err_nbr == -1) {
> +				dev_err(host->dev, "PMECC: Too many errors\n");
> +				mtd->ecc_stats.failed++;
> +				return -EIO;
> +			} else {
> +				pmecc_correct_data(mtd, buf_pos, 0, err_nbr);
> +				mtd->ecc_stats.corrected += err_nbr;
> +			}
> +		}
> +		pmecc_stat >>= 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
> +		struct nand_chip *chip, uint8_t *buf, int page)
> +{
> +	uint32_t		stat;
> +	int			timeout_count = 0;
> +	int			eccsize = chip->ecc.size;
> +	uint8_t			*oob = chip->oob_poi;
> +	struct atmel_nand_host	*host = chip->priv;
> +	uint32_t		*eccpos = chip->ecc.layout->eccpos;
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
> +	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG)
> +		& ~PMECC_CFG_WRITE_OP) | PMECC_CFG_AUTO_ENABLE);
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
> +
> +	chip->read_buf(mtd, buf, eccsize);
> +	chip->read_buf(mtd, oob, mtd->oobsize);
> +
> +	while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
> +		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT))
> +			return -EIO;	/* Time out */
> +		cpu_relax();
> +	}
> +
> +	stat = pmecc_readl_relaxed(host->ecc, ISR);
> +	if (stat != 0) {
> +		if (pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]) != 0)
> +			return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static void atmel_nand_pmecc_write_page(struct mtd_info *mtd,
> +		struct nand_chip *chip, const uint8_t *buf)
> +{
> +	int			i, j;
> +	int			timeout_count = 0;
> +	struct atmel_nand_host	*host = chip->priv;
> +	uint32_t		*eccpos = chip->ecc.layout->eccpos;
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
> +
> +	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG) |
> +		PMECC_CFG_WRITE_OP) & ~PMECC_CFG_AUTO_ENABLE);
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
> +
> +	chip->write_buf(mtd, (u8 *)buf, mtd->writesize);
> +
> +	while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
> +		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT)) {
> +			dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
> +			return;	/* Time out */
> +		}
> +		cpu_relax();
> +	}
> +
> +	for (i = 0; i < host->pmecc_sector_number; i++) {
> +		for (j = 0; j < host->pmecc_bytes_per_sector; j++) {
> +			int pos;
> +
> +			pos = i * host->pmecc_bytes_per_sector + j;
> +			chip->oob_poi[eccpos[pos]] =
> +				pmecc_readb_ecc_relaxed(host->ecc, i, j);
> +		}
> +	}
> +	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return;
> +}
> +
> +static void atmel_pmecc_core_init(struct mtd_info *mtd)
> +{
> +	uint32_t		val = 0;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +	struct nand_ecclayout	*ecc_layout;
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
> +
> +	switch (host->pmecc_corr_cap) {
> +	case 2:
> +		val = PMECC_CFG_BCH_ERR2;
> +		break;
> +	case 4:
> +		val = PMECC_CFG_BCH_ERR4;
> +		break;
> +	case 8:
> +		val = PMECC_CFG_BCH_ERR8;
> +		break;
> +	case 12:
> +		val = PMECC_CFG_BCH_ERR12;
> +		break;
> +	case 24:
> +		val = PMECC_CFG_BCH_ERR24;
> +		break;
> +	}
> +
> +	if (host->pmecc_sector_size == 512)
> +		val |= PMECC_CFG_SECTOR512;
> +	else if (host->pmecc_sector_size == 1024)
> +		val |= PMECC_CFG_SECTOR1024;
> +
> +	switch (host->pmecc_sector_number) {
> +	case 1:
> +		val |= PMECC_CFG_PAGE_1SECTOR;
> +		break;
> +	case 2:
> +		val |= PMECC_CFG_PAGE_2SECTORS;
> +		break;
> +	case 4:
> +		val |= PMECC_CFG_PAGE_4SECTORS;
> +		break;
> +	case 8:
> +		val |= PMECC_CFG_PAGE_8SECTORS;
> +		break;
> +	}
> +
> +	val |= (PMECC_CFG_READ_OP | PMECC_CFG_SPARE_DISABLE
> +		| PMECC_CFG_AUTO_DISABLE);
> +	pmecc_writel(host->ecc, CFG, val);
> +
> +	ecc_layout = nand_chip->ecc.layout;
> +	pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
> +	pmecc_writel(host->ecc, SADDR, ecc_layout->eccpos[0]);
> +	pmecc_writel(host->ecc, EADDR,
> +			ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
> +	/* See datasheet about PMECC Clock Control Register */
> +	pmecc_writel(host->ecc, CLK, 2);
> +	pmecc_writel(host->ecc, IDR, 0xff);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
> +}
> +
> +static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
> +					 struct atmel_nand_host *host)
> +{
> +	int			cap, sector_size, err_no;
> +	struct mtd_info		*mtd;
> +	struct nand_chip	*nand_chip;
> +	struct resource		*regs;
> +	struct resource		*regs_pmerr, *regs_rom;
> +
> +	cap = host->pmecc_corr_cap;
> +	sector_size = host->pmecc_sector_size;
> +	dev_info(host->dev, "Initialize PMECC params, cap: %d, sector: %d\n",
> +		 cap, sector_size);
> +
> +	/* Sanity check */
> +	if ((sector_size != 512) && (sector_size != 1024)) {
> +		dev_err(host->dev,
> +			"Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
> +			sector_size);
> +		return -EINVAL;
> +	}
> +	if ((cap != 2) && (cap != 4) && (cap != 8) && (cap != 12) &&
> +	    (cap != 24)) {
> +		dev_err(host->dev,
> +			"Unsupported PMECC correction capability, should be 2, 4, 8, 12 or 24\n");
> +		return -EINVAL;
> +	}
> +
> +	nand_chip = &host->nand_chip;
> +	mtd = &host->mtd;
> +
> +	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* By default */
> +
> +	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!regs) {
> +		dev_warn(host->dev,
> +			"Can't get I/O resource regs, rolling back on software ECC\n");
> +		return 0;
> +	}
> +
> +	host->ecc = ioremap(regs->start, resource_size(regs));
> +	if (host->ecc == NULL) {
> +		dev_err(host->dev, "ioremap failed\n");
> +		err_no = -EIO;
> +		goto err_pmecc_ioremap;
> +	}
> +
> +	regs_pmerr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> +	if (regs_pmerr && regs_rom) {
> +		host->pmerrloc_base = ioremap(regs_pmerr->start,
> +			resource_size(regs_pmerr));
> +		host->pmecc_rom_base = ioremap(regs_rom->start,
> +			resource_size(regs_rom));
> +
> +		if (host->pmerrloc_base && host->pmecc_rom_base) {
> +			nand_chip->ecc.mode = NAND_ECC_HW;
> +			nand_chip->ecc.read_page =
> +				atmel_nand_pmecc_read_page;
> +			nand_chip->ecc.write_page =
> +				atmel_nand_pmecc_write_page;
> +		} else {
> +			dev_err(host->dev,
> +				"Can not get I/O resource for PMECC controller!\n");
> +			err_no = -EIO;
> +			goto err_pmloc_ioremap;
> +		}
> +	}
> +
> +	/* ECC is calculated for the whole page (1 step) */
> +	nand_chip->ecc.size = mtd->writesize;
> +
> +	/* set ECC page size and oob layout */
> +	switch (mtd->writesize) {
> +	case 2048:
> +		host->pmecc_degree = PMECC_GF_DIMENSION_13;
> +		host->pmecc_cw_len = (1 << host->pmecc_degree) - 1;
> +		host->pmecc_corr_cap = cap;
> +		host->pmecc_sector_number = mtd->writesize / sector_size;
> +		host->pmecc_bytes_per_sector = pmecc_get_ecc_bytes(
> +			cap, sector_size);
> +		host->pmecc_alpha_to = pmecc_get_alpha_to(host);
> +		host->pmecc_index_of = pmecc_get_index_of(host);
> +
> +		nand_chip->ecc.steps = 1;
> +		nand_chip->ecc.strength = cap;
> +		nand_chip->ecc.bytes = host->pmecc_bytes_per_sector *
> +				       host->pmecc_sector_number;
> +		if (nand_chip->ecc.bytes > mtd->oobsize - 2) {
> +			dev_err(host->dev, "No room for ECC bytes\n");
> +			err_no = -EINVAL;
> +			goto err;
> +		}
> +		pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
> +					mtd->oobsize,
> +					nand_chip->ecc.bytes);
> +		nand_chip->ecc.layout = &atmel_pmecc_oobinfo;
> +		break;
> +	case 512:
> +	case 1024:
> +	case 4096:
> +		/* TODO */
> +		dev_warn(host->dev,
> +			"Unsupported page size for PMECC, use Software ECC\n");
> +	default:
> +		/* page size not handled by HW ECC */
> +		/* switching back to soft ECC */
> +		nand_chip->ecc.mode = NAND_ECC_SOFT;
> +		nand_chip->ecc.calculate = NULL;
> +		nand_chip->ecc.correct = NULL;
> +		nand_chip->ecc.hwctl = NULL;
> +		nand_chip->ecc.read_page = NULL;
> +		nand_chip->ecc.write_page = NULL;
> +		nand_chip->ecc.postpad = 0;
> +		nand_chip->ecc.prepad = 0;
> +		nand_chip->ecc.bytes = 0;
> +		err_no = 0;
> +		goto err;
> +	}
> +
> +	/* Allocate data for PMECC computation */
> +	host->pmecc_data = kzalloc(sizeof(struct atmel_pmecc_data), GFP_KERNEL);
why do you always allocate the pmecc_data?

you need to allocate it only if you use it

Best Regards,
J.
> +	if (!host->pmecc_data) {
> +		dev_err(host->dev,
> +				"Cannot allocate memory for PMECC computation!\n");
> +		err_no = -ENOMEM;
> +		goto err;
> +	}
> +
> +	atmel_pmecc_core_init(mtd);
> +
> +	return 0;
> +
> +err:
> +err_pmloc_ioremap:
> +	iounmap(host->ecc);
> +	if (host->pmerrloc_base)
> +		iounmap(host->pmerrloc_base);
> +	if (host->pmecc_rom_base)
> +		iounmap(host->pmecc_rom_base);
> +err_pmecc_ioremap:
> +	return err_no;
> +}
> +
> +/*
>   * Calculate HW ECC
>   *
>   * function called after a write
> @@ -720,7 +1456,11 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
>  	}
>  
>  	if (nand_chip->ecc.mode == NAND_ECC_HW) {
> -		res = atmel_hw_nand_init_params(pdev, host);
> +		if (host->has_pmecc)
> +			res = atmel_pmecc_nand_init_params(pdev, host);
> +		else
> +			res = atmel_hw_nand_init_params(pdev, host);
> +
>  		if (res != 0)
>  			goto err_hw_ecc;
>  	}
> @@ -741,6 +1481,12 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
>  err_scan_tail:
>  	if (host->ecc)
>  		iounmap(host->ecc);
> +	if (host->has_pmecc) {
no need to check if you have teh pmecc

if it's no the case pmerrloc_base will be NULLo

don't you need to disable it in the error path?

Best Regards,
J.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
@ 2012-05-28  6:58     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-28  6:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 21:24 Sat 26 May     , Josh Wu wrote:
> The Programmable Multibit ECC (PMECC) controller is a programmable binary
> BCH(Bose, Chaudhuri and Hocquenghem) encoder and decoder. This controller
> can be used to support both SLC and MLC NAND Flash devices. It supports to
> generate ECC to correct 2, 4, 8, 12 or 24 bits of error per sector of data.
> 
> To use this driver, the user needs to pass in the correction capability and
> the sector size.
> 
> This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2,
> YAFFS2, UBIFS and mtd-utils.
> 
> Signed-off-by: Hong Xu <hong.xu@atmel.com>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>  drivers/mtd/nand/atmel_nand.c     |  761 ++++++++++++++++++++++++++++++++++++-
>  drivers/mtd/nand/atmel_nand_ecc.h |  116 ++++++
>  2 files changed, 876 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 9a9bfbf..ddcf1ed 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -15,6 +15,8 @@
>   *     		(u-boot-1.1.5/board/atmel/at91sam9263ek/nand.c)
>   *     (C) Copyright 2006 ATMEL Rousset, Lacressonniere Nicolas
>   *
> + *  Add Programmable Multibit ECC support for various AT91 SoC
> + *     (C) Copyright 2012 ATMEL, Hong Xu
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
> @@ -77,6 +79,21 @@ static struct nand_ecclayout atmel_oobinfo_small = {
>  	},
>  };
>  
> +/* a structure includes datas for PMECC computation */
> +struct atmel_pmecc_data {
> +	int16_t			partial_syn[2 * PMECC_MAX_ERROR_NB + 1];
> +	int16_t			si[2 * PMECC_MAX_ERROR_NB + 1];
> +
> +	/* Sigma table */
> +	int16_t		smu[PMECC_MAX_ERROR_NB + 2][2 * PMECC_MAX_ERROR_NB + 1];
you still hardcode the array in the struct

and if the pmecc evolve we will have to touch again
please allocate them
> +	/* polynomal order */
> +	int16_t			lmu[PMECC_MAX_ERROR_NB + 1];
> +
> +	int			mu[PMECC_MAX_ERROR_NB + 1];
> +	int			dmu[PMECC_MAX_ERROR_NB + 1];
> +	int			delta[PMECC_MAX_ERROR_NB + 1];
> +};
> +
>  struct atmel_nand_host {
>  	struct nand_chip	nand_chip;
>  	struct mtd_info		mtd;
> @@ -92,8 +109,25 @@ struct atmel_nand_host {
>  	bool			has_pmecc;
>  	u8			pmecc_corr_cap;
>  	u16			pmecc_sector_size;
> +
> +	int			pmecc_bytes_per_sector;
> +	int			pmecc_sector_number;
> +	int			pmecc_degree;	/* Degree of remainders */
> +	int			pmecc_cw_len;	/* Length of codeword */
> +
> +	void __iomem		*pmerrloc_base;
> +	void __iomem		*pmecc_rom_base;
> +
> +	/* lookup table for alpha_to and index_of */
> +	void __iomem		*pmecc_alpha_to;
> +	void __iomem		*pmecc_index_of;
> +
> +	/* data for pmecc computation */
> +	struct atmel_pmecc_data	*pmecc_data;
>  };
>  
> +static struct nand_ecclayout atmel_pmecc_oobinfo;
> +
>  static int cpu_has_dma(void)
>  {
>  	return cpu_is_at91sam9rl() || cpu_is_at91sam9g45();
> @@ -287,6 +321,708 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
>  }
>  
>  /*
> + * Return number of ecc bytes per sector according to sector size and
> + * correction capability
> + *
> + * Following table shows what at91 PMECC supported:
> + * Correction Capability	Sector_512_bytes	Sector_1024_bytes
> + * =====================	================	=================
> + *                2-bits                 4-bytes                  4-bytes
> + *                4-bits                 7-bytes                  7-bytes
> + *                8-bits                13-bytes                 14-bytes
> + *               12-bits                20-bytes                 21-bytes
> + *               24-bits                39-bytes                 42-bytes
> + */
> +static int pmecc_get_ecc_bytes(int cap, int sector_size)
> +{
> +	int m = 12 + sector_size / 512;
> +	return (m * cap + 7) / 8;
> +}
> +
> +static void pmecc_config_ecc_layout(struct nand_ecclayout *layout, int oobsize,
> +	int ecc_len)
> +{
> +	int i;
> +
> +	layout->eccbytes = ecc_len;
> +
> +	/* ECC will occupy the last ecc_len bytes continuously */
> +	for (i = 0; i < ecc_len; i++)
> +		layout->eccpos[i] = oobsize - ecc_len + i;
> +
> +	layout->oobfree[0].offset = 2;
> +	layout->oobfree[0].length =
> +		oobsize - ecc_len - layout->oobfree[0].offset;
> +}
> +
> +static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
> +{
> +	void __iomem *p;
> +
> +	switch (host->pmecc_sector_size) {
> +	case 512:
> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512 +
> +			PMECC_LOOKUP_TABLE_SIZE_512 * sizeof(int16_t);
> +		break;
> +	case 1024:
> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024 +
> +			PMECC_LOOKUP_TABLE_SIZE_1024 * sizeof(int16_t);
> +		break;
> +	default:
> +		BUG();
> +	}
> +
> +	return p;
> +}
> +
> +static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
this is a __dev_init function plese check the other too

btw you need to use __dev_init and not __init
> +{
> +	void __iomem *p;
> +
> +	switch (host->pmecc_sector_size) {
> +	case 512:
> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512;
> +		break;
> +	case 1024:
> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024;
> +		break;
> +	default:
> +		BUG();
> +	}
> +
> +	return p;
> +}
> +
> +static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
> +{
> +	int			i;
> +	uint32_t		value;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +
> +	/* Fill odd syndromes */
> +	for (i = 0; i < host->pmecc_corr_cap; i++) {
> +		value = pmecc_readl_rem_relaxed(host->ecc, sector, i / 2);
> +		value = (i & 1) ? (value & 0xffff0000) >> 16 : value & 0xffff;
simplify by 
		if (i & 1)
			val >>= 16;
		value &= 0xffff;

> +		host->pmecc_data->partial_syn[(2 * i) + 1] = (int16_t)value;
> +	}
> +}
> +
> +static void pmecc_substitute(struct mtd_info *mtd)
> +{
> +	int16_t			*si;
> +	int			i, j;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +	int16_t __iomem		*alpha_to = host->pmecc_alpha_to;
> +	int16_t __iomem		*index_of = host->pmecc_index_of;
> +	int16_t			*partial_syn = host->pmecc_data->partial_syn;
> +
> +	/* si[] is a table that holds the current syndrome value,
> +	 * an element of that table belongs to the field
> +	 */
> +	si = host->pmecc_data->si;
> +
> +	for (i = 1; i < 2 * PMECC_MAX_ERROR_NB; i++)
> +		si[i] = 0;
please use memset
> +
> +	/* Computation 2t syndromes based on S(x) */
> +	/* Odd syndromes */
> +	for (i = 1; i < 2 * host->pmecc_corr_cap; i += 2) {
> +		si[i] = 0;
shy this you already init the array at 0 before
> +		for (j = 0; j < host->pmecc_degree; j++) {
> +			if (partial_syn[i] & ((unsigned short)0x1 << j))
> +				si[i] = readw_relaxed(alpha_to + i * j) ^ si[i];
> +		}
> +	}
> +	/* Even syndrome = (Odd syndrome) ** 2 */
> +	for (i = 2; i <= 2 * host->pmecc_corr_cap; i += 2) {
> +		j = i / 2;
> +		if (si[j] == 0)
here if {
} else {
}
> +			si[i] = 0;
> +		else {
> +			int16_t tmp;
missing blank line
> +			tmp = readw_relaxed(index_of + si[j]);
> +			tmp = (tmp * 2) % host->pmecc_cw_len;
> +			si[i] = readw_relaxed(alpha_to + tmp);
> +		}
> +	}
> +
> +	return;
> +}
> +
> +static void pmecc_get_sigma(struct mtd_info *mtd)
> +{
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +
> +	int		i, j, k;
> +	uint32_t	dmu_0_count, tmp;
> +	int16_t		(*smu)[2 * PMECC_MAX_ERROR_NB + 1];
> +	int16_t		*lmu = host->pmecc_data->lmu;
> +	int16_t		*si = host->pmecc_data->si;
> +	int		*mu = host->pmecc_data->mu;
> +	int		*dmu = host->pmecc_data->dmu;	  /* Discrepancy */
> +	int		*delta = host->pmecc_data->delta; /* Delta order */
> +	int		cw_len = host->pmecc_cw_len;
> +	int16_t		cap = host->pmecc_corr_cap;
> +
> +	int16_t __iomem	*index_of = host->pmecc_index_of;
> +	int16_t __iomem	*alpha_to = host->pmecc_alpha_to;
> +
> +	/* index of largest delta */
> +	int ro;
> +	int largest;
> +	int diff;
> +
> +	dmu_0_count = 0;
> +	smu = host->pmecc_data->smu;
> +
> +	/* First Row */
> +
> +	/* Mu */
> +	mu[0] = -1;
> +
> +	memset(&smu[0][0], 0,
> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
> +	smu[0][0] = 1;
> +
> +	/* discrepancy set to 1 */
> +	dmu[0] = 1;
> +	/* polynom order set to 0 */
> +	lmu[0] = 0;
> +	delta[0]  = (mu[0] * 2 - lmu[0]) >> 1;
> +
> +	/* Second Row */
> +
> +	/* Mu */
> +	mu[1]  = 0;
> +	/* Sigma(x) set to 1 */
> +	memset(&smu[1][0], 0,
> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
> +	smu[1][0] = 1;
> +
> +	/* discrepancy set to S1 */
> +	dmu[1] = si[1];
> +
> +	/* polynom order set to 0 */
> +	lmu[1] = 0;
> +
> +	delta[1]  = (mu[1] * 2 - lmu[1]) >> 1;
> +
> +	/* Init the Sigma(x) last row */
> +	memset(&smu[cap + 1][0], 0,
> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
> +
> +	for (i = 1; i <= cap; i++) {
> +		mu[i+1] = i << 1;
> +		/* Begin Computing Sigma (Mu+1) and L(mu) */
> +		/* check if discrepancy is set to 0 */
> +		if (dmu[i] == 0) {
> +			dmu_0_count++;
> +
> +			tmp = ((cap - (lmu[i] >> 1) - 1) / 2);
> +			if ((cap - (lmu[i] >> 1) - 1) & 0x1)
> +				tmp += 2;
> +			else
> +				tmp += 1;
> +
> +			if (dmu_0_count == tmp) {
> +				for (j = 0; j <= (lmu[i] >> 1) + 1; j++)
> +					smu[cap + 1][j] = smu[i][j];
> +				lmu[cap + 1] = lmu[i];
> +				return;
> +			}
> +
> +			/* copy polynom */
> +			for (j = 0; j <= lmu[i] >> 1; j++)
> +				smu[i + 1][j] = smu[i][j];
> +
> +			/* copy previous polynom order to the next */
> +			lmu[i + 1] = lmu[i];
> +		} else {
> +			ro = 0;
> +			largest = -1;
> +			/* find largest delta with dmu != 0 */
> +			for (j = 0; j < i; j++) {
> +				if ((dmu[j]) && (delta[j] > largest)) {
> +					largest = delta[j];
> +					ro = j;
> +				}
> +			}
> +
> +			/* compute difference */
> +			diff = (mu[i] - mu[ro]);
> +
> +			/* Compute degree of the new smu polynomial */
> +			if ((lmu[i] >> 1) > ((lmu[ro] >> 1) + diff))
> +				lmu[i + 1] = lmu[i];
> +			else
> +				lmu[i + 1] = ((lmu[ro] >> 1) + diff) * 2;
> +
> +			/* Init smu[i+1] with 0 */
> +			for (k = 0; k < (2 * PMECC_MAX_ERROR_NB + 1); k++)
> +				smu[i+1][k] = 0;
> +
> +			/* Compute smu[i+1] */
> +			for (k = 0; k <= lmu[ro] >> 1; k++) {
> +				int16_t a, b, c;
> +
> +				if (!(smu[ro][k] && dmu[i]))
> +					continue;
> +				a = readw_relaxed(index_of + dmu[i]);
> +				b = readw_relaxed(index_of + dmu[ro]);
> +				c = readw_relaxed(index_of + smu[ro][k]);
> +				tmp = a + (cw_len - b) + c;
> +				a = readw_relaxed(alpha_to + tmp % cw_len);
> +				smu[i + 1][k + diff] = a;
> +			}
> +
> +			for (k = 0; k <= lmu[i] >> 1; k++)
> +				smu[i + 1][k] ^= smu[i][k];
> +		}
> +
> +		/* End Computing Sigma (Mu+1) and L(mu) */
> +		/* In either case compute delta */
> +		delta[i + 1]  = (mu[i + 1] * 2 - lmu[i + 1]) >> 1;
> +
> +		/* Do not compute discrepancy for the last iteration */
> +		if (i >= cap)
> +			continue;
> +
> +		for (k = 0 ; k <= (lmu[i + 1] >> 1); k++) {
> +			tmp = 2 * (i - 1);
> +			if (k == 0)
> +				dmu[i + 1] = si[tmp + 3];
> +			else if (smu[i+1][k] && si[tmp + 3 - k]) {
> +				int16_t a, b, c;
> +				a = readw_relaxed(index_of + smu[i + 1][k]);
> +				b = si[2 * (i - 1) + 3 - k];
> +				c = readw_relaxed(index_of + b);
> +				tmp = a + c;
> +				tmp %= cw_len;
> +				dmu[i + 1] = readw_relaxed(alpha_to + tmp) ^
> +					dmu[i + 1];
> +			}
> +		}
> +	}
> +
> +	return;
> +}
> +
> +static int pmecc_err_location(struct mtd_info *mtd)
> +{
> +	int			i;
> +	int			err_nbr;	/* number of error */
> +	int			roots_nbr;	/* number of roots */
> +	int			sector_size;
> +	uint32_t		val;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +	int			timeout_count = 0;
> +	int			cap = host->pmecc_corr_cap;
> +
> +	err_nbr = 0;
> +	sector_size = host->pmecc_sector_size;
> +
> +	pmerrloc_writel(host->pmerrloc_base, ELDIS, PMERRLOC_DISABLE);
> +
> +	for (i = 0; i <= host->pmecc_data->lmu[cap + 1] >> 1; i++) {
> +		pmerrloc_writel_sigma_relaxed(host->pmerrloc_base, i,
> +				      host->pmecc_data->smu[cap + 1][i]);
> +		err_nbr++;
> +	}
> +
> +	val = (err_nbr - 1) << 16;
> +	if (sector_size == 1024)
> +		val |= 1;
> +
> +	pmerrloc_writel(host->pmerrloc_base, ELCFG, val);
> +	pmerrloc_writel(host->pmerrloc_base, ELEN,
> +			sector_size * 8 + host->pmecc_degree * cap);
> +
> +	while (!(pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
> +		 & PMERRLOC_CALC_DONE)) {
> +		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT))
> +			return -1;	/* Time out */
> +		cpu_relax();
> +	}
> +
> +	roots_nbr = (pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
> +		& PMERRLOC_ERR_NUM_MASK) >> 8;
> +	/* Number of roots == degree of smu hence <= cap */
> +	if (roots_nbr == host->pmecc_data->lmu[cap + 1] >> 1)
> +		return err_nbr - 1;
> +
> +	/* Number of roots does not match the degree of smu
> +	 * unable to correct error */
> +	return -1;
> +}
> +
> +static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf,
> +		int extra_bytes, int err_nbr)
> +{
> +	int			i = 0;
> +	int			byte_pos, bit_pos;
> +	int			sector_size, ecc_size;
> +	uint32_t		tmp;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +
> +	sector_size = host->pmecc_sector_size;
> +	ecc_size = nand_chip->ecc.bytes;
> +
> +	while (err_nbr) {
> +		tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
> +		byte_pos = tmp / 8;
> +		bit_pos  = tmp % 8;
> +		dev_info(host->dev, "PMECC correction, byte_pos: %d bit_pos: %d\n",
> +					byte_pos, bit_pos);
> +
> +		if (byte_pos < (sector_size + extra_bytes)) {
> +			tmp = sector_size +
> +				pmecc_readl_relaxed(host->ecc, SADDR);
> +
> +			if (byte_pos < tmp)
> +				*(buf + byte_pos) ^= (1 << bit_pos);
> +			else
> +				*(buf + byte_pos + ecc_size) ^= (1 << bit_pos);
> +		}
> +
> +		i++;
> +		err_nbr--;
> +	}
> +
> +	return;
> +}
> +
> +static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
> +	u8 *ecc)
> +{
> +	int			i, err_nbr;
> +	uint8_t			*buf_pos;
> +	int			eccbytes;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +
> +	eccbytes = nand_chip->ecc.bytes;
> +	for (i = 0; i < eccbytes; i++)
> +		if (ecc[i] != 0xff)
> +			goto normal_check;
> +	/* Erased page, return OK */
> +	return 0;
> +
> +normal_check:
> +	for (i = 0; i < host->pmecc_sector_number; i++) {
> +		err_nbr = 0;
> +		if (pmecc_stat & 0x1) {
> +			buf_pos = buf + i * host->pmecc_sector_size;
> +
> +			pmecc_gen_syndrome(mtd, i);
> +			pmecc_substitute(mtd);
> +			pmecc_get_sigma(mtd);
> +
> +			err_nbr = pmecc_err_location(mtd);
> +			if (err_nbr == -1) {
> +				dev_err(host->dev, "PMECC: Too many errors\n");
> +				mtd->ecc_stats.failed++;
> +				return -EIO;
> +			} else {
> +				pmecc_correct_data(mtd, buf_pos, 0, err_nbr);
> +				mtd->ecc_stats.corrected += err_nbr;
> +			}
> +		}
> +		pmecc_stat >>= 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
> +		struct nand_chip *chip, uint8_t *buf, int page)
> +{
> +	uint32_t		stat;
> +	int			timeout_count = 0;
> +	int			eccsize = chip->ecc.size;
> +	uint8_t			*oob = chip->oob_poi;
> +	struct atmel_nand_host	*host = chip->priv;
> +	uint32_t		*eccpos = chip->ecc.layout->eccpos;
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
> +	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG)
> +		& ~PMECC_CFG_WRITE_OP) | PMECC_CFG_AUTO_ENABLE);
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
> +
> +	chip->read_buf(mtd, buf, eccsize);
> +	chip->read_buf(mtd, oob, mtd->oobsize);
> +
> +	while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
> +		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT))
> +			return -EIO;	/* Time out */
> +		cpu_relax();
> +	}
> +
> +	stat = pmecc_readl_relaxed(host->ecc, ISR);
> +	if (stat != 0) {
> +		if (pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]) != 0)
> +			return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static void atmel_nand_pmecc_write_page(struct mtd_info *mtd,
> +		struct nand_chip *chip, const uint8_t *buf)
> +{
> +	int			i, j;
> +	int			timeout_count = 0;
> +	struct atmel_nand_host	*host = chip->priv;
> +	uint32_t		*eccpos = chip->ecc.layout->eccpos;
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
> +
> +	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG) |
> +		PMECC_CFG_WRITE_OP) & ~PMECC_CFG_AUTO_ENABLE);
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
> +
> +	chip->write_buf(mtd, (u8 *)buf, mtd->writesize);
> +
> +	while ((pmecc_readl_relaxed(host->ecc, SR) & PMECC_SR_BUSY)) {
> +		if (unlikely(timeout_count++ > PMECC_MAX_TIMEOUT_COUNT)) {
> +			dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
> +			return;	/* Time out */
> +		}
> +		cpu_relax();
> +	}
> +
> +	for (i = 0; i < host->pmecc_sector_number; i++) {
> +		for (j = 0; j < host->pmecc_bytes_per_sector; j++) {
> +			int pos;
> +
> +			pos = i * host->pmecc_bytes_per_sector + j;
> +			chip->oob_poi[eccpos[pos]] =
> +				pmecc_readb_ecc_relaxed(host->ecc, i, j);
> +		}
> +	}
> +	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return;
> +}
> +
> +static void atmel_pmecc_core_init(struct mtd_info *mtd)
> +{
> +	uint32_t		val = 0;
> +	struct nand_chip	*nand_chip = mtd->priv;
> +	struct atmel_nand_host	*host = nand_chip->priv;
> +	struct nand_ecclayout	*ecc_layout;
> +
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
> +
> +	switch (host->pmecc_corr_cap) {
> +	case 2:
> +		val = PMECC_CFG_BCH_ERR2;
> +		break;
> +	case 4:
> +		val = PMECC_CFG_BCH_ERR4;
> +		break;
> +	case 8:
> +		val = PMECC_CFG_BCH_ERR8;
> +		break;
> +	case 12:
> +		val = PMECC_CFG_BCH_ERR12;
> +		break;
> +	case 24:
> +		val = PMECC_CFG_BCH_ERR24;
> +		break;
> +	}
> +
> +	if (host->pmecc_sector_size == 512)
> +		val |= PMECC_CFG_SECTOR512;
> +	else if (host->pmecc_sector_size == 1024)
> +		val |= PMECC_CFG_SECTOR1024;
> +
> +	switch (host->pmecc_sector_number) {
> +	case 1:
> +		val |= PMECC_CFG_PAGE_1SECTOR;
> +		break;
> +	case 2:
> +		val |= PMECC_CFG_PAGE_2SECTORS;
> +		break;
> +	case 4:
> +		val |= PMECC_CFG_PAGE_4SECTORS;
> +		break;
> +	case 8:
> +		val |= PMECC_CFG_PAGE_8SECTORS;
> +		break;
> +	}
> +
> +	val |= (PMECC_CFG_READ_OP | PMECC_CFG_SPARE_DISABLE
> +		| PMECC_CFG_AUTO_DISABLE);
> +	pmecc_writel(host->ecc, CFG, val);
> +
> +	ecc_layout = nand_chip->ecc.layout;
> +	pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
> +	pmecc_writel(host->ecc, SADDR, ecc_layout->eccpos[0]);
> +	pmecc_writel(host->ecc, EADDR,
> +			ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
> +	/* See datasheet about PMECC Clock Control Register */
> +	pmecc_writel(host->ecc, CLK, 2);
> +	pmecc_writel(host->ecc, IDR, 0xff);
> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
> +}
> +
> +static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
> +					 struct atmel_nand_host *host)
> +{
> +	int			cap, sector_size, err_no;
> +	struct mtd_info		*mtd;
> +	struct nand_chip	*nand_chip;
> +	struct resource		*regs;
> +	struct resource		*regs_pmerr, *regs_rom;
> +
> +	cap = host->pmecc_corr_cap;
> +	sector_size = host->pmecc_sector_size;
> +	dev_info(host->dev, "Initialize PMECC params, cap: %d, sector: %d\n",
> +		 cap, sector_size);
> +
> +	/* Sanity check */
> +	if ((sector_size != 512) && (sector_size != 1024)) {
> +		dev_err(host->dev,
> +			"Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
> +			sector_size);
> +		return -EINVAL;
> +	}
> +	if ((cap != 2) && (cap != 4) && (cap != 8) && (cap != 12) &&
> +	    (cap != 24)) {
> +		dev_err(host->dev,
> +			"Unsupported PMECC correction capability, should be 2, 4, 8, 12 or 24\n");
> +		return -EINVAL;
> +	}
> +
> +	nand_chip = &host->nand_chip;
> +	mtd = &host->mtd;
> +
> +	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* By default */
> +
> +	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!regs) {
> +		dev_warn(host->dev,
> +			"Can't get I/O resource regs, rolling back on software ECC\n");
> +		return 0;
> +	}
> +
> +	host->ecc = ioremap(regs->start, resource_size(regs));
> +	if (host->ecc == NULL) {
> +		dev_err(host->dev, "ioremap failed\n");
> +		err_no = -EIO;
> +		goto err_pmecc_ioremap;
> +	}
> +
> +	regs_pmerr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> +	if (regs_pmerr && regs_rom) {
> +		host->pmerrloc_base = ioremap(regs_pmerr->start,
> +			resource_size(regs_pmerr));
> +		host->pmecc_rom_base = ioremap(regs_rom->start,
> +			resource_size(regs_rom));
> +
> +		if (host->pmerrloc_base && host->pmecc_rom_base) {
> +			nand_chip->ecc.mode = NAND_ECC_HW;
> +			nand_chip->ecc.read_page =
> +				atmel_nand_pmecc_read_page;
> +			nand_chip->ecc.write_page =
> +				atmel_nand_pmecc_write_page;
> +		} else {
> +			dev_err(host->dev,
> +				"Can not get I/O resource for PMECC controller!\n");
> +			err_no = -EIO;
> +			goto err_pmloc_ioremap;
> +		}
> +	}
> +
> +	/* ECC is calculated for the whole page (1 step) */
> +	nand_chip->ecc.size = mtd->writesize;
> +
> +	/* set ECC page size and oob layout */
> +	switch (mtd->writesize) {
> +	case 2048:
> +		host->pmecc_degree = PMECC_GF_DIMENSION_13;
> +		host->pmecc_cw_len = (1 << host->pmecc_degree) - 1;
> +		host->pmecc_corr_cap = cap;
> +		host->pmecc_sector_number = mtd->writesize / sector_size;
> +		host->pmecc_bytes_per_sector = pmecc_get_ecc_bytes(
> +			cap, sector_size);
> +		host->pmecc_alpha_to = pmecc_get_alpha_to(host);
> +		host->pmecc_index_of = pmecc_get_index_of(host);
> +
> +		nand_chip->ecc.steps = 1;
> +		nand_chip->ecc.strength = cap;
> +		nand_chip->ecc.bytes = host->pmecc_bytes_per_sector *
> +				       host->pmecc_sector_number;
> +		if (nand_chip->ecc.bytes > mtd->oobsize - 2) {
> +			dev_err(host->dev, "No room for ECC bytes\n");
> +			err_no = -EINVAL;
> +			goto err;
> +		}
> +		pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
> +					mtd->oobsize,
> +					nand_chip->ecc.bytes);
> +		nand_chip->ecc.layout = &atmel_pmecc_oobinfo;
> +		break;
> +	case 512:
> +	case 1024:
> +	case 4096:
> +		/* TODO */
> +		dev_warn(host->dev,
> +			"Unsupported page size for PMECC, use Software ECC\n");
> +	default:
> +		/* page size not handled by HW ECC */
> +		/* switching back to soft ECC */
> +		nand_chip->ecc.mode = NAND_ECC_SOFT;
> +		nand_chip->ecc.calculate = NULL;
> +		nand_chip->ecc.correct = NULL;
> +		nand_chip->ecc.hwctl = NULL;
> +		nand_chip->ecc.read_page = NULL;
> +		nand_chip->ecc.write_page = NULL;
> +		nand_chip->ecc.postpad = 0;
> +		nand_chip->ecc.prepad = 0;
> +		nand_chip->ecc.bytes = 0;
> +		err_no = 0;
> +		goto err;
> +	}
> +
> +	/* Allocate data for PMECC computation */
> +	host->pmecc_data = kzalloc(sizeof(struct atmel_pmecc_data), GFP_KERNEL);
why do you always allocate the pmecc_data?

you need to allocate it only if you use it

Best Regards,
J.
> +	if (!host->pmecc_data) {
> +		dev_err(host->dev,
> +				"Cannot allocate memory for PMECC computation!\n");
> +		err_no = -ENOMEM;
> +		goto err;
> +	}
> +
> +	atmel_pmecc_core_init(mtd);
> +
> +	return 0;
> +
> +err:
> +err_pmloc_ioremap:
> +	iounmap(host->ecc);
> +	if (host->pmerrloc_base)
> +		iounmap(host->pmerrloc_base);
> +	if (host->pmecc_rom_base)
> +		iounmap(host->pmecc_rom_base);
> +err_pmecc_ioremap:
> +	return err_no;
> +}
> +
> +/*
>   * Calculate HW ECC
>   *
>   * function called after a write
> @@ -720,7 +1456,11 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
>  	}
>  
>  	if (nand_chip->ecc.mode == NAND_ECC_HW) {
> -		res = atmel_hw_nand_init_params(pdev, host);
> +		if (host->has_pmecc)
> +			res = atmel_pmecc_nand_init_params(pdev, host);
> +		else
> +			res = atmel_hw_nand_init_params(pdev, host);
> +
>  		if (res != 0)
>  			goto err_hw_ecc;
>  	}
> @@ -741,6 +1481,12 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
>  err_scan_tail:
>  	if (host->ecc)
>  		iounmap(host->ecc);
> +	if (host->has_pmecc) {
no need to check if you have teh pmecc

if it's no the case pmerrloc_base will be NULLo

don't you need to disable it in the error path?

Best Regards,
J.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
  2012-05-28  6:58     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-05-28  8:34       ` Josh Wu
  -1 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-28  8:34 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: hongxu.cn, dedekind1, nicolas.ferre, linux-mtd, ivan.djelic,
	linux-arm-kernel

On 5/28/2012 2:58 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 21:24 Sat 26 May     , Josh Wu wrote:
>> The Programmable Multibit ECC (PMECC) controller is a programmable binary
>> BCH(Bose, Chaudhuri and Hocquenghem) encoder and decoder. This controller
>> can be used to support both SLC and MLC NAND Flash devices. It supports to
>> generate ECC to correct 2, 4, 8, 12 or 24 bits of error per sector of data.
>>
>> To use this driver, the user needs to pass in the correction capability and
>> the sector size.
>>
>> This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2,
>> YAFFS2, UBIFS and mtd-utils.
>>
>> Signed-off-by: Hong Xu<hong.xu@atmel.com>
>> Signed-off-by: Josh Wu<josh.wu@atmel.com>
>> ---
>>   drivers/mtd/nand/atmel_nand.c     |  761 ++++++++++++++++++++++++++++++++++++-
>>   drivers/mtd/nand/atmel_nand_ecc.h |  116 ++++++
>>   2 files changed, 876 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index 9a9bfbf..ddcf1ed 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -15,6 +15,8 @@
>>    *     		(u-boot-1.1.5/board/atmel/at91sam9263ek/nand.c)
>>    *     (C) Copyright 2006 ATMEL Rousset, Lacressonniere Nicolas
>>    *
>> + *  Add Programmable Multibit ECC support for various AT91 SoC
>> + *     (C) Copyright 2012 ATMEL, Hong Xu
>>    *
>>    * This program is free software; you can redistribute it and/or modify
>>    * it under the terms of the GNU General Public License version 2 as
>> @@ -77,6 +79,21 @@ static struct nand_ecclayout atmel_oobinfo_small = {
>>   	},
>>   };
>>
>> +/* a structure includes datas for PMECC computation */
>> +struct atmel_pmecc_data {
>> +	int16_t			partial_syn[2 * PMECC_MAX_ERROR_NB + 1];
>> +	int16_t			si[2 * PMECC_MAX_ERROR_NB + 1];
>> +
>> +	/* Sigma table */
>> +	int16_t		smu[PMECC_MAX_ERROR_NB + 2][2 * PMECC_MAX_ERROR_NB + 1];
> you still hardcode the array in the struct
>
> and if the pmecc evolve we will have to touch again
> please allocate them

ok.

>> +	/* polynomal order */
>> +	int16_t			lmu[PMECC_MAX_ERROR_NB + 1];
>> +
>> +	int			mu[PMECC_MAX_ERROR_NB + 1];
>> +	int			dmu[PMECC_MAX_ERROR_NB + 1];
>> +	int			delta[PMECC_MAX_ERROR_NB + 1];
>> +};
>> +
>>   struct atmel_nand_host {
>>   	struct nand_chip	nand_chip;
>>   	struct mtd_info		mtd;
>> @@ -92,8 +109,25 @@ struct atmel_nand_host {
>>   	bool			has_pmecc;
>>   	u8			pmecc_corr_cap;
>>   	u16			pmecc_sector_size;
>> +
>> +	int			pmecc_bytes_per_sector;
>> +	int			pmecc_sector_number;
>> +	int			pmecc_degree;	/* Degree of remainders */
>> +	int			pmecc_cw_len;	/* Length of codeword */
>> +
>> +	void __iomem		*pmerrloc_base;
>> +	void __iomem		*pmecc_rom_base;
>> +
>> +	/* lookup table for alpha_to and index_of */
>> +	void __iomem		*pmecc_alpha_to;
>> +	void __iomem		*pmecc_index_of;
>> +
>> +	/* data for pmecc computation */
>> +	struct atmel_pmecc_data	*pmecc_data;
>>   };
>>
>> +static struct nand_ecclayout atmel_pmecc_oobinfo;
>> +
>>   static int cpu_has_dma(void)
>>   {
>>   	return cpu_is_at91sam9rl() || cpu_is_at91sam9g45();
>> @@ -287,6 +321,708 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
>>   }
>>
>>   /*
>> + * Return number of ecc bytes per sector according to sector size and
>> + * correction capability
>> + *
>> + * Following table shows what at91 PMECC supported:
>> + * Correction Capability	Sector_512_bytes	Sector_1024_bytes
>> + * =====================	================	=================
>> + *                2-bits                 4-bytes                  4-bytes
>> + *                4-bits                 7-bytes                  7-bytes
>> + *                8-bits                13-bytes                 14-bytes
>> + *               12-bits                20-bytes                 21-bytes
>> + *               24-bits                39-bytes                 42-bytes
>> + */
>> +static int pmecc_get_ecc_bytes(int cap, int sector_size)
>> +{
>> +	int m = 12 + sector_size / 512;
>> +	return (m * cap + 7) / 8;
>> +}
>> +
>> +static void pmecc_config_ecc_layout(struct nand_ecclayout *layout, int oobsize,
>> +	int ecc_len)
>> +{
>> +	int i;
>> +
>> +	layout->eccbytes = ecc_len;
>> +
>> +	/* ECC will occupy the last ecc_len bytes continuously */
>> +	for (i = 0; i<  ecc_len; i++)
>> +		layout->eccpos[i] = oobsize - ecc_len + i;
>> +
>> +	layout->oobfree[0].offset = 2;
>> +	layout->oobfree[0].length =
>> +		oobsize - ecc_len - layout->oobfree[0].offset;
>> +}
>> +
>> +static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
>> +{
>> +	void __iomem *p;
>> +
>> +	switch (host->pmecc_sector_size) {
>> +	case 512:
>> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512 +
>> +			PMECC_LOOKUP_TABLE_SIZE_512 * sizeof(int16_t);
>> +		break;
>> +	case 1024:
>> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024 +
>> +			PMECC_LOOKUP_TABLE_SIZE_1024 * sizeof(int16_t);
>> +		break;
>> +	default:
>> +		BUG();
>> +	}
>> +
>> +	return p;
>> +}
>> +
>> +static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
> this is a __dev_init function plese check the other too
>
> btw you need to use __dev_init and not __init

Here I use pmecc_get_index_of() function to get a lookup table base 
which is in ROM.
I'm not clear about how should I need __dev_init here. Can you give more 
information?

>> +{
>> +	void __iomem *p;
>> +
>> +	switch (host->pmecc_sector_size) {
>> +	case 512:
>> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512;
>> +		break;
>> +	case 1024:
>> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024;
>> +		break;
>> +	default:
>> +		BUG();
>> +	}
>> +
>> +	return p;
>> +}
>> +
>> +static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
>> +{
>> +	int			i;
>> +	uint32_t		value;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +
>> +	/* Fill odd syndromes */
>> +	for (i = 0; i<  host->pmecc_corr_cap; i++) {
>> +		value = pmecc_readl_rem_relaxed(host->ecc, sector, i / 2);
>> +		value = (i&  1) ? (value&  0xffff0000)>>  16 : value&  0xffff;
> simplify by
> 		if (i&  1)
> 			val>>= 16;
> 		value&= 0xffff;

I'll fix it.

>
>> +		host->pmecc_data->partial_syn[(2 * i) + 1] = (int16_t)value;
>> +	}
>> +}
>> +
>> +static void pmecc_substitute(struct mtd_info *mtd)
>> +{
>> +	int16_t			*si;
>> +	int			i, j;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +	int16_t __iomem		*alpha_to = host->pmecc_alpha_to;
>> +	int16_t __iomem		*index_of = host->pmecc_index_of;
>> +	int16_t			*partial_syn = host->pmecc_data->partial_syn;
>> +
>> +	/* si[] is a table that holds the current syndrome value,
>> +	 * an element of that table belongs to the field
>> +	 */
>> +	si = host->pmecc_data->si;
>> +
>> +	for (i = 1; i<  2 * PMECC_MAX_ERROR_NB; i++)
>> +		si[i] = 0;
> please use memset

ok

>> +
>> +	/* Computation 2t syndromes based on S(x) */
>> +	/* Odd syndromes */
>> +	for (i = 1; i<  2 * host->pmecc_corr_cap; i += 2) {
>> +		si[i] = 0;
> shy this you already init the array at 0 before

yes, I'll fix it.

>> +		for (j = 0; j<  host->pmecc_degree; j++) {
>> +			if (partial_syn[i]&  ((unsigned short)0x1<<  j))
>> +				si[i] = readw_relaxed(alpha_to + i * j) ^ si[i];
>> +		}
>> +	}
>> +	/* Even syndrome = (Odd syndrome) ** 2 */
>> +	for (i = 2; i<= 2 * host->pmecc_corr_cap; i += 2) {
>> +		j = i / 2;
>> +		if (si[j] == 0)
> here if {
> } else {
> }

ok

>> +			si[i] = 0;
>> +		else {
>> +			int16_t tmp;
> missing blank line

ok.

>> +			tmp = readw_relaxed(index_of + si[j]);
>> +			tmp = (tmp * 2) % host->pmecc_cw_len;
>> +			si[i] = readw_relaxed(alpha_to + tmp);
>> +		}
>> +	}
>> +
>> +	return;
>> +}
>> +
>> +static void pmecc_get_sigma(struct mtd_info *mtd)
>> +{
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +
>> +	int		i, j, k;
>> +	uint32_t	dmu_0_count, tmp;
>> +	int16_t		(*smu)[2 * PMECC_MAX_ERROR_NB + 1];
>> +	int16_t		*lmu = host->pmecc_data->lmu;
>> +	int16_t		*si = host->pmecc_data->si;
>> +	int		*mu = host->pmecc_data->mu;
>> +	int		*dmu = host->pmecc_data->dmu;	  /* Discrepancy */
>> +	int		*delta = host->pmecc_data->delta; /* Delta order */
>> +	int		cw_len = host->pmecc_cw_len;
>> +	int16_t		cap = host->pmecc_corr_cap;
>> +
>> +	int16_t __iomem	*index_of = host->pmecc_index_of;
>> +	int16_t __iomem	*alpha_to = host->pmecc_alpha_to;
>> +
>> +	/* index of largest delta */
>> +	int ro;
>> +	int largest;
>> +	int diff;
>> +
>> +	dmu_0_count = 0;
>> +	smu = host->pmecc_data->smu;
>> +
>> +	/* First Row */
>> +
>> +	/* Mu */
>> +	mu[0] = -1;
>> +
>> +	memset(&smu[0][0], 0,
>> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
>> +	smu[0][0] = 1;
>> +
>> +	/* discrepancy set to 1 */
>> +	dmu[0] = 1;
>> +	/* polynom order set to 0 */
>> +	lmu[0] = 0;
>> +	delta[0]  = (mu[0] * 2 - lmu[0])>>  1;
>> +
>> +	/* Second Row */
>> +
>> +	/* Mu */
>> +	mu[1]  = 0;
>> +	/* Sigma(x) set to 1 */
>> +	memset(&smu[1][0], 0,
>> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
>> +	smu[1][0] = 1;
>> +
>> +	/* discrepancy set to S1 */
>> +	dmu[1] = si[1];
>> +
>> +	/* polynom order set to 0 */
>> +	lmu[1] = 0;
>> +
>> +	delta[1]  = (mu[1] * 2 - lmu[1])>>  1;
>> +
>> +	/* Init the Sigma(x) last row */
>> +	memset(&smu[cap + 1][0], 0,
>> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
>> +
>> +	for (i = 1; i<= cap; i++) {
>> +		mu[i+1] = i<<  1;
>> +		/* Begin Computing Sigma (Mu+1) and L(mu) */
>> +		/* check if discrepancy is set to 0 */
>> +		if (dmu[i] == 0) {
>> +			dmu_0_count++;
>> +
>> +			tmp = ((cap - (lmu[i]>>  1) - 1) / 2);
>> +			if ((cap - (lmu[i]>>  1) - 1)&  0x1)
>> +				tmp += 2;
>> +			else
>> +				tmp += 1;
>> +
>> +			if (dmu_0_count == tmp) {
>> +				for (j = 0; j<= (lmu[i]>>  1) + 1; j++)
>> +					smu[cap + 1][j] = smu[i][j];
>> +				lmu[cap + 1] = lmu[i];
>> +				return;
>> +			}
>> +
>> +			/* copy polynom */
>> +			for (j = 0; j<= lmu[i]>>  1; j++)
>> +				smu[i + 1][j] = smu[i][j];
>> +
>> +			/* copy previous polynom order to the next */
>> +			lmu[i + 1] = lmu[i];
>> +		} else {
>> +			ro = 0;
>> +			largest = -1;
>> +			/* find largest delta with dmu != 0 */
>> +			for (j = 0; j<  i; j++) {
>> +				if ((dmu[j])&&  (delta[j]>  largest)) {
>> +					largest = delta[j];
>> +					ro = j;
>> +				}
>> +			}
>> +
>> +			/* compute difference */
>> +			diff = (mu[i] - mu[ro]);
>> +
>> +			/* Compute degree of the new smu polynomial */
>> +			if ((lmu[i]>>  1)>  ((lmu[ro]>>  1) + diff))
>> +				lmu[i + 1] = lmu[i];
>> +			else
>> +				lmu[i + 1] = ((lmu[ro]>>  1) + diff) * 2;
>> +
>> +			/* Init smu[i+1] with 0 */
>> +			for (k = 0; k<  (2 * PMECC_MAX_ERROR_NB + 1); k++)
>> +				smu[i+1][k] = 0;
>> +
>> +			/* Compute smu[i+1] */
>> +			for (k = 0; k<= lmu[ro]>>  1; k++) {
>> +				int16_t a, b, c;
>> +
>> +				if (!(smu[ro][k]&&  dmu[i]))
>> +					continue;
>> +				a = readw_relaxed(index_of + dmu[i]);
>> +				b = readw_relaxed(index_of + dmu[ro]);
>> +				c = readw_relaxed(index_of + smu[ro][k]);
>> +				tmp = a + (cw_len - b) + c;
>> +				a = readw_relaxed(alpha_to + tmp % cw_len);
>> +				smu[i + 1][k + diff] = a;
>> +			}
>> +
>> +			for (k = 0; k<= lmu[i]>>  1; k++)
>> +				smu[i + 1][k] ^= smu[i][k];
>> +		}
>> +
>> +		/* End Computing Sigma (Mu+1) and L(mu) */
>> +		/* In either case compute delta */
>> +		delta[i + 1]  = (mu[i + 1] * 2 - lmu[i + 1])>>  1;
>> +
>> +		/* Do not compute discrepancy for the last iteration */
>> +		if (i>= cap)
>> +			continue;
>> +
>> +		for (k = 0 ; k<= (lmu[i + 1]>>  1); k++) {
>> +			tmp = 2 * (i - 1);
>> +			if (k == 0)
>> +				dmu[i + 1] = si[tmp + 3];
>> +			else if (smu[i+1][k]&&  si[tmp + 3 - k]) {
>> +				int16_t a, b, c;
>> +				a = readw_relaxed(index_of + smu[i + 1][k]);
>> +				b = si[2 * (i - 1) + 3 - k];
>> +				c = readw_relaxed(index_of + b);
>> +				tmp = a + c;
>> +				tmp %= cw_len;
>> +				dmu[i + 1] = readw_relaxed(alpha_to + tmp) ^
>> +					dmu[i + 1];
>> +			}
>> +		}
>> +	}
>> +
>> +	return;
>> +}
>> +
>> +static int pmecc_err_location(struct mtd_info *mtd)
>> +{
>> +	int			i;
>> +	int			err_nbr;	/* number of error */
>> +	int			roots_nbr;	/* number of roots */
>> +	int			sector_size;
>> +	uint32_t		val;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +	int			timeout_count = 0;
>> +	int			cap = host->pmecc_corr_cap;
>> +
>> +	err_nbr = 0;
>> +	sector_size = host->pmecc_sector_size;
>> +
>> +	pmerrloc_writel(host->pmerrloc_base, ELDIS, PMERRLOC_DISABLE);
>> +
>> +	for (i = 0; i<= host->pmecc_data->lmu[cap + 1]>>  1; i++) {
>> +		pmerrloc_writel_sigma_relaxed(host->pmerrloc_base, i,
>> +				      host->pmecc_data->smu[cap + 1][i]);
>> +		err_nbr++;
>> +	}
>> +
>> +	val = (err_nbr - 1)<<  16;
>> +	if (sector_size == 1024)
>> +		val |= 1;
>> +
>> +	pmerrloc_writel(host->pmerrloc_base, ELCFG, val);
>> +	pmerrloc_writel(host->pmerrloc_base, ELEN,
>> +			sector_size * 8 + host->pmecc_degree * cap);
>> +
>> +	while (!(pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
>> +		&  PMERRLOC_CALC_DONE)) {
>> +		if (unlikely(timeout_count++>  PMECC_MAX_TIMEOUT_COUNT))
>> +			return -1;	/* Time out */
>> +		cpu_relax();
>> +	}
>> +
>> +	roots_nbr = (pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
>> +		&  PMERRLOC_ERR_NUM_MASK)>>  8;
>> +	/* Number of roots == degree of smu hence<= cap */
>> +	if (roots_nbr == host->pmecc_data->lmu[cap + 1]>>  1)
>> +		return err_nbr - 1;
>> +
>> +	/* Number of roots does not match the degree of smu
>> +	 * unable to correct error */
>> +	return -1;
>> +}
>> +
>> +static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf,
>> +		int extra_bytes, int err_nbr)
>> +{
>> +	int			i = 0;
>> +	int			byte_pos, bit_pos;
>> +	int			sector_size, ecc_size;
>> +	uint32_t		tmp;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +
>> +	sector_size = host->pmecc_sector_size;
>> +	ecc_size = nand_chip->ecc.bytes;
>> +
>> +	while (err_nbr) {
>> +		tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
>> +		byte_pos = tmp / 8;
>> +		bit_pos  = tmp % 8;
>> +		dev_info(host->dev, "PMECC correction, byte_pos: %d bit_pos: %d\n",
>> +					byte_pos, bit_pos);
>> +
>> +		if (byte_pos<  (sector_size + extra_bytes)) {
>> +			tmp = sector_size +
>> +				pmecc_readl_relaxed(host->ecc, SADDR);
>> +
>> +			if (byte_pos<  tmp)
>> +				*(buf + byte_pos) ^= (1<<  bit_pos);
>> +			else
>> +				*(buf + byte_pos + ecc_size) ^= (1<<  bit_pos);
>> +		}
>> +
>> +		i++;
>> +		err_nbr--;
>> +	}
>> +
>> +	return;
>> +}
>> +
>> +static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
>> +	u8 *ecc)
>> +{
>> +	int			i, err_nbr;
>> +	uint8_t			*buf_pos;
>> +	int			eccbytes;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +
>> +	eccbytes = nand_chip->ecc.bytes;
>> +	for (i = 0; i<  eccbytes; i++)
>> +		if (ecc[i] != 0xff)
>> +			goto normal_check;
>> +	/* Erased page, return OK */
>> +	return 0;
>> +
>> +normal_check:
>> +	for (i = 0; i<  host->pmecc_sector_number; i++) {
>> +		err_nbr = 0;
>> +		if (pmecc_stat&  0x1) {
>> +			buf_pos = buf + i * host->pmecc_sector_size;
>> +
>> +			pmecc_gen_syndrome(mtd, i);
>> +			pmecc_substitute(mtd);
>> +			pmecc_get_sigma(mtd);
>> +
>> +			err_nbr = pmecc_err_location(mtd);
>> +			if (err_nbr == -1) {
>> +				dev_err(host->dev, "PMECC: Too many errors\n");
>> +				mtd->ecc_stats.failed++;
>> +				return -EIO;
>> +			} else {
>> +				pmecc_correct_data(mtd, buf_pos, 0, err_nbr);
>> +				mtd->ecc_stats.corrected += err_nbr;
>> +			}
>> +		}
>> +		pmecc_stat>>= 1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
>> +		struct nand_chip *chip, uint8_t *buf, int page)
>> +{
>> +	uint32_t		stat;
>> +	int			timeout_count = 0;
>> +	int			eccsize = chip->ecc.size;
>> +	uint8_t			*oob = chip->oob_poi;
>> +	struct atmel_nand_host	*host = chip->priv;
>> +	uint32_t		*eccpos = chip->ecc.layout->eccpos;
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
>> +	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG)
>> +		&  ~PMECC_CFG_WRITE_OP) | PMECC_CFG_AUTO_ENABLE);
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
>> +
>> +	chip->read_buf(mtd, buf, eccsize);
>> +	chip->read_buf(mtd, oob, mtd->oobsize);
>> +
>> +	while ((pmecc_readl_relaxed(host->ecc, SR)&  PMECC_SR_BUSY)) {
>> +		if (unlikely(timeout_count++>  PMECC_MAX_TIMEOUT_COUNT))
>> +			return -EIO;	/* Time out */
>> +		cpu_relax();
>> +	}
>> +
>> +	stat = pmecc_readl_relaxed(host->ecc, ISR);
>> +	if (stat != 0) {
>> +		if (pmecc_correction(mtd, stat, buf,&oob[eccpos[0]]) != 0)
>> +			return -EIO;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void atmel_nand_pmecc_write_page(struct mtd_info *mtd,
>> +		struct nand_chip *chip, const uint8_t *buf)
>> +{
>> +	int			i, j;
>> +	int			timeout_count = 0;
>> +	struct atmel_nand_host	*host = chip->priv;
>> +	uint32_t		*eccpos = chip->ecc.layout->eccpos;
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
>> +
>> +	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG) |
>> +		PMECC_CFG_WRITE_OP)&  ~PMECC_CFG_AUTO_ENABLE);
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
>> +
>> +	chip->write_buf(mtd, (u8 *)buf, mtd->writesize);
>> +
>> +	while ((pmecc_readl_relaxed(host->ecc, SR)&  PMECC_SR_BUSY)) {
>> +		if (unlikely(timeout_count++>  PMECC_MAX_TIMEOUT_COUNT)) {
>> +			dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
>> +			return;	/* Time out */
>> +		}
>> +		cpu_relax();
>> +	}
>> +
>> +	for (i = 0; i<  host->pmecc_sector_number; i++) {
>> +		for (j = 0; j<  host->pmecc_bytes_per_sector; j++) {
>> +			int pos;
>> +
>> +			pos = i * host->pmecc_bytes_per_sector + j;
>> +			chip->oob_poi[eccpos[pos]] =
>> +				pmecc_readb_ecc_relaxed(host->ecc, i, j);
>> +		}
>> +	}
>> +	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
>> +
>> +	return;
>> +}
>> +
>> +static void atmel_pmecc_core_init(struct mtd_info *mtd)
>> +{
>> +	uint32_t		val = 0;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +	struct nand_ecclayout	*ecc_layout;
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
>> +
>> +	switch (host->pmecc_corr_cap) {
>> +	case 2:
>> +		val = PMECC_CFG_BCH_ERR2;
>> +		break;
>> +	case 4:
>> +		val = PMECC_CFG_BCH_ERR4;
>> +		break;
>> +	case 8:
>> +		val = PMECC_CFG_BCH_ERR8;
>> +		break;
>> +	case 12:
>> +		val = PMECC_CFG_BCH_ERR12;
>> +		break;
>> +	case 24:
>> +		val = PMECC_CFG_BCH_ERR24;
>> +		break;
>> +	}
>> +
>> +	if (host->pmecc_sector_size == 512)
>> +		val |= PMECC_CFG_SECTOR512;
>> +	else if (host->pmecc_sector_size == 1024)
>> +		val |= PMECC_CFG_SECTOR1024;
>> +
>> +	switch (host->pmecc_sector_number) {
>> +	case 1:
>> +		val |= PMECC_CFG_PAGE_1SECTOR;
>> +		break;
>> +	case 2:
>> +		val |= PMECC_CFG_PAGE_2SECTORS;
>> +		break;
>> +	case 4:
>> +		val |= PMECC_CFG_PAGE_4SECTORS;
>> +		break;
>> +	case 8:
>> +		val |= PMECC_CFG_PAGE_8SECTORS;
>> +		break;
>> +	}
>> +
>> +	val |= (PMECC_CFG_READ_OP | PMECC_CFG_SPARE_DISABLE
>> +		| PMECC_CFG_AUTO_DISABLE);
>> +	pmecc_writel(host->ecc, CFG, val);
>> +
>> +	ecc_layout = nand_chip->ecc.layout;
>> +	pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
>> +	pmecc_writel(host->ecc, SADDR, ecc_layout->eccpos[0]);
>> +	pmecc_writel(host->ecc, EADDR,
>> +			ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
>> +	/* See datasheet about PMECC Clock Control Register */
>> +	pmecc_writel(host->ecc, CLK, 2);
>> +	pmecc_writel(host->ecc, IDR, 0xff);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
>> +}
>> +
>> +static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
>> +					 struct atmel_nand_host *host)
>> +{
>> +	int			cap, sector_size, err_no;
>> +	struct mtd_info		*mtd;
>> +	struct nand_chip	*nand_chip;
>> +	struct resource		*regs;
>> +	struct resource		*regs_pmerr, *regs_rom;
>> +
>> +	cap = host->pmecc_corr_cap;
>> +	sector_size = host->pmecc_sector_size;
>> +	dev_info(host->dev, "Initialize PMECC params, cap: %d, sector: %d\n",
>> +		 cap, sector_size);
>> +
>> +	/* Sanity check */
>> +	if ((sector_size != 512)&&  (sector_size != 1024)) {
>> +		dev_err(host->dev,
>> +			"Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
>> +			sector_size);
>> +		return -EINVAL;
>> +	}
>> +	if ((cap != 2)&&  (cap != 4)&&  (cap != 8)&&  (cap != 12)&&
>> +	    (cap != 24)) {
>> +		dev_err(host->dev,
>> +			"Unsupported PMECC correction capability, should be 2, 4, 8, 12 or 24\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	nand_chip =&host->nand_chip;
>> +	mtd =&host->mtd;
>> +
>> +	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* By default */
>> +
>> +	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	if (!regs) {
>> +		dev_warn(host->dev,
>> +			"Can't get I/O resource regs, rolling back on software ECC\n");
>> +		return 0;
>> +	}
>> +
>> +	host->ecc = ioremap(regs->start, resource_size(regs));
>> +	if (host->ecc == NULL) {
>> +		dev_err(host->dev, "ioremap failed\n");
>> +		err_no = -EIO;
>> +		goto err_pmecc_ioremap;
>> +	}
>> +
>> +	regs_pmerr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>> +	regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
>> +	if (regs_pmerr&&  regs_rom) {
>> +		host->pmerrloc_base = ioremap(regs_pmerr->start,
>> +			resource_size(regs_pmerr));
>> +		host->pmecc_rom_base = ioremap(regs_rom->start,
>> +			resource_size(regs_rom));
>> +
>> +		if (host->pmerrloc_base&&  host->pmecc_rom_base) {
>> +			nand_chip->ecc.mode = NAND_ECC_HW;
>> +			nand_chip->ecc.read_page =
>> +				atmel_nand_pmecc_read_page;
>> +			nand_chip->ecc.write_page =
>> +				atmel_nand_pmecc_write_page;
>> +		} else {
>> +			dev_err(host->dev,
>> +				"Can not get I/O resource for PMECC controller!\n");
>> +			err_no = -EIO;
>> +			goto err_pmloc_ioremap;
>> +		}
>> +	}
>> +
>> +	/* ECC is calculated for the whole page (1 step) */
>> +	nand_chip->ecc.size = mtd->writesize;
>> +
>> +	/* set ECC page size and oob layout */
>> +	switch (mtd->writesize) {
>> +	case 2048:
>> +		host->pmecc_degree = PMECC_GF_DIMENSION_13;
>> +		host->pmecc_cw_len = (1<<  host->pmecc_degree) - 1;
>> +		host->pmecc_corr_cap = cap;
>> +		host->pmecc_sector_number = mtd->writesize / sector_size;
>> +		host->pmecc_bytes_per_sector = pmecc_get_ecc_bytes(
>> +			cap, sector_size);
>> +		host->pmecc_alpha_to = pmecc_get_alpha_to(host);
>> +		host->pmecc_index_of = pmecc_get_index_of(host);
>> +
>> +		nand_chip->ecc.steps = 1;
>> +		nand_chip->ecc.strength = cap;
>> +		nand_chip->ecc.bytes = host->pmecc_bytes_per_sector *
>> +				       host->pmecc_sector_number;
>> +		if (nand_chip->ecc.bytes>  mtd->oobsize - 2) {
>> +			dev_err(host->dev, "No room for ECC bytes\n");
>> +			err_no = -EINVAL;
>> +			goto err;
>> +		}
>> +		pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
>> +					mtd->oobsize,
>> +					nand_chip->ecc.bytes);
>> +		nand_chip->ecc.layout =&atmel_pmecc_oobinfo;
>> +		break;
>> +	case 512:
>> +	case 1024:
>> +	case 4096:
>> +		/* TODO */
>> +		dev_warn(host->dev,
>> +			"Unsupported page size for PMECC, use Software ECC\n");
>> +	default:
>> +		/* page size not handled by HW ECC */
>> +		/* switching back to soft ECC */
>> +		nand_chip->ecc.mode = NAND_ECC_SOFT;
>> +		nand_chip->ecc.calculate = NULL;
>> +		nand_chip->ecc.correct = NULL;
>> +		nand_chip->ecc.hwctl = NULL;
>> +		nand_chip->ecc.read_page = NULL;
>> +		nand_chip->ecc.write_page = NULL;
>> +		nand_chip->ecc.postpad = 0;
>> +		nand_chip->ecc.prepad = 0;
>> +		nand_chip->ecc.bytes = 0;
>> +		err_no = 0;
>> +		goto err;
>> +	}
>> +
>> +	/* Allocate data for PMECC computation */
>> +	host->pmecc_data = kzalloc(sizeof(struct atmel_pmecc_data), GFP_KERNEL);
> why do you always allocate the pmecc_data?
>
> you need to allocate it only if you use it

If not using PMECC, the code will never come to here. So I think it's 
right time to allocate all the computation data here.

>
> Best Regards,
> J.
>> +	if (!host->pmecc_data) {
>> +		dev_err(host->dev,
>> +				"Cannot allocate memory for PMECC computation!\n");
>> +		err_no = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	atmel_pmecc_core_init(mtd);
>> +
>> +	return 0;
>> +
>> +err:
>> +err_pmloc_ioremap:
>> +	iounmap(host->ecc);
>> +	if (host->pmerrloc_base)
>> +		iounmap(host->pmerrloc_base);
>> +	if (host->pmecc_rom_base)
>> +		iounmap(host->pmecc_rom_base);
>> +err_pmecc_ioremap:
>> +	return err_no;
>> +}
>> +
>> +/*
>>    * Calculate HW ECC
>>    *
>>    * function called after a write
>> @@ -720,7 +1456,11 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
>>   	}
>>
>>   	if (nand_chip->ecc.mode == NAND_ECC_HW) {
>> -		res = atmel_hw_nand_init_params(pdev, host);
>> +		if (host->has_pmecc)
>> +			res = atmel_pmecc_nand_init_params(pdev, host);
>> +		else
>> +			res = atmel_hw_nand_init_params(pdev, host);
>> +
>>   		if (res != 0)
>>   			goto err_hw_ecc;
>>   	}
>> @@ -741,6 +1481,12 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
>>   err_scan_tail:
>>   	if (host->ecc)
>>   		iounmap(host->ecc);
>> +	if (host->has_pmecc) {
> no need to check if you have teh pmecc
>
> if it's no the case pmerrloc_base will be NULLo
>
> don't you need to disable it in the error path?

right, I should disable the hardware in this error path.

>
> Best Regards,
> J.

Best Regards,
Josh Wu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
@ 2012-05-28  8:34       ` Josh Wu
  0 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-28  8:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 5/28/2012 2:58 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 21:24 Sat 26 May     , Josh Wu wrote:
>> The Programmable Multibit ECC (PMECC) controller is a programmable binary
>> BCH(Bose, Chaudhuri and Hocquenghem) encoder and decoder. This controller
>> can be used to support both SLC and MLC NAND Flash devices. It supports to
>> generate ECC to correct 2, 4, 8, 12 or 24 bits of error per sector of data.
>>
>> To use this driver, the user needs to pass in the correction capability and
>> the sector size.
>>
>> This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2,
>> YAFFS2, UBIFS and mtd-utils.
>>
>> Signed-off-by: Hong Xu<hong.xu@atmel.com>
>> Signed-off-by: Josh Wu<josh.wu@atmel.com>
>> ---
>>   drivers/mtd/nand/atmel_nand.c     |  761 ++++++++++++++++++++++++++++++++++++-
>>   drivers/mtd/nand/atmel_nand_ecc.h |  116 ++++++
>>   2 files changed, 876 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index 9a9bfbf..ddcf1ed 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -15,6 +15,8 @@
>>    *     		(u-boot-1.1.5/board/atmel/at91sam9263ek/nand.c)
>>    *     (C) Copyright 2006 ATMEL Rousset, Lacressonniere Nicolas
>>    *
>> + *  Add Programmable Multibit ECC support for various AT91 SoC
>> + *     (C) Copyright 2012 ATMEL, Hong Xu
>>    *
>>    * This program is free software; you can redistribute it and/or modify
>>    * it under the terms of the GNU General Public License version 2 as
>> @@ -77,6 +79,21 @@ static struct nand_ecclayout atmel_oobinfo_small = {
>>   	},
>>   };
>>
>> +/* a structure includes datas for PMECC computation */
>> +struct atmel_pmecc_data {
>> +	int16_t			partial_syn[2 * PMECC_MAX_ERROR_NB + 1];
>> +	int16_t			si[2 * PMECC_MAX_ERROR_NB + 1];
>> +
>> +	/* Sigma table */
>> +	int16_t		smu[PMECC_MAX_ERROR_NB + 2][2 * PMECC_MAX_ERROR_NB + 1];
> you still hardcode the array in the struct
>
> and if the pmecc evolve we will have to touch again
> please allocate them

ok.

>> +	/* polynomal order */
>> +	int16_t			lmu[PMECC_MAX_ERROR_NB + 1];
>> +
>> +	int			mu[PMECC_MAX_ERROR_NB + 1];
>> +	int			dmu[PMECC_MAX_ERROR_NB + 1];
>> +	int			delta[PMECC_MAX_ERROR_NB + 1];
>> +};
>> +
>>   struct atmel_nand_host {
>>   	struct nand_chip	nand_chip;
>>   	struct mtd_info		mtd;
>> @@ -92,8 +109,25 @@ struct atmel_nand_host {
>>   	bool			has_pmecc;
>>   	u8			pmecc_corr_cap;
>>   	u16			pmecc_sector_size;
>> +
>> +	int			pmecc_bytes_per_sector;
>> +	int			pmecc_sector_number;
>> +	int			pmecc_degree;	/* Degree of remainders */
>> +	int			pmecc_cw_len;	/* Length of codeword */
>> +
>> +	void __iomem		*pmerrloc_base;
>> +	void __iomem		*pmecc_rom_base;
>> +
>> +	/* lookup table for alpha_to and index_of */
>> +	void __iomem		*pmecc_alpha_to;
>> +	void __iomem		*pmecc_index_of;
>> +
>> +	/* data for pmecc computation */
>> +	struct atmel_pmecc_data	*pmecc_data;
>>   };
>>
>> +static struct nand_ecclayout atmel_pmecc_oobinfo;
>> +
>>   static int cpu_has_dma(void)
>>   {
>>   	return cpu_is_at91sam9rl() || cpu_is_at91sam9g45();
>> @@ -287,6 +321,708 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
>>   }
>>
>>   /*
>> + * Return number of ecc bytes per sector according to sector size and
>> + * correction capability
>> + *
>> + * Following table shows what at91 PMECC supported:
>> + * Correction Capability	Sector_512_bytes	Sector_1024_bytes
>> + * =====================	================	=================
>> + *                2-bits                 4-bytes                  4-bytes
>> + *                4-bits                 7-bytes                  7-bytes
>> + *                8-bits                13-bytes                 14-bytes
>> + *               12-bits                20-bytes                 21-bytes
>> + *               24-bits                39-bytes                 42-bytes
>> + */
>> +static int pmecc_get_ecc_bytes(int cap, int sector_size)
>> +{
>> +	int m = 12 + sector_size / 512;
>> +	return (m * cap + 7) / 8;
>> +}
>> +
>> +static void pmecc_config_ecc_layout(struct nand_ecclayout *layout, int oobsize,
>> +	int ecc_len)
>> +{
>> +	int i;
>> +
>> +	layout->eccbytes = ecc_len;
>> +
>> +	/* ECC will occupy the last ecc_len bytes continuously */
>> +	for (i = 0; i<  ecc_len; i++)
>> +		layout->eccpos[i] = oobsize - ecc_len + i;
>> +
>> +	layout->oobfree[0].offset = 2;
>> +	layout->oobfree[0].length =
>> +		oobsize - ecc_len - layout->oobfree[0].offset;
>> +}
>> +
>> +static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
>> +{
>> +	void __iomem *p;
>> +
>> +	switch (host->pmecc_sector_size) {
>> +	case 512:
>> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512 +
>> +			PMECC_LOOKUP_TABLE_SIZE_512 * sizeof(int16_t);
>> +		break;
>> +	case 1024:
>> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024 +
>> +			PMECC_LOOKUP_TABLE_SIZE_1024 * sizeof(int16_t);
>> +		break;
>> +	default:
>> +		BUG();
>> +	}
>> +
>> +	return p;
>> +}
>> +
>> +static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
> this is a __dev_init function plese check the other too
>
> btw you need to use __dev_init and not __init

Here I use pmecc_get_index_of() function to get a lookup table base 
which is in ROM.
I'm not clear about how should I need __dev_init here. Can you give more 
information?

>> +{
>> +	void __iomem *p;
>> +
>> +	switch (host->pmecc_sector_size) {
>> +	case 512:
>> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_512;
>> +		break;
>> +	case 1024:
>> +		p = host->pmecc_rom_base + PMECC_LOOKUP_TABLE_OFFSET_1024;
>> +		break;
>> +	default:
>> +		BUG();
>> +	}
>> +
>> +	return p;
>> +}
>> +
>> +static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
>> +{
>> +	int			i;
>> +	uint32_t		value;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +
>> +	/* Fill odd syndromes */
>> +	for (i = 0; i<  host->pmecc_corr_cap; i++) {
>> +		value = pmecc_readl_rem_relaxed(host->ecc, sector, i / 2);
>> +		value = (i&  1) ? (value&  0xffff0000)>>  16 : value&  0xffff;
> simplify by
> 		if (i&  1)
> 			val>>= 16;
> 		value&= 0xffff;

I'll fix it.

>
>> +		host->pmecc_data->partial_syn[(2 * i) + 1] = (int16_t)value;
>> +	}
>> +}
>> +
>> +static void pmecc_substitute(struct mtd_info *mtd)
>> +{
>> +	int16_t			*si;
>> +	int			i, j;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +	int16_t __iomem		*alpha_to = host->pmecc_alpha_to;
>> +	int16_t __iomem		*index_of = host->pmecc_index_of;
>> +	int16_t			*partial_syn = host->pmecc_data->partial_syn;
>> +
>> +	/* si[] is a table that holds the current syndrome value,
>> +	 * an element of that table belongs to the field
>> +	 */
>> +	si = host->pmecc_data->si;
>> +
>> +	for (i = 1; i<  2 * PMECC_MAX_ERROR_NB; i++)
>> +		si[i] = 0;
> please use memset

ok

>> +
>> +	/* Computation 2t syndromes based on S(x) */
>> +	/* Odd syndromes */
>> +	for (i = 1; i<  2 * host->pmecc_corr_cap; i += 2) {
>> +		si[i] = 0;
> shy this you already init the array at 0 before

yes, I'll fix it.

>> +		for (j = 0; j<  host->pmecc_degree; j++) {
>> +			if (partial_syn[i]&  ((unsigned short)0x1<<  j))
>> +				si[i] = readw_relaxed(alpha_to + i * j) ^ si[i];
>> +		}
>> +	}
>> +	/* Even syndrome = (Odd syndrome) ** 2 */
>> +	for (i = 2; i<= 2 * host->pmecc_corr_cap; i += 2) {
>> +		j = i / 2;
>> +		if (si[j] == 0)
> here if {
> } else {
> }

ok

>> +			si[i] = 0;
>> +		else {
>> +			int16_t tmp;
> missing blank line

ok.

>> +			tmp = readw_relaxed(index_of + si[j]);
>> +			tmp = (tmp * 2) % host->pmecc_cw_len;
>> +			si[i] = readw_relaxed(alpha_to + tmp);
>> +		}
>> +	}
>> +
>> +	return;
>> +}
>> +
>> +static void pmecc_get_sigma(struct mtd_info *mtd)
>> +{
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +
>> +	int		i, j, k;
>> +	uint32_t	dmu_0_count, tmp;
>> +	int16_t		(*smu)[2 * PMECC_MAX_ERROR_NB + 1];
>> +	int16_t		*lmu = host->pmecc_data->lmu;
>> +	int16_t		*si = host->pmecc_data->si;
>> +	int		*mu = host->pmecc_data->mu;
>> +	int		*dmu = host->pmecc_data->dmu;	  /* Discrepancy */
>> +	int		*delta = host->pmecc_data->delta; /* Delta order */
>> +	int		cw_len = host->pmecc_cw_len;
>> +	int16_t		cap = host->pmecc_corr_cap;
>> +
>> +	int16_t __iomem	*index_of = host->pmecc_index_of;
>> +	int16_t __iomem	*alpha_to = host->pmecc_alpha_to;
>> +
>> +	/* index of largest delta */
>> +	int ro;
>> +	int largest;
>> +	int diff;
>> +
>> +	dmu_0_count = 0;
>> +	smu = host->pmecc_data->smu;
>> +
>> +	/* First Row */
>> +
>> +	/* Mu */
>> +	mu[0] = -1;
>> +
>> +	memset(&smu[0][0], 0,
>> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
>> +	smu[0][0] = 1;
>> +
>> +	/* discrepancy set to 1 */
>> +	dmu[0] = 1;
>> +	/* polynom order set to 0 */
>> +	lmu[0] = 0;
>> +	delta[0]  = (mu[0] * 2 - lmu[0])>>  1;
>> +
>> +	/* Second Row */
>> +
>> +	/* Mu */
>> +	mu[1]  = 0;
>> +	/* Sigma(x) set to 1 */
>> +	memset(&smu[1][0], 0,
>> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
>> +	smu[1][0] = 1;
>> +
>> +	/* discrepancy set to S1 */
>> +	dmu[1] = si[1];
>> +
>> +	/* polynom order set to 0 */
>> +	lmu[1] = 0;
>> +
>> +	delta[1]  = (mu[1] * 2 - lmu[1])>>  1;
>> +
>> +	/* Init the Sigma(x) last row */
>> +	memset(&smu[cap + 1][0], 0,
>> +		sizeof(int16_t) * (2 * PMECC_MAX_ERROR_NB + 1));
>> +
>> +	for (i = 1; i<= cap; i++) {
>> +		mu[i+1] = i<<  1;
>> +		/* Begin Computing Sigma (Mu+1) and L(mu) */
>> +		/* check if discrepancy is set to 0 */
>> +		if (dmu[i] == 0) {
>> +			dmu_0_count++;
>> +
>> +			tmp = ((cap - (lmu[i]>>  1) - 1) / 2);
>> +			if ((cap - (lmu[i]>>  1) - 1)&  0x1)
>> +				tmp += 2;
>> +			else
>> +				tmp += 1;
>> +
>> +			if (dmu_0_count == tmp) {
>> +				for (j = 0; j<= (lmu[i]>>  1) + 1; j++)
>> +					smu[cap + 1][j] = smu[i][j];
>> +				lmu[cap + 1] = lmu[i];
>> +				return;
>> +			}
>> +
>> +			/* copy polynom */
>> +			for (j = 0; j<= lmu[i]>>  1; j++)
>> +				smu[i + 1][j] = smu[i][j];
>> +
>> +			/* copy previous polynom order to the next */
>> +			lmu[i + 1] = lmu[i];
>> +		} else {
>> +			ro = 0;
>> +			largest = -1;
>> +			/* find largest delta with dmu != 0 */
>> +			for (j = 0; j<  i; j++) {
>> +				if ((dmu[j])&&  (delta[j]>  largest)) {
>> +					largest = delta[j];
>> +					ro = j;
>> +				}
>> +			}
>> +
>> +			/* compute difference */
>> +			diff = (mu[i] - mu[ro]);
>> +
>> +			/* Compute degree of the new smu polynomial */
>> +			if ((lmu[i]>>  1)>  ((lmu[ro]>>  1) + diff))
>> +				lmu[i + 1] = lmu[i];
>> +			else
>> +				lmu[i + 1] = ((lmu[ro]>>  1) + diff) * 2;
>> +
>> +			/* Init smu[i+1] with 0 */
>> +			for (k = 0; k<  (2 * PMECC_MAX_ERROR_NB + 1); k++)
>> +				smu[i+1][k] = 0;
>> +
>> +			/* Compute smu[i+1] */
>> +			for (k = 0; k<= lmu[ro]>>  1; k++) {
>> +				int16_t a, b, c;
>> +
>> +				if (!(smu[ro][k]&&  dmu[i]))
>> +					continue;
>> +				a = readw_relaxed(index_of + dmu[i]);
>> +				b = readw_relaxed(index_of + dmu[ro]);
>> +				c = readw_relaxed(index_of + smu[ro][k]);
>> +				tmp = a + (cw_len - b) + c;
>> +				a = readw_relaxed(alpha_to + tmp % cw_len);
>> +				smu[i + 1][k + diff] = a;
>> +			}
>> +
>> +			for (k = 0; k<= lmu[i]>>  1; k++)
>> +				smu[i + 1][k] ^= smu[i][k];
>> +		}
>> +
>> +		/* End Computing Sigma (Mu+1) and L(mu) */
>> +		/* In either case compute delta */
>> +		delta[i + 1]  = (mu[i + 1] * 2 - lmu[i + 1])>>  1;
>> +
>> +		/* Do not compute discrepancy for the last iteration */
>> +		if (i>= cap)
>> +			continue;
>> +
>> +		for (k = 0 ; k<= (lmu[i + 1]>>  1); k++) {
>> +			tmp = 2 * (i - 1);
>> +			if (k == 0)
>> +				dmu[i + 1] = si[tmp + 3];
>> +			else if (smu[i+1][k]&&  si[tmp + 3 - k]) {
>> +				int16_t a, b, c;
>> +				a = readw_relaxed(index_of + smu[i + 1][k]);
>> +				b = si[2 * (i - 1) + 3 - k];
>> +				c = readw_relaxed(index_of + b);
>> +				tmp = a + c;
>> +				tmp %= cw_len;
>> +				dmu[i + 1] = readw_relaxed(alpha_to + tmp) ^
>> +					dmu[i + 1];
>> +			}
>> +		}
>> +	}
>> +
>> +	return;
>> +}
>> +
>> +static int pmecc_err_location(struct mtd_info *mtd)
>> +{
>> +	int			i;
>> +	int			err_nbr;	/* number of error */
>> +	int			roots_nbr;	/* number of roots */
>> +	int			sector_size;
>> +	uint32_t		val;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +	int			timeout_count = 0;
>> +	int			cap = host->pmecc_corr_cap;
>> +
>> +	err_nbr = 0;
>> +	sector_size = host->pmecc_sector_size;
>> +
>> +	pmerrloc_writel(host->pmerrloc_base, ELDIS, PMERRLOC_DISABLE);
>> +
>> +	for (i = 0; i<= host->pmecc_data->lmu[cap + 1]>>  1; i++) {
>> +		pmerrloc_writel_sigma_relaxed(host->pmerrloc_base, i,
>> +				      host->pmecc_data->smu[cap + 1][i]);
>> +		err_nbr++;
>> +	}
>> +
>> +	val = (err_nbr - 1)<<  16;
>> +	if (sector_size == 1024)
>> +		val |= 1;
>> +
>> +	pmerrloc_writel(host->pmerrloc_base, ELCFG, val);
>> +	pmerrloc_writel(host->pmerrloc_base, ELEN,
>> +			sector_size * 8 + host->pmecc_degree * cap);
>> +
>> +	while (!(pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
>> +		&  PMERRLOC_CALC_DONE)) {
>> +		if (unlikely(timeout_count++>  PMECC_MAX_TIMEOUT_COUNT))
>> +			return -1;	/* Time out */
>> +		cpu_relax();
>> +	}
>> +
>> +	roots_nbr = (pmerrloc_readl_relaxed(host->pmerrloc_base, ELISR)
>> +		&  PMERRLOC_ERR_NUM_MASK)>>  8;
>> +	/* Number of roots == degree of smu hence<= cap */
>> +	if (roots_nbr == host->pmecc_data->lmu[cap + 1]>>  1)
>> +		return err_nbr - 1;
>> +
>> +	/* Number of roots does not match the degree of smu
>> +	 * unable to correct error */
>> +	return -1;
>> +}
>> +
>> +static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf,
>> +		int extra_bytes, int err_nbr)
>> +{
>> +	int			i = 0;
>> +	int			byte_pos, bit_pos;
>> +	int			sector_size, ecc_size;
>> +	uint32_t		tmp;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +
>> +	sector_size = host->pmecc_sector_size;
>> +	ecc_size = nand_chip->ecc.bytes;
>> +
>> +	while (err_nbr) {
>> +		tmp = pmerrloc_readl_el_relaxed(host->pmerrloc_base, i) - 1;
>> +		byte_pos = tmp / 8;
>> +		bit_pos  = tmp % 8;
>> +		dev_info(host->dev, "PMECC correction, byte_pos: %d bit_pos: %d\n",
>> +					byte_pos, bit_pos);
>> +
>> +		if (byte_pos<  (sector_size + extra_bytes)) {
>> +			tmp = sector_size +
>> +				pmecc_readl_relaxed(host->ecc, SADDR);
>> +
>> +			if (byte_pos<  tmp)
>> +				*(buf + byte_pos) ^= (1<<  bit_pos);
>> +			else
>> +				*(buf + byte_pos + ecc_size) ^= (1<<  bit_pos);
>> +		}
>> +
>> +		i++;
>> +		err_nbr--;
>> +	}
>> +
>> +	return;
>> +}
>> +
>> +static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
>> +	u8 *ecc)
>> +{
>> +	int			i, err_nbr;
>> +	uint8_t			*buf_pos;
>> +	int			eccbytes;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +
>> +	eccbytes = nand_chip->ecc.bytes;
>> +	for (i = 0; i<  eccbytes; i++)
>> +		if (ecc[i] != 0xff)
>> +			goto normal_check;
>> +	/* Erased page, return OK */
>> +	return 0;
>> +
>> +normal_check:
>> +	for (i = 0; i<  host->pmecc_sector_number; i++) {
>> +		err_nbr = 0;
>> +		if (pmecc_stat&  0x1) {
>> +			buf_pos = buf + i * host->pmecc_sector_size;
>> +
>> +			pmecc_gen_syndrome(mtd, i);
>> +			pmecc_substitute(mtd);
>> +			pmecc_get_sigma(mtd);
>> +
>> +			err_nbr = pmecc_err_location(mtd);
>> +			if (err_nbr == -1) {
>> +				dev_err(host->dev, "PMECC: Too many errors\n");
>> +				mtd->ecc_stats.failed++;
>> +				return -EIO;
>> +			} else {
>> +				pmecc_correct_data(mtd, buf_pos, 0, err_nbr);
>> +				mtd->ecc_stats.corrected += err_nbr;
>> +			}
>> +		}
>> +		pmecc_stat>>= 1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
>> +		struct nand_chip *chip, uint8_t *buf, int page)
>> +{
>> +	uint32_t		stat;
>> +	int			timeout_count = 0;
>> +	int			eccsize = chip->ecc.size;
>> +	uint8_t			*oob = chip->oob_poi;
>> +	struct atmel_nand_host	*host = chip->priv;
>> +	uint32_t		*eccpos = chip->ecc.layout->eccpos;
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
>> +	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG)
>> +		&  ~PMECC_CFG_WRITE_OP) | PMECC_CFG_AUTO_ENABLE);
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
>> +
>> +	chip->read_buf(mtd, buf, eccsize);
>> +	chip->read_buf(mtd, oob, mtd->oobsize);
>> +
>> +	while ((pmecc_readl_relaxed(host->ecc, SR)&  PMECC_SR_BUSY)) {
>> +		if (unlikely(timeout_count++>  PMECC_MAX_TIMEOUT_COUNT))
>> +			return -EIO;	/* Time out */
>> +		cpu_relax();
>> +	}
>> +
>> +	stat = pmecc_readl_relaxed(host->ecc, ISR);
>> +	if (stat != 0) {
>> +		if (pmecc_correction(mtd, stat, buf,&oob[eccpos[0]]) != 0)
>> +			return -EIO;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void atmel_nand_pmecc_write_page(struct mtd_info *mtd,
>> +		struct nand_chip *chip, const uint8_t *buf)
>> +{
>> +	int			i, j;
>> +	int			timeout_count = 0;
>> +	struct atmel_nand_host	*host = chip->priv;
>> +	uint32_t		*eccpos = chip->ecc.layout->eccpos;
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
>> +
>> +	pmecc_writel(host->ecc, CFG, (pmecc_readl_relaxed(host->ecc, CFG) |
>> +		PMECC_CFG_WRITE_OP)&  ~PMECC_CFG_AUTO_ENABLE);
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
>> +
>> +	chip->write_buf(mtd, (u8 *)buf, mtd->writesize);
>> +
>> +	while ((pmecc_readl_relaxed(host->ecc, SR)&  PMECC_SR_BUSY)) {
>> +		if (unlikely(timeout_count++>  PMECC_MAX_TIMEOUT_COUNT)) {
>> +			dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
>> +			return;	/* Time out */
>> +		}
>> +		cpu_relax();
>> +	}
>> +
>> +	for (i = 0; i<  host->pmecc_sector_number; i++) {
>> +		for (j = 0; j<  host->pmecc_bytes_per_sector; j++) {
>> +			int pos;
>> +
>> +			pos = i * host->pmecc_bytes_per_sector + j;
>> +			chip->oob_poi[eccpos[pos]] =
>> +				pmecc_readb_ecc_relaxed(host->ecc, i, j);
>> +		}
>> +	}
>> +	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
>> +
>> +	return;
>> +}
>> +
>> +static void atmel_pmecc_core_init(struct mtd_info *mtd)
>> +{
>> +	uint32_t		val = 0;
>> +	struct nand_chip	*nand_chip = mtd->priv;
>> +	struct atmel_nand_host	*host = nand_chip->priv;
>> +	struct nand_ecclayout	*ecc_layout;
>> +
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
>> +
>> +	switch (host->pmecc_corr_cap) {
>> +	case 2:
>> +		val = PMECC_CFG_BCH_ERR2;
>> +		break;
>> +	case 4:
>> +		val = PMECC_CFG_BCH_ERR4;
>> +		break;
>> +	case 8:
>> +		val = PMECC_CFG_BCH_ERR8;
>> +		break;
>> +	case 12:
>> +		val = PMECC_CFG_BCH_ERR12;
>> +		break;
>> +	case 24:
>> +		val = PMECC_CFG_BCH_ERR24;
>> +		break;
>> +	}
>> +
>> +	if (host->pmecc_sector_size == 512)
>> +		val |= PMECC_CFG_SECTOR512;
>> +	else if (host->pmecc_sector_size == 1024)
>> +		val |= PMECC_CFG_SECTOR1024;
>> +
>> +	switch (host->pmecc_sector_number) {
>> +	case 1:
>> +		val |= PMECC_CFG_PAGE_1SECTOR;
>> +		break;
>> +	case 2:
>> +		val |= PMECC_CFG_PAGE_2SECTORS;
>> +		break;
>> +	case 4:
>> +		val |= PMECC_CFG_PAGE_4SECTORS;
>> +		break;
>> +	case 8:
>> +		val |= PMECC_CFG_PAGE_8SECTORS;
>> +		break;
>> +	}
>> +
>> +	val |= (PMECC_CFG_READ_OP | PMECC_CFG_SPARE_DISABLE
>> +		| PMECC_CFG_AUTO_DISABLE);
>> +	pmecc_writel(host->ecc, CFG, val);
>> +
>> +	ecc_layout = nand_chip->ecc.layout;
>> +	pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
>> +	pmecc_writel(host->ecc, SADDR, ecc_layout->eccpos[0]);
>> +	pmecc_writel(host->ecc, EADDR,
>> +			ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
>> +	/* See datasheet about PMECC Clock Control Register */
>> +	pmecc_writel(host->ecc, CLK, 2);
>> +	pmecc_writel(host->ecc, IDR, 0xff);
>> +	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
>> +}
>> +
>> +static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
>> +					 struct atmel_nand_host *host)
>> +{
>> +	int			cap, sector_size, err_no;
>> +	struct mtd_info		*mtd;
>> +	struct nand_chip	*nand_chip;
>> +	struct resource		*regs;
>> +	struct resource		*regs_pmerr, *regs_rom;
>> +
>> +	cap = host->pmecc_corr_cap;
>> +	sector_size = host->pmecc_sector_size;
>> +	dev_info(host->dev, "Initialize PMECC params, cap: %d, sector: %d\n",
>> +		 cap, sector_size);
>> +
>> +	/* Sanity check */
>> +	if ((sector_size != 512)&&  (sector_size != 1024)) {
>> +		dev_err(host->dev,
>> +			"Unsupported PMECC sector size: %d; should be 512 or 1024 bytes\n",
>> +			sector_size);
>> +		return -EINVAL;
>> +	}
>> +	if ((cap != 2)&&  (cap != 4)&&  (cap != 8)&&  (cap != 12)&&
>> +	    (cap != 24)) {
>> +		dev_err(host->dev,
>> +			"Unsupported PMECC correction capability, should be 2, 4, 8, 12 or 24\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	nand_chip =&host->nand_chip;
>> +	mtd =&host->mtd;
>> +
>> +	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* By default */
>> +
>> +	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	if (!regs) {
>> +		dev_warn(host->dev,
>> +			"Can't get I/O resource regs, rolling back on software ECC\n");
>> +		return 0;
>> +	}
>> +
>> +	host->ecc = ioremap(regs->start, resource_size(regs));
>> +	if (host->ecc == NULL) {
>> +		dev_err(host->dev, "ioremap failed\n");
>> +		err_no = -EIO;
>> +		goto err_pmecc_ioremap;
>> +	}
>> +
>> +	regs_pmerr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>> +	regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
>> +	if (regs_pmerr&&  regs_rom) {
>> +		host->pmerrloc_base = ioremap(regs_pmerr->start,
>> +			resource_size(regs_pmerr));
>> +		host->pmecc_rom_base = ioremap(regs_rom->start,
>> +			resource_size(regs_rom));
>> +
>> +		if (host->pmerrloc_base&&  host->pmecc_rom_base) {
>> +			nand_chip->ecc.mode = NAND_ECC_HW;
>> +			nand_chip->ecc.read_page =
>> +				atmel_nand_pmecc_read_page;
>> +			nand_chip->ecc.write_page =
>> +				atmel_nand_pmecc_write_page;
>> +		} else {
>> +			dev_err(host->dev,
>> +				"Can not get I/O resource for PMECC controller!\n");
>> +			err_no = -EIO;
>> +			goto err_pmloc_ioremap;
>> +		}
>> +	}
>> +
>> +	/* ECC is calculated for the whole page (1 step) */
>> +	nand_chip->ecc.size = mtd->writesize;
>> +
>> +	/* set ECC page size and oob layout */
>> +	switch (mtd->writesize) {
>> +	case 2048:
>> +		host->pmecc_degree = PMECC_GF_DIMENSION_13;
>> +		host->pmecc_cw_len = (1<<  host->pmecc_degree) - 1;
>> +		host->pmecc_corr_cap = cap;
>> +		host->pmecc_sector_number = mtd->writesize / sector_size;
>> +		host->pmecc_bytes_per_sector = pmecc_get_ecc_bytes(
>> +			cap, sector_size);
>> +		host->pmecc_alpha_to = pmecc_get_alpha_to(host);
>> +		host->pmecc_index_of = pmecc_get_index_of(host);
>> +
>> +		nand_chip->ecc.steps = 1;
>> +		nand_chip->ecc.strength = cap;
>> +		nand_chip->ecc.bytes = host->pmecc_bytes_per_sector *
>> +				       host->pmecc_sector_number;
>> +		if (nand_chip->ecc.bytes>  mtd->oobsize - 2) {
>> +			dev_err(host->dev, "No room for ECC bytes\n");
>> +			err_no = -EINVAL;
>> +			goto err;
>> +		}
>> +		pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
>> +					mtd->oobsize,
>> +					nand_chip->ecc.bytes);
>> +		nand_chip->ecc.layout =&atmel_pmecc_oobinfo;
>> +		break;
>> +	case 512:
>> +	case 1024:
>> +	case 4096:
>> +		/* TODO */
>> +		dev_warn(host->dev,
>> +			"Unsupported page size for PMECC, use Software ECC\n");
>> +	default:
>> +		/* page size not handled by HW ECC */
>> +		/* switching back to soft ECC */
>> +		nand_chip->ecc.mode = NAND_ECC_SOFT;
>> +		nand_chip->ecc.calculate = NULL;
>> +		nand_chip->ecc.correct = NULL;
>> +		nand_chip->ecc.hwctl = NULL;
>> +		nand_chip->ecc.read_page = NULL;
>> +		nand_chip->ecc.write_page = NULL;
>> +		nand_chip->ecc.postpad = 0;
>> +		nand_chip->ecc.prepad = 0;
>> +		nand_chip->ecc.bytes = 0;
>> +		err_no = 0;
>> +		goto err;
>> +	}
>> +
>> +	/* Allocate data for PMECC computation */
>> +	host->pmecc_data = kzalloc(sizeof(struct atmel_pmecc_data), GFP_KERNEL);
> why do you always allocate the pmecc_data?
>
> you need to allocate it only if you use it

If not using PMECC, the code will never come to here. So I think it's 
right time to allocate all the computation data here.

>
> Best Regards,
> J.
>> +	if (!host->pmecc_data) {
>> +		dev_err(host->dev,
>> +				"Cannot allocate memory for PMECC computation!\n");
>> +		err_no = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	atmel_pmecc_core_init(mtd);
>> +
>> +	return 0;
>> +
>> +err:
>> +err_pmloc_ioremap:
>> +	iounmap(host->ecc);
>> +	if (host->pmerrloc_base)
>> +		iounmap(host->pmerrloc_base);
>> +	if (host->pmecc_rom_base)
>> +		iounmap(host->pmecc_rom_base);
>> +err_pmecc_ioremap:
>> +	return err_no;
>> +}
>> +
>> +/*
>>    * Calculate HW ECC
>>    *
>>    * function called after a write
>> @@ -720,7 +1456,11 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
>>   	}
>>
>>   	if (nand_chip->ecc.mode == NAND_ECC_HW) {
>> -		res = atmel_hw_nand_init_params(pdev, host);
>> +		if (host->has_pmecc)
>> +			res = atmel_pmecc_nand_init_params(pdev, host);
>> +		else
>> +			res = atmel_hw_nand_init_params(pdev, host);
>> +
>>   		if (res != 0)
>>   			goto err_hw_ecc;
>>   	}
>> @@ -741,6 +1481,12 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
>>   err_scan_tail:
>>   	if (host->ecc)
>>   		iounmap(host->ecc);
>> +	if (host->has_pmecc) {
> no need to check if you have teh pmecc
>
> if it's no the case pmerrloc_base will be NULLo
>
> don't you need to disable it in the error path?

right, I should disable the hardware in this error path.

>
> Best Regards,
> J.

Best Regards,
Josh Wu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
  2012-05-27 12:50     ` Artem Bityutskiy
@ 2012-05-28  8:43       ` Josh Wu
  -1 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-28  8:43 UTC (permalink / raw)
  To: Artem Bityutskiy
  Cc: hongxu.cn, nicolas.ferre, linux-mtd, ivan.djelic, plagnioj,
	linux-arm-kernel

On 5/27/2012 8:50 PM, Artem Bityutskiy wrote:
> On Sat, 2012-05-26 at 21:24 +0800, Josh Wu wrote:
>> +       while ((pmecc_readl_relaxed(host->ecc, SR)&  PMECC_SR_BUSY)) {
>> +               if (unlikely(timeout_count++>  PMECC_MAX_TIMEOUT_COUNT)) {
>> +                       dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
>> +                       return; /* Time out */
> How this error is communicated then up the the user?

If this error happened, that should mean the PMECC is not configurated 
correctly. I think I only I can do is that prompt to user and then call 
BUG() here.

>
>> +               }
>> +               cpu_relax();
>> +       }
> I see this pattern all over the place - why people consider it reliable?
> Is this code guaranteed to run on the same CPU?
>
> Why not to use loops_per_jiffie * msecs_to_jiffies(TIMOUT) instead to
> calculate how many iterations to do? Yes, due to HW register reading and
> cpu_relax() the real timeout will be larger, but this is about error
> anyway, so it does not hurt to iterate longer?
>

I will fix that in next version. thanks.

Best Regards,
Josh Wu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
@ 2012-05-28  8:43       ` Josh Wu
  0 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-28  8:43 UTC (permalink / raw)
  To: linux-arm-kernel

On 5/27/2012 8:50 PM, Artem Bityutskiy wrote:
> On Sat, 2012-05-26 at 21:24 +0800, Josh Wu wrote:
>> +       while ((pmecc_readl_relaxed(host->ecc, SR)&  PMECC_SR_BUSY)) {
>> +               if (unlikely(timeout_count++>  PMECC_MAX_TIMEOUT_COUNT)) {
>> +                       dev_err(host->dev, "PMECC: Timeout to get ECC value.\n");
>> +                       return; /* Time out */
> How this error is communicated then up the the user?

If this error happened, that should mean the PMECC is not configurated 
correctly. I think I only I can do is that prompt to user and then call 
BUG() here.

>
>> +               }
>> +               cpu_relax();
>> +       }
> I see this pattern all over the place - why people consider it reliable?
> Is this code guaranteed to run on the same CPU?
>
> Why not to use loops_per_jiffie * msecs_to_jiffies(TIMOUT) instead to
> calculate how many iterations to do? Yes, due to HW register reading and
> cpu_relax() the real timeout will be larger, but this is about error
> anyway, so it does not hurt to iterate longer?
>

I will fix that in next version. thanks.

Best Regards,
Josh Wu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v9 1/3] MTD: at91: extract hw ecc initialization to one function and use relaxed read/write
  2012-05-27 12:44     ` Artem Bityutskiy
@ 2012-05-28  8:50       ` Josh Wu
  -1 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-28  8:50 UTC (permalink / raw)
  To: Artem Bityutskiy
  Cc: hongxu.cn, nicolas.ferre, linux-mtd, ivan.djelic, plagnioj,
	linux-arm-kernel

On 5/27/2012 8:44 PM, Artem Bityutskiy wrote:
> On Sat, 2012-05-26 at 21:24 +0800, Josh Wu wrote:
>> use _relaxed read/write in most place. And use writel in operations of Control Register since it needs memory barrier.
>>
>> Signed-off-by: Hong Xu<hong.xu@atmel.com>
>> Signed-off-by: Josh Wu<josh.wu@atmel.com>
> This should be split on 2 or even 3 parts:
>
> 1. You move definitions and things around.
> 2. You separate out the init stuff into a function
> 3. You start using _relaxed helpers.
>
> And you should provide better commit messages. You should explain why
> you do things - just cleanup? improvement? fix? preparation for
> something?

My goal here is to prepare for the PMECC support.
    Point 1 is consistent for the later PMECC patch.
    Point 2 is prepare for PMECC support.

So I think I will not add the _relaxed helpers for HW ECC operation in 
this patch series. Since that is out of the goal of this patch series.

>
> I think this patch is too big, commit message is poor, patch does more
> things than documented in the commit message.
>

Indeed the commit message is poor. I will fix it.

Best Regards,
Josh Wu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 1/3] MTD: at91: extract hw ecc initialization to one function and use relaxed read/write
@ 2012-05-28  8:50       ` Josh Wu
  0 siblings, 0 replies; 24+ messages in thread
From: Josh Wu @ 2012-05-28  8:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 5/27/2012 8:44 PM, Artem Bityutskiy wrote:
> On Sat, 2012-05-26 at 21:24 +0800, Josh Wu wrote:
>> use _relaxed read/write in most place. And use writel in operations of Control Register since it needs memory barrier.
>>
>> Signed-off-by: Hong Xu<hong.xu@atmel.com>
>> Signed-off-by: Josh Wu<josh.wu@atmel.com>
> This should be split on 2 or even 3 parts:
>
> 1. You move definitions and things around.
> 2. You separate out the init stuff into a function
> 3. You start using _relaxed helpers.
>
> And you should provide better commit messages. You should explain why
> you do things - just cleanup? improvement? fix? preparation for
> something?

My goal here is to prepare for the PMECC support.
    Point 1 is consistent for the later PMECC patch.
    Point 2 is prepare for PMECC support.

So I think I will not add the _relaxed helpers for HW ECC operation in 
this patch series. Since that is out of the goal of this patch series.

>
> I think this patch is too big, commit message is poor, patch does more
> things than documented in the commit message.
>

Indeed the commit message is poor. I will fix it.

Best Regards,
Josh Wu

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
  2012-05-28  8:34       ` Josh Wu
@ 2012-05-29 16:01         ` Jean-Christophe PLAGNIOL-VILLARD
  -1 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-29 16:01 UTC (permalink / raw)
  To: Josh Wu
  Cc: hongxu.cn, dedekind1, nicolas.ferre, linux-mtd, ivan.djelic,
	linux-arm-kernel

> >>+
> >>+static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
> >this is a __dev_init function plese check the other too
> >
> >btw you need to use __dev_init and not __init
> 
> Here I use pmecc_get_index_of() function to get a lookup table base
> which is in ROM.
> I'm not clear about how should I need __dev_init here. Can you give
> more information?

it is the section where the function is going to be stored

pmecc_get_index_of is use at probe time so it's __devinit section

as it's supposed to be for the probe function too

Best Regards,
J.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller
@ 2012-05-29 16:01         ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-05-29 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

> >>+
> >>+static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
> >this is a __dev_init function plese check the other too
> >
> >btw you need to use __dev_init and not __init
> 
> Here I use pmecc_get_index_of() function to get a lookup table base
> which is in ROM.
> I'm not clear about how should I need __dev_init here. Can you give
> more information?

it is the section where the function is going to be stored

pmecc_get_index_of is use at probe time so it's __devinit section

as it's supposed to be for the probe function too

Best Regards,
J.

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2012-05-29 16:01 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-26 13:24 [PATCH v9 0/3] MTD: at91: Add PMECC support for at91 nand flash driver Josh Wu
2012-05-26 13:24 ` Josh Wu
2012-05-26 13:24 ` [PATCH v9 1/3] MTD: at91: extract hw ecc initialization to one function and use relaxed read/write Josh Wu
2012-05-26 13:24   ` Josh Wu
2012-05-27 12:44   ` Artem Bityutskiy
2012-05-27 12:44     ` Artem Bityutskiy
2012-05-28  8:50     ` Josh Wu
2012-05-28  8:50       ` Josh Wu
2012-05-26 13:24 ` [PATCH v9 2/3] MTD: at91: add dt parameters for PMECC Josh Wu
2012-05-26 13:24   ` Josh Wu
2012-05-26 13:24 ` [PATCH v9 3/3] MTD: at91: atmel_nand: Update driver to support Programmable Multibit ECC controller Josh Wu
2012-05-26 13:24   ` Josh Wu
2012-05-27 12:50   ` Artem Bityutskiy
2012-05-27 12:50     ` Artem Bityutskiy
2012-05-28  8:43     ` Josh Wu
2012-05-28  8:43       ` Josh Wu
2012-05-28  6:58   ` Jean-Christophe PLAGNIOL-VILLARD
2012-05-28  6:58     ` Jean-Christophe PLAGNIOL-VILLARD
2012-05-28  8:34     ` Josh Wu
2012-05-28  8:34       ` Josh Wu
2012-05-29 16:01       ` Jean-Christophe PLAGNIOL-VILLARD
2012-05-29 16:01         ` Jean-Christophe PLAGNIOL-VILLARD
2012-05-28  6:39 ` [PATCH v9 0/3] MTD: at91: Add PMECC support for at91 nand flash driver Jean-Christophe PLAGNIOL-VILLARD
2012-05-28  6:39   ` Jean-Christophe PLAGNIOL-VILLARD

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.