[U-Boot] [PATCH v2 0/3] mtd: nand: omap: add support for BCH16_ECC

[U-Boot] [PATCH v2 0/3] mtd: nand: omap: add support for BCH16_ECC

[Pekon Gupta]

*changes in v2*
- rebased for http://lists.denx.de/pipermail/u-boot/2013-September/162107.html
- minor code cleanup

*original v1*
This patch series add support of BCH16_ECC scheme.
As BCH16_ECC scheme generates 26bytes of ECC syndrome per 512B data,
hence this scheme is usable only for NAND devices having 4K or above
page-size, as their OOB/spare area has enough space to accomodate ECC.

This patch series is applicable over an above following series:
http://lists.denx.de/pipermail/u-boot/2013-September/161859.html

Pekon Gupta (3):
[PATCH 1/3] am33xx: elm: add support for BCH16_ECC - ELM driver updates
[PATCH 2/3] mtd: nand: omap: add support for BCH16_ECC - NAND driver updates
[PATCH 3/3] am335x: update README for BCH16

 arch/arm/cpu/armv7/am33xx/elm.c              | 95 ++++++++++++++++++----------
 arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
 arch/arm/include/asm/arch-am33xx/elm.h       | 16 ++---
 arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
 board/ti/am335x/README                       | 43 ++++++++++++-
 drivers/mtd/nand/omap_gpmc.c                 | 87 ++++++++++++++++++++-----
 include/mtd/mtd-abi.h                        |  3 +-
 7 files changed, 200 insertions(+), 63 deletions(-)

-- 
1.8.1

[U-Boot] [PATCH v2 1/3] am33xx: elm: add support for BCH16_ECC - ELM driver updates

[Pekon Gupta]

With increase in NAND flash densities occurence of bit-flips has increased.
Thus stronger ECC schemes are required for detecting and correcting multiple
simultaneous bit-flips in same NAND page. But stronger ECC schemes have large
ECC syndrome which require more space in OOB/Spare.
This patch add support for BCH16_ECC:
(a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
(b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.

Due to (b) this scheme can only be used with NAND devices which have enough
OOB to satisfy following equation:
OOBsize per page >= 26 * (page-size / 512)

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 arch/arm/cpu/armv7/am33xx/elm.c        | 95 ++++++++++++++++++++++------------
 arch/arm/include/asm/arch-am33xx/elm.h | 16 +++---
 2 files changed, 69 insertions(+), 42 deletions(-)

diff --git a/arch/arm/cpu/armv7/am33xx/elm.c b/arch/arm/cpu/armv7/am33xx/elm.c
index 8f1d6af..0196034 100644
--- a/arch/arm/cpu/armv7/am33xx/elm.c
+++ b/arch/arm/cpu/armv7/am33xx/elm.c
@@ -25,31 +25,27 @@
 struct elm *elm_cfg;
 
 /**
- * elm_load_syndromes - Load BCH syndromes based on nibble selection
+ * elm_load_syndromes - Load calc_ecc in ELM registers for error detection
  * @syndrome: BCH syndrome
- * @nibbles:
  * @poly: Syndrome Polynomial set to use
- *
- * Load BCH syndromes based on nibble selection
  */
-static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
+static int elm_load_syndromes(u8 *syndrome, u32 bch_type, u8 poly)
 {
 	u32 *ptr;
 	u32 val;
 
-	/* reg 0 */
-	ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[0];
-	val = syndrome[0] | (syndrome[1] << 8) | (syndrome[2] << 16) |
-				(syndrome[3] << 24);
-	writel(val, ptr);
-	/* reg 1 */
-	ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[1];
-	val = syndrome[4] | (syndrome[5] << 8) | (syndrome[6] << 16) |
-				(syndrome[7] << 24);
-	writel(val, ptr);
-
-	/* BCH 8-bit with 26 nibbles (4*8=32) */
-	if (nibbles > 13) {
+	switch (bch_type) {
+	case ECC_BCH16:
+		/* reg 0 */
+		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[0];
+		val = syndrome[0] | (syndrome[1] << 8) | (syndrome[2] << 16) |
+					(syndrome[3] << 24);
+		writel(val, ptr);
+		/* reg 1 */
+		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[1];
+		val = syndrome[4] | (syndrome[5] << 8) | (syndrome[6] << 16) |
+					(syndrome[7] << 24);
+		writel(val, ptr);
 		/* reg 2 */
 		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[2];
 		val = syndrome[8] | (syndrome[9] << 8) | (syndrome[10] << 16) |
@@ -60,34 +56,65 @@ static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
 		val = syndrome[12] | (syndrome[13] << 8) |
 			(syndrome[14] << 16) | (syndrome[15] << 24);
 		writel(val, ptr);
-	}
-
-	/* BCH 16-bit with 52 nibbles (7*8=56) */
-	if (nibbles > 26) {
 		/* reg 4 */
 		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[4];
 		val = syndrome[16] | (syndrome[17] << 8) |
 			(syndrome[18] << 16) | (syndrome[19] << 24);
 		writel(val, ptr);
-
 		/* reg 5 */
 		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[5];
 		val = syndrome[20] | (syndrome[21] << 8) |
 			(syndrome[22] << 16) | (syndrome[23] << 24);
 		writel(val, ptr);
-
 		/* reg 6 */
 		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[6];
-		val = syndrome[24] | (syndrome[25] << 8) |
-			(syndrome[26] << 16) | (syndrome[27] << 24);
+		val = syndrome[24] | (syndrome[25] << 8);
 		writel(val, ptr);
+		break;
+	case ECC_BCH8:
+		/* reg 0 */
+		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[0];
+		val = syndrome[0] | (syndrome[1] << 8) | (syndrome[2] << 16) |
+					(syndrome[3] << 24);
+		writel(val, ptr);
+		/* reg 1 */
+		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[1];
+		val = syndrome[4] | (syndrome[5] << 8) | (syndrome[6] << 16) |
+					(syndrome[7] << 24);
+		writel(val, ptr);
+		/* reg 2 */
+		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[2];
+		val = syndrome[8] | (syndrome[9] << 8) | (syndrome[10] << 16) |
+				(syndrome[11] << 24);
+		writel(val, ptr);
+		/* reg 3 */
+		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[3];
+		val = syndrome[12];
+		writel(val, ptr);
+		break;
+	case ECC_BCH4:
+		/* reg 0 */
+		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[0];
+		val = syndrome[0] | (syndrome[1] << 8) | (syndrome[2] << 16) |
+					(syndrome[3] << 24);
+		writel(val, ptr);
+		/* reg 1 */
+		ptr = &elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[1];
+		val = syndrome[4] | (syndrome[5] << 8) | (syndrome[6] << 16) |
+					(syndrome[7] << 24);
+		writel(val, ptr);
+		break;
+	default:
+		return -1;
 	}
+
+	return 0;
 }
 
 /**
- * elm_check_errors - Check for BCH errors and return error locations
+ * elm_check_error - Check for BCH errors and return error locations
  * @syndrome: BCH syndrome
- * @nibbles:
+ * @bch_type: BCH4/BCH8/BCH16
  * @error_count: Returns number of errrors in the syndrome
  * @error_locations: Returns error locations (in decimal) in this array
  *
@@ -95,15 +122,17 @@ static void elm_load_syndromes(u8 *syndrome, u32 nibbles, u8 poly)
  * and locations in the array passed. Returns -1 if error is not correctable,
  * else returns 0
  */
-int elm_check_error(u8 *syndrome, u32 nibbles, u32 *error_count,
+int elm_check_error(u8 *syndrome, u32 bch_type, u32 *error_count,
 		u32 *error_locations)
 {
 	u8 poly = ELM_DEFAULT_POLY;
 	s8 i;
 	u32 location_status;
 
-	elm_load_syndromes(syndrome, nibbles, poly);
-
+	if (elm_load_syndromes(syndrome, bch_type, poly)) {
+		printf("ELM: *Error: invalid driver configuration\n");
+		return -1;
+	}
 	/* start processing */
 	writel((readl(&elm_cfg->syndrome_fragments[poly].syndrome_fragment_x[6])
 				| ELM_SYNDROME_FRAGMENT_6_SYNDROME_VALID),
@@ -143,14 +172,14 @@ int elm_check_error(u8 *syndrome, u32 nibbles, u32 *error_count,
  * Currently we are using only syndrome 0 and syndromes 1 to 6 are not used.
  * Also, the mode is set only for syndrome 0
  */
-int elm_config(enum bch_level level)
+int elm_config(u32 bch_type)
 {
 	u32 val;
 	u8 poly = ELM_DEFAULT_POLY;
 	u32 buffer_size = 0x7FF;
 
 	/* config size and level */
-	val = (u32)(level) & ELM_LOCATION_CONFIG_ECC_BCH_LEVEL_MASK;
+	val = bch_type & ELM_LOCATION_CONFIG_ECC_BCH_LEVEL_MASK;
 	val |= ((buffer_size << ELM_LOCATION_CONFIG_ECC_SIZE_POS) &
 				ELM_LOCATION_CONFIG_ECC_SIZE_MASK);
 	writel(val, &elm_cfg->location_config);
diff --git a/arch/arm/include/asm/arch-am33xx/elm.h b/arch/arm/include/asm/arch-am33xx/elm.h
index 45454ea..857dc27 100644
--- a/arch/arm/include/asm/arch-am33xx/elm.h
+++ b/arch/arm/include/asm/arch-am33xx/elm.h
@@ -24,14 +24,12 @@
 #define ELM_LOCATION_STATUS_ECC_CORRECTABLE_MASK	(0x100)
 #define ELM_LOCATION_STATUS_ECC_NB_ERRORS_MASK		(0x1F)
 
-#ifndef __ASSEMBLY__
-
-enum bch_level {
-	BCH_4_BIT = 0,
-	BCH_8_BIT,
-	BCH_16_BIT
-};
+/* bch types */
+#define ECC_BCH4	0
+#define ECC_BCH8	1
+#define ECC_BCH16	2
 
+#ifndef __ASSEMBLY__
 
 /* BCH syndrome registers */
 struct syndrome {
@@ -68,9 +66,9 @@ struct elm {
 	struct location  error_location[8];	/* 0x800 */
 };
 
-int elm_check_error(u8 *syndrome, u32 nibbles, u32 *error_count,
+int elm_check_error(u8 *syndrome, u32 bch_type, u32 *error_count,
 		u32 *error_locations);
-int elm_config(enum bch_level level);
+int elm_config(u32 bch_type);
 void elm_reset(void);
 void elm_init(void);
 #endif /* __ASSEMBLY__ */
-- 
1.8.1

[U-Boot] [PATCH v2 2/3] mtd: nand: omap: add support for BCH16_ECC - NAND driver updates

[Pekon Gupta]

With increase in NAND flash densities occurence of bit-flips has increased.
Thus stronger ECC schemes are required for detecting and correcting multiple
simultaneous bit-flips in same NAND page. But stronger ECC schemes have large
ECC syndrome which require more space in OOB/Spare.
This patch add support for BCH16_ECC:
(a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
(b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.

Due to (b) this scheme can only be used with NAND devices which have enough
OOB to satisfy following equation:
OOBsize per page >= 26 * (page-size / 512)

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
 arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
 drivers/mtd/nand/omap_gpmc.c                 | 87 +++++++++++++++++++++++-----
 include/mtd/mtd-abi.h                        |  3 +-
 4 files changed, 90 insertions(+), 19 deletions(-)

diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h
index 10b56e0..1de92e6 100644
--- a/arch/arm/include/asm/arch-am33xx/cpu.h
+++ b/arch/arm/include/asm/arch-am33xx/cpu.h
@@ -63,7 +63,16 @@ struct gpmc_cs {
 };
 
 struct bch_res_0_3 {
-	u32 bch_result_x[4];
+	u32 bch_result0;
+	u32 bch_result1;
+	u32 bch_result2;
+	u32 bch_result3;
+};
+
+struct bch_res_4_6 {
+	u32 bch_result4;
+	u32 bch_result5;
+	u32 bch_result6;
 };
 
 struct gpmc {
@@ -95,7 +104,9 @@ struct gpmc {
 	u8 res7[12];		/* 0x224 */
 	u32 testmomde_ctrl;	/* 0x230 */
 	u8 res8[12];		/* 0x234 */
-	struct bch_res_0_3 bch_result_0_3[2];	/* 0x240 */
+	struct bch_res_0_3 bch_result_0_3[8];	/* 0x240 - 0x2BF */
+	u8 res9[16 * 4];	/* 0x2C0 - 0x2FF */
+	struct bch_res_4_6 bch_result_4_6[8];	/* 0x300 - 0x37F */
 };
 
 /* Used for board specific gpmc initialization */
diff --git a/arch/arm/include/asm/arch-am33xx/omap_gpmc.h b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
index 69982c1..ade0ca4 100644
--- a/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
+++ b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
@@ -16,7 +16,9 @@ enum omap_ecc {
 	/* 8-bit  ECC calculation by GPMC, Error detection by Software */
 	OMAP_ECC_BCH8_CODE_HW_DETECTION_SW,
 	/* 8-bit  ECC calculation by GPMC, Error detection by ELM */
-	OMAP_ECC_BCH8_CODE_HW
+	OMAP_ECC_BCH8_CODE_HW,
+	/* 16-bit  ECC calculation by GPMC, Error detection by ELM */
+	OMAP_ECC_BCH16_CODE_HW,
 };
 
 #endif /* __ASM_ARCH_OMAP_GPMC_H */
diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index c1a7317..b044f00 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -154,21 +154,10 @@ static int __maybe_unused omap_correct_data(struct mtd_info *mtd, uint8_t *dat,
 struct nand_bch_priv {
 	uint8_t mode;
 	uint8_t type;
-	uint8_t nibbles;
 	struct bch_control *control;
 	enum omap_ecc ecc_scheme;
 };
 
-/* bch types */
-#define ECC_BCH4	0
-#define ECC_BCH8	1
-#define ECC_BCH16	2
-
-/* BCH nibbles for diff bch levels */
-#define ECC_BCH4_NIBBLES	13
-#define ECC_BCH8_NIBBLES	26
-#define ECC_BCH16_NIBBLES	52
-
 /*
  * This can be a single instance cause all current users have only one NAND
  * with nearly the same setup (BCH8, some with ELM and others with sw BCH
@@ -177,7 +166,6 @@ struct nand_bch_priv {
  */
 static __maybe_unused struct nand_bch_priv bch_priv = {
 	.type = ECC_BCH8,
-	.nibbles = ECC_BCH8_NIBBLES,
 	.control = NULL
 };
 
@@ -243,6 +231,19 @@ static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
 			eccsize1 = 2;  /* non-ECC bits in nibbles per sector */
 		}
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		ecc_algo = 0x1;
+		bch_type = 0x2;
+		if (mode == NAND_ECC_WRITE) {
+			bch_wrapmode = 0x01;
+			eccsize0 = 0;  /* extra bits in nibbles per sector */
+			eccsize1 = 52; /* OOB bits in nibbles per sector */
+		} else {
+			bch_wrapmode = 0x01;
+			eccsize0 = 52; /* ECC bits in nibbles per sector */
+			eccsize1 = 0;  /* non-ECC bits in nibbles per sector */
+		}
+		break;
 	default:
 		return;
 	}
@@ -295,7 +296,7 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
 		break;
 	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
 	case OMAP_ECC_BCH8_CODE_HW:
-		ptr = &gpmc_cfg->bch_result_0_3[0].bch_result_x[3];
+		ptr = &gpmc_cfg->bch_result_0_3[0].bch_result3;
 		val = readl(ptr);
 		ecc_code[i++] = (val >>  0) & 0xFF;
 		ptr--;
@@ -308,6 +309,27 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
 			ptr--;
 		}
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		ptr = &gpmc_cfg->bch_result_4_6[0].bch_result6;
+		ecc_code[i++] = (readl(ptr) >>  8) & 0xFF;
+		ecc_code[i++] = (readl(ptr) >>  0) & 0xFF;
+		ptr--;
+		for (j = 0; j < 2; j++) {
+			ecc_code[i++] = (readl(ptr) >> 24) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >> 16) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >>  8) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >>  0) & 0xFF;
+			ptr--;
+		}
+		ptr = &gpmc_cfg->bch_result_0_3[0].bch_result3;
+		for (j = 0; j < 4; j++) {
+			ecc_code[i++] = (readl(ptr) >> 24) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >> 16) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >>  8) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >>  0) & 0xFF;
+			ptr--;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -326,6 +348,8 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
 	case OMAP_ECC_BCH8_CODE_HW:
 		ecc_code[chip->ecc.bytes - 1] = 0x00;
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		break;
 	}
 	return 0;
 }
@@ -376,12 +400,15 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	case OMAP_ECC_BCH8_CODE_HW:
 		omap_reverse_list(calc_ecc, eccbytes - 1);
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		omap_reverse_list(calc_ecc, eccbytes);
+		break;
 	default:
 		return -EINVAL;
 	}
 	/* use elm module to check for errors */
-	elm_config((enum bch_level)(bch->type));
-	if (elm_check_error(calc_ecc, bch->nibbles, &error_count, error_loc)) {
+	elm_config(bch->type);
+	if (elm_check_error(calc_ecc, bch->type, &error_count, error_loc)) {
 		printf("nand: error: uncorrectable ECC errors\n");
 		return -EINVAL;
 	}
@@ -392,6 +419,9 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 			/* 14th byte in ECC is reserved to match ROM layout */
 			error_max = SECTOR_BYTES + (eccbytes - 1);
 			break;
+		case ECC_BCH16:
+			error_max = SECTOR_BYTES + eccbytes;
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -614,6 +644,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme,
 			printf("nand: error: could not init_bch()\n");
 			return -ENODEV;
 		}
+		bch_priv.type		= ECC_BCH8;
 		/* define ecc-layout */
 		ecclayout->eccbytes	= nand->ecc.bytes *
 						(pagesize / SECTOR_BYTES);
@@ -638,6 +669,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme,
 		nand->ecc.read_page	= omap_read_page_bch;
 		/* ELM is used for ECC error detection */
 		elm_init();
+		bch_priv.type		= ECC_BCH8;
 		/* define ecc-layout */
 		ecclayout->eccbytes	= nand->ecc.bytes *
 						(pagesize / SECTOR_BYTES);
@@ -650,6 +682,31 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme,
 						BADBLOCK_MARKER_LENGTH;
 		bch->ecc_scheme		= OMAP_ECC_BCH8_CODE_HW;
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		debug("nand: using OMAP_ECC_BCH16_CODE_HW\n");
+		nand->ecc.mode		= NAND_ECC_HW;
+		nand->ecc.size		= SECTOR_BYTES;
+		nand->ecc.bytes		= 26;
+		nand->ecc.strength	= 16;
+		nand->ecc.hwctl		= omap_enable_hwecc;
+		nand->ecc.correct	= omap_correct_data_bch;
+		nand->ecc.calculate	= omap_calculate_ecc;
+		nand->ecc.read_page	= omap_read_page_bch;
+		/* ELM is used for ECC error detection */
+		elm_init();
+		bch_priv.type		= ECC_BCH16;
+		/* define ecc-layout */
+		ecclayout->eccbytes	= nand->ecc.bytes *
+						(pagesize / SECTOR_BYTES);
+		for (i = 0; i < ecclayout->eccbytes; i++)
+			ecclayout->eccpos[i] = i +
+						BADBLOCK_MARKER_LENGTH;
+		ecclayout->oobfree[0].offset = i +
+						BADBLOCK_MARKER_LENGTH;
+		ecclayout->oobfree[0].length = oobsize - nand->ecc.bytes -
+						BADBLOCK_MARKER_LENGTH;
+		bch->ecc_scheme		= OMAP_ECC_BCH16_CODE_HW;
+		break;
 	default:
 		debug("nand: error: ecc scheme not enabled or supported\n");
 		return -EINVAL;
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index d51c1ab..f7040be 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -156,13 +156,14 @@ struct nand_oobfree {
 };
 
 #define MTD_MAX_OOBFREE_ENTRIES	8
+#define MTD_MAX_ECCPOS_ENTRIES	256
 /*
  * ECC layout control structure. Exported to userspace for
  * diagnosis and to allow creation of raw images
  */
 struct nand_ecclayout {
 	uint32_t eccbytes;
-	uint32_t eccpos[128];
+	uint32_t eccpos[MTD_MAX_ECCPOS_ENTRIES];
 	uint32_t oobavail;
 	struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES];
 };
-- 
1.8.1

[U-Boot] [PATCH v2 3/3] am335x: update README for BCH16

[Pekon Gupta]

Adds explanation on how to select ECC scheme.

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 board/ti/am335x/README | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/board/ti/am335x/README b/board/ti/am335x/README
index 941dfbb..05e53b1 100644
--- a/board/ti/am335x/README
+++ b/board/ti/am335x/README
@@ -30,12 +30,13 @@ Step-1: Building u-boot for NAND boot
 	CONFIG_SYS_NAND_BLOCK_SIZE	number of bytes in NAND erase-block
 	CONFIG_SYS_NAND_ECCPOS		ECC map for NAND page
 	CONFIG_SYS_NAND_ECCSCHEME	ECC scheme used by NAND
-					0 - HAM1_SW
-					1 - HAM1_HW
+					0 - HAM1_SW  (for legacy devices)
+					1 - HAM1_HW  (for legacy devices)
 					2 - BCH4_SW  (unsupported)
 					3 - BCH4_HW  (unsupported)
 					4 - BCH8_SW
 					5 - BCH8_HW
+					6 - BCH16_HW
 
 Step-2: Flashing NAND via MMC/SD
 	# select BOOTSEL to MMC/SD boot and boot from MMC/SD card
@@ -63,6 +64,44 @@ Step-2: Flashing NAND via MMC/SD
 Step-3: Set BOOTSEL pin to select NAND boot, and POR the device.
 	The device should boot from images flashed on NAND device.
 
+
+How to select ECC scheme ?
+--------------------------
+Though higher ECC schemes have more capability to detect and correct
+bit-flips, but still selection of ECC scheme is dependent on following
+- hardware engines present in SoC.
+	Some legacy OMAP SoC do not have ELM h/w engine thus such
+	SoC cannot support BCHx_HW ECC schemes.
+- size of OOB/Spare region
+	With higher ECC schemes, more OOB/Spare area is required to
+	store ECC. So choice of ECC scheme is limited by NAND oobsize.
+
+In general following expression can help:
+	NAND_OOBSIZE >= 2 + (NAND_PAGESIZE / 512) * ECC_BYTES
+where
+	NAND_OOBSIZE	= number of bytes available in
+			OOB/spare area per NAND page.
+	NAND_PAGESIZE	= bytes in main-area of NAND page.
+	ECC_BYTES	= number of ECC bytes generated to
+			protect 512 bytes of data, which is:
+			3 for HAM1_xx ecc schemes
+			7 for BCH4_xx ecc schemes
+			14 for BCH8_xx ecc schemes
+			26 for BCH16_xx ecc schemes
+
+	example to check for BCH16 on 2K page NAND
+	NAND_PAGESIZE = 2048
+	NAND_OOBSIZE = 64
+	2 + (2048 / 512) * 26 = 106 > NAND_OOBSIZE
+	Thus BCH16 cannot be supported on 2K page NAND.
+
+	However, for 4K pagesize NAND
+	NAND_PAGESIZE = 4096
+	NAND_OOBSIZE = 64
+	ECC_BYTES = 26
+	2 + (4096 / 512) * 26 = 210 < NAND_OOBSIZE
+	Thus BCH16 can be supported on 4K page NAND.
+
 NOR
 ===
 
-- 
1.8.1

[U-Boot] [U-Boot, v2, 1/3] am33xx: elm: add support for BCH16_ECC - ELM driver updates

[Scott Wood]

On Tue, Sep 10, 2013 at 12:55:06PM +0530, pekon gupta wrote:
> With increase in NAND flash densities occurence of bit-flips has increased.
> Thus stronger ECC schemes are required for detecting and correcting multiple
> simultaneous bit-flips in same NAND page. But stronger ECC schemes have large
> ECC syndrome which require more space in OOB/Spare.
> This patch add support for BCH16_ECC:
> (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> 
> Due to (b) this scheme can only be used with NAND devices which have enough
> OOB to satisfy following equation:
> OOBsize per page >= 26 * (page-size / 512)
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> 
> ---
> arch/arm/cpu/armv7/am33xx/elm.c        | 95 ++++++++++++++++++++++------------
>  arch/arm/include/asm/arch-am33xx/elm.h | 16 +++---
>  2 files changed, 69 insertions(+), 42 deletions(-)

omap_gpmc.c: In function 'omap_correct_data_bch':
omap_gpmc.c:550:19: error: conversion to incomplete type

It looks like this is fixed in patch 2/3, but this breaks bisectability.

Also, does this code really belong under arch/arm rather than
drivers/mtd/nand?

-Scott

[U-Boot] [U-Boot, v2, 2/3] mtd: nand: omap: add support for BCH16_ECC - NAND driver updates

[Scott Wood]

On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> With increase in NAND flash densities occurence of bit-flips has increased.
> Thus stronger ECC schemes are required for detecting and correcting multiple
> simultaneous bit-flips in same NAND page. But stronger ECC schemes have large
> ECC syndrome which require more space in OOB/Spare.
> This patch add support for BCH16_ECC:
> (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> 
> Due to (b) this scheme can only be used with NAND devices which have enough
> OOB to satisfy following equation:
> OOBsize per page >= 26 * (page-size / 512)
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> 
> ---
> arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
>  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
>  drivers/mtd/nand/omap_gpmc.c                 | 87 +++++++++++++++++++++++-----
>  include/mtd/mtd-abi.h                        |  3 +-
>  4 files changed, 90 insertions(+), 19 deletions(-)

This doesn't apply cleanly.

> diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h
> index 10b56e0..1de92e6 100644
> --- a/arch/arm/include/asm/arch-am33xx/cpu.h
> +++ b/arch/arm/include/asm/arch-am33xx/cpu.h
> @@ -63,7 +63,16 @@ struct gpmc_cs {
>  };
>  
>  struct bch_res_0_3 {
> -	u32 bch_result_x[4];
> +	u32 bch_result0;
> +	u32 bch_result1;
> +	u32 bch_result2;
> +	u32 bch_result3;
> +};

Is this really an improvement?

It would also be nice if headers for things in drivers/mtd/nand weren't
in arch.

-Scott

[U-Boot] [U-Boot, v2, 2/3] mtd: nand: omap: add support for BCH16_ECC - NAND driver updates

[Gupta, Pekon]

> From: Scott Wood [mailto:scottwood at freescale.com]
> > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> > With increase in NAND flash densities occurence of bit-flips has increased.
> > Thus stronger ECC schemes are required for detecting and correcting
> multiple
> > simultaneous bit-flips in same NAND page. But stronger ECC schemes have
> large
> > ECC syndrome which require more space in OOB/Spare.
> > This patch add support for BCH16_ECC:
> > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> >
> > Due to (b) this scheme can only be used with NAND devices which have
> enough
> > OOB to satisfy following equation:
> > OOBsize per page >= 26 * (page-size / 512)
> >
> > Signed-off-by: Pekon Gupta <pekon@ti.com>
> >
> > ---
> > arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
> >  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
> >  drivers/mtd/nand/omap_gpmc.c                 | 87
> +++++++++++++++++++++++-----
> >  include/mtd/mtd-abi.h                        |  3 +-
> >  4 files changed, 90 insertions(+), 19 deletions(-)
> 
> This doesn't apply cleanly.
> 
This one is on top of previous patch series like..
[Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html
[Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html

So once the above series are cleaned and accepted. I'll rebase and re-send this
series for BCH16 ecc-scheme again..


> > diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h
> b/arch/arm/include/asm/arch-am33xx/cpu.h
> > index 10b56e0..1de92e6 100644
> > --- a/arch/arm/include/asm/arch-am33xx/cpu.h
> > +++ b/arch/arm/include/asm/arch-am33xx/cpu.h
> > @@ -63,7 +63,16 @@ struct gpmc_cs {
> >  };
> >
> >  struct bch_res_0_3 {
> > -	u32 bch_result_x[4];
> > +	u32 bch_result0;
> > +	u32 bch_result1;
> > +	u32 bch_result2;
> > +	u32 bch_result3;
> > +};
> 
> Is this really an improvement?
> 
Good for readability to match it to actual IP spec.. 
Each of bch_resultx map to individual 32-bit registers named accordingly
in IP spec.


> It would also be nice if headers for things in drivers/mtd/nand weren't
> in arch.
> 
I agree.. but I would take that separately, as I need to test all other
platforms also for this change .. (like am335x, am43xx, omap3.. )


with regards, pekon

[U-Boot] [U-Boot, v2, 2/3] mtd: nand: omap: add support for BCH16_ECC - NAND driver updates

[Scott Wood]

On Tue, 2013-11-19 at 13:21 +0000, Gupta, Pekon wrote:
> > From: Scott Wood [mailto:scottwood at freescale.com]
> > > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> > > With increase in NAND flash densities occurence of bit-flips has increased.
> > > Thus stronger ECC schemes are required for detecting and correcting
> > multiple
> > > simultaneous bit-flips in same NAND page. But stronger ECC schemes have
> > large
> > > ECC syndrome which require more space in OOB/Spare.
> > > This patch add support for BCH16_ECC:
> > > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> > > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> > >
> > > Due to (b) this scheme can only be used with NAND devices which have
> > enough
> > > OOB to satisfy following equation:
> > > OOBsize per page >= 26 * (page-size / 512)
> > >
> > > Signed-off-by: Pekon Gupta <pekon@ti.com>
> > >
> > > ---
> > > arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
> > >  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
> > >  drivers/mtd/nand/omap_gpmc.c                 | 87
> > +++++++++++++++++++++++-----
> > >  include/mtd/mtd-abi.h                        |  3 +-
> > >  4 files changed, 90 insertions(+), 19 deletions(-)
> > 
> > This doesn't apply cleanly.
> > 
> This one is on top of previous patch series like..
> [Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html
> [Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html
> 
> So once the above series are cleaned and accepted. I'll rebase and re-send this
> series for BCH16 ecc-scheme again..
> 
> 
> > > diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h
> > b/arch/arm/include/asm/arch-am33xx/cpu.h
> > > index 10b56e0..1de92e6 100644
> > > --- a/arch/arm/include/asm/arch-am33xx/cpu.h
> > > +++ b/arch/arm/include/asm/arch-am33xx/cpu.h
> > > @@ -63,7 +63,16 @@ struct gpmc_cs {
> > >  };
> > >
> > >  struct bch_res_0_3 {
> > > -	u32 bch_result_x[4];
> > > +	u32 bch_result0;
> > > +	u32 bch_result1;
> > > +	u32 bch_result2;
> > > +	u32 bch_result3;
> > > +};
> > 
> > Is this really an improvement?
> > 
> Good for readability to match it to actual IP spec.. 
> Each of bch_resultx map to individual 32-bit registers named accordingly
> in IP spec.

Hardware often names things "FOO1, FOO2, etc", but in C it's often
better to express as foo[n] rather than foo1, foo2, etc.

> > It would also be nice if headers for things in drivers/mtd/nand weren't
> > in arch.
> > 
> I agree.. but I would take that separately, as I need to test all other
> platforms also for this change .. (like am335x, am43xx, omap3.. )

Yes, of course it would be separate.

-Scott

[U-Boot] [U-Boot, v2, 2/3] mtd: nand: omap: add support for BCH16_ECC - NAND driver updates

[Scott Wood]

On Tue, 2013-11-19 at 13:21 +0000, Gupta, Pekon wrote:
> > From: Scott Wood [mailto:scottwood at freescale.com]
> > > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> > > With increase in NAND flash densities occurence of bit-flips has increased.
> > > Thus stronger ECC schemes are required for detecting and correcting
> > multiple
> > > simultaneous bit-flips in same NAND page. But stronger ECC schemes have
> > large
> > > ECC syndrome which require more space in OOB/Spare.
> > > This patch add support for BCH16_ECC:
> > > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> > > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> > >
> > > Due to (b) this scheme can only be used with NAND devices which have
> > enough
> > > OOB to satisfy following equation:
> > > OOBsize per page >= 26 * (page-size / 512)
> > >
> > > Signed-off-by: Pekon Gupta <pekon@ti.com>
> > >
> > > ---
> > > arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
> > >  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
> > >  drivers/mtd/nand/omap_gpmc.c                 | 87
> > +++++++++++++++++++++++-----
> > >  include/mtd/mtd-abi.h                        |  3 +-
> > >  4 files changed, 90 insertions(+), 19 deletions(-)
> > 
> > This doesn't apply cleanly.
> > 
> This one is on top of previous patch series like..
> [Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html
> [Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html
> 
> So once the above series are cleaned and accepted. I'll rebase and re-send this
> series for BCH16 ecc-scheme again..

Please be sure to mention such dependencies in the future.

-Scott