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

[U-Boot] [PATCH v2 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.

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 v2 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 v2 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 v2 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 v2 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>
---
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..3cd403f 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(NULL, "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 v2 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>
---
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 3cd403f..ceba562 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))
+			pr_debug("ONFI params, minimum required ECC: %d bits in %d bytes\n",
+				*cap, *sector_size);
+		else
+			dev_info(NULL, "NAND chip ECC reqirement is in Extended ONFI parameter, we don't support yet.\n");
+	} else {
+		dev_info(NULL, "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(NULL, "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(NULL, "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(NULL, "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 v2 5/5] ARM: at91: atmel_nand: add code to check the ONFI parameter ECC requirement

[Scott Wood]

On 06/17/2013 05:51:21 AM, Josh Wu wrote:
> +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))
> +			pr_debug("ONFI params, minimum required ECC: %d  
> bits in %d bytes\n",
> +				*cap, *sector_size);
> +		else
> +			dev_info(NULL, "NAND chip ECC reqirement is in  
> Extended ONFI parameter, we don't support yet.\n");
> +	} else {
> +		dev_info(NULL, "NAND chip is not ONFI compliant, assume  
> ecc_bits is 2 in 512 bytes");
> +	}

Don't pass NULL to dev_info().  Either pass host->dev as the Linux code  
does (and use dev_dbg rather than pr_debug), or just use printf() (and  
debug()) if there's no intent to sync this change back to Linux.  It  
doesn't matter if host->dev doesn't exist in U-Boot, as the macro  
doesn't use that parameter, but the only reason to use Linux-style  
prints is to keep the code common with Linux.

-Scott

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

[Josh Wu]

Hi, Dear Scott

On 6/21/2013 4:57 AM, Scott Wood wrote:
> On 06/17/2013 05:51:21 AM, Josh Wu wrote:
>> +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))
>> +            pr_debug("ONFI params, minimum required ECC: %d bits in 
>> %d bytes\n",
>> +                *cap, *sector_size);
>> +        else
>> +            dev_info(NULL, "NAND chip ECC reqirement is in Extended 
>> ONFI parameter, we don't support yet.\n");
>> +    } else {
>> +        dev_info(NULL, "NAND chip is not ONFI compliant, assume 
>> ecc_bits is 2 in 512 bytes");
>> +    }
>
> Don't pass NULL to dev_info().  Either pass host->dev as the Linux 
> code does (and use dev_dbg rather than pr_debug), or just use printf() 
> (and debug()) if there's no intent to sync this change back to Linux.  
> It doesn't matter if host->dev doesn't exist in U-Boot, as the macro 
> doesn't use that parameter, but the only reason to use Linux-style 
> prints is to keep the code common with Linux.

understood. I prefer to use dev_xxx(host->dev, ...) to keep consistent 
with kernel.

I use pr_debug because it is defined as MTDDEBUG in mtd/mtd.h, that is 
exact what I want.
Since if use debug(), then when I enable debug, there are many mtd 
non-related message shows up.

Best Regards,
Josh Wu

>
> -Scott

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

[Scott Wood]

On 06/25/2013 01:33:32 AM, Josh Wu wrote:
> Hi, Dear Scott
> 
> On 6/21/2013 4:57 AM, Scott Wood wrote:
>> On 06/17/2013 05:51:21 AM, Josh Wu wrote:
>>> +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))
>>> +            pr_debug("ONFI params, minimum required ECC: %d bits  
>>> in %d bytes\n",
>>> +                *cap, *sector_size);
>>> +        else
>>> +            dev_info(NULL, "NAND chip ECC reqirement is in  
>>> Extended ONFI parameter, we don't support yet.\n");
>>> +    } else {
>>> +        dev_info(NULL, "NAND chip is not ONFI compliant, assume  
>>> ecc_bits is 2 in 512 bytes");
>>> +    }
>> 
>> Don't pass NULL to dev_info().  Either pass host->dev as the Linux  
>> code does (and use dev_dbg rather than pr_debug), or just use  
>> printf() (and debug()) if there's no intent to sync this change back  
>> to Linux.  It doesn't matter if host->dev doesn't exist in U-Boot,  
>> as the macro doesn't use that parameter, but the only reason to use  
>> Linux-style prints is to keep the code common with Linux.
> 
> understood. I prefer to use dev_xxx(host->dev, ...) to keep  
> consistent with kernel.
> 
> I use pr_debug because it is defined as MTDDEBUG in mtd/mtd.h, that  
> is exact what I want.
> Since if use debug(), then when I enable debug, there are many mtd  
> non-related message shows up.

If pr_debug() and dev_dbg() are behaving differently in U-Boot, that's  
a bug that should be fixed.

-Scott