Skip to contentSkip to conversation

ARM: at91: atmel_nand: check ONFI ecc minimum requirement

PATCH Superseded Josh Wu 3 participants 18 messages Jul 3, 2013 – Aug 22, 2013

Subsystems (5)
Arm (32-bit) USB Memory technology devices (MTD) Arm/Microchip (AT91) SoC support +1 more subsystems
lore
Share & tools
lore

Josh Wu

author
#0 lore raw

v3 cover ARM: at91: atmel_nand: check ONFI ecc minimum requirement

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(-)
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH v3 1/5 ARM: at91: atmel_nand: pmecc driver will select the galois table by sector size

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 <[email protected]>
---
 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(-)
arch/arm/include/asm/arch-at91/at91sam9x5.h +6 -0
index 85e42f5..de9fa50 100644
@@ -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__
doc/README.atmel_pmecc +0 -14
index b483744..41f3bd7 100644
@@ -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
drivers/mtd/nand/atmel_nand.c +4 -1
index 3bfbaf8..139a479 100644
@@ -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",
include/configs/at91sam9x5ek.h +0 -1
index ee6e3fc..4364026 100644
@@ -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
include/configs/sama5d3xek.h +0 -1
index c13e983..67f8180 100644
@@ -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
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH v3 1/5 ARM: at91: atmel_nand: pmecc driver will select the galois table by sector size

Dear Josh Wu,

Quoted 7 lines · 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 <[email protected]>
Acked-by Scott Wood <[email protected]>
[rebased on master]
Signed-off-by Andreas Bie?mann <[email protected]>
Quoted 7 lines

---

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

author
#3 lore raw

PATCH v3 2/5 ARM: at91: sama5d3: remove unused definition about PMECC alpha table offset

In reply to Josh Wu
Signed-off-by Josh Wu <[email protected]>
---
 arch/arm/include/asm/arch-at91/sama5d3.h |    2 --
 1 file changed, 2 deletions(-)
arch/arm/include/asm/arch-at91/sama5d3.h +0 -2
index 883b932..aff12a6 100644
@@ -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
Signature · 1 line
1.7.9.5
author
#5 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/usb/musb-new/linux-compat.h |   16 ----------------
 include/linux/compat.h              |    8 ++++++++
 2 files changed, 8 insertions(+), 16 deletions(-)
drivers/usb/musb-new/linux-compat.h +0 -16
index 72c8c2b..d7a5663 100644
@@ -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)
include/linux/compat.h +8 -0
index e1338bf..3fdfb39 100644
@@ -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
Signature · 1 line
1.7.9.5
#6 lore raw

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

Dear Josh Wu,

Quoted 4 lines · 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.

Quoted 8 lines

This patch also removed duplicated macro definitions in usb driver.

Signed-off-by Josh Wu <[email protected]>
Acked-by Scott Wood <[email protected]>

---
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

author
#7 lore raw

PATCH v3 4/5 mtd: atmel_nand: alloc memory instead of use static array for pmecc data

In reply to Josh Wu

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

Signed-off-by Josh Wu <[email protected]>

---
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(-)
drivers/mtd/nand/atmel_nand.c +56 -7
index 139a479..a4107fd 100644
@@ -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;
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH v3 4/5 mtd: atmel_nand: alloc memory instead of use static array for pmecc data

Dear Josh Wu,

Quoted 8 lines · Josh Wu

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

Signed-off-by Josh Wu <[email protected]>
Acked-by Scott Wood <[email protected]>

---
v2 --> v3:
use dev_err(host->dev) instead of using dev_err(NULL).

Quoted 7 lines

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

author
#9 lore raw

PATCH v3 5/5 ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

In reply to 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 <[email protected]>

---
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(-)
drivers/mtd/nand/atmel_nand.c +127 -2
index a4107fd..91afe5c 100644
@@ -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
Signature · 1 line
1.7.9.5
#10 lore raw

PATCH v3 5/5 ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

drivers/mtd/nand/atmel_nand.c static void atmel_pmecc_core_init(struct mtd_info *mtd) · line 735

Quoted 4 lines · Josh Wu
(quoted diff) +2 -0
+		if (!get_onfi_ecc_param(chip, cap, sector_size))
+			MTDDEBUG(MTD_DEBUG_LEVEL1, "ONFI params,

minimum required ECC: %d bits in %d bytes\n",

drivers/mtd/nand/atmel_nand.c +1 -0
+				*cap, *sector_size);

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

Otherwise, ACK this patchset.

-Scott

author
#11 lore raw

PATCH v3 5/5 ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

Quoted 10 lines · Scott Wood
Quoted 4 lines · Josh Wu
(quoted diff) +2 -0
+        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.

Quoted 2 lines

-Scott

Best Regards,
Josh Wu

author
#12 lore raw

PATCH RESEND v3 5/5 ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

In reply to Scott Wood

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 <[email protected]>

---
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(-)
drivers/mtd/nand/atmel_nand.c +128 -2
index a4107fd..3906be0 100644
@@ -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
Signature · 1 line
1.7.9.5
#13 lore raw

PATCH RESEND v3 5/5 ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

Dear Josh Wu,

Quoted 28 lines · 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 <[email protected]>
Acked-by Scott Wood <[email protected]>

---
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().

Quoted 6 lines

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

author
#14 lore raw

v3 cover ARM: at91: atmel_nand: check ONFI ecc minimum requirement

In reply to Josh Wu

Hi,

Any feedback for those patch series?

Best Regards,
Josh Wu

Quoted 35 lines · 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(-)
author
#16 lore raw

v3 cover ARM: at91: atmel_nand: check ONFI ecc minimum requirement

In reply to 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

Quoted 35 lines · 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(-)
#17 lore raw

v3 cover ARM: at91: atmel_nand: check ONFI ecc minimum requirement

Hi Josh,

Quoted 4 lines · Josh Wu

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