[U-Boot] [PATCH v3 0/5] ARM: at91: atmel_nand: check ONFI ecc minimum requirement

[U-Boot] [PATCH v3 0/5] ARM: at91: atmel_nand: check ONFI ecc minimum requirement

[Josh Wu]

In this patch series, first we make the nand driver can dynamic change sector
size and ecc correct bits.
Then we enable ONFI ecc parameters check.

v2 --> v3: use dev_err/info(host->dev) instead of dev_err/info(NULL).
           use MTDDEBUG instead of pr_debug().

v1 --> v2: Fixes according to Scoot Wood's suggestion
  - add a new patch which make all driver can use dev_err, dev_info, dev_dbg.
  - replace all printk to dev_err or dev_info.
  - fix pmecc data allocate error handler.
  - fix the pmecc ecc requirement selection, should use <= instead of use <.

Josh Wu (5):
  ARM: at91: atmel_nand: pmecc driver will select the galois table by
    sector size
  ARM: at91: sama5d3: remove unused definition about PMECC alpha table
    offset
  linux/compat.h: move dev_err, dev_info and dev_dbg from usb driver to
    compat.h
  mtd: atmel_nand: alloc memory instead of use static array for pmecc
    data
  ARM: at91: atmel_nand: add code to check the ONFI parameter ECC
    requirement

 arch/arm/include/asm/arch-at91/at91sam9x5.h |    6 +
 arch/arm/include/asm/arch-at91/sama5d3.h    |    2 -
 doc/README.atmel_pmecc                      |   14 --
 drivers/mtd/nand/atmel_nand.c               |  197 +++++++++++++++++++++++++--
 drivers/usb/musb-new/linux-compat.h         |   16 ---
 include/configs/at91sam9x5ek.h              |    1 -
 include/configs/sama5d3xek.h                |    1 -
 include/linux/compat.h                      |    8 ++
 8 files changed, 201 insertions(+), 44 deletions(-)

-- 
1.7.9.5

[U-Boot] [PATCH v3 1/5] ARM: at91: atmel_nand: pmecc driver will select the galois table by sector size

[Josh Wu]

Define the galois index table offset in chip head file. So user do not need
to set by himself. Driver will set it correctly according to sector_size.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 arch/arm/include/asm/arch-at91/at91sam9x5.h |    6 ++++++
 doc/README.atmel_pmecc                      |   14 --------------
 drivers/mtd/nand/atmel_nand.c               |    5 ++++-
 include/configs/at91sam9x5ek.h              |    1 -
 include/configs/sama5d3xek.h                |    1 -
 5 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/arch/arm/include/asm/arch-at91/at91sam9x5.h b/arch/arm/include/asm/arch-at91/at91sam9x5.h
index 85e42f5..de9fa50 100644
--- a/arch/arm/include/asm/arch-at91/at91sam9x5.h
+++ b/arch/arm/include/asm/arch-at91/at91sam9x5.h
@@ -165,6 +165,12 @@
 #define ATMEL_ID_UHP		ATMEL_ID_UHPHS
 
 /*
+ * PMECC table in ROM
+ */
+#define ATMEL_PMECC_INDEX_OFFSET_512	0x8000
+#define ATMEL_PMECC_INDEX_OFFSET_1024	0x10000
+
+/*
  * at91sam9x5 specific prototypes
  */
 #ifndef __ASSEMBLY__
diff --git a/doc/README.atmel_pmecc b/doc/README.atmel_pmecc
index b483744..41f3bd7 100644
--- a/doc/README.atmel_pmecc
+++ b/doc/README.atmel_pmecc
@@ -19,17 +19,6 @@ To use PMECC in this driver, the user needs to set:
 	   It can be 2, 4, 8, 12 or 24.
 	2. The PMECC sector size: CONFIG_PMECC_SECTOR_SIZE.
 	   It only can be 512 or 1024.
-	3. The PMECC index lookup table's offsets in ROM code: CONFIG_PMECC_INDEX_TABLE_OFFSET.
-	   In the chip datasheet section "Boot Stragegies", you can find
-	   two Galois Field Table in the ROM code. One table is for 512-bytes
-	   sector. Another is for 1024-byte sector. Each Galois Field includes
-	   two sub-table: indext table & alpha table.
-	   In the beginning of each Galois Field Table is the index table,
-	   Alpha table is in the following.
-	   So the index table's offset is same as the Galois Field Table.
-
-	   Please set CONFIG_PMECC_INDEX_TABLE_OFFSET correctly according the
-	   Galois Field Table's offset base on the sector size you used.
 
 Take AT91SAM9X5EK as an example, the board definition file likes:
 
@@ -38,7 +27,4 @@ Take AT91SAM9X5EK as an example, the board definition file likes:
 #define CONFIG_ATMEL_NAND_HW_PMECC	1
 #define CONFIG_PMECC_CAP		2
 #define CONFIG_PMECC_SECTOR_SIZE	512
-#define CONFIG_PMECC_INDEX_TABLE_OFFSET	0x8000
 
-NOTE: If you use 1024 as the sector size, then need set 0x10000 as the
- CONFIG_PMECC_INDEX_TABLE_OFFSET
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 3bfbaf8..139a479 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -653,7 +653,10 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 
 	cap = host->pmecc_corr_cap = CONFIG_PMECC_CAP;
 	sector_size = host->pmecc_sector_size = CONFIG_PMECC_SECTOR_SIZE;
-	host->pmecc_index_table_offset = CONFIG_PMECC_INDEX_TABLE_OFFSET;
+	if (host->pmecc_sector_size == 512)
+		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_512;
+	else
+		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_1024;
 
 	MTDDEBUG(MTD_DEBUG_LEVEL1,
 		"Initialize PMECC params, cap: %d, sector: %d\n",
diff --git a/include/configs/at91sam9x5ek.h b/include/configs/at91sam9x5ek.h
index ee6e3fc..4364026 100644
--- a/include/configs/at91sam9x5ek.h
+++ b/include/configs/at91sam9x5ek.h
@@ -137,7 +137,6 @@
 #define CONFIG_ATMEL_NAND_HW_PMECC	1
 #define CONFIG_PMECC_CAP		2
 #define CONFIG_PMECC_SECTOR_SIZE	512
-#define CONFIG_PMECC_INDEX_TABLE_OFFSET	0x8000
 
 #define CONFIG_MTD_DEVICE
 #define CONFIG_CMD_MTDPARTS
diff --git a/include/configs/sama5d3xek.h b/include/configs/sama5d3xek.h
index c13e983..67f8180 100644
--- a/include/configs/sama5d3xek.h
+++ b/include/configs/sama5d3xek.h
@@ -142,7 +142,6 @@
 #define CONFIG_ATMEL_NAND_HW_PMECC
 #define CONFIG_PMECC_CAP		4
 #define CONFIG_PMECC_SECTOR_SIZE	512
-#define CONFIG_PMECC_INDEX_TABLE_OFFSET	ATMEL_PMECC_INDEX_OFFSET_512
 #define CONFIG_CMD_NAND_TRIMFFS
 #endif
 
-- 
1.7.9.5

[U-Boot] [PATCH v3 2/5] ARM: at91: sama5d3: remove unused definition about PMECC alpha table offset

[Josh Wu]

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 arch/arm/include/asm/arch-at91/sama5d3.h |    2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-at91/sama5d3.h b/arch/arm/include/asm/arch-at91/sama5d3.h
index 883b932..aff12a6 100644
--- a/arch/arm/include/asm/arch-at91/sama5d3.h
+++ b/arch/arm/include/asm/arch-at91/sama5d3.h
@@ -194,8 +194,6 @@
  */
 #define ATMEL_PMECC_INDEX_OFFSET_512	0x10000
 #define ATMEL_PMECC_INDEX_OFFSET_1024	0x18000
-#define ATMEL_PMECC_ALPHA_OFFSET_512	0x10000
-#define ATMEL_PMECC_ALPHA_OFFSET_1024	0x18000
 
 /*
  * SAMA5D3 specific prototypes
-- 
1.7.9.5

[U-Boot] [PATCH v3 3/5] linux/compat.h: move dev_err, dev_info and dev_dbg from usb driver to compat.h

[Josh Wu]

Since kernel code current use many dev_xxx() instead of using printk. To
compatible, move those dev_xxx from usb driver to linux/compat.h. Then all
driver code can use dev_err, dev_info and dev_vdbg.

This patch also removed duplicated macro definitions in usb driver.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/usb/musb-new/linux-compat.h |   16 ----------------
 include/linux/compat.h              |    8 ++++++++
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/musb-new/linux-compat.h b/drivers/usb/musb-new/linux-compat.h
index 72c8c2b..d7a5663 100644
--- a/drivers/usb/musb-new/linux-compat.h
+++ b/drivers/usb/musb-new/linux-compat.h
@@ -39,15 +39,6 @@ typedef unsigned long dmaaddr_t;
 #define cpu_relax() do {} while (0)
 
 #define pr_debug(fmt, args...) debug(fmt, ##args)
-#define dev_dbg(dev, fmt, args...)		\
-	debug(fmt, ##args)
-#define dev_vdbg(dev, fmt, args...)		\
-	debug(fmt, ##args)
-#define dev_info(dev, fmt, args...)		\
-	printf(fmt, ##args)
-#define dev_err(dev, fmt, args...)		\
-	printf(fmt, ##args)
-#define printk printf
 
 #define WARN(condition, fmt, args...) ({	\
 	int ret_warn = !!condition;		\
@@ -55,13 +46,6 @@ typedef unsigned long dmaaddr_t;
 		printf(fmt, ##args);		\
 	ret_warn; })
 
-#define KERN_DEBUG
-#define KERN_NOTICE
-#define KERN_WARNING
-#define KERN_ERR
-
-#define kfree(ptr) free(ptr)
-
 #define pm_runtime_get_sync(dev) do {} while (0)
 #define pm_runtime_put(dev) do {} while (0)
 #define pm_runtime_put_sync(dev) do {} while (0)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index e1338bf..3fdfb39 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -3,6 +3,14 @@
 
 #define ndelay(x)	udelay(1)
 
+#define dev_dbg(dev, fmt, args...)		\
+	debug(fmt, ##args)
+#define dev_vdbg(dev, fmt, args...)		\
+	debug(fmt, ##args)
+#define dev_info(dev, fmt, args...)		\
+	printf(fmt, ##args)
+#define dev_err(dev, fmt, args...)		\
+	printf(fmt, ##args)
 #define printk	printf
 
 #define KERN_EMERG
-- 
1.7.9.5

[U-Boot] [PATCH v3 4/5] mtd: atmel_nand: alloc memory instead of use static array for pmecc data

[Josh Wu]

In this way, the pmecc corraction capbility can change in run time.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
v2 --> v3:
  use dev_err(host->dev) instead of using dev_err(NULL).

v1 --> v2:
  replace printk with dev_err.
  fix coding style.
  free memory before return ENOMEM.

 drivers/mtd/nand/atmel_nand.c |   63 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 56 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 139a479..a4107fd 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -32,6 +32,7 @@
 #include <asm/arch/gpio.h>
 #include <asm/arch/at91_pio.h>
 
+#include <malloc.h>
 #include <nand.h>
 #include <watchdog.h>
 
@@ -66,13 +67,13 @@ struct atmel_nand_host {
 	void __iomem	*pmecc_index_of;
 
 	/* data for pmecc computation */
-	int16_t	pmecc_smu[(CONFIG_PMECC_CAP + 2) * (2 * CONFIG_PMECC_CAP + 1)];
-	int16_t	pmecc_partial_syn[2 * CONFIG_PMECC_CAP + 1];
-	int16_t	pmecc_si[2 * CONFIG_PMECC_CAP + 1];
-	int16_t	pmecc_lmu[CONFIG_PMECC_CAP + 1]; /* polynomal order */
-	int	pmecc_mu[CONFIG_PMECC_CAP + 1];
-	int	pmecc_dmu[CONFIG_PMECC_CAP + 1];
-	int	pmecc_delta[CONFIG_PMECC_CAP + 1];
+	int16_t	*pmecc_smu;
+	int16_t	*pmecc_partial_syn;
+	int16_t	*pmecc_si;
+	int16_t	*pmecc_lmu; /* polynomal order */
+	int	*pmecc_mu;
+	int	*pmecc_dmu;
+	int	*pmecc_delta;
 };
 
 static struct atmel_nand_host pmecc_host;
@@ -125,6 +126,48 @@ static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
 			table_size * sizeof(int16_t);
 }
 
+static void pmecc_data_free(struct atmel_nand_host *host)
+{
+	free(host->pmecc_partial_syn);
+	free(host->pmecc_si);
+	free(host->pmecc_lmu);
+	free(host->pmecc_smu);
+	free(host->pmecc_mu);
+	free(host->pmecc_dmu);
+	free(host->pmecc_delta);
+}
+
+static int pmecc_data_alloc(struct atmel_nand_host *host)
+{
+	const int cap = host->pmecc_corr_cap;
+	int size;
+
+	size = (2 * cap + 1) * sizeof(int16_t);
+	host->pmecc_partial_syn = malloc(size);
+	host->pmecc_si = malloc(size);
+	host->pmecc_lmu = malloc((cap + 1) * sizeof(int16_t));
+	host->pmecc_smu = malloc((cap + 2) * size);
+
+	size = (cap + 1) * sizeof(int);
+	host->pmecc_mu = malloc(size);
+	host->pmecc_dmu = malloc(size);
+	host->pmecc_delta = malloc(size);
+
+	if (host->pmecc_partial_syn &&
+			host->pmecc_si &&
+			host->pmecc_lmu &&
+			host->pmecc_smu &&
+			host->pmecc_mu &&
+			host->pmecc_dmu &&
+			host->pmecc_delta)
+		return 0;
+
+	/* error happened */
+	pmecc_data_free(host);
+	return -ENOMEM;
+
+}
+
 static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
 {
 	struct nand_chip *nand_chip = mtd->priv;
@@ -710,6 +753,12 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 		return 0;
 	}
 
+	/* Allocate data for PMECC computation */
+	if (pmecc_data_alloc(host)) {
+		dev_err(host->dev, "Cannot allocate memory for PMECC computation!\n");
+		return -ENOMEM;
+	}
+
 	nand->ecc.read_page = atmel_nand_pmecc_read_page;
 	nand->ecc.write_page = atmel_nand_pmecc_write_page;
 	nand->ecc.strength = cap;
-- 
1.7.9.5

[U-Boot] [PATCH v3 5/5] ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

[Josh Wu]

1. if CONFIG_SYS_NAND_ONFI_DETECTION is defined, driver will check NAND flash's
   ecc minimum requirement in ONFI parameter.

  a) if CONFIG_PMECC_CAP, CONFIG_PMECC_SECTOR_SIZE are defined. then use it.
     Driver will display a WARNING if the values are different from ONFI
     parameters.

  b) if CONFIG_PMECC_CAP, CONFIG_PMECC_SECTOR_SIZE are not defined, then use
      the value from ONFI parameters.
      * If ONFI ECC parameters are in ONFI extended parameter page, since we
        are not support it, so assume the minimum ecc requirement is 2 bits
        in 512 bytes.
      * For non-ONFI support nand flash, also assume the minimum ecc
        requirement is 2 bits in 512 bytes.

2. if CONFIG_SYS_NAND_ONFI_DETECTION is not defined, just use CONFIG_PMECC_CAP
   and CONFIG_PMECC_SECTOR_SIZE.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
v2 --> v3:
  use dev_err/info(host->dev) instead of dev_err/info(NULL).
  use MTDDEBUG() instead of pr_debug().

v1 --> v2:
  use dev_err/info to replace printk.
  fix typo mistake, that should use <= instead of use <.

 drivers/mtd/nand/atmel_nand.c |  129 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 127 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index a4107fd..91afe5c 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -681,6 +681,98 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd)
 	pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_ENABLE);
 }
 
+#ifdef CONFIG_SYS_NAND_ONFI_DETECTION
+/*
+ * get_onfi_ecc_param - Get ECC requirement from ONFI parameters
+ * @ecc_bits: store the ONFI ECC correct bits capbility
+ * @sector_size: in how many bytes that ONFI require to correct @ecc_bits
+ *
+ * Returns -1 if ONFI parameters is not supported. In this case @ecc_bits,
+ * @sector_size are initialize to 0.
+ * Return 0 if success to get the ECC requirement.
+ */
+static int get_onfi_ecc_param(struct nand_chip *chip,
+		int *ecc_bits, int *sector_size)
+{
+	*ecc_bits = *sector_size = 0;
+
+	if (chip->onfi_params.ecc_bits == 0xff)
+		/* TODO: the sector_size and ecc_bits need to be find in
+		 * extended ecc parameter, currently we don't support it.
+		 */
+		return -1;
+
+	*ecc_bits = chip->onfi_params.ecc_bits;
+
+	/* The default sector size (ecc codeword size) is 512 */
+	*sector_size = 512;
+
+	return 0;
+}
+
+/*
+ * pmecc_choose_ecc - Get ecc requirement from ONFI parameters. If
+ *                    pmecc_corr_cap or pmecc_sector_size is 0, then set it as
+ *                    ONFI ECC parameters.
+ * @host: point to an atmel_nand_host structure.
+ *        if host->pmecc_corr_cap is 0 then set it as the ONFI ecc_bits.
+ *        if host->pmecc_sector_size is 0 then set it as the ONFI sector_size.
+ * @chip: point to an nand_chip structure.
+ * @cap: store the ONFI ECC correct bits capbility
+ * @sector_size: in how many bytes that ONFI require to correct @ecc_bits
+ *
+ * Return 0 if success. otherwise return the error code.
+ */
+static int pmecc_choose_ecc(struct atmel_nand_host *host,
+		struct nand_chip *chip,
+		int *cap, int *sector_size)
+{
+	/* Get ECC requirement from ONFI parameters */
+	*cap = *sector_size = 0;
+	if (chip->onfi_version) {
+		if (!get_onfi_ecc_param(chip, cap, sector_size))
+			MTDDEBUG(MTD_DEBUG_LEVEL1, "ONFI params, minimum required ECC: %d bits in %d bytes\n",
+				*cap, *sector_size);
+		else
+			dev_info(host->dev, "NAND chip ECC reqirement is in Extended ONFI parameter, we don't support yet.\n");
+	} else {
+		dev_info(host->dev, "NAND chip is not ONFI compliant, assume ecc_bits is 2 in 512 bytes");
+	}
+	if (*cap == 0 && *sector_size == 0) {
+		/* Non-ONFI compliant or use extended ONFI parameters */
+		*cap = 2;
+		*sector_size = 512;
+	}
+
+	/* If head file doesn't specify then use the one in ONFI parameters */
+	if (host->pmecc_corr_cap == 0) {
+		/* use the most fitable ecc bits (the near bigger one ) */
+		if (*cap <= 2)
+			host->pmecc_corr_cap = 2;
+		else if (*cap <= 4)
+			host->pmecc_corr_cap = 4;
+		else if (*cap <= 8)
+			host->pmecc_corr_cap = 8;
+		else if (*cap <= 12)
+			host->pmecc_corr_cap = 12;
+		else if (*cap <= 24)
+			host->pmecc_corr_cap = 24;
+		else
+			return -EINVAL;
+	}
+	if (host->pmecc_sector_size == 0) {
+		/* use the most fitable sector size (the near smaller one ) */
+		if (*sector_size >= 1024)
+			host->pmecc_sector_size = 1024;
+		else if (*sector_size >= 512)
+			host->pmecc_sector_size = 512;
+		else
+			return -EINVAL;
+	}
+	return 0;
+}
+#endif
+
 static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 		struct mtd_info *mtd)
 {
@@ -694,8 +786,41 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 	nand->ecc.correct = NULL;
 	nand->ecc.hwctl = NULL;
 
-	cap = host->pmecc_corr_cap = CONFIG_PMECC_CAP;
-	sector_size = host->pmecc_sector_size = CONFIG_PMECC_SECTOR_SIZE;
+#ifdef CONFIG_SYS_NAND_ONFI_DETECTION
+	host->pmecc_corr_cap = host->pmecc_sector_size = 0;
+
+#ifdef CONFIG_PMECC_CAP
+	host->pmecc_corr_cap = CONFIG_PMECC_CAP;
+#endif
+#ifdef CONFIG_PMECC_SECTOR_SIZE
+	host->pmecc_sector_size = CONFIG_PMECC_SECTOR_SIZE;
+#endif
+	/* Get ECC requirement of ONFI parameters. And if CONFIG_PMECC_CAP or
+	 * CONFIG_PMECC_SECTOR_SIZE not defined, then use ecc_bits, sector_size
+	 * from ONFI.
+	 */
+	if (pmecc_choose_ecc(host, nand, &cap, &sector_size)) {
+		dev_err(host->dev, "The NAND flash's ECC requirement(ecc_bits: %d, sector_size: %d) are not support!",
+				cap, sector_size);
+		return -EINVAL;
+	}
+
+	if (cap > host->pmecc_corr_cap)
+		dev_info(host->dev, "WARNING: Using different ecc correct bits(%d bit) from Nand ONFI ECC reqirement (%d bit).\n",
+				host->pmecc_corr_cap, cap);
+	if (sector_size < host->pmecc_sector_size)
+		dev_info(host->dev, "WARNING: Using different ecc correct sector size (%d bytes) from Nand ONFI ECC reqirement (%d bytes).\n",
+				host->pmecc_sector_size, sector_size);
+#else	/* CONFIG_SYS_NAND_ONFI_DETECTION */
+	host->pmecc_corr_cap = CONFIG_PMECC_CAP;
+	host->pmecc_sector_size = CONFIG_PMECC_SECTOR_SIZE;
+#endif
+
+	cap = host->pmecc_corr_cap;
+	sector_size = host->pmecc_sector_size;
+
+	/* TODO: need check whether cap & sector_size is validate */
+
 	if (host->pmecc_sector_size == 512)
 		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_512;
 	else
-- 
1.7.9.5

[U-Boot] [PATCH v3 5/5] ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

[Scott Wood]

On 07/02/2013 10:11:49 PM, Josh Wu wrote:
> +		if (!get_onfi_ecc_param(chip, cap, sector_size))
> +			MTDDEBUG(MTD_DEBUG_LEVEL1, "ONFI params,  
> minimum required ECC: %d bits in %d bytes\n",
> +				*cap, *sector_size);

Use braces around multi-line if/loop bodies, even if it's a single  
statement.

Otherwise, ACK this patchset.

-Scott

[U-Boot] [PATCH v3 5/5] ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

[Josh Wu]

On 7/4/2013 6:12 AM, Scott Wood wrote:
> On 07/02/2013 10:11:49 PM, Josh Wu wrote:
>> +        if (!get_onfi_ecc_param(chip, cap, sector_size))
>> +            MTDDEBUG(MTD_DEBUG_LEVEL1, "ONFI params, minimum 
>> required ECC: %d bits in %d bytes\n",
>> +                *cap, *sector_size);
>
> Use braces around multi-line if/loop bodies, even if it's a single 
> statement.
>
> Otherwise, ACK this patchset.

thanks, I'll resend this patch with braces around.

>
> -Scott

Best Regards,
Josh Wu

[U-Boot] [resend][PATCH v3 5/5] ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

[Josh Wu]

1. if CONFIG_SYS_NAND_ONFI_DETECTION is defined, driver will check NAND flash's
   ecc minimum requirement in ONFI parameter.

  a) if CONFIG_PMECC_CAP, CONFIG_PMECC_SECTOR_SIZE are defined. then use it.
     Driver will display a WARNING if the values are different from ONFI
     parameters.

  b) if CONFIG_PMECC_CAP, CONFIG_PMECC_SECTOR_SIZE are not defined, then use
      the value from ONFI parameters.
      * If ONFI ECC parameters are in ONFI extended parameter page, since we
        are not support it, so assume the minimum ecc requirement is 2 bits
        in 512 bytes.
      * For non-ONFI support nand flash, also assume the minimum ecc
        requirement is 2 bits in 512 bytes.

2. if CONFIG_SYS_NAND_ONFI_DETECTION is not defined, just use CONFIG_PMECC_CAP
   and CONFIG_PMECC_SECTOR_SIZE.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
resend:
  use braces around muti-line if/loop body.

v2 --> v3:
  use dev_err/info(host->dev) instead of dev_err/info(NULL).
  use MTDDEBUG() instead of pr_debug().

v1 --> v2:
  use dev_err/info to replace printk.
  fix typo mistake, that should use <= instead of use <.

 drivers/mtd/nand/atmel_nand.c |  130 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 128 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index a4107fd..3906be0 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -681,6 +681,99 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd)
 	pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_ENABLE);
 }
 
+#ifdef CONFIG_SYS_NAND_ONFI_DETECTION
+/*
+ * get_onfi_ecc_param - Get ECC requirement from ONFI parameters
+ * @ecc_bits: store the ONFI ECC correct bits capbility
+ * @sector_size: in how many bytes that ONFI require to correct @ecc_bits
+ *
+ * Returns -1 if ONFI parameters is not supported. In this case @ecc_bits,
+ * @sector_size are initialize to 0.
+ * Return 0 if success to get the ECC requirement.
+ */
+static int get_onfi_ecc_param(struct nand_chip *chip,
+		int *ecc_bits, int *sector_size)
+{
+	*ecc_bits = *sector_size = 0;
+
+	if (chip->onfi_params.ecc_bits == 0xff)
+		/* TODO: the sector_size and ecc_bits need to be find in
+		 * extended ecc parameter, currently we don't support it.
+		 */
+		return -1;
+
+	*ecc_bits = chip->onfi_params.ecc_bits;
+
+	/* The default sector size (ecc codeword size) is 512 */
+	*sector_size = 512;
+
+	return 0;
+}
+
+/*
+ * pmecc_choose_ecc - Get ecc requirement from ONFI parameters. If
+ *                    pmecc_corr_cap or pmecc_sector_size is 0, then set it as
+ *                    ONFI ECC parameters.
+ * @host: point to an atmel_nand_host structure.
+ *        if host->pmecc_corr_cap is 0 then set it as the ONFI ecc_bits.
+ *        if host->pmecc_sector_size is 0 then set it as the ONFI sector_size.
+ * @chip: point to an nand_chip structure.
+ * @cap: store the ONFI ECC correct bits capbility
+ * @sector_size: in how many bytes that ONFI require to correct @ecc_bits
+ *
+ * Return 0 if success. otherwise return the error code.
+ */
+static int pmecc_choose_ecc(struct atmel_nand_host *host,
+		struct nand_chip *chip,
+		int *cap, int *sector_size)
+{
+	/* Get ECC requirement from ONFI parameters */
+	*cap = *sector_size = 0;
+	if (chip->onfi_version) {
+		if (!get_onfi_ecc_param(chip, cap, sector_size)) {
+			MTDDEBUG(MTD_DEBUG_LEVEL1, "ONFI params, minimum required ECC: %d bits in %d bytes\n",
+				*cap, *sector_size);
+		} else {
+			dev_info(host->dev, "NAND chip ECC reqirement is in Extended ONFI parameter, we don't support yet.\n");
+		}
+	} else {
+		dev_info(host->dev, "NAND chip is not ONFI compliant, assume ecc_bits is 2 in 512 bytes");
+	}
+	if (*cap == 0 && *sector_size == 0) {
+		/* Non-ONFI compliant or use extended ONFI parameters */
+		*cap = 2;
+		*sector_size = 512;
+	}
+
+	/* If head file doesn't specify then use the one in ONFI parameters */
+	if (host->pmecc_corr_cap == 0) {
+		/* use the most fitable ecc bits (the near bigger one ) */
+		if (*cap <= 2)
+			host->pmecc_corr_cap = 2;
+		else if (*cap <= 4)
+			host->pmecc_corr_cap = 4;
+		else if (*cap <= 8)
+			host->pmecc_corr_cap = 8;
+		else if (*cap <= 12)
+			host->pmecc_corr_cap = 12;
+		else if (*cap <= 24)
+			host->pmecc_corr_cap = 24;
+		else
+			return -EINVAL;
+	}
+	if (host->pmecc_sector_size == 0) {
+		/* use the most fitable sector size (the near smaller one ) */
+		if (*sector_size >= 1024)
+			host->pmecc_sector_size = 1024;
+		else if (*sector_size >= 512)
+			host->pmecc_sector_size = 512;
+		else
+			return -EINVAL;
+	}
+	return 0;
+}
+#endif
+
 static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 		struct mtd_info *mtd)
 {
@@ -694,8 +787,41 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 	nand->ecc.correct = NULL;
 	nand->ecc.hwctl = NULL;
 
-	cap = host->pmecc_corr_cap = CONFIG_PMECC_CAP;
-	sector_size = host->pmecc_sector_size = CONFIG_PMECC_SECTOR_SIZE;
+#ifdef CONFIG_SYS_NAND_ONFI_DETECTION
+	host->pmecc_corr_cap = host->pmecc_sector_size = 0;
+
+#ifdef CONFIG_PMECC_CAP
+	host->pmecc_corr_cap = CONFIG_PMECC_CAP;
+#endif
+#ifdef CONFIG_PMECC_SECTOR_SIZE
+	host->pmecc_sector_size = CONFIG_PMECC_SECTOR_SIZE;
+#endif
+	/* Get ECC requirement of ONFI parameters. And if CONFIG_PMECC_CAP or
+	 * CONFIG_PMECC_SECTOR_SIZE not defined, then use ecc_bits, sector_size
+	 * from ONFI.
+	 */
+	if (pmecc_choose_ecc(host, nand, &cap, &sector_size)) {
+		dev_err(host->dev, "The NAND flash's ECC requirement(ecc_bits: %d, sector_size: %d) are not support!",
+				cap, sector_size);
+		return -EINVAL;
+	}
+
+	if (cap > host->pmecc_corr_cap)
+		dev_info(host->dev, "WARNING: Using different ecc correct bits(%d bit) from Nand ONFI ECC reqirement (%d bit).\n",
+				host->pmecc_corr_cap, cap);
+	if (sector_size < host->pmecc_sector_size)
+		dev_info(host->dev, "WARNING: Using different ecc correct sector size (%d bytes) from Nand ONFI ECC reqirement (%d bytes).\n",
+				host->pmecc_sector_size, sector_size);
+#else	/* CONFIG_SYS_NAND_ONFI_DETECTION */
+	host->pmecc_corr_cap = CONFIG_PMECC_CAP;
+	host->pmecc_sector_size = CONFIG_PMECC_SECTOR_SIZE;
+#endif
+
+	cap = host->pmecc_corr_cap;
+	sector_size = host->pmecc_sector_size;
+
+	/* TODO: need check whether cap & sector_size is validate */
+
 	if (host->pmecc_sector_size == 512)
 		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_512;
 	else
-- 
1.7.9.5

[U-Boot] [PATCH v3 0/5] ARM: at91: atmel_nand: check ONFI ecc minimum requirement

[Josh Wu]

Hi,

Any feedback for those patch series?

Best Regards,
Josh Wu

On 7/3/2013 11:11 AM, Josh Wu wrote:
> In this patch series, first we make the nand driver can dynamic change sector
> size and ecc correct bits.
> Then we enable ONFI ecc parameters check.
>
> v2 --> v3: use dev_err/info(host->dev) instead of dev_err/info(NULL).
>             use MTDDEBUG instead of pr_debug().
>
> v1 --> v2: Fixes according to Scoot Wood's suggestion
>    - add a new patch which make all driver can use dev_err, dev_info, dev_dbg.
>    - replace all printk to dev_err or dev_info.
>    - fix pmecc data allocate error handler.
>    - fix the pmecc ecc requirement selection, should use <= instead of use <.
>
> Josh Wu (5):
>    ARM: at91: atmel_nand: pmecc driver will select the galois table by
>      sector size
>    ARM: at91: sama5d3: remove unused definition about PMECC alpha table
>      offset
>    linux/compat.h: move dev_err, dev_info and dev_dbg from usb driver to
>      compat.h
>    mtd: atmel_nand: alloc memory instead of use static array for pmecc
>      data
>    ARM: at91: atmel_nand: add code to check the ONFI parameter ECC
>      requirement
>
>   arch/arm/include/asm/arch-at91/at91sam9x5.h |    6 +
>   arch/arm/include/asm/arch-at91/sama5d3.h    |    2 -
>   doc/README.atmel_pmecc                      |   14 --
>   drivers/mtd/nand/atmel_nand.c               |  197 +++++++++++++++++++++++++--
>   drivers/usb/musb-new/linux-compat.h         |   16 ---
>   include/configs/at91sam9x5ek.h              |    1 -
>   include/configs/sama5d3xek.h                |    1 -
>   include/linux/compat.h                      |    8 ++
>   8 files changed, 201 insertions(+), 44 deletions(-)
>

[U-Boot] [PATCH v3 0/5] ARM: at91: atmel_nand: check ONFI ecc minimum requirement

[Scott Wood]

On Fri, 2013-08-09 at 18:00 +0800, Josh Wu wrote:
> Hi,
> 
> Any feedback for those patch series?
> 
> Best Regards,
> Josh Wu

Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

[U-Boot] [PATCH v3 0/5] ARM: at91: atmel_nand: check ONFI ecc minimum requirement

[Josh Wu]

Hi, Andreas

I am wondering will you take this series in your git tree? Since this 
patch is pending here for weeks.

Best Regards,
Josh Wu

On 7/3/2013 11:11 AM, Josh Wu wrote:
> In this patch series, first we make the nand driver can dynamic change sector
> size and ecc correct bits.
> Then we enable ONFI ecc parameters check.
>
> v2 --> v3: use dev_err/info(host->dev) instead of dev_err/info(NULL).
>             use MTDDEBUG instead of pr_debug().
>
> v1 --> v2: Fixes according to Scoot Wood's suggestion
>    - add a new patch which make all driver can use dev_err, dev_info, dev_dbg.
>    - replace all printk to dev_err or dev_info.
>    - fix pmecc data allocate error handler.
>    - fix the pmecc ecc requirement selection, should use <= instead of use <.
>
> Josh Wu (5):
>    ARM: at91: atmel_nand: pmecc driver will select the galois table by
>      sector size
>    ARM: at91: sama5d3: remove unused definition about PMECC alpha table
>      offset
>    linux/compat.h: move dev_err, dev_info and dev_dbg from usb driver to
>      compat.h
>    mtd: atmel_nand: alloc memory instead of use static array for pmecc
>      data
>    ARM: at91: atmel_nand: add code to check the ONFI parameter ECC
>      requirement
>
>   arch/arm/include/asm/arch-at91/at91sam9x5.h |    6 +
>   arch/arm/include/asm/arch-at91/sama5d3.h    |    2 -
>   doc/README.atmel_pmecc                      |   14 --
>   drivers/mtd/nand/atmel_nand.c               |  197 +++++++++++++++++++++++++--
>   drivers/usb/musb-new/linux-compat.h         |   16 ---
>   include/configs/at91sam9x5ek.h              |    1 -
>   include/configs/sama5d3xek.h                |    1 -
>   include/linux/compat.h                      |    8 ++
>   8 files changed, 201 insertions(+), 44 deletions(-)
>

[U-Boot] [PATCH v3 0/5] ARM: at91: atmel_nand: check ONFI ecc minimum requirement

[Andreas Bießmann]

Hi Josh,

On 08/22/2013 12:07 PM, Josh Wu wrote:
> Hi, Andreas
> 
> I am wondering will you take this series in your git tree? Since this
> patch is pending here for weeks.

I'm on it. Scott ACK'ed the series some time ago and I'm test building
it currently. Should be applied this evening.

Best regards

Andreas Bie?mann

[U-Boot] [U-Boot, v3, 1/5] ARM: at91: atmel_nand: pmecc driver will select the galois table by sector size

[Andreas Bießmann]

Dear Josh Wu,

Josh Wu <Josh.wu@atmel.com> writes:
>Define the galois index table offset in chip head file. So user do not need
>to set by himself. Driver will set it correctly according to sector_size.
>
>Signed-off-by: Josh Wu <josh.wu@atmel.com>
>Acked-by: Scott Wood <scottwood@freescale.com>
>[rebased on master]
>Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>---
> arch/arm/include/asm/arch-at91/at91sam9x5.h |    6 ++++++
> doc/README.atmel_pmecc                      |   14 --------------
> drivers/mtd/nand/atmel_nand.c               |    5 ++++-
> include/configs/at91sam9x5ek.h              |    1 -
> include/configs/sama5d3xek.h                |    1 -
> 5 files changed, 10 insertions(+), 17 deletions(-)

applied to u-boot-atmel/master, thanks!

Best regards,
Andreas Bie?mann

[U-Boot] [U-Boot, v3, 2/5] ARM: at91: sama5d3: remove unused definition about PMECC alpha table offset

[Andreas Bießmann]

Dear Josh Wu,

Josh Wu <Josh.wu@atmel.com> writes:
>Signed-off-by: Josh Wu <josh.wu@atmel.com>
>Acked-by: Scott Wood <scottwood@freescale.com>
>---
>arch/arm/include/asm/arch-at91/sama5d3.h |    2 --
> 1 file changed, 2 deletions(-)

applied to u-boot-atmel/master, thanks!

Best regards,
Andreas Bie?mann

[U-Boot] [U-Boot, v3, 3/5] linux/compat.h: move dev_err, dev_info and dev_dbg from usb driver to compat.h

[Andreas Bießmann]

Dear Josh Wu,

Josh Wu <Josh.wu@atmel.com> writes:
>Since kernel code current use many dev_xxx() instead of using printk. To
>compatible, move those dev_xxx from usb driver to linux/compat.h. Then all
>driver code can use dev_err, dev_info and dev_vdbg.
>
>This patch also removed duplicated macro definitions in usb driver.
>
>Signed-off-by: Josh Wu <josh.wu@atmel.com>
>Acked-by: Scott Wood <scottwood@freescale.com>
>---
>drivers/usb/musb-new/linux-compat.h |   16 ----------------
> include/linux/compat.h              |    8 ++++++++
> 2 files changed, 8 insertions(+), 16 deletions(-)

applied to u-boot-atmel/master, thanks!

Best regards,
Andreas Bie?mann

[U-Boot] [U-Boot, v3, 4/5] mtd: atmel_nand: alloc memory instead of use static array for pmecc data

[Andreas Bießmann]

Dear Josh Wu,

Josh Wu <Josh.wu@atmel.com> writes:
>In this way, the pmecc corraction capbility can change in run time.
>
>Signed-off-by: Josh Wu <josh.wu@atmel.com>
>Acked-by: Scott Wood <scottwood@freescale.com>
>---
>v2 --> v3:
>  use dev_err(host->dev) instead of using dev_err(NULL).
>
>v1 --> v2:
>  replace printk with dev_err.
>  fix coding style.
>  free memory before return ENOMEM.
>
> drivers/mtd/nand/atmel_nand.c |   63 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 56 insertions(+), 7 deletions(-)

applied to u-boot-atmel/master, thanks!

Best regards,
Andreas Bie?mann

[U-Boot] [U-Boot, resend, v3, 5/5] ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

[Andreas Bießmann]

Dear Josh Wu,

Josh Wu <Josh.wu@atmel.com> writes:
>1. if CONFIG_SYS_NAND_ONFI_DETECTION is defined, driver will check NAND flash's
>   ecc minimum requirement in ONFI parameter.
>
>  a) if CONFIG_PMECC_CAP, CONFIG_PMECC_SECTOR_SIZE are defined. then use it.
>     Driver will display a WARNING if the values are different from ONFI
>     parameters.
>
>  b) if CONFIG_PMECC_CAP, CONFIG_PMECC_SECTOR_SIZE are not defined, then use
>      the value from ONFI parameters.
>      * If ONFI ECC parameters are in ONFI extended parameter page, since we
>        are not support it, so assume the minimum ecc requirement is 2 bits
>        in 512 bytes.
>      * For non-ONFI support nand flash, also assume the minimum ecc
>        requirement is 2 bits in 512 bytes.
>
>2. if CONFIG_SYS_NAND_ONFI_DETECTION is not defined, just use CONFIG_PMECC_CAP
>   and CONFIG_PMECC_SECTOR_SIZE.
>
>Signed-off-by: Josh Wu <josh.wu@atmel.com>
>Acked-by: Scott Wood <scottwood@freescale.com>
>---
>resend:
>  use braces around muti-line if/loop body.
>
>v2 --> v3:
>  use dev_err/info(host->dev) instead of dev_err/info(NULL).
>  use MTDDEBUG() instead of pr_debug().
>
>v1 --> v2:
>  use dev_err/info to replace printk.
>  fix typo mistake, that should use <= instead of use <.
>
> drivers/mtd/nand/atmel_nand.c |  130 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 128 insertions(+), 2 deletions(-)

applied to u-boot-atmel/master, thanks!

Best regards,
Andreas Bie?mann