All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot-Users] [PATCH RFC] ARM: Davinci: NAND fix for large page ECC and linux compatibility
@ 2008-06-27  5:12 Bernard Blackham
  2008-06-27  6:44 ` Jean-Christophe PLAGNIOL-VILLARD
  2008-08-30 21:06 ` [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND Hugo Villeneuve
  0 siblings, 2 replies; 6+ messages in thread
From: Bernard Blackham @ 2008-06-27  5:12 UTC (permalink / raw)
  To: u-boot

U-boot's HW ECC support for large page NAND on Davinci is completely
broken.  Some kernels, such as the 2.6.10 one supported by
MontaVista for Davinci, rely upon this broken behaviour as they
share the same code for ECCs. In the existing scheme, error
detection *might* work on large page, but error correction
definitely does not.  Small page ECC correction works, but the
format is not compatible with the mainline git kernel.

This patch adds ECC code that matches what is currently in the
Davinci git repository (since NAND support was added in 2.6.24).
This makes the ECC and OOB layout written by u-boot compatible with
Linux for both small page and large page devices and fixes ECC
correction for large page devices.

The code depends on a #define CFG_LINUX_COMPATIBLE_ECC, which is
undefined by default, making the default state backwards compatible.
I have verified this by compiling without the #define and producing
a binary byte-for-byte identical to one without this patch.

[NOTE: I have not yet been able to get my hands on a board with
small-page NAND to test, but large page does work. If anybody is
interested in testing it, please do and let me know if it works for
you (i.e. uboot with this patch and davinci git kernel can
read/write the same NAND).]

Signed-off-by: Bernard Blackham <bernard@largestprime.net>

---
 cpu/arm926ejs/davinci/nand.c    |   79 ++++++++++++++++++++++++++++++++++++++--
 include/configs/davinci_dvevm.h |   12 ++++++
 2 files changed, 89 insertions(+), 2 deletions(-)

Index: u-boot-1.3.3/cpu/arm926ejs/davinci/nand.c
===================================================================
--- u-boot-1.3.3.orig/cpu/arm926ejs/davinci/nand.c	2008-05-19 18:47:11.000000000 +0800
+++ u-boot-1.3.3/cpu/arm926ejs/davinci/nand.c	2008-06-27 13:04:03.000000000 +0800
@@ -87,6 +87,10 @@ static void nand_davinci_select_chip(str
 }
 
 #ifdef CFG_NAND_HW_ECC
+
+#ifndef CFG_LINUX_COMPATIBLE_ECC
+/* Linux-compatible ECC uses MTD defaults. */
+/* These layouts are not compatible with Linux or RBL/UBL. */
 #ifdef CFG_NAND_LARGEPAGE
 static struct nand_oobinfo davinci_nand_oobinfo = {
 	.useecc = MTD_NANDECC_AUTOPLACE,
@@ -104,6 +108,7 @@ static struct nand_oobinfo davinci_nand_
 #else
 #error "Either CFG_NAND_LARGEPAGE or CFG_NAND_SMALLPAGE must be defined!"
 #endif
+#endif
 
 static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
 {
@@ -141,12 +146,29 @@ static u_int32_t nand_davinci_readecc(st
 
 static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat, u_char *ecc_code)
 {
+#ifdef CFG_LINUX_COMPATIBLE_ECC
+	unsigned int ecc_val = nand_davinci_readecc(mtd, 1);
+	/* squeeze 0 middle bits out so that it fits in 3 bytes */
+	unsigned int tmp = (ecc_val&0x0fff)|((ecc_val&0x0fff0000)>>4);
+	/* invert so that erased block ecc is correct */
+	tmp = ~tmp;
+	ecc_code[0] = (u_char)(tmp);
+	ecc_code[1] = (u_char)(tmp >> 8);
+	ecc_code[2] = (u_char)(tmp >> 16);
+#else
 	u_int32_t		tmp;
 	int			region, n;
 	struct nand_chip	*this = mtd->priv;
 
 	n = (this->eccmode == NAND_ECC_HW12_2048) ? 4 : 1;
 
+	/*
+	 * This is not how you should read ECCs on large page Davinci devices.
+	 * The region parameter gets you ECCs for flash chips on different chip
+	 * selects, not the 4x512 byte pages in a 2048 byte page.
+	 *
+	 * Preserved for backwards compatibility though.
+	 */
 	region = 1;
 	while (n--) {
 		tmp = nand_davinci_readecc(mtd, region);
@@ -155,9 +177,51 @@ static int nand_davinci_calculate_ecc(st
 		*ecc_code++ = ((tmp >> 8) & 0x0f) | ((tmp >> 20) & 0xf0);
 		region++;
 	}
+#endif
+
 	return(0);
 }
 
+#ifdef CFG_LINUX_COMPATIBLE_ECC
+static int nand_davinci_correct_data(struct mtd_info *mtd, u_char *dat,
+				u_char *read_ecc, u_char *calc_ecc)
+{
+	struct nand_chip *chip = mtd->priv;
+	u_int32_t ecc_nand = read_ecc[0] | (read_ecc[1] << 8) |
+					  (read_ecc[2] << 16);
+	u_int32_t ecc_calc = calc_ecc[0] | (calc_ecc[1] << 8) |
+					  (calc_ecc[2] << 16);
+	u_int32_t diff = ecc_calc ^ ecc_nand;
+
+	if (diff) {
+		if ((((diff>>12)^diff) & 0xfff) == 0xfff) {
+			/* Correctable error */
+			if ((diff>>(12+3)) < chip->eccsize) {
+				uint8_t find_bit = 1 << ((diff>>12)&7);
+				uint32_t find_byte = diff>>(12+3);
+				dat[find_byte] ^= find_bit;
+				DEBUG (MTD_DEBUG_LEVEL0, "Correcting single bit ECC error at offset: %d, bit: %d\n", find_byte, find_bit);
+				return 1;
+			} else {
+				return -1;
+			}
+		} else if (!(diff & (diff-1))) {
+			/* Single bit ECC error in the ECC itself,
+			   nothing to fix */
+			DEBUG (MTD_DEBUG_LEVEL0, "Single bit ECC error in ECC.\n");
+			return 1;
+		} else {
+			/* Uncorrectable error */
+			DEBUG (MTD_DEBUG_LEVEL0, "ECC UNCORRECTED_ERROR 1\n");
+			return -1;
+		}
+
+	}
+	return 0;
+}
+
+#else
+
 static void nand_davinci_gen_true_ecc(u_int8_t *ecc_buf)
 {
 	u_int32_t	tmp = ecc_buf[0] | (ecc_buf[1] << 16) | ((ecc_buf[2] & 0xf0) << 20) | ((ecc_buf[2] & 0x0f) << 8);
@@ -291,7 +355,9 @@ static int nand_davinci_correct_data(str
 	}
 	return(0);
 }
-#endif
+#endif /* CFG_LINUX_COMPATIBLE_ECC */
+
+#endif /* CFG_NAND_HW_ECC */
 
 static int nand_davinci_dev_ready(struct mtd_info *mtd)
 {
@@ -356,7 +422,13 @@ int board_nand_init(struct nand_chip *na
 #ifdef CFG_NAND_USE_FLASH_BBT
 	nand->options	  = NAND_USE_FLASH_BBT;
 #endif
+
 #ifdef CFG_NAND_HW_ECC
+
+#ifdef CFG_LINUX_COMPATIBLE_ECC
+	nand->eccmode     = NAND_ECC_HW3_512;
+#else
+
 #ifdef CFG_NAND_LARGEPAGE
 	nand->eccmode     = NAND_ECC_HW12_2048;
 #elif defined(CFG_NAND_SMALLPAGE)
@@ -365,12 +437,15 @@ int board_nand_init(struct nand_chip *na
 #error "Either CFG_NAND_LARGEPAGE or CFG_NAND_SMALLPAGE must be defined!"
 #endif
 	nand->autooob	  = &davinci_nand_oobinfo;
+
+#endif /* CFG_LINUX_COMPATIBLE_ECC */
+
 	nand->calculate_ecc = nand_davinci_calculate_ecc;
 	nand->correct_data  = nand_davinci_correct_data;
 	nand->enable_hwecc  = nand_davinci_enable_hwecc;
 #else
 	nand->eccmode     = NAND_ECC_SOFT;
-#endif
+#endif /* CFG_NAND_HW_ECC */
 
 	/* Set address of hardware control function */
 	nand->hwcontrol = nand_davinci_hwcontrol;
Index: u-boot-1.3.3/include/configs/davinci_dvevm.h
===================================================================
--- u-boot-1.3.3.orig/include/configs/davinci_dvevm.h	2008-05-19 18:47:11.000000000 +0800
+++ u-boot-1.3.3/include/configs/davinci_dvevm.h	2008-06-27 13:04:07.000000000 +0800
@@ -46,6 +46,18 @@
 #define CONFIG_NOR_UART_BOOT
  */
 
+/*
+ * Previous versions of u-boot (1.3.3 and prior) and Montavista Linux kernels
+ * generated bogus ECCs on large-page NAND. Both large and small page NAND ECCs
+ * were incompatible with the Linux davinci git tree (since NAND was integrated
+ * in 2.6.24).
+ * Don't turn this on if you want backwards compatibility.
+ * Do turn this on if you want u-boot to be able to read and write NAND
+ * that can be written or read by the Linux davinci git kernel.
+ *
+#define CFG_LINUX_COMPATIBLE_ECC
+ */
+
 /*=======*/
 /* Board */
 /*=======*/

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

* [U-Boot-Users] [PATCH RFC] ARM: Davinci: NAND fix for large page ECC and linux compatibility
  2008-06-27  5:12 [U-Boot-Users] [PATCH RFC] ARM: Davinci: NAND fix for large page ECC and linux compatibility Bernard Blackham
@ 2008-06-27  6:44 ` Jean-Christophe PLAGNIOL-VILLARD
  2008-08-30 21:06 ` [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND Hugo Villeneuve
  1 sibling, 0 replies; 6+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2008-06-27  6:44 UTC (permalink / raw)
  To: u-boot

>  #endif
> +#endif
>  
>  static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
>  {
> @@ -141,12 +146,29 @@ static u_int32_t nand_davinci_readecc(st
>  
>  static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat, u_char *ecc_code)
>  {
> +#ifdef CFG_LINUX_COMPATIBLE_ECC
> +	unsigned int ecc_val = nand_davinci_readecc(mtd, 1);
> +	/* squeeze 0 middle bits out so that it fits in 3 bytes */
> +	unsigned int tmp = (ecc_val&0x0fff)|((ecc_val&0x0fff0000)>>4);
	unsigned int tmp = (ecc_val & 0x0fff) | ((ecc_val & 0x0fff0000) >> 4);
please and space between operator

plese use the same alignement
add an empty line
> +	/* invert so that erased block ecc is correct */
> +	tmp = ~tmp;
> +	ecc_code[0] = (u_char)(tmp);
> +	ecc_code[1] = (u_char)(tmp >> 8);
> +	ecc_code[2] = (u_char)(tmp >> 16);
> +#else
>  	u_int32_t		tmp;
>  	int			region, n;
>  	struct nand_chip	*this = mtd->priv;
>  
>  	n = (this->eccmode == NAND_ECC_HW12_2048) ? 4 : 1;
>  
> +				u_char *read_ecc, u_char *calc_ecc)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	u_int32_t ecc_nand = read_ecc[0] | (read_ecc[1] << 8) |
> +					  (read_ecc[2] << 16);
> +	u_int32_t ecc_calc = calc_ecc[0] | (calc_ecc[1] << 8) |
> +					  (calc_ecc[2] << 16);
> +	u_int32_t diff = ecc_calc ^ ecc_nand;
> +
> +	if (diff) {
> +		if ((((diff>>12)^diff) & 0xfff) == 0xfff) {
please and space between operator
> +			/* Correctable error */
please and space between operator
> +			if ((diff>>(12+3)) < chip->eccsize) {
> +				uint8_t find_bit = 1 << ((diff>>12)&7);
please and space between operator
> +				uint32_t find_byte = diff>>(12+3);
				uint32_t find_byte = diff >> 15;
please and space between operator
> +				dat[find_byte] ^= find_bit;
> +				DEBUG (MTD_DEBUG_LEVEL0, "Correcting single bit ECC error at offset: %d, bit: %d\n", find_byte, find_bit);
too long please split
> +				return 1;
> +			} else {
> +				return -1;
> +			}
> +		} else if (!(diff & (diff-1))) {
please and space between operator
> +			/* Single bit ECC error in the ECC itself,
> +			   nothing to fix */
please use this style of comment
/*
 *
 */
> +			DEBUG (MTD_DEBUG_LEVEL0, "Single bit ECC error in ECC.\n");
> +			return 1;
> +		} else {
> +			/* Uncorrectable error */
> +			DEBUG (MTD_DEBUG_LEVEL0, "ECC UNCORRECTED_ERROR 1\n");
> +			return -1;
>  	/* Set address of hardware control function */
>  	nand->hwcontrol = nand_davinci_hwcontrol;
> Index: u-boot-1.3.3/include/configs/davinci_dvevm.h
> ===================================================================
> --- u-boot-1.3.3.orig/include/configs/davinci_dvevm.h	2008-05-19 18:47:11.000000000 +0800
> +++ u-boot-1.3.3/include/configs/davinci_dvevm.h	2008-06-27 13:04:07.000000000 +0800
> @@ -46,6 +46,18 @@
>  #define CONFIG_NOR_UART_BOOT
>   */
>  
> +/*
> + * Previous versions of u-boot (1.3.3 and prior) and Montavista Linux kernels
> + * generated bogus ECCs on large-page NAND. Both large and small page NAND ECCs
> + * were incompatible with the Linux davinci git tree (since NAND was integrated
> + * in 2.6.24).
> + * Don't turn this on if you want backwards compatibility.
> + * Do turn this on if you want u-boot to be able to read and write NAND
> + * that can be written or read by the Linux davinci git kernel.
> + *
> +#define CFG_LINUX_COMPATIBLE_ECC
> + */
please move this in README.davinci
> +

Best Regards,
J.

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

* [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND.
  2008-06-27  5:12 [U-Boot-Users] [PATCH RFC] ARM: Davinci: NAND fix for large page ECC and linux compatibility Bernard Blackham
  2008-06-27  6:44 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2008-08-30 21:06 ` Hugo Villeneuve
  2008-08-30 22:27   ` ksi at koi8.net
                     ` (2 more replies)
  1 sibling, 3 replies; 6+ messages in thread
From: Hugo Villeneuve @ 2008-08-30 21:06 UTC (permalink / raw)
  To: u-boot

ARM DaVinci: Fix broken HW ECC for large page NAND.

Signed-off-by: Hugo Villeneuve <hugo.villeneuve@lyrtech.com>

---

Based on original patch by Bernard Blackham <bernard@largestprime.net>

U-boot's HW ECC support for large page NAND on Davinci is completely
broken.  Some kernels, such as the 2.6.10 one supported by
MontaVista for DaVinci, rely upon this broken behaviour as they
share the same code for ECCs. In the existing scheme, error
detection *might* work on large page, but error correction
definitely does not.  Small page ECC correction works, but the
format is not compatible with the mainline git kernel.

This patch adds ECC code that matches what is currently in the
Davinci git repository (since NAND support was added in 2.6.24).
This makes the ECC and OOB layout written by u-boot compatible with
Linux for both small page and large page devices and fixes ECC
correction for large page devices.

The old behaviour can be restored by defining the macro
CFG_DAVINCI_BROKEN_ECC, which is undefined by default.

 cpu/arm926ejs/davinci/nand.c |   81 +++++++++++++++++++++++++++++++++++++++---
 doc/README.nand              |    8 ++++
 2 files changed, 84 insertions(+), 5 deletions(-)

diff --git a/cpu/arm926ejs/davinci/nand.c b/cpu/arm926ejs/davinci/nand.c
index 5a1da63..f7cf0c9 100644
--- a/cpu/arm926ejs/davinci/nand.c
+++ b/cpu/arm926ejs/davinci/nand.c
@@ -88,6 +88,9 @@ static void nand_davinci_select_chip(struct mtd_info *mtd, int chip)
 }
 
 #ifdef CFG_NAND_HW_ECC
+#ifdef CFG_DAVINCI_BROKEN_ECC
+/* Linux-compatible ECC uses MTD defaults. */
+/* These layouts are not compatible with Linux or RBL/UBL. */
 #ifdef CFG_NAND_LARGEPAGE
 static struct nand_ecclayout davinci_nand_ecclayout = {
 	.eccbytes = 12,
@@ -112,6 +115,7 @@ static struct nand_ecclayout davinci_nand_ecclayout = {
 #else
 #error "Either CFG_NAND_LARGEPAGE or CFG_NAND_SMALLPAGE must be defined!"
 #endif
+#endif /* CFG_DAVINCI_BROKEN_ECC */
 
 static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
 {
@@ -150,6 +154,15 @@ static u_int32_t nand_davinci_readecc(struct mtd_info *mtd, u_int32_t region)
 static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat, u_char *ecc_code)
 {
 	u_int32_t		tmp;
+#ifdef CFG_DAVINCI_BROKEN_ECC
+	/*
+	 * This is not how you should read ECCs on large page Davinci devices.
+	 * The region parameter gets you ECCs for flash chips on different chip
+	 * selects, not the 4x512 byte pages in a 2048 byte page.
+	 *
+	 * Preserved for backwards compatibility though.
+	 */
+
 	int			region, n;
 	struct nand_chip	*this = mtd->priv;
 
@@ -163,9 +176,26 @@ static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat, u
 		*ecc_code++ = ((tmp >> 8) & 0x0f) | ((tmp >> 20) & 0xf0);
 		region++;
 	}
+#else
+	const int region = 1;
+
+	tmp = nand_davinci_readecc(mtd, region);
+
+	/* Squeeze 4 bytes ECC into 3 bytes by removing RESERVED bits
+	 * and shifting. RESERVED bits are 31 to 28 and 15 to 12. */
+	tmp = (tmp & 0x00000fff) | ((tmp & 0x0fff0000) >> 4);
+
+	/* Invert so that erased block ECC is correct */
+	tmp = ~tmp;
+
+	*ecc_code++ = tmp;
+	*ecc_code++ = tmp >>  8;
+	*ecc_code++ = tmp >> 16;
+#endif /* CFG_DAVINCI_BROKEN_ECC */
 	return(0);
 }
 
+#ifdef CFG_DAVINCI_BROKEN_ECC
 static void nand_davinci_gen_true_ecc(u_int8_t *ecc_buf)
 {
 	u_int32_t	tmp = ecc_buf[0] | (ecc_buf[1] << 16) | ((ecc_buf[2] & 0xf0) << 20) | ((ecc_buf[2] & 0x0f) << 8);
@@ -282,13 +312,14 @@ static int nand_davinci_compare_ecc(u_int8_t *ecc_nand, u_int8_t *ecc_calc, u_in
 			return(-1);
 	}
 }
+#endif /* CFG_DAVINCI_BROKEN_ECC */
 
 static int nand_davinci_correct_data(struct mtd_info *mtd, u_char *dat, u_char *read_ecc, u_char *calc_ecc)
 {
-	struct nand_chip	*this;
+	struct nand_chip *this = mtd->priv;
+#ifdef CFG_DAVINCI_BROKEN_ECC
 	int			block_count = 0, i, rc;
 
-	this = mtd->priv;
 	block_count = (this->ecc.size/512);
 	for (i = 0; i < block_count; i++) {
 		if (memcmp(read_ecc, calc_ecc, 3) != 0) {
@@ -301,9 +332,44 @@ static int nand_davinci_correct_data(struct mtd_info *mtd, u_char *dat, u_char *
 		calc_ecc += 3;
 		dat += 512;
 	}
+#else
+	u_int32_t ecc_nand = read_ecc[0] | (read_ecc[1] << 8) |
+					  (read_ecc[2] << 16);
+	u_int32_t ecc_calc = calc_ecc[0] | (calc_ecc[1] << 8) |
+					  (calc_ecc[2] << 16);
+	u_int32_t diff = ecc_calc ^ ecc_nand;
+
+	if (diff) {
+		if ((((diff >> 12) ^ diff) & 0xfff) == 0xfff) {
+			/* Correctable error */
+			if ((diff >> (12 + 3)) < this->ecc.size) {
+				uint8_t find_bit = 1 << ((diff >> 12) & 7);
+				uint32_t find_byte = diff >> (12 + 3);
+
+				dat[find_byte] ^= find_bit;
+				MTDDEBUG(MTD_DEBUG_LEVEL0, "Correcting single "
+					 "bit ECC error at offset: %d, bit: "
+					 "%d\n", find_byte, find_bit);
+				return 1;
+			} else {
+				return -1;
+			}
+		} else if (!(diff & (diff - 1))) {
+			/* Single bit ECC error in the ECC itself,
+			   nothing to fix */
+			MTDDEBUG(MTD_DEBUG_LEVEL0, "Single bit ECC error in "
+				 "ECC.\n");
+			return 1;
+		} else {
+			/* Uncorrectable error */
+			MTDDEBUG(MTD_DEBUG_LEVEL0, "ECC UNCORRECTED_ERROR 1\n");
+			return -1;
+		}
+	}
+#endif /* CFG_DAVINCI_BROKEN_ECC */
 	return(0);
 }
-#endif
+#endif /* CFG_NAND_HW_ECC */
 
 static int nand_davinci_dev_ready(struct mtd_info *mtd)
 {
@@ -370,6 +436,8 @@ int board_nand_init(struct nand_chip *nand)
 #endif
 #ifdef CFG_NAND_HW_ECC
 	nand->ecc.mode = NAND_ECC_HW;
+#ifdef CFG_DAVINCI_BROKEN_ECC
+	nand->ecc.layout  = &davinci_nand_ecclayout;
 #ifdef CFG_NAND_LARGEPAGE
 	nand->ecc.size = 2048;
 	nand->ecc.bytes = 12;
@@ -379,13 +447,16 @@ int board_nand_init(struct nand_chip *nand)
 #else
 #error "Either CFG_NAND_LARGEPAGE or CFG_NAND_SMALLPAGE must be defined!"
 #endif
-	nand->ecc.layout  = &davinci_nand_ecclayout;
+#else
+	nand->ecc.size = 512;
+	nand->ecc.bytes = 3;
+#endif /* CFG_DAVINCI_BROKEN_ECC */
 	nand->ecc.calculate = nand_davinci_calculate_ecc;
 	nand->ecc.correct  = nand_davinci_correct_data;
 	nand->ecc.hwctl  = nand_davinci_enable_hwecc;
 #else
 	nand->ecc.mode = NAND_ECC_SOFT;
-#endif
+#endif /* CFG_NAND_HW_ECC */
 
 	/* Set address of hardware control function */
 	nand->cmd_ctrl = nand_davinci_hwcontrol;
diff --git a/doc/README.nand b/doc/README.nand
index 171380e..6dac24c 100644
--- a/doc/README.nand
+++ b/doc/README.nand
@@ -174,6 +174,14 @@ More Definitions:
    #define NAND_MAX_FLOORS 1
    #define NAND_MAX_CHIPS 1
 
+   #define CFG_DAVINCI_BROKEN_ECC
+      Versions of U-Boot <= 1.3.3 and Montavista Linux kernels
+      generated bogus ECCs on large-page NAND. Both large and small page
+      NAND ECCs were incompatible with the Linux davinci git tree (since
+      NAND was integrated in 2.6.24).
+      Turn this ON if you want backwards compatibility.
+      Turn this OFF if you want U-Boot and the Linux davinci git kernel
+      to use the same ECC format.
 
 NOTE:
 =====

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

* [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND.
  2008-08-30 21:06 ` [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND Hugo Villeneuve
@ 2008-08-30 22:27   ` ksi at koi8.net
  2008-08-31  2:21   ` Bernard Blackham
  2008-09-08 17:48   ` Scott Wood
  2 siblings, 0 replies; 6+ messages in thread
From: ksi at koi8.net @ 2008-08-30 22:27 UTC (permalink / raw)
  To: u-boot

On Sat, 30 Aug 2008, Hugo Villeneuve wrote:

ACK-by: Sergey Kubushyn <ksi@koi8.net>

> ARM DaVinci: Fix broken HW ECC for large page NAND.
>
> Signed-off-by: Hugo Villeneuve <hugo.villeneuve@lyrtech.com>
>
> ---
>
> Based on original patch by Bernard Blackham <bernard@largestprime.net>
>
> U-boot's HW ECC support for large page NAND on Davinci is completely
> broken.  Some kernels, such as the 2.6.10 one supported by
> MontaVista for DaVinci, rely upon this broken behaviour as they
> share the same code for ECCs. In the existing scheme, error
> detection *might* work on large page, but error correction
> definitely does not.  Small page ECC correction works, but the
> format is not compatible with the mainline git kernel.
>
> This patch adds ECC code that matches what is currently in the
> Davinci git repository (since NAND support was added in 2.6.24).
> This makes the ECC and OOB layout written by u-boot compatible with
> Linux for both small page and large page devices and fixes ECC
> correction for large page devices.
>
> The old behaviour can be restored by defining the macro
> CFG_DAVINCI_BROKEN_ECC, which is undefined by default.
>
> cpu/arm926ejs/davinci/nand.c |   81
> +++++++++++++++++++++++++++++++++++++++---
> doc/README.nand              |    8 ++++
> 2 files changed, 84 insertions(+), 5 deletions(-)
>
> diff --git a/cpu/arm926ejs/davinci/nand.c b/cpu/arm926ejs/davinci/nand.c
> index 5a1da63..f7cf0c9 100644
> --- a/cpu/arm926ejs/davinci/nand.c
> +++ b/cpu/arm926ejs/davinci/nand.c
> @@ -88,6 +88,9 @@ static void nand_davinci_select_chip(struct mtd_info
> *mtd, int chip)
> }
>
> #ifdef CFG_NAND_HW_ECC
> +#ifdef CFG_DAVINCI_BROKEN_ECC
> +/* Linux-compatible ECC uses MTD defaults. */
> +/* These layouts are not compatible with Linux or RBL/UBL. */
> #ifdef CFG_NAND_LARGEPAGE
> static struct nand_ecclayout davinci_nand_ecclayout = {
> 	.eccbytes = 12,
> @@ -112,6 +115,7 @@ static struct nand_ecclayout davinci_nand_ecclayout
> = {
> #else
> #error "Either CFG_NAND_LARGEPAGE or CFG_NAND_SMALLPAGE must be
> defined!"
> #endif
> +#endif /* CFG_DAVINCI_BROKEN_ECC */
>
> static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
> {
> @@ -150,6 +154,15 @@ static u_int32_t nand_davinci_readecc(struct
> mtd_info *mtd, u_int32_t region)
> static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const
> u_char *dat, u_char *ecc_code)
> {
> 	u_int32_t		tmp;
> +#ifdef CFG_DAVINCI_BROKEN_ECC
> +	/*
> +	 * This is not how you should read ECCs on large page Davinci
> devices.
> +	 * The region parameter gets you ECCs for flash chips on
> different chip
> +	 * selects, not the 4x512 byte pages in a 2048 byte page.
> +	 *
> +	 * Preserved for backwards compatibility though.
> +	 */
> +
> 	int			region, n;
> 	struct nand_chip	*this = mtd->priv;
>
> @@ -163,9 +176,26 @@ static int nand_davinci_calculate_ecc(struct
> mtd_info *mtd, const u_char *dat, u
> 		*ecc_code++ = ((tmp >> 8) & 0x0f) | ((tmp >> 20) &
> 0xf0);
> 		region++;
> 	}
> +#else
> +	const int region = 1;
> +
> +	tmp = nand_davinci_readecc(mtd, region);
> +
> +	/* Squeeze 4 bytes ECC into 3 bytes by removing RESERVED bits
> +	 * and shifting. RESERVED bits are 31 to 28 and 15 to 12. */
> +	tmp = (tmp & 0x00000fff) | ((tmp & 0x0fff0000) >> 4);
> +
> +	/* Invert so that erased block ECC is correct */
> +	tmp = ~tmp;
> +
> +	*ecc_code++ = tmp;
> +	*ecc_code++ = tmp >>  8;
> +	*ecc_code++ = tmp >> 16;
> +#endif /* CFG_DAVINCI_BROKEN_ECC */
> 	return(0);
> }
>
> +#ifdef CFG_DAVINCI_BROKEN_ECC
> static void nand_davinci_gen_true_ecc(u_int8_t *ecc_buf)
> {
> 	u_int32_t	tmp = ecc_buf[0] | (ecc_buf[1] << 16) |
> ((ecc_buf[2] & 0xf0) << 20) | ((ecc_buf[2] & 0x0f) << 8);
> @@ -282,13 +312,14 @@ static int nand_davinci_compare_ecc(u_int8_t
> *ecc_nand, u_int8_t *ecc_calc, u_in
> 			return(-1);
> 	}
> }
> +#endif /* CFG_DAVINCI_BROKEN_ECC */
>
> static int nand_davinci_correct_data(struct mtd_info *mtd, u_char *dat,
> u_char *read_ecc, u_char *calc_ecc)
> {
> -	struct nand_chip	*this;
> +	struct nand_chip *this = mtd->priv;
> +#ifdef CFG_DAVINCI_BROKEN_ECC
> 	int			block_count = 0, i, rc;
>
> -	this = mtd->priv;
> 	block_count = (this->ecc.size/512);
> 	for (i = 0; i < block_count; i++) {
> 		if (memcmp(read_ecc, calc_ecc, 3) != 0) {
> @@ -301,9 +332,44 @@ static int nand_davinci_correct_data(struct
> mtd_info *mtd, u_char *dat, u_char *
> 		calc_ecc += 3;
> 		dat += 512;
> 	}
> +#else
> +	u_int32_t ecc_nand = read_ecc[0] | (read_ecc[1] << 8) |
> +					  (read_ecc[2] << 16);
> +	u_int32_t ecc_calc = calc_ecc[0] | (calc_ecc[1] << 8) |
> +					  (calc_ecc[2] << 16);
> +	u_int32_t diff = ecc_calc ^ ecc_nand;
> +
> +	if (diff) {
> +		if ((((diff >> 12) ^ diff) & 0xfff) == 0xfff) {
> +			/* Correctable error */
> +			if ((diff >> (12 + 3)) < this->ecc.size) {
> +				uint8_t find_bit = 1 << ((diff >> 12) &
> 7);
> +				uint32_t find_byte = diff >> (12 + 3);
> +
> +				dat[find_byte] ^= find_bit;
> +				MTDDEBUG(MTD_DEBUG_LEVEL0, "Correcting
> single "
> +					 "bit ECC error at offset: %d,
> bit: "
> +					 "%d\n", find_byte, find_bit);
> +				return 1;
> +			} else {
> +				return -1;
> +			}
> +		} else if (!(diff & (diff - 1))) {
> +			/* Single bit ECC error in the ECC itself,
> +			   nothing to fix */
> +			MTDDEBUG(MTD_DEBUG_LEVEL0, "Single bit ECC error
> in "
> +				 "ECC.\n");
> +			return 1;
> +		} else {
> +			/* Uncorrectable error */
> +			MTDDEBUG(MTD_DEBUG_LEVEL0, "ECC
> UNCORRECTED_ERROR 1\n");
> +			return -1;
> +		}
> +	}
> +#endif /* CFG_DAVINCI_BROKEN_ECC */
> 	return(0);
> }
> -#endif
> +#endif /* CFG_NAND_HW_ECC */
>
> static int nand_davinci_dev_ready(struct mtd_info *mtd)
> {
> @@ -370,6 +436,8 @@ int board_nand_init(struct nand_chip *nand)
> #endif
> #ifdef CFG_NAND_HW_ECC
> 	nand->ecc.mode = NAND_ECC_HW;
> +#ifdef CFG_DAVINCI_BROKEN_ECC
> +	nand->ecc.layout  = &davinci_nand_ecclayout;
> #ifdef CFG_NAND_LARGEPAGE
> 	nand->ecc.size = 2048;
> 	nand->ecc.bytes = 12;
> @@ -379,13 +447,16 @@ int board_nand_init(struct nand_chip *nand)
> #else
> #error "Either CFG_NAND_LARGEPAGE or CFG_NAND_SMALLPAGE must be
> defined!"
> #endif
> -	nand->ecc.layout  = &davinci_nand_ecclayout;
> +#else
> +	nand->ecc.size = 512;
> +	nand->ecc.bytes = 3;
> +#endif /* CFG_DAVINCI_BROKEN_ECC */
> 	nand->ecc.calculate = nand_davinci_calculate_ecc;
> 	nand->ecc.correct  = nand_davinci_correct_data;
> 	nand->ecc.hwctl  = nand_davinci_enable_hwecc;
> #else
> 	nand->ecc.mode = NAND_ECC_SOFT;
> -#endif
> +#endif /* CFG_NAND_HW_ECC */
>
> 	/* Set address of hardware control function */
> 	nand->cmd_ctrl = nand_davinci_hwcontrol;
> diff --git a/doc/README.nand b/doc/README.nand
> index 171380e..6dac24c 100644
> --- a/doc/README.nand
> +++ b/doc/README.nand
> @@ -174,6 +174,14 @@ More Definitions:
>    #define NAND_MAX_FLOORS 1
>    #define NAND_MAX_CHIPS 1
>
> +   #define CFG_DAVINCI_BROKEN_ECC
> +      Versions of U-Boot <= 1.3.3 and Montavista Linux kernels
> +      generated bogus ECCs on large-page NAND. Both large and small
> page
> +      NAND ECCs were incompatible with the Linux davinci git tree
> (since
> +      NAND was integrated in 2.6.24).
> +      Turn this ON if you want backwards compatibility.
> +      Turn this OFF if you want U-Boot and the Linux davinci git kernel
> +      to use the same ECC format.
>
> NOTE:
> =====
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

---
******************************************************************
*  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************

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

* [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND.
  2008-08-30 21:06 ` [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND Hugo Villeneuve
  2008-08-30 22:27   ` ksi at koi8.net
@ 2008-08-31  2:21   ` Bernard Blackham
  2008-09-08 17:48   ` Scott Wood
  2 siblings, 0 replies; 6+ messages in thread
From: Bernard Blackham @ 2008-08-31  2:21 UTC (permalink / raw)
  To: u-boot

Hugo Villeneuve wrote:
> ARM DaVinci: Fix broken HW ECC for large page NAND.
> 
> Signed-off-by: Hugo Villeneuve <hugo.villeneuve@lyrtech.com>

Thanks for following up on this, Hugo.

> +   #define CFG_DAVINCI_BROKEN_ECC
> +      Versions of U-Boot <= 1.3.3 and Montavista Linux kernels
> +      generated bogus ECCs on large-page NAND. Both large and small page

This version should now be 1.3.4. Otherwise, ack.

Cheers,
Bernard.

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

* [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND.
  2008-08-30 21:06 ` [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND Hugo Villeneuve
  2008-08-30 22:27   ` ksi at koi8.net
  2008-08-31  2:21   ` Bernard Blackham
@ 2008-09-08 17:48   ` Scott Wood
  2 siblings, 0 replies; 6+ messages in thread
From: Scott Wood @ 2008-09-08 17:48 UTC (permalink / raw)
  To: u-boot

On Sat, Aug 30, 2008 at 05:06:55PM -0400, Hugo Villeneuve wrote:
> ARM DaVinci: Fix broken HW ECC for large page NAND.
> 
> Signed-off-by: Hugo Villeneuve <hugo.villeneuve@lyrtech.com>
> 
> ---
> 
> Based on original patch by Bernard Blackham <bernard@largestprime.net>
> 
> U-boot's HW ECC support for large page NAND on Davinci is completely
> broken.  Some kernels, such as the 2.6.10 one supported by
> MontaVista for DaVinci, rely upon this broken behaviour as they
> share the same code for ECCs. In the existing scheme, error
> detection *might* work on large page, but error correction
> definitely does not.  Small page ECC correction works, but the
> format is not compatible with the mainline git kernel.
> 
> This patch adds ECC code that matches what is currently in the
> Davinci git repository (since NAND support was added in 2.6.24).
> This makes the ECC and OOB layout written by u-boot compatible with
> Linux for both small page and large page devices and fixes ECC
> correction for large page devices.
> 
> The old behaviour can be restored by defining the macro
> CFG_DAVINCI_BROKEN_ECC, which is undefined by default.

Applied to u-boot-nand-flash.  Next time, please put the commit message
above the "---", except for extra commentary that isn't intended to go
into the git history.

-Scott

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

end of thread, other threads:[~2008-09-08 17:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-27  5:12 [U-Boot-Users] [PATCH RFC] ARM: Davinci: NAND fix for large page ECC and linux compatibility Bernard Blackham
2008-06-27  6:44 ` Jean-Christophe PLAGNIOL-VILLARD
2008-08-30 21:06 ` [U-Boot] [PATCH] ARM DaVinci: Fix broken HW ECC for large page NAND Hugo Villeneuve
2008-08-30 22:27   ` ksi at koi8.net
2008-08-31  2:21   ` Bernard Blackham
2008-09-08 17:48   ` Scott Wood

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.