[PATCH v5 0/4] mtd:nand:omap2: clean-up of supported ECC schemes

[PATCH v5 0/4] mtd:nand:omap2: clean-up of supported ECC schemes

[Pekon Gupta]

Changes v4 -> v5
- Rebased to linux-next 
IMPORTANT: Need to revert commit fb1585b, [PATCH 2/4] part of previous version
	http://lists.infradead.org/pipermail/linux-mtd/2013-July/047441.html

- Swapped PATCH-1 & PATCH-2 to maintain bisectibility & compilation dependency
	http://lists.infradead.org/pipermail/linux-mtd/2013-July/047461.html

- PATCH-2: re-ordered call to is_elm_present() for later updates ELM driver
	- dropped changes in include/linux/platform_data/elm.h (not needed)
- PATCH-3: re-ordered call to is_elm_present() for later updates ELM driver
- Re-formated patch description (replaced tabs with white-spaces)


Changes v3 -> v4
(Resent with CC: devicetree-discuss@lists.ozlabs.org)
- [Patch 1/3] removed MTD_NAND_OMAP_BCH8 & MTD_NAND_OMAP_BCH4 from nand/Kconfig
	ECC scheme selectable via nand DT (nand-ecc-opt).
- [*] rebased for l2-mtd.git


Changes v2 -> v3
(Resent with Author Name fixed)
- PATCH-1: re-arranged code to remove redundancy, added NAND_BUSWIDTH_AUTO
- PATCH-2: updated nand-ecc-opt DT mapping and Documentation
- PATCH-3: code-cleaning + changes to match PATCH-1
- PATCH-4 <DROPPED> update DT attribute for ti,nand-ecc-opt 
	- received feedback to keep DT mapping independent of linuxism
- PATCH-4:<NEW> : ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-opt
	- independent patch for AM335x-evm.dts update based on PATCH-2


Changes v1 -> v2
	added 	[PATCH 3/4] and [PATCH 4/4]


Patches in this series:
[PATCH 1/4]->[PATCH v5 2/4]: clean-up and optimization for supported ECC schemes.
[PATCH 2/4]->[PATCH v5 1/4]: add separate DT options each supported ECC scheme.
[PATCH 3/4]: update BCH4 ECC implementation (using ELM or using lib/bch.h)
[PATCH 4/4]: ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-opt

After this patch series, omap2-nand driver will supports following ECC schemes:
+---------------------------------------+---------------+---------------+
| ECC scheme                            |ECC calculation|Error detection|
+---------------------------------------+---------------+---------------+
|OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
|OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
|OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH4_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W (lib/bch.h)|
|OMAP_ECC_BCH4_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W (lib/bch.h)|
|OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
+---------------------------------------+---------------+---------------+
- Selection of OMAP_ECC_BCHx_CODE_HW_DETECTION_SW requires,
	Kconfig: CONFIG_MTD_NAND_ECC_BCH: enables S/W based BCH ECC algorithm.

- Selection of OMAP_ECC_BCHx_CODE_HW requires,
	Kconfig: CONFIG_MTD_NAND_OMAP_BCH: enables ELM H/W module.

Pekon Gupta (4):
  ARM: OMAP2+: cleaned-up DT support of various ECC schemes
  mtd:nand:omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in
    device_probe
  mtd:nand:omap2: updated support for BCH4 ECC scheme
  ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-opt

 .../devicetree/bindings/mtd/gpmc-nand.txt          |  62 ++-
 arch/arm/boot/dts/am335x-evm.dts                   |   2 +-
 arch/arm/mach-omap2/gpmc.c                         |  14 +-
 drivers/mtd/nand/Kconfig                           |  30 +-
 drivers/mtd/nand/omap2.c                           | 483 ++++++++++-----------
 include/linux/platform_data/mtd-nand-omap2.h       |  22 +-
 6 files changed, 304 insertions(+), 309 deletions(-)

-- 
1.8.1

[PATCH v5 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

[Pekon Gupta]

ECC scheme on NAND devices can be implemented in multiple ways.Some using
Software algorithm, while others using in-build Hardware engines.
omap2-nand driver currently supports following flavours of ECC schemes,
selectable via DTB.

+---------------------------------------+---------------+---------------+
| ECC scheme                            |ECC calculation|Error detection|
+---------------------------------------+---------------+---------------+
|OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
|OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
|OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|(requires CONFIG_MTD_NAND_ECC_BCH)     |               |               |
|OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|(requires CONFIG_MTD_NAND_OMAP_BCH)    |               |               |
|OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
+---------------------------------------+---------------+---------------+

Selection of some ECC schemes also require enabling following Kconfig options.
This was done to optimize footprint of omap2-nand driver.
-Kconfig:CONFIG_MTD_NAND_ECC_BCH	enables S/W based BCH ECC algorithm
-Kconfig:CONFIG_MTD_NAND_OMAP_BCH	enables H/W based BCH ECC algorithm

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 .../devicetree/bindings/mtd/gpmc-nand.txt          | 45 ++++++++++++++++------
 arch/arm/mach-omap2/gpmc.c                         | 14 ++++---
 include/linux/platform_data/mtd-nand-omap2.h       | 22 +++++++----
 3 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
index df338cb..c6551b6 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -17,17 +17,42 @@ Required properties:
 
 Optional properties:
 
- - nand-bus-width: 		Set this numeric value to 16 if the hardware
-				is wired that way. If not specified, a bus
-				width of 8 is assumed.
+ - nand-bus-width: 		Determines data-width of the connected device
+				x16 = "16"
+				x8  = "8" (default)
 
- - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
 
-		"sw"		Software method (default)
-		"hw"		Hardware method
-		"hw-romcode"	gpmc hamming mode method & romcode layout
-		"bch4"		4-bit BCH ecc code
-		"bch8"		8-bit BCH ecc code
+ - ti,nand-ecc-opt:		Determines the ECC scheme used by driver.
+				It can be any of the following strings:
+
+	"hamming_code_sw"		1-bit Hamming ECC
+					- ECC calculation in software
+					- Error detection in software
+
+	"hamming_code_hw"		1-bit Hamming ECC
+					- ECC calculation in hardware
+					- Error detection in software
+
+	"hamming_code_hw_romcode"	1-bit Hamming ECC
+					- ECC calculation in hardware
+					- Error detection in software
+					- ECC layout compatible to ROM code
+
+	"bch8_hw_code_detection_sw"	8-bit BCH ECC
+					- ECC calculation in hardware
+					- Error detection in software
+					- depends on CONFIG_MTD_NAND_ECC_BCH
+
+	"bch8_code_hw"             	8-bit BCH ECC
+					- ECC calculation in hardware
+					- Error detection in hardware
+					- depends on CONFIG_MTD_NAND_OMAP_BCH
+					- requires <elm_id> to be specified
+
+
+ - elm_id:			Specifies elm device node. This is required to
+				support some BCH ECC schemes mentioned above.
+
 
  - ti,nand-xfer-type:		A string setting the data transfer type. One of:
 
@@ -36,8 +61,6 @@ Optional properties:
 		"prefetch-dma"		Prefetch enabled sDMA mode
 		"prefetch-irq"		Prefetch enabled irq mode
 
- - elm_id:	Specifies elm device node. This is required to support BCH
- 		error correction using ELM module.
 
 For inline partiton table parsing (optional):
 
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 1c7969e..e19de21 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1342,11 +1342,13 @@ static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
 #ifdef CONFIG_MTD_NAND
 
 static const char * const nand_ecc_opts[] = {
-	[OMAP_ECC_HAMMING_CODE_DEFAULT]		= "sw",
-	[OMAP_ECC_HAMMING_CODE_HW]		= "hw",
-	[OMAP_ECC_HAMMING_CODE_HW_ROMCODE]	= "hw-romcode",
-	[OMAP_ECC_BCH4_CODE_HW]			= "bch4",
-	[OMAP_ECC_BCH8_CODE_HW]			= "bch8",
+	[OMAP_ECC_HAMMING_CODE_DEFAULT]		= "hamming_code_sw",
+	[OMAP_ECC_HAMMING_CODE_HW]		= "hamming_code_hw",
+	[OMAP_ECC_HAMMING_CODE_HW_ROMCODE]	= "hamming_code_hw_romcode",
+	[OMAP_ECC_BCH4_CODE_HW]			= "bch4_code_hw",
+	[OMAP_ECC_BCH4_CODE_HW_DETECTION_SW]	= "bch4_code_hw_detection_sw",
+	[OMAP_ECC_BCH8_CODE_HW]			= "bch8_code_hw",
+	[OMAP_ECC_BCH8_CODE_HW_DETECTION_SW]	= "bch8_code_hw_detection_sw"
 };
 
 static const char * const nand_xfer_types[] = {
@@ -1380,7 +1382,7 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
 
 	if (!of_property_read_string(child, "ti,nand-ecc-opt", &s))
 		for (val = 0; val < ARRAY_SIZE(nand_ecc_opts); val++)
-			if (!strcasecmp(s, nand_ecc_opts[val])) {
+			if (!strcmp(s, nand_ecc_opts[val])) {
 				gpmc_nand_data->ecc_opt = val;
 				break;
 			}
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 6bf9ef4..ce74576 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -23,13 +23,21 @@ enum nand_io {
 };
 
 enum omap_ecc {
-		/* 1-bit ecc: stored at end of spare area */
-	OMAP_ECC_HAMMING_CODE_DEFAULT = 0, /* Default, s/w method */
-	OMAP_ECC_HAMMING_CODE_HW, /* gpmc to detect the error */
-		/* 1-bit ecc: stored at beginning of spare area as romcode */
-	OMAP_ECC_HAMMING_CODE_HW_ROMCODE, /* gpmc method & romcode layout */
-	OMAP_ECC_BCH4_CODE_HW, /* 4-bit BCH ecc code */
-	OMAP_ECC_BCH8_CODE_HW, /* 8-bit BCH ecc code */
+	/* 1-bit  ECC calculation by Software, Error detection by Software */
+	OMAP_ECC_HAMMING_CODE_DEFAULT = 0,
+	/* 1-bit  ECC calculation by GPMC, Error detection by Software */
+	OMAP_ECC_HAMMING_CODE_HW,
+	/* 1-bit  ECC calculation by GPMC, Error detection by Software */
+	/* ECC layout compatible to legacy ROMCODE. */
+	OMAP_ECC_HAMMING_CODE_HW_ROMCODE,
+	/* 4-bit  ECC calculation by GPMC, Error detection by ELM */
+	OMAP_ECC_BCH4_CODE_HW,
+	/* 4-bit  ECC calculation by GPMC, Error detection by Software */
+	OMAP_ECC_BCH4_CODE_HW_DETECTION_SW,
+	/* 8-bit  ECC calculation by GPMC, Error detection by ELM */
+	OMAP_ECC_BCH8_CODE_HW,
+	/* 8-bit  ECC calculation by GPMC, Error detection by Software */
+	OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
 };
 
 struct gpmc_nand_regs {
-- 
1.8.1

[PATCH v5 2/4] mtd:nand:omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in device_probe

[Pekon Gupta]

ECC scheme on NAND devices can be implemented in multiple ways.Some using
Software algorithm, while others using in-build Hardware engines.
omap2-nand driver currently supports following flavours of ECC schemes.

+---------------------------------------+---------------+---------------+
| ECC scheme                            |ECC calculation|Error detection|
+---------------------------------------+---------------+---------------+
|OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
|OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
|OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|(requires CONFIG_MTD_NAND_ECC_BCH)     |               |               |
|OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|(requires CONFIG_MTD_NAND_OMAP_BCH)    |               |               |
|OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
+---------------------------------------+---------------+---------------+

This patch
- separates the configurations for various ECC schemes.
- fixes dependency issues based on Kconfig options.
- cleans up redundant code

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/omap2.c | 505 +++++++++++++++++++++++------------------------
 1 file changed, 249 insertions(+), 256 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index daa3dfc..ea857cc 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -25,8 +25,10 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
+#ifdef CONFIG_MTD_NAND_ECC_BCH
 #include <linux/bch.h>
+#endif
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
 #include <linux/platform_data/elm.h>
 #endif
 
@@ -141,6 +143,9 @@
 #define BCH_ECC_SIZE0		0x0	/* ecc_size0 = 0, no oob protection */
 #define BCH_ECC_SIZE1		0x20	/* ecc_size1 = 32 */
 
+#define BADBLOCK_MARKER_LENGTH		0x2
+#define OMAP_ECC_BCH8_POLYNOMIAL	0x201b
+
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
 	0xac, 0x6b, 0xff, 0x99, 0x7b};
@@ -182,14 +187,11 @@ struct omap_nand_info {
 	u_char				*buf;
 	int					buf_len;
 	struct gpmc_nand_regs		reg;
-
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
-	struct bch_control             *bch;
-	struct nand_ecclayout           ecclayout;
+	/* fields specific for BCHx_HW ECC scheme */
+	struct bch_control              *bch;
 	bool				is_elm_used;
 	struct device			*elm_dev;
 	struct device_node		*of_node;
-#endif
 };
 
 /**
@@ -1058,8 +1060,6 @@ static int omap_dev_ready(struct mtd_info *mtd)
 	}
 }
 
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
-
 /**
  * omap3_enable_hwecc_bch - Program OMAP3 GPMC to perform BCH ECC correction
  * @mtd: MTD device structure
@@ -1141,6 +1141,7 @@ static void omap3_enable_hwecc_bch(struct mtd_info *mtd, int mode)
 	writel(ECCCLEAR | ECC1, info->reg.gpmc_ecc_control);
 }
 
+#ifdef CONFIG_MTD_NAND_ECC_BCH
 /**
  * omap3_calculate_ecc_bch4 - Generate 7 bytes of ECC bytes
  * @mtd: MTD device structure
@@ -1227,6 +1228,62 @@ static int omap3_calculate_ecc_bch8(struct mtd_info *mtd, const u_char *dat,
 }
 
 /**
+ * omap3_correct_data_bch - Decode received data and correct errors
+ * @mtd: MTD device structure
+ * @data: page data
+ * @read_ecc: ecc read from nand flash
+ * @calc_ecc: ecc read from HW ECC registers
+ */
+static int omap3_correct_data_bch(struct mtd_info *mtd, u_char *data,
+				  u_char *read_ecc, u_char *calc_ecc)
+{
+	int i, count;
+	/* cannot correct more than 8 errors */
+	unsigned int errloc[8];
+	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
+						   mtd);
+
+	count = decode_bch(info->bch, NULL, 512, read_ecc, calc_ecc, NULL,
+			   errloc);
+	if (count > 0) {
+		/* correct errors */
+		for (i = 0; i < count; i++) {
+			/* correct data only, not ecc bytes */
+			if (errloc[i] < 8*512)
+				data[errloc[i]/8] ^= 1 << (errloc[i] & 7);
+			pr_debug("corrected bitflip %u\n", errloc[i]);
+		}
+	} else if (count < 0) {
+		pr_err("ecc unrecoverable error\n");
+	}
+	return count;
+}
+
+/**
+ * omap3_free_bch - Release BCH ecc resources
+ * @mtd: MTD device structure
+ */
+static void omap3_free_bch(struct mtd_info *mtd)
+{
+	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
+						   mtd);
+	if (info->bch) {
+		free_bch(info->bch);
+		info->bch = NULL;
+	}
+}
+
+#else
+
+static void omap3_free_bch(struct mtd_info *mtd)
+{
+}
+
+#endif /* CONFIG_MTD_NAND_ECC_BCH */
+
+
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+/**
  * omap3_calculate_ecc_bch - Generate bytes of ECC bytes
  * @mtd:	MTD device structure
  * @dat:	The pointer to data on which ecc is computed
@@ -1519,38 +1576,6 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
 }
 
 /**
- * omap3_correct_data_bch - Decode received data and correct errors
- * @mtd: MTD device structure
- * @data: page data
- * @read_ecc: ecc read from nand flash
- * @calc_ecc: ecc read from HW ECC registers
- */
-static int omap3_correct_data_bch(struct mtd_info *mtd, u_char *data,
-				  u_char *read_ecc, u_char *calc_ecc)
-{
-	int i, count;
-	/* cannot correct more than 8 errors */
-	unsigned int errloc[8];
-	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
-						   mtd);
-
-	count = decode_bch(info->bch, NULL, 512, read_ecc, calc_ecc, NULL,
-			   errloc);
-	if (count > 0) {
-		/* correct errors */
-		for (i = 0; i < count; i++) {
-			/* correct data only, not ecc bytes */
-			if (errloc[i] < 8*512)
-				data[errloc[i]/8] ^= 1 << (errloc[i] & 7);
-			pr_debug("corrected bitflip %u\n", errloc[i]);
-		}
-	} else if (count < 0) {
-		pr_err("ecc unrecoverable error\n");
-	}
-	return count;
-}
-
-/**
  * omap_write_page_bch - BCH ecc based write page function for entire page
  * @mtd:		mtd info structure
  * @chip:		nand chip info structure
@@ -1637,197 +1662,47 @@ static int omap_read_page_bch(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /**
- * omap3_free_bch - Release BCH ecc resources
- * @mtd: MTD device structure
- */
-static void omap3_free_bch(struct mtd_info *mtd)
-{
-	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
-						   mtd);
-	if (info->bch) {
-		free_bch(info->bch);
-		info->bch = NULL;
-	}
-}
-
-/**
- * omap3_init_bch - Initialize BCH ECC
- * @mtd: MTD device structure
- * @ecc_opt: OMAP ECC mode (OMAP_ECC_BCH4_CODE_HW or OMAP_ECC_BCH8_CODE_HW)
+ * is_elm_present - checks for presence of ELM module by scanning DT nodes
+ * @omap_nand_info: NAND device structure containing platform data
+ * @bch_type: 0x0=BCH4, 0x1=BCH8, 0x2=BCH16
  */
-static int omap3_init_bch(struct mtd_info *mtd, int ecc_opt)
+static int is_elm_present(struct omap_nand_info *info, enum bch_ecc bch_type)
 {
-	int max_errors;
-	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
-						   mtd);
-#ifdef CONFIG_MTD_NAND_OMAP_BCH8
-	const int hw_errors = BCH8_MAX_ERROR;
-#else
-	const int hw_errors = BCH4_MAX_ERROR;
-#endif
-	enum bch_ecc bch_type;
 	const __be32 *parp;
 	int lenp;
 	struct device_node *elm_node;
-
-	info->bch = NULL;
-
-	max_errors = (ecc_opt == OMAP_ECC_BCH8_CODE_HW) ?
-		BCH8_MAX_ERROR : BCH4_MAX_ERROR;
-	if (max_errors != hw_errors) {
-		pr_err("cannot configure %d-bit BCH ecc, only %d-bit supported",
-		       max_errors, hw_errors);
-		goto fail;
-	}
-
-	info->nand.ecc.size = 512;
-	info->nand.ecc.hwctl = omap3_enable_hwecc_bch;
-	info->nand.ecc.mode = NAND_ECC_HW;
-	info->nand.ecc.strength = max_errors;
-
-	if (hw_errors == BCH8_MAX_ERROR)
-		bch_type = BCH8_ECC;
-	else
-		bch_type = BCH4_ECC;
+	struct platform_device *pdev;
+	info->is_elm_used = false;
 
 	/* Detect availability of ELM module */
 	parp = of_get_property(info->of_node, "elm_id", &lenp);
 	if ((parp == NULL) && (lenp != (sizeof(void *) * 2))) {
 		pr_err("Missing elm_id property, fall back to Software BCH\n");
-		info->is_elm_used = false;
 	} else {
-		struct platform_device *pdev;
-
 		elm_node = of_find_node_by_phandle(be32_to_cpup(parp));
 		pdev = of_find_device_by_node(elm_node);
 		info->elm_dev = &pdev->dev;
-
-		if (elm_config(info->elm_dev, bch_type) == 0)
-			info->is_elm_used = true;
-	}
-
-	if (info->is_elm_used && (mtd->writesize <= 4096)) {
-
-		if (hw_errors == BCH8_MAX_ERROR)
-			info->nand.ecc.bytes = BCH8_SIZE;
-		else
-			info->nand.ecc.bytes = BCH4_SIZE;
-
-		info->nand.ecc.correct = omap_elm_correct_data;
-		info->nand.ecc.calculate = omap3_calculate_ecc_bch;
-		info->nand.ecc.read_page = omap_read_page_bch;
-		info->nand.ecc.write_page = omap_write_page_bch;
-	} else {
-		/*
-		 * software bch library is only used to detect and
-		 * locate errors
-		 */
-		info->bch = init_bch(13, max_errors,
-				0x201b /* hw polynomial */);
-		if (!info->bch)
-			goto fail;
-
-		info->nand.ecc.correct = omap3_correct_data_bch;
-
-		/*
-		 * The number of corrected errors in an ecc block that will
-		 * trigger block scrubbing defaults to the ecc strength (4 or 8)
-		 * Set mtd->bitflip_threshold here to define a custom threshold.
-		 */
-
-		if (max_errors == 8) {
-			info->nand.ecc.bytes = 13;
-			info->nand.ecc.calculate = omap3_calculate_ecc_bch8;
-		} else {
-			info->nand.ecc.bytes = 7;
-			info->nand.ecc.calculate = omap3_calculate_ecc_bch4;
-		}
-	}
-
-	pr_info("enabling NAND BCH ecc with %d-bit correction\n", max_errors);
-	return 0;
-fail:
-	omap3_free_bch(mtd);
-	return -1;
-}
-
-/**
- * omap3_init_bch_tail - Build an oob layout for BCH ECC correction.
- * @mtd: MTD device structure
- */
-static int omap3_init_bch_tail(struct mtd_info *mtd)
-{
-	int i, steps, offset;
-	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
-						   mtd);
-	struct nand_ecclayout *layout = &info->ecclayout;
-
-	/* build oob layout */
-	steps = mtd->writesize/info->nand.ecc.size;
-	layout->eccbytes = steps*info->nand.ecc.bytes;
-
-	/* do not bother creating special oob layouts for small page devices */
-	if (mtd->oobsize < 64) {
-		pr_err("BCH ecc is not supported on small page devices\n");
-		goto fail;
-	}
-
-	/* reserve 2 bytes for bad block marker */
-	if (layout->eccbytes+2 > mtd->oobsize) {
-		pr_err("no oob layout available for oobsize %d eccbytes %u\n",
-		       mtd->oobsize, layout->eccbytes);
-		goto fail;
+		/* ELM module available, now configure it */
+		elm_config(info->elm_dev, bch_type);
+		info->is_elm_used = true;
+		return 0;
 	}
 
-	/* ECC layout compatible with RBL for BCH8 */
-	if (info->is_elm_used && (info->nand.ecc.bytes == BCH8_SIZE))
-		offset = 2;
-	else
-		offset = mtd->oobsize - layout->eccbytes;
-
-	/* put ecc bytes at oob tail */
-	for (i = 0; i < layout->eccbytes; i++)
-		layout->eccpos[i] = offset + i;
-
-	if (info->is_elm_used && (info->nand.ecc.bytes == BCH8_SIZE))
-		layout->oobfree[0].offset = 2 + layout->eccbytes * steps;
-	else
-		layout->oobfree[0].offset = 2;
-
-	layout->oobfree[0].length = mtd->oobsize-2-layout->eccbytes;
-	info->nand.ecc.layout = layout;
-
-	if (!(info->nand.options & NAND_BUSWIDTH_16))
-		info->nand.badblock_pattern = &bb_descrip_flashbased;
-	return 0;
-fail:
-	omap3_free_bch(mtd);
-	return -1;
-}
-
-#else
-static int omap3_init_bch(struct mtd_info *mtd, int ecc_opt)
-{
-	pr_err("CONFIG_MTD_NAND_OMAP_BCH is not enabled\n");
-	return -1;
-}
-static int omap3_init_bch_tail(struct mtd_info *mtd)
-{
-	return -1;
-}
-static void omap3_free_bch(struct mtd_info *mtd)
-{
+	return -ENODEV;
 }
 #endif /* CONFIG_MTD_NAND_OMAP_BCH */
 
+
 static int omap_nand_probe(struct platform_device *pdev)
 {
 	struct omap_nand_info		*info;
 	struct omap_nand_platform_data	*pdata;
+	struct mtd_info			*mtd;
+	struct nand_chip		*chip;
 	int				err;
-	int				i, offset;
-	dma_cap_mask_t mask;
-	unsigned sig;
+	int				i;
+	dma_cap_mask_t			mask;
+	unsigned			sig;
 	struct resource			*res;
 	struct mtd_part_parser_data	ppdata = {};
 
@@ -1846,16 +1721,18 @@ static int omap_nand_probe(struct platform_device *pdev)
 	spin_lock_init(&info->controller.lock);
 	init_waitqueue_head(&info->controller.wq);
 
-	info->pdev = pdev;
+	mtd			= &info->mtd;
+	mtd->name		= dev_name(&pdev->dev);
+	mtd->owner		= THIS_MODULE;
+	mtd->priv		= &info->nand;
+	chip			= mtd->priv;
 
+	info->pdev		= pdev;
 	info->gpmc_cs		= pdata->cs;
 	info->reg		= pdata->reg;
+	info->bch		= NULL;
 
-	info->mtd.priv		= &info->nand;
-	info->mtd.name		= dev_name(&pdev->dev);
-	info->mtd.owner		= THIS_MODULE;
-
-	info->nand.options	= pdata->devsize;
+	info->nand.options	= NAND_BUSWIDTH_AUTO;
 	info->nand.options	|= NAND_SKIP_BBTSCAN;
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 	info->of_node		= pdata->of_node;
@@ -1903,6 +1780,31 @@ static int omap_nand_probe(struct platform_device *pdev)
 		info->nand.chip_delay = 50;
 	}
 
+	/* scan NAND device conncted to controller */
+	if (nand_scan_ident(mtd, 1, NULL)) {
+		err = -ENXIO;
+		goto out_release_mem_region;
+	}
+	pr_info("%s: detected %s NAND flash\n", DRIVER_NAME,
+			(info->nand.options & NAND_BUSWIDTH_16) ? "x16" : "x8");
+	if ((info->nand.options & NAND_BUSWIDTH_16) !=
+			(pdata->devsize & NAND_BUSWIDTH_16)) {
+		pr_err("%s: but incorrectly configured as %s", DRIVER_NAME,
+			(pdata->devsize & NAND_BUSWIDTH_16) ? "x16" : "x8");
+		err = -EINVAL;
+		goto out_release_mem_region;
+	}
+
+	/* check for small page devices */
+	if ((mtd->oobsize < 64) &&
+		(pdata->ecc_opt != OMAP_ECC_HAMMING_CODE_DEFAULT) &&
+		(pdata->ecc_opt != OMAP_ECC_HAMMING_CODE_HW)) {
+		pr_err("small page devices are not supported\n");
+		err = -EINVAL;
+		goto out_release_mem_region;
+	}
+
+	/* populate read & write API based on xfer_type selected */
 	switch (pdata->xfer_type) {
 	case NAND_OMAP_PREFETCH_POLLED:
 		info->nand.read_buf   = omap_read_buf_pref;
@@ -1992,66 +1894,155 @@ static int omap_nand_probe(struct platform_device *pdev)
 		goto out_release_mem_region;
 	}
 
-	/* select the ecc type */
-	if (pdata->ecc_opt == OMAP_ECC_HAMMING_CODE_DEFAULT)
-		info->nand.ecc.mode = NAND_ECC_SOFT;
-	else if ((pdata->ecc_opt == OMAP_ECC_HAMMING_CODE_HW) ||
-		(pdata->ecc_opt == OMAP_ECC_HAMMING_CODE_HW_ROMCODE)) {
+	/* populate MTD interface based on ECC scheme */
+	switch (pdata->ecc_opt) {
+	case OMAP_ECC_HAMMING_CODE_DEFAULT:
+		pr_info("using OMAP_ECC_HAMMING_CODE_DEFAULT ECC scheme\n");
+		info->nand.ecc.mode		= NAND_ECC_SOFT;
+		goto generic_ecc_layout;
+
+	case OMAP_ECC_HAMMING_CODE_HW:
+		pr_info("using OMAP_ECC_HAMMING_CODE_HW ECC scheme\n");
+		info->nand.ecc.mode             = NAND_ECC_HW;
 		info->nand.ecc.bytes            = 3;
 		info->nand.ecc.size             = 512;
 		info->nand.ecc.strength         = 1;
 		info->nand.ecc.calculate        = omap_calculate_ecc;
 		info->nand.ecc.hwctl            = omap_enable_hwecc;
 		info->nand.ecc.correct          = omap_correct_data;
+		goto generic_ecc_layout;
+
+	case OMAP_ECC_HAMMING_CODE_HW_ROMCODE:
+		pr_info("using OMAP_ECC_HAMMING_CODE_HW_ROMCODE ECC scheme\n");
 		info->nand.ecc.mode             = NAND_ECC_HW;
-	} else if ((pdata->ecc_opt == OMAP_ECC_BCH4_CODE_HW) ||
-		   (pdata->ecc_opt == OMAP_ECC_BCH8_CODE_HW)) {
-		err = omap3_init_bch(&info->mtd, pdata->ecc_opt);
-		if (err) {
+		info->nand.ecc.bytes            = 3;
+		info->nand.ecc.size             = 512;
+		info->nand.ecc.strength         = 1;
+		info->nand.ecc.calculate        = omap_calculate_ecc;
+		info->nand.ecc.hwctl            = omap_enable_hwecc;
+		info->nand.ecc.correct          = omap_correct_data;
+		/* define custom ECC layout */
+		omap_oobinfo.eccbytes		= info->nand.ecc.bytes *
+							(mtd->writesize /
+							info->nand.ecc.size);
+		if (info->nand.options & NAND_BUSWIDTH_16)
+			omap_oobinfo.eccpos[0]	= BADBLOCK_MARKER_LENGTH;
+		else
+			omap_oobinfo.eccpos[0]	= 1;
+		omap_oobinfo.oobfree->offset	= omap_oobinfo.eccpos[0] +
+							omap_oobinfo.eccbytes;
+		goto custom_ecc_layout;
+
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+	case OMAP_ECC_BCH8_CODE_HW:
+		pr_info("using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
+		info->nand.ecc.mode		= NAND_ECC_HW;
+		info->nand.ecc.size		= 512;
+		/* 14th bit is kept reserved for ROM-code compatibility */
+		info->nand.ecc.bytes		= 13 + 1;
+		info->nand.ecc.strength		= 8;
+		info->nand.ecc.hwctl		= omap3_enable_hwecc_bch;
+		info->nand.ecc.correct		= omap_elm_correct_data;
+		info->nand.ecc.calculate	= omap3_calculate_ecc_bch;
+		info->nand.ecc.read_page	= omap_read_page_bch;
+		info->nand.ecc.write_page	= omap_write_page_bch;
+		/* This ECC scheme requires ELM H/W block */
+		if (is_elm_present(info, BCH8_ECC) < 0) {
+			pr_err("ELM module not detected, required for ECC\n");
 			err = -EINVAL;
 			goto out_release_mem_region;
 		}
-	}
-
-	/* DIP switches on some boards change between 8 and 16 bit
-	 * bus widths for flash.  Try the other width if the first try fails.
-	 */
-	if (nand_scan_ident(&info->mtd, 1, NULL)) {
-		info->nand.options ^= NAND_BUSWIDTH_16;
-		if (nand_scan_ident(&info->mtd, 1, NULL)) {
-			err = -ENXIO;
+		/* define custom ECC layout */
+		omap_oobinfo.eccbytes		= info->nand.ecc.bytes *
+							(mtd->writesize /
+							info->nand.ecc.size);
+		omap_oobinfo.eccpos[0]		= BADBLOCK_MARKER_LENGTH;
+		omap_oobinfo.oobfree->offset	= omap_oobinfo.eccpos[0] +
+							omap_oobinfo.eccbytes;
+		goto custom_ecc_layout;
+#endif
+#ifdef CONFIG_MTD_NAND_ECC_BCH
+	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
+		pr_info("using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW ECC\n");
+		info->nand.ecc.mode		= NAND_ECC_HW;
+		info->nand.ecc.size		= 512;
+		info->nand.ecc.bytes		= 13;
+		info->nand.ecc.strength		= 8;
+		info->nand.ecc.hwctl		= omap3_enable_hwecc_bch;
+		info->nand.ecc.correct		= omap3_correct_data_bch;
+		info->nand.ecc.calculate	= omap3_calculate_ecc_bch8;
+		/* define custom ECC layout */
+		omap_oobinfo.eccbytes		= info->nand.ecc.bytes *
+							(mtd->writesize /
+							info->nand.ecc.size);
+		omap_oobinfo.eccpos[0]		= info->mtd.oobsize -
+							omap_oobinfo.eccbytes;
+		omap_oobinfo.oobfree->offset	= BADBLOCK_MARKER_LENGTH;
+		/* software bch library is used for locating errors */
+		info->bch = init_bch(info->nand.ecc.bytes,
+					info->nand.ecc.strength,
+					OMAP_ECC_BCH8_POLYNOMIAL);
+		if (!info->bch) {
+			pr_err("unable initialize S/W BCH logic\n");
+			err = -EINVAL;
 			goto out_release_mem_region;
 		}
-	}
-
-	/* rom code layout */
-	if (pdata->ecc_opt == OMAP_ECC_HAMMING_CODE_HW_ROMCODE) {
-
-		if (info->nand.options & NAND_BUSWIDTH_16)
-			offset = 2;
-		else {
-			offset = 1;
-			info->nand.badblock_pattern = &bb_descrip_flashbased;
-		}
-		omap_oobinfo.eccbytes = 3 * (info->mtd.oobsize/16);
-		for (i = 0; i < omap_oobinfo.eccbytes; i++)
-			omap_oobinfo.eccpos[i] = i+offset;
-
-		omap_oobinfo.oobfree->offset = offset + omap_oobinfo.eccbytes;
-		omap_oobinfo.oobfree->length = info->mtd.oobsize -
-					(offset + omap_oobinfo.eccbytes);
-
-		info->nand.ecc.layout = &omap_oobinfo;
-	} else if ((pdata->ecc_opt == OMAP_ECC_BCH4_CODE_HW) ||
-		   (pdata->ecc_opt == OMAP_ECC_BCH8_CODE_HW)) {
-		/* build OOB layout for BCH ECC correction */
-		err = omap3_init_bch_tail(&info->mtd);
-		if (err) {
+		goto custom_ecc_layout;
+
+	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
+		pr_info("using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW ECC\n");
+		info->nand.ecc.mode		= NAND_ECC_HW;
+		info->nand.ecc.size		= 512;
+		info->nand.ecc.bytes		= 7;
+		info->nand.ecc.strength		= 4;
+		info->nand.ecc.hwctl		= omap3_enable_hwecc_bch;
+		info->nand.ecc.correct		= omap3_correct_data_bch;
+		info->nand.ecc.calculate	= omap3_calculate_ecc_bch4;
+		/* define custom ECC layout */
+		omap_oobinfo.eccbytes		= info->nand.ecc.bytes *
+							(mtd->writesize /
+							info->nand.ecc.size);
+		omap_oobinfo.eccpos[0]		= info->mtd.oobsize -
+							omap_oobinfo.eccbytes;
+		omap_oobinfo.oobfree->offset	= BADBLOCK_MARKER_LENGTH;
+		/* software bch library is used for locating errors */
+		info->bch = init_bch(info->nand.ecc.bytes,
+					info->nand.ecc.strength,
+					OMAP_ECC_BCH8_POLYNOMIAL);
+		if (!info->bch) {
+			pr_err("unable initialize S/W BCH logic\n");
 			err = -EINVAL;
 			goto out_release_mem_region;
 		}
+		goto custom_ecc_layout;
+#endif
+	default:
+		pr_err("selected ECC scheme not supported or not enabled\n");
+		err = -EINVAL;
+		goto out_release_mem_region;
+	}
+
+custom_ecc_layout:
+	/* populate remaining info for custom ecc layout */
+	pr_info("%s: using custom ecc layout\n", DRIVER_NAME);
+	omap_oobinfo.oobfree->length = mtd->oobsize - BADBLOCK_MARKER_LENGTH
+						- omap_oobinfo.eccbytes;
+	if (!(info->nand.options & NAND_BUSWIDTH_16))
+		info->nand.badblock_pattern = &bb_descrip_flashbased;
+	for (i = 1; i < omap_oobinfo.eccbytes; i++)
+		omap_oobinfo.eccpos[i] = omap_oobinfo.eccpos[0] + i;
+
+	/* check if NAND OOBSIZE meets ECC scheme requirement */
+	if (mtd->oobsize < (omap_oobinfo.eccbytes +
+					BADBLOCK_MARKER_LENGTH)) {
+		pr_err("not enough OOB bytes required = %d, available=%d\n",
+		       mtd->oobsize, omap_oobinfo.eccbytes);
+		err = -EINVAL;
+		goto out_release_mem_region;
 	}
+	info->nand.ecc.layout = &omap_oobinfo;
 
+generic_ecc_layout:
 	/* second phase scan */
 	if (nand_scan_tail(&info->mtd)) {
 		err = -ENXIO;
@@ -2075,11 +2066,13 @@ out_release_mem_region:
 		free_irq(info->gpmc_irq_fifo, info);
 	release_mem_region(info->phys_base, info->mem_size);
 out_free_info:
+	omap3_free_bch(&info->mtd);
 	kfree(info);
 
 	return err;
 }
 
+
 static int omap_nand_remove(struct platform_device *pdev)
 {
 	struct mtd_info *mtd = platform_get_drvdata(pdev);
-- 
1.8.1

[PATCH v5 3/4] mtd:nand:omap2: updated support for BCH4 ECC scheme

[Pekon Gupta]

This patch adds following two flavours of BCH4 ECC scheme in omap2-nand driver
- OMAP_ECC_BCH4_CODE_HW_DETECTION_SW
	- uses GPMC H/W engine for calculating ECC.
	- uses software library (lib/bch.h & nand_bch.h) for error correction.

- OMAP_ECC_BCH4_CODE_HW
	- uses GPMC H/W engine for calculating ECC.
	- uses ELM H/W engine for error correction.

With this patch omap2-nand driver supports following ECC schemes:
+---------------------------------------+---------------+---------------+
| ECC scheme                            |ECC calculation|Error detection|
+---------------------------------------+---------------+---------------+
|OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
|OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
|OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH4_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W (lib/bch.h)|
|OMAP_ECC_BCH4_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W (lib/bch.h)|
|OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
+---------------------------------------+---------------+---------------+
Important:
- Selection of OMAP_ECC_BCHx_CODE_HW_DETECTION_SW requires,
	Kconfig: CONFIG_MTD_NAND_ECC_BCH: enables S/W based BCH ECC algorithm.

- Selection of OMAP_ECC_BCHx_CODE_HW requires,
	Kconfig: CONFIG_MTD_NAND_OMAP_BCH: enables ELM H/W module.

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  27 +++-
 drivers/mtd/nand/Kconfig                           |  30 +---
 drivers/mtd/nand/omap2.c                           | 168 +++++++++------------
 include/linux/platform_data/mtd-nand-omap2.h       |  10 +-
 4 files changed, 102 insertions(+), 133 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
index c6551b6..d307c67 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -28,26 +28,43 @@ Optional properties:
 	"hamming_code_sw"		1-bit Hamming ECC
 					- ECC calculation in software
 					- Error detection in software
+					- ECC layout compatible with S/W scheme
 
 	"hamming_code_hw"		1-bit Hamming ECC
 					- ECC calculation in hardware
 					- Error detection in software
+					- ECC layout compatible with S/W scheme
 
 	"hamming_code_hw_romcode"	1-bit Hamming ECC
 					- ECC calculation in hardware
 					- Error detection in software
-					- ECC layout compatible to ROM code
+					- ECC layout compatible with ROM code
 
-	"bch8_hw_code_detection_sw"	8-bit BCH ECC
+	"bch4_code_hw_detection_sw"	4-bit BCH ECC
 					- ECC calculation in hardware
 					- Error detection in software
-					- depends on CONFIG_MTD_NAND_ECC_BCH
+					- ECC layout compatible with S/W scheme
+					* depends on CONFIG_MTD_NAND_ECC_BCH
+
+	"bch4_code_hw"             	4-bit BCH ECC
+					- ECC calculation in hardware
+					- Error detection in hardware
+					- ECC layout compatible with ROM code
+					* depends on CONFIG_MTD_NAND_OMAP_BCH
+					* requires <elm_id> to be specified
+
+	"bch8_code_hw_detection_sw"	8-bit BCH ECC
+					- ECC calculation in hardware
+					- Error detection in software
+					- ECC layout compatible with S/W scheme
+					* depends on CONFIG_MTD_NAND_ECC_BCH
 
 	"bch8_code_hw"             	8-bit BCH ECC
 					- ECC calculation in hardware
 					- Error detection in hardware
-					- depends on CONFIG_MTD_NAND_OMAP_BCH
-					- requires <elm_id> to be specified
+					- ECC layout compatible with ROM code
+					* depends on CONFIG_MTD_NAND_OMAP_BCH
+					* requires <elm_id> to be specified
 
 
  - elm_id:			Specifies elm device node. This is required to
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5ef8f5e..64830f9 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -95,35 +95,13 @@ config MTD_NAND_OMAP2
 
 config MTD_NAND_OMAP_BCH
 	depends on MTD_NAND && MTD_NAND_OMAP2 && ARCH_OMAP3
-	tristate "Enable support for hardware BCH error correction"
+	tristate "Support hardware based BCH error correction"
 	default n
 	select BCH
-	select BCH_CONST_PARAMS
 	help
-	 Support for hardware BCH error correction.
-
-choice
-	prompt "BCH error correction capability"
-	depends on MTD_NAND_OMAP_BCH
-
-config MTD_NAND_OMAP_BCH8
-	bool "8 bits / 512 bytes (recommended)"
-	help
-	 Support correcting up to 8 bitflips per 512-byte block.
-	 This will use 13 bytes of spare area per 512 bytes of page data.
-	 This is the recommended mode, as 4-bit mode does not work
-	 on some OMAP3 revisions, due to a hardware bug.
-
-config MTD_NAND_OMAP_BCH4
-	bool "4 bits / 512 bytes"
-	help
-	 Support correcting up to 4 bitflips per 512-byte block.
-	 This will use 7 bytes of spare area per 512 bytes of page data.
-	 Note that this mode does not work on some OMAP3 revisions, due to a
-	 hardware bug. Please check your OMAP datasheet before selecting this
-	 mode.
-
-endchoice
+	  Some devices have built-in ELM hardware engine, which can be used to
+	  locate and correct errors when using BCH ECC scheme. This enables the
+	  driver support for same.
 
 if MTD_NAND_OMAP_BCH
 config BCH_CONST_M
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index ea857cc..e9ef743 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -27,6 +27,7 @@
 
 #ifdef CONFIG_MTD_NAND_ECC_BCH
 #include <linux/bch.h>
+#include <linux/mtd/nand_bch.h>
 #endif
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 #include <linux/platform_data/elm.h>
@@ -144,7 +145,6 @@
 #define BCH_ECC_SIZE1		0x20	/* ecc_size1 = 32 */
 
 #define BADBLOCK_MARKER_LENGTH		0x2
-#define OMAP_ECC_BCH8_POLYNOMIAL	0x201b
 
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
@@ -185,10 +185,9 @@ struct omap_nand_info {
 		OMAP_NAND_IO_WRITE,	/* write */
 	} iomode;
 	u_char				*buf;
-	int					buf_len;
+	int				buf_len;
 	struct gpmc_nand_regs		reg;
 	/* fields specific for BCHx_HW ECC scheme */
-	struct bch_control              *bch;
 	bool				is_elm_used;
 	struct device			*elm_dev;
 	struct device_node		*of_node;
@@ -1227,58 +1226,6 @@ static int omap3_calculate_ecc_bch8(struct mtd_info *mtd, const u_char *dat,
 	return 0;
 }
 
-/**
- * omap3_correct_data_bch - Decode received data and correct errors
- * @mtd: MTD device structure
- * @data: page data
- * @read_ecc: ecc read from nand flash
- * @calc_ecc: ecc read from HW ECC registers
- */
-static int omap3_correct_data_bch(struct mtd_info *mtd, u_char *data,
-				  u_char *read_ecc, u_char *calc_ecc)
-{
-	int i, count;
-	/* cannot correct more than 8 errors */
-	unsigned int errloc[8];
-	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
-						   mtd);
-
-	count = decode_bch(info->bch, NULL, 512, read_ecc, calc_ecc, NULL,
-			   errloc);
-	if (count > 0) {
-		/* correct errors */
-		for (i = 0; i < count; i++) {
-			/* correct data only, not ecc bytes */
-			if (errloc[i] < 8*512)
-				data[errloc[i]/8] ^= 1 << (errloc[i] & 7);
-			pr_debug("corrected bitflip %u\n", errloc[i]);
-		}
-	} else if (count < 0) {
-		pr_err("ecc unrecoverable error\n");
-	}
-	return count;
-}
-
-/**
- * omap3_free_bch - Release BCH ecc resources
- * @mtd: MTD device structure
- */
-static void omap3_free_bch(struct mtd_info *mtd)
-{
-	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
-						   mtd);
-	if (info->bch) {
-		free_bch(info->bch);
-		info->bch = NULL;
-	}
-}
-
-#else
-
-static void omap3_free_bch(struct mtd_info *mtd)
-{
-}
-
 #endif /* CONFIG_MTD_NAND_ECC_BCH */
 
 
@@ -1730,13 +1677,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 	info->pdev		= pdev;
 	info->gpmc_cs		= pdata->cs;
 	info->reg		= pdata->reg;
-	info->bch		= NULL;
 
 	info->nand.options	= NAND_BUSWIDTH_AUTO;
 	info->nand.options	|= NAND_SKIP_BBTSCAN;
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 	info->of_node		= pdata->of_node;
 #endif
+	info->nand.ecc.priv			= NULL;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL) {
@@ -1933,21 +1880,43 @@ static int omap_nand_probe(struct platform_device *pdev)
 							omap_oobinfo.eccbytes;
 		goto custom_ecc_layout;
 
+#ifdef CONFIG_MTD_NAND_ECC_BCH
+	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
+		pr_info("using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW ECC scheme");
+		info->nand.ecc.mode		= NAND_ECC_HW;
+		info->nand.ecc.size		= 512;
+		info->nand.ecc.bytes		= 7;
+		info->nand.ecc.strength		= 4;
+		info->nand.ecc.hwctl		= omap3_enable_hwecc_bch;
+		info->nand.ecc.correct		= nand_bch_correct_data;
+		info->nand.ecc.calculate	= omap3_calculate_ecc_bch4;
+		/* software bch library is used for locating errors */
+		info->nand.ecc.priv		= nand_bch_init(mtd,
+						info->nand.ecc.size,
+						info->nand.ecc.bytes,
+						&info->nand.ecc.layout);
+		if (!info->nand.ecc.priv) {
+			pr_err("unable initialize S/W BCH logic\n");
+			err = -EINVAL;
+			goto out_release_mem_region;
+		}
+		goto generic_ecc_layout;
+#endif
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
-	case OMAP_ECC_BCH8_CODE_HW:
-		pr_info("using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
+	case OMAP_ECC_BCH4_CODE_HW:
+		pr_info("using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
 		info->nand.ecc.mode		= NAND_ECC_HW;
 		info->nand.ecc.size		= 512;
-		/* 14th bit is kept reserved for ROM-code compatibility */
-		info->nand.ecc.bytes		= 13 + 1;
-		info->nand.ecc.strength		= 8;
+		/* 8th bit is kept reserved for ROM-code compatibility */
+		info->nand.ecc.bytes		= 7 + 1;
+		info->nand.ecc.strength		= 4;
 		info->nand.ecc.hwctl		= omap3_enable_hwecc_bch;
 		info->nand.ecc.correct		= omap_elm_correct_data;
 		info->nand.ecc.calculate	= omap3_calculate_ecc_bch;
 		info->nand.ecc.read_page	= omap_read_page_bch;
 		info->nand.ecc.write_page	= omap_write_page_bch;
-		/* This ECC scheme requires ELM H/W block */
-		if (is_elm_present(info, BCH8_ECC) < 0) {
+		/* ELM H/W engine is used for locating errors */
+		if (is_elm_present(info, BCH4_ECC) < 0) {
 			pr_err("ELM module not detected, required for ECC\n");
 			err = -EINVAL;
 			goto out_release_mem_region;
@@ -1969,51 +1938,46 @@ static int omap_nand_probe(struct platform_device *pdev)
 		info->nand.ecc.bytes		= 13;
 		info->nand.ecc.strength		= 8;
 		info->nand.ecc.hwctl		= omap3_enable_hwecc_bch;
-		info->nand.ecc.correct		= omap3_correct_data_bch;
+		info->nand.ecc.correct		= nand_bch_correct_data;
 		info->nand.ecc.calculate	= omap3_calculate_ecc_bch8;
-		/* define custom ECC layout */
-		omap_oobinfo.eccbytes		= info->nand.ecc.bytes *
-							(mtd->writesize /
-							info->nand.ecc.size);
-		omap_oobinfo.eccpos[0]		= info->mtd.oobsize -
-							omap_oobinfo.eccbytes;
-		omap_oobinfo.oobfree->offset	= BADBLOCK_MARKER_LENGTH;
 		/* software bch library is used for locating errors */
-		info->bch = init_bch(info->nand.ecc.bytes,
-					info->nand.ecc.strength,
-					OMAP_ECC_BCH8_POLYNOMIAL);
-		if (!info->bch) {
+		info->nand.ecc.priv		= nand_bch_init(mtd,
+						info->nand.ecc.size,
+						info->nand.ecc.bytes,
+						&info->nand.ecc.layout);
+		if (!info->nand.ecc.priv) {
 			pr_err("unable initialize S/W BCH logic\n");
 			err = -EINVAL;
 			goto out_release_mem_region;
 		}
-		goto custom_ecc_layout;
-
-	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
-		pr_info("using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW ECC\n");
+		goto generic_ecc_layout;
+#endif
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+	case OMAP_ECC_BCH8_CODE_HW:
+		pr_info("using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
 		info->nand.ecc.mode		= NAND_ECC_HW;
 		info->nand.ecc.size		= 512;
-		info->nand.ecc.bytes		= 7;
-		info->nand.ecc.strength		= 4;
+		/* 14th bit is kept reserved for ROM-code compatibility */
+		info->nand.ecc.bytes		= 13 + 1;
+		info->nand.ecc.strength		= 8;
 		info->nand.ecc.hwctl		= omap3_enable_hwecc_bch;
-		info->nand.ecc.correct		= omap3_correct_data_bch;
-		info->nand.ecc.calculate	= omap3_calculate_ecc_bch4;
+		info->nand.ecc.correct		= omap_elm_correct_data;
+		info->nand.ecc.calculate	= omap3_calculate_ecc_bch;
+		info->nand.ecc.read_page	= omap_read_page_bch;
+		info->nand.ecc.write_page	= omap_write_page_bch;
+		/* ELM H/W engine is used for locating errors */
+		if (is_elm_present(info, BCH8_ECC) < 0) {
+			pr_err("ELM module not detected, required for ECC\n");
+			err = -EINVAL;
+			goto out_release_mem_region;
+		}
 		/* define custom ECC layout */
 		omap_oobinfo.eccbytes		= info->nand.ecc.bytes *
 							(mtd->writesize /
 							info->nand.ecc.size);
-		omap_oobinfo.eccpos[0]		= info->mtd.oobsize -
+		omap_oobinfo.eccpos[0]		= BADBLOCK_MARKER_LENGTH;
+		omap_oobinfo.oobfree->offset	= omap_oobinfo.eccpos[0] +
 							omap_oobinfo.eccbytes;
-		omap_oobinfo.oobfree->offset	= BADBLOCK_MARKER_LENGTH;
-		/* software bch library is used for locating errors */
-		info->bch = init_bch(info->nand.ecc.bytes,
-					info->nand.ecc.strength,
-					OMAP_ECC_BCH8_POLYNOMIAL);
-		if (!info->bch) {
-			pr_err("unable initialize S/W BCH logic\n");
-			err = -EINVAL;
-			goto out_release_mem_region;
-		}
 		goto custom_ecc_layout;
 #endif
 	default:
@@ -2065,8 +2029,14 @@ out_release_mem_region:
 	if (info->gpmc_irq_fifo > 0)
 		free_irq(info->gpmc_irq_fifo, info);
 	release_mem_region(info->phys_base, info->mem_size);
+
 out_free_info:
-	omap3_free_bch(&info->mtd);
+#ifdef CONFIG_MTD_NAND_ECC_BCH
+	if (info->nand.ecc.priv) {
+		nand_bch_free(info->nand.ecc.priv);
+		info->nand.ecc.priv = NULL;
+	}
+#endif
 	kfree(info);
 
 	return err;
@@ -2078,8 +2048,12 @@ static int omap_nand_remove(struct platform_device *pdev)
 	struct mtd_info *mtd = platform_get_drvdata(pdev);
 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
 							mtd);
-	omap3_free_bch(&info->mtd);
-
+#ifdef CONFIG_MTD_NAND_ECC_BCH
+	if (info->nand.ecc.priv) {
+		nand_bch_free(info->nand.ecc.priv);
+		info->nand.ecc.priv = NULL;
+	}
+#endif
 	if (info->dma)
 		dma_release_channel(info->dma);
 
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index ce74576..9fcee61 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -30,14 +30,14 @@ enum omap_ecc {
 	/* 1-bit  ECC calculation by GPMC, Error detection by Software */
 	/* ECC layout compatible to legacy ROMCODE. */
 	OMAP_ECC_HAMMING_CODE_HW_ROMCODE,
-	/* 4-bit  ECC calculation by GPMC, Error detection by ELM */
-	OMAP_ECC_BCH4_CODE_HW,
 	/* 4-bit  ECC calculation by GPMC, Error detection by Software */
 	OMAP_ECC_BCH4_CODE_HW_DETECTION_SW,
-	/* 8-bit  ECC calculation by GPMC, Error detection by ELM */
-	OMAP_ECC_BCH8_CODE_HW,
+	/* 4-bit  ECC calculation by GPMC, Error detection by ELM */
+	OMAP_ECC_BCH4_CODE_HW,
 	/* 8-bit  ECC calculation by GPMC, Error detection by Software */
-	OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
+	OMAP_ECC_BCH8_CODE_HW_DETECTION_SW,
+	/* 8-bit  ECC calculation by GPMC, Error detection by ELM */
+	OMAP_ECC_BCH8_CODE_HW
 };
 
 struct gpmc_nand_regs {
-- 
1.8.1

[PATCH v5 4/4] ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-opt

[Pekon Gupta]

DT property values for OMAP based gpmc-nand have been updated
to match changes in commit:
	6faf096 ARM: OMAP2+: cleaned-up DT support of various ECC schemes
Refer: Documentation/devicetree/bindings/mtd/gpmc-nand.txt

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 arch/arm/boot/dts/am335x-evm.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 3aee1a4..7077a2f 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -239,7 +239,7 @@
 			nand@0,0 {
 				reg = <0 0 0>; /* CS0, offset 0 */
 				nand-bus-width = <8>;
-				ti,nand-ecc-opt = "bch8";
+				ti,nand-ecc-opt = "bch8_code_hw";
 				gpmc,device-nand = "true";
 				gpmc,device-width = <1>;
 				gpmc,sync-clk-ps = <0>;
-- 
1.8.1

RE: [PATCH v5 0/4] mtd:nand:omap2: clean-up of supported ECC schemes

[Gupta, Pekon]

> 
> Changes v4 -> v5
> - Rebased to linux-next
> IMPORTANT: Need to revert commit fb1585b, [PATCH 2/4] part of previous
> version
> 	http://lists.infradead.org/pipermail/linux-mtd/2013-July/047441.html
> 
> - Swapped PATCH-1 & PATCH-2 to maintain bisectibility & compilation
> dependency
> 	http://lists.infradead.org/pipermail/linux-mtd/2013-July/047461.html
> 
> - PATCH-2: re-ordered call to is_elm_present() for later updates ELM driver
> 	- dropped changes in include/linux/platform_data/elm.h (not
> needed)
> - PATCH-3: re-ordered call to is_elm_present() for later updates ELM driver
> - Re-formated patch description (replaced tabs with white-spaces)
> 

Hi Olof, Arnd,

I have re-submitted patch with minor updates and re-ordered PATCH-1 & 2
to preserve bisectibility. And there is no comment | objection from
maintainers about the DT bindings. So request you to please Ack | Review
this series for it to be picked by Artem | David Woodhouse in MTD tree.

Also please revert commit fb1585b in linux-next, as its left-over piece of
same series v4.

- This series is the base for further clean-up of omap2-nand driver, hence
its important for it to be up-streamed for future work.


with regards, pekon

RE: [PATCH v5 0/4] mtd:nand:omap2: clean-up of supported ECC schemes

[Gupta, Pekon]

Hi Artem, Brian,

> 
> Changes v4 -> v5
> - Rebased to linux-next
> IMPORTANT: Need to revert commit fb1585b, [PATCH 2/4] part of previous
> version
> 	http://lists.infradead.org/pipermail/linux-mtd/2013-July/047441.html
> 
> - Swapped PATCH-1 & PATCH-2 to maintain bisectibility & compilation
> dependency
> 	http://lists.infradead.org/pipermail/linux-mtd/2013-July/047461.html
> 
> - PATCH-2: re-ordered call to is_elm_present() for later updates ELM driver
> 	- dropped changes in include/linux/platform_data/elm.h (not
> needed)
> - PATCH-3: re-ordered call to is_elm_present() for later updates ELM driver
> - Re-formated patch description (replaced tabs with white-spaces)
> 
> 
> Changes v3 -> v4
> (Resent with CC: devicetree-discuss@lists.ozlabs.org)
> - [Patch 1/3] removed MTD_NAND_OMAP_BCH8 &
> MTD_NAND_OMAP_BCH4 from nand/Kconfig
> 	ECC scheme selectable via nand DT (nand-ecc-opt).
> - [*] rebased for l2-mtd.git
> 
> 
> Changes v2 -> v3
> (Resent with Author Name fixed)
> - PATCH-1: re-arranged code to remove redundancy, added
> NAND_BUSWIDTH_AUTO
> - PATCH-2: updated nand-ecc-opt DT mapping and Documentation
> - PATCH-3: code-cleaning + changes to match PATCH-1
> - PATCH-4 <DROPPED> update DT attribute for ti,nand-ecc-opt
> 	- received feedback to keep DT mapping independent of linuxism
> - PATCH-4:<NEW> : ARM: dts: AM33xx: updated default ECC scheme in nand-
> ecc-opt
> 	- independent patch for AM335x-evm.dts update based on PATCH-2
> 
> 
> Changes v1 -> v2
> 	added 	[PATCH 3/4] and [PATCH 4/4]
> 
> 
> Patches in this series:
> [PATCH 1/4]->[PATCH v5 2/4]: clean-up and optimization for supported ECC
> schemes.
> [PATCH 2/4]->[PATCH v5 1/4]: add separate DT options each supported ECC
> scheme.
> [PATCH 3/4]: update BCH4 ECC implementation (using ELM or using lib/bch.h)
> [PATCH 4/4]: ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-
> opt
> 
> After this patch series, omap2-nand driver will supports following ECC
> schemes:
> +---------------------------------------+---------------+---------------+
> | ECC scheme                            |ECC calculation|Error detection|
> +---------------------------------------+---------------+---------------+
> |OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
> |OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
> |OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W
> |
> +---------------------------------------+---------------+---------------+
> |OMAP_ECC_BCH4_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W
> (lib/bch.h)|
> |OMAP_ECC_BCH4_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
> +---------------------------------------+---------------+---------------+
> |OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W
> (lib/bch.h)|
> |OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
> +---------------------------------------+---------------+---------------+
> - Selection of OMAP_ECC_BCHx_CODE_HW_DETECTION_SW requires,
> 	Kconfig: CONFIG_MTD_NAND_ECC_BCH: enables S/W based BCH
> ECC algorithm.
> 
> - Selection of OMAP_ECC_BCHx_CODE_HW requires,
> 	Kconfig: CONFIG_MTD_NAND_OMAP_BCH: enables ELM H/W
> module.
> 

Request to please pick-up this patch series, as following two more
patch series queued up depending on this.

http://lists.infradead.org/pipermail/linux-mtd/2013-July/047538.html

http://lists.infradead.org/pipermail/linux-mtd/2013-July/047562.html


with regards, pekon

Re: [PATCH v5 0/4] mtd:nand:omap2: clean-up of supported ECC schemes

[Artem Bityutskiy]

On Tue, 2013-08-20 at 13:20 +0000, Gupta, Pekon wrote:
> Hi Artem, Brian,

I'll try to go through old e-mails now, and especially ubi/ubifs ones,
including yours, and I'll let Brian handle your patches, if I can? :-)

I checked them, and they looked good to me:

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH v5 0/4] mtd:nand:omap2: clean-up of supported ECC schemes

[Brian Norris]

On Tue, Aug 20, 2013 at 7:02 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Tue, 2013-08-20 at 13:20 +0000, Gupta, Pekon wrote:
>> Hi Artem, Brian,
>
> I'll try to go through old e-mails now, and especially ubi/ubifs ones,
> including yours, and I'll let Brian handle your patches, if I can? :-)

Yes, these are on my radar, but I haven't spent any time on them yet.
I will handle them soon.

> I checked them, and they looked good to me:
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Brian

Re: [PATCH v5 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

[Brian Norris]

Including the correct device-tree list.

On Sun, Jul 14, 2013 at 02:24:48AM +0530, Pekon Gupta wrote:
> ECC scheme on NAND devices can be implemented in multiple ways.Some using
> Software algorithm, while others using in-build Hardware engines.
> omap2-nand driver currently supports following flavours of ECC schemes,
> selectable via DTB.
> 
> +---------------------------------------+---------------+---------------+
> | ECC scheme                            |ECC calculation|Error detection|
> +---------------------------------------+---------------+---------------+
> |OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
> |OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
> |OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W            |
> +---------------------------------------+---------------+---------------+
> |(requires CONFIG_MTD_NAND_ECC_BCH)     |               |               |
> |OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W            |
> +---------------------------------------+---------------+---------------+
> |(requires CONFIG_MTD_NAND_OMAP_BCH)    |               |               |
> |OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
> +---------------------------------------+---------------+---------------+
> 
> Selection of some ECC schemes also require enabling following Kconfig options.
> This was done to optimize footprint of omap2-nand driver.
> -Kconfig:CONFIG_MTD_NAND_ECC_BCH	enables S/W based BCH ECC algorithm
> -Kconfig:CONFIG_MTD_NAND_OMAP_BCH	enables H/W based BCH ECC algorithm
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  .../devicetree/bindings/mtd/gpmc-nand.txt          | 45 ++++++++++++++++------
>  arch/arm/mach-omap2/gpmc.c                         | 14 ++++---
>  include/linux/platform_data/mtd-nand-omap2.h       | 22 +++++++----
>  3 files changed, 57 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> index df338cb..c6551b6 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> @@ -17,17 +17,42 @@ Required properties:
>  
>  Optional properties:
>  
> - - nand-bus-width: 		Set this numeric value to 16 if the hardware
> -				is wired that way. If not specified, a bus
> -				width of 8 is assumed.
> + - nand-bus-width: 		Determines data-width of the connected device
> +				x16 = "16"
> +				x8  = "8" (default)
>  
> - - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
>  
> -		"sw"		Software method (default)
> -		"hw"		Hardware method
> -		"hw-romcode"	gpmc hamming mode method & romcode layout
> -		"bch4"		4-bit BCH ecc code
> -		"bch8"		8-bit BCH ecc code
> + - ti,nand-ecc-opt:		Determines the ECC scheme used by driver.
> +				It can be any of the following strings:
> +
> +	"hamming_code_sw"		1-bit Hamming ECC
> +					- ECC calculation in software
> +					- Error detection in software
> +
> +	"hamming_code_hw"		1-bit Hamming ECC
> +					- ECC calculation in hardware
> +					- Error detection in software
> +
> +	"hamming_code_hw_romcode"	1-bit Hamming ECC
> +					- ECC calculation in hardware
> +					- Error detection in software
> +					- ECC layout compatible to ROM code
> +
> +	"bch8_hw_code_detection_sw"	8-bit BCH ECC
> +					- ECC calculation in hardware
> +					- Error detection in software
> +					- depends on CONFIG_MTD_NAND_ECC_BCH
> +
> +	"bch8_code_hw"             	8-bit BCH ECC
> +					- ECC calculation in hardware
> +					- Error detection in hardware
> +					- depends on CONFIG_MTD_NAND_OMAP_BCH
> +					- requires <elm_id> to be specified

I'm pretty sure this is an ABI breakage, which is now considered
unacceptable. AFAIK, you can support new ECC types (or new aliases for
old ones), but you can't remove/rename old ones.

> +
> +
> + - elm_id:			Specifies elm device node. This is required to
> +				support some BCH ECC schemes mentioned above.
> +
>  
>   - ti,nand-xfer-type:		A string setting the data transfer type. One of:
>  
> @@ -36,8 +61,6 @@ Optional properties:
>  		"prefetch-dma"		Prefetch enabled sDMA mode
>  		"prefetch-irq"		Prefetch enabled irq mode
>  
> - - elm_id:	Specifies elm device node. This is required to support BCH
> - 		error correction using ELM module.
>  
>  For inline partiton table parsing (optional):
>  
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 1c7969e..e19de21 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -1342,11 +1342,13 @@ static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
>  #ifdef CONFIG_MTD_NAND
>  
>  static const char * const nand_ecc_opts[] = {
> -	[OMAP_ECC_HAMMING_CODE_DEFAULT]		= "sw",
> -	[OMAP_ECC_HAMMING_CODE_HW]		= "hw",
> -	[OMAP_ECC_HAMMING_CODE_HW_ROMCODE]	= "hw-romcode",
> -	[OMAP_ECC_BCH4_CODE_HW]			= "bch4",
> -	[OMAP_ECC_BCH8_CODE_HW]			= "bch8",
> +	[OMAP_ECC_HAMMING_CODE_DEFAULT]		= "hamming_code_sw",
> +	[OMAP_ECC_HAMMING_CODE_HW]		= "hamming_code_hw",
> +	[OMAP_ECC_HAMMING_CODE_HW_ROMCODE]	= "hamming_code_hw_romcode",
> +	[OMAP_ECC_BCH4_CODE_HW]			= "bch4_code_hw",
> +	[OMAP_ECC_BCH4_CODE_HW_DETECTION_SW]	= "bch4_code_hw_detection_sw",
> +	[OMAP_ECC_BCH8_CODE_HW]			= "bch8_code_hw",
> +	[OMAP_ECC_BCH8_CODE_HW_DETECTION_SW]	= "bch8_code_hw_detection_sw"
>  };
>  
>  static const char * const nand_xfer_types[] = {
> @@ -1380,7 +1382,7 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>  
>  	if (!of_property_read_string(child, "ti,nand-ecc-opt", &s))
>  		for (val = 0; val < ARRAY_SIZE(nand_ecc_opts); val++)
> -			if (!strcasecmp(s, nand_ecc_opts[val])) {
> +			if (!strcmp(s, nand_ecc_opts[val])) {
>  				gpmc_nand_data->ecc_opt = val;
>  				break;
>  			}
> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
> index 6bf9ef4..ce74576 100644
> --- a/include/linux/platform_data/mtd-nand-omap2.h
> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> @@ -23,13 +23,21 @@ enum nand_io {
>  };
>  
>  enum omap_ecc {
> -		/* 1-bit ecc: stored at end of spare area */
> -	OMAP_ECC_HAMMING_CODE_DEFAULT = 0, /* Default, s/w method */
> -	OMAP_ECC_HAMMING_CODE_HW, /* gpmc to detect the error */
> -		/* 1-bit ecc: stored at beginning of spare area as romcode */
> -	OMAP_ECC_HAMMING_CODE_HW_ROMCODE, /* gpmc method & romcode layout */
> -	OMAP_ECC_BCH4_CODE_HW, /* 4-bit BCH ecc code */
> -	OMAP_ECC_BCH8_CODE_HW, /* 8-bit BCH ecc code */
> +	/* 1-bit  ECC calculation by Software, Error detection by Software */
> +	OMAP_ECC_HAMMING_CODE_DEFAULT = 0,
> +	/* 1-bit  ECC calculation by GPMC, Error detection by Software */
> +	OMAP_ECC_HAMMING_CODE_HW,
> +	/* 1-bit  ECC calculation by GPMC, Error detection by Software */
> +	/* ECC layout compatible to legacy ROMCODE. */
> +	OMAP_ECC_HAMMING_CODE_HW_ROMCODE,
> +	/* 4-bit  ECC calculation by GPMC, Error detection by ELM */
> +	OMAP_ECC_BCH4_CODE_HW,
> +	/* 4-bit  ECC calculation by GPMC, Error detection by Software */
> +	OMAP_ECC_BCH4_CODE_HW_DETECTION_SW,
> +	/* 8-bit  ECC calculation by GPMC, Error detection by ELM */
> +	OMAP_ECC_BCH8_CODE_HW,
> +	/* 8-bit  ECC calculation by GPMC, Error detection by Software */
> +	OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
>  };
>  
>  struct gpmc_nand_regs {

Brian

Re: [PATCH v5 2/4] mtd:nand:omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in device_probe

[Brian Norris]

On Sun, Jul 14, 2013 at 02:24:49AM +0530, Pekon Gupta wrote:
> ECC scheme on NAND devices can be implemented in multiple ways.Some using
> Software algorithm, while others using in-build Hardware engines.
> omap2-nand driver currently supports following flavours of ECC schemes.
> 
> +---------------------------------------+---------------+---------------+
> | ECC scheme                            |ECC calculation|Error detection|
> +---------------------------------------+---------------+---------------+
> |OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
> |OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
> |OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W            |
> +---------------------------------------+---------------+---------------+
> |(requires CONFIG_MTD_NAND_ECC_BCH)     |               |               |
> |OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W            |
> +---------------------------------------+---------------+---------------+
> |(requires CONFIG_MTD_NAND_OMAP_BCH)    |               |               |
> |OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
> +---------------------------------------+---------------+---------------+
> 
> This patch
> - separates the configurations for various ECC schemes.
> - fixes dependency issues based on Kconfig options.
> - cleans up redundant code
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  drivers/mtd/nand/omap2.c | 505 +++++++++++++++++++++++------------------------
>  1 file changed, 249 insertions(+), 256 deletions(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index daa3dfc..ea857cc 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c

[...]

> @@ -2075,11 +2066,13 @@ out_release_mem_region:
>  		free_irq(info->gpmc_irq_fifo, info);
>  	release_mem_region(info->phys_base, info->mem_size);
>  out_free_info:
> +	omap3_free_bch(&info->mtd);
>  	kfree(info);
>  
>  	return err;
>  }
>  
> +

Extra blank line?

>  static int omap_nand_remove(struct platform_device *pdev)
>  {
>  	struct mtd_info *mtd = platform_get_drvdata(pdev);

Brian

Re: [PATCH v5 0/4] mtd:nand:omap2: clean-up of supported ECC schemes

[Brian Norris]

Hi Pekon,

I don't think I will take this series (at least not yet), because of the
device-tree ABI breakage.

On Sun, Jul 14, 2013 at 02:24:47AM +0530, Pekon Gupta wrote:
> 
> Changes v4 -> v5
> - Rebased to linux-next 
> IMPORTANT: Need to revert commit fb1585b, [PATCH 2/4] part of previous version
> 	http://lists.infradead.org/pipermail/linux-mtd/2013-July/047441.html
> 
> - Swapped PATCH-1 & PATCH-2 to maintain bisectibility & compilation dependency
> 	http://lists.infradead.org/pipermail/linux-mtd/2013-July/047461.html
> 
> - PATCH-2: re-ordered call to is_elm_present() for later updates ELM driver
> 	- dropped changes in include/linux/platform_data/elm.h (not needed)
> - PATCH-3: re-ordered call to is_elm_present() for later updates ELM driver
> - Re-formated patch description (replaced tabs with white-spaces)
> 
> 
> Changes v3 -> v4
> (Resent with CC: devicetree-discuss@lists.ozlabs.org)

Make sure to use devicetree@vger.kernel.org for future series.

> - [Patch 1/3] removed MTD_NAND_OMAP_BCH8 & MTD_NAND_OMAP_BCH4 from nand/Kconfig
> 	ECC scheme selectable via nand DT (nand-ecc-opt).
> - [*] rebased for l2-mtd.git
> 
> 
> Changes v2 -> v3
> (Resent with Author Name fixed)
> - PATCH-1: re-arranged code to remove redundancy, added NAND_BUSWIDTH_AUTO
> - PATCH-2: updated nand-ecc-opt DT mapping and Documentation
> - PATCH-3: code-cleaning + changes to match PATCH-1
> - PATCH-4 <DROPPED> update DT attribute for ti,nand-ecc-opt 
> 	- received feedback to keep DT mapping independent of linuxism
> - PATCH-4:<NEW> : ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-opt
> 	- independent patch for AM335x-evm.dts update based on PATCH-2
> 
> 
> Changes v1 -> v2
> 	added 	[PATCH 3/4] and [PATCH 4/4]

[...]

You might also consider a future patch for utilizing devm_* functions
for the probe/remove routines in drivers/mtd/nand/omap2.c. That could
improve some of the stuff I looked at in this series.

Brian

Re: [PATCH v5 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

[Olof Johansson]

On Sun, Jul 14, 2013 at 02:24:48AM +0530, Pekon Gupta wrote:
> ECC scheme on NAND devices can be implemented in multiple ways.Some using
> Software algorithm, while others using in-build Hardware engines.
> omap2-nand driver currently supports following flavours of ECC schemes,
> selectable via DTB.
> 
> +---------------------------------------+---------------+---------------+
> | ECC scheme                            |ECC calculation|Error detection|
> +---------------------------------------+---------------+---------------+
> |OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
> |OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
> |OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W            |
> +---------------------------------------+---------------+---------------+
> |(requires CONFIG_MTD_NAND_ECC_BCH)     |               |               |
> |OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W            |
> +---------------------------------------+---------------+---------------+
> |(requires CONFIG_MTD_NAND_OMAP_BCH)    |               |               |
> |OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
> +---------------------------------------+---------------+---------------+
> 
> Selection of some ECC schemes also require enabling following Kconfig options.
> This was done to optimize footprint of omap2-nand driver.
> -Kconfig:CONFIG_MTD_NAND_ECC_BCH	enables S/W based BCH ECC algorithm
> -Kconfig:CONFIG_MTD_NAND_OMAP_BCH	enables H/W based BCH ECC algorithm
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  .../devicetree/bindings/mtd/gpmc-nand.txt          | 45 ++++++++++++++++------
>  arch/arm/mach-omap2/gpmc.c                         | 14 ++++---
>  include/linux/platform_data/mtd-nand-omap2.h       | 22 +++++++----
>  3 files changed, 57 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> index df338cb..c6551b6 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> @@ -17,17 +17,42 @@ Required properties:
>  
>  Optional properties:
>  
> - - nand-bus-width: 		Set this numeric value to 16 if the hardware
> -				is wired that way. If not specified, a bus
> -				width of 8 is assumed.
> + - nand-bus-width: 		Determines data-width of the connected device
> +				x16 = "16"
> +				x8  = "8" (default)

This change doesn't look like an improvement to me.

> - - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
>  
> -		"sw"		Software method (default)
> -		"hw"		Hardware method
> -		"hw-romcode"	gpmc hamming mode method & romcode layout
> -		"bch4"		4-bit BCH ecc code
> -		"bch8"		8-bit BCH ecc code
> + - ti,nand-ecc-opt:		Determines the ECC scheme used by driver.
> +				It can be any of the following strings:

The device tree should define the hardware, not configure the software.

Also, if it defines the scheme then you should probably call it something like
"ti,nand-ecc-scheme" instead.

> +
> +	"hamming_code_sw"		1-bit Hamming ECC
> +					- ECC calculation in software
> +					- Error detection in software
> +
> +	"hamming_code_hw"		1-bit Hamming ECC
> +					- ECC calculation in hardware
> +					- Error detection in software

Bzzt. Same here. It really doesn't matter to the hardware if the ECC is
calculated in HW or SW, and it doesn't belong in the device tree.
> +
> +	"hamming_code_hw_romcode"	1-bit Hamming ECC
> +					- ECC calculation in hardware
> +					- Error detection in software
> +					- ECC layout compatible to ROM code
> +
> +	"bch8_hw_code_detection_sw"	8-bit BCH ECC
> +					- ECC calculation in hardware
> +					- Error detection in software
> +					- depends on CONFIG_MTD_NAND_ECC_BCH

Configuration options don't belong in here either.

> +
> +	"bch8_code_hw"             	8-bit BCH ECC
> +					- ECC calculation in hardware
> +					- Error detection in hardware
> +					- depends on CONFIG_MTD_NAND_OMAP_BCH
> +					- requires <elm_id> to be specified

Some of the above clearly shouldn't be described in the device tree at all, but
it's also not very convenient to describe them as strings. Since you have
a binding, it's probably easier to give them integer values and just define
what those mean.

> +
> +
> + - elm_id:			Specifies elm device node. This is required to
> +				support some BCH ECC schemes mentioned above.

Use dashes instead of underscores for properties. if all other properties are
prepended with "ti,", then so should this.

What's an ELM device node? How is it specified? A phandle?



-Olof

RE: [PATCH v5 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

[Gupta, Pekon]

> 
> On Sun, Jul 14, 2013 at 02:24:48AM +0530, Pekon Gupta wrote:
> > ECC scheme on NAND devices can be implemented in multiple ways.Some
> using
> > Software algorithm, while others using in-build Hardware engines.
> > omap2-nand driver currently supports following flavours of ECC schemes,
> > selectable via DTB.
> >
> > +---------------------------------------+---------------+---------------+
> > | ECC scheme                            |ECC calculation|Error detection|
> > +---------------------------------------+---------------+---------------+
> > |OMAP_ECC_HAMMING_CODE_DEFAULT          |S/W            |S/W            |
> > |OMAP_ECC_HAMMING_CODE_HW               |H/W (GPMC)     |S/W            |
> > |OMAP_ECC_HAMMING_CODE_HW_ROMCODE       |H/W (GPMC)     |S/W
> |
> > +---------------------------------------+---------------+---------------+
> > |(requires CONFIG_MTD_NAND_ECC_BCH)     |               |               |
> > |OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W
> |
> > +---------------------------------------+---------------+---------------+
> > |(requires CONFIG_MTD_NAND_OMAP_BCH)    |               |               |
> > |OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
> > +---------------------------------------+---------------+---------------+
> >
> > Selection of some ECC schemes also require enabling following Kconfig
> options.
> > This was done to optimize footprint of omap2-nand driver.
> > -Kconfig:CONFIG_MTD_NAND_ECC_BCH	enables S/W based BCH ECC
> algorithm
> > -Kconfig:CONFIG_MTD_NAND_OMAP_BCH	enables H/W based BCH ECC
> algorithm
> >
> > Signed-off-by: Pekon Gupta <pekon@ti.com>
> > ---
> >  .../devicetree/bindings/mtd/gpmc-nand.txt          | 45
> ++++++++++++++++------
> >  arch/arm/mach-omap2/gpmc.c                         | 14 ++++---
> >  include/linux/platform_data/mtd-nand-omap2.h       | 22 +++++++----
> >  3 files changed, 57 insertions(+), 24 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> > index df338cb..c6551b6 100644
> > --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> > @@ -17,17 +17,42 @@ Required properties:
> >
> >  Optional properties:
> >
> > - - nand-bus-width: 		Set this numeric value to 16 if the hardware
> > -				is wired that way. If not specified, a bus
> > -				width of 8 is assumed.
> > + - nand-bus-width: 		Determines data-width of the connected
> device
> > +				x16 = "16"
> > +				x8  = "8" (default)
> 
> This change doesn't look like an improvement to me.
> 
[Pekon]: Accepted. I'll drop this. However, this is not a new binding.
I was just elaborating & formatting the description because code allows only
two values "16" or "8".


> > - - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
> >
> > -		"sw"		Software method (default)
> > -		"hw"		Hardware method
> > -		"hw-romcode"	gpmc hamming mode method & romcode
> layout
> > -		"bch4"		4-bit BCH ecc code
> > -		"bch8"		8-bit BCH ecc code
> > + - ti,nand-ecc-opt:		Determines the ECC scheme used by driver.
> > +				It can be any of the following strings:
> 
> The device tree should define the hardware, not configure the software.
> 
> Also, if it defines the scheme then you should probably call it something like
> "ti,nand-ecc-scheme" instead.
> 
[Pekon]: Again, ti,nand-ecc-opt is not new DT binding, This was added in
	Commit bc6b1e7b86f5d8e4a6fc1c0189e64bba4077efe0
	Daniel Mack <zonque@gmail.com>
	ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
I just expanded its options..

Also I'll try to explain how below ecc-scheme selection is linked to TI Hardware.
TI SoC uses two separate H/W engines for calculating and correcting ECC.
(a) GPMC: General Purpose Memory Controller which calculates ECC also.
(b) ELM: Error Locator Module which just locates errors in BCHx code only.

*Reason-1*: All OMAP platforms have (a) GPMC h/w engine, but some 
legacy OMAP  platforms do not have (b) ELM h/w engine. Such older 
platforms use S/W lib/bch.c libraries for ECC error detection.
Therefore in-order to keep the driver consistent for all platforms we 
needed to keep so many ECC options alive. Like below you would see
two versions of BCH8 and BCH4
- bch8_code_hw (supported on new devices with ELM h/w engine)
- bch8_code_hw_detection_sw (kept for legacy devices)

*Reason-2*: Also H/W ECC schemes have different ECC layout, which is
compatible to ROM code. Thus end-to-end NAND boot would work
only with xx_code_hw schemes only.

Hence ECC selection is dependent on H/W, hence put as DT binding.
But I agree going forward we should deprecate most of legacy options
when there is common H/W platform for all devices.


> > +
> > +	"hamming_code_sw"		1-bit Hamming ECC
> > +					- ECC calculation in software
> > +					- Error detection in software
> > +
> > +	"hamming_code_hw"		1-bit Hamming ECC
> > +					- ECC calculation in hardware
> > +					- Error detection in software
> 
> Bzzt. Same here. It really doesn't matter to the hardware if the ECC is
> calculated in HW or SW, and it doesn't belong in the device tree.

[Pekon]: Not a new bindings option just renamed, and kept for legacy
 compatibility (please refer change diff below)
> > -		"sw"		Software method (default)
	renamed to "hamming_code_sw"

> > -		"hw"		Hardware method 
	renamed to "hamming_code_hw"


> > +
> > +	"hamming_code_hw_romcode"	1-bit Hamming ECC
> > +					- ECC calculation in hardware
> > +					- Error detection in software
> > +					- ECC layout compatible to ROM code
> > +
> > +	"bch8_hw_code_detection_sw"	8-bit BCH ECC
> > +					- ECC calculation in hardware
> > +					- Error detection in software
> > +					- depends on
> CONFIG_MTD_NAND_ECC_BCH
> 
> Configuration options don't belong in here either.
> 
[Pekon]: As explained above, legacy platforms do not have ELM h/w.
So they depend on lib/bch.c libraries for ECC detection. But that bloats
the driver code-footprint a lot, just to maintain compatibility for legacy
device platforms. Therefore I had to use KConfigs to keep driver
code-footprint small.

I can remove KConfig from documentation, but then it would confuse
the users when they get NAND probing errors because:
- bchx_hw_code requires ELM h/w engine, which is not present on 
	legacy devices.
-  bchx_hw_code_detection_sw requires lib/bch.c which is only
	 enabled by CONFIG_MTD_NAND_ECC_BCH.

Will removing KConfig from DT Documentation be acceptable ?

> > +
> > +	"bch8_code_hw"             	8-bit BCH ECC
> > +					- ECC calculation in hardware
> > +					- Error detection in hardware
> > +					- depends on
> CONFIG_MTD_NAND_OMAP_BCH
> > +					- requires <elm_id> to be specified
> 
> Some of the above clearly shouldn't be described in the device tree at all, but
> it's also not very convenient to describe them as strings. Since you have
> a binding, it's probably easier to give them integer values and just define
> what those mean.
> 
[Pekon]: Do you mean something like below ?
ti,nand-ecc-scheme(n)  where 'n' means
	0 ==  1-bit Hamming in S/W
	1 ==  1-bit Hamming in H/W
	2 ==  BCH4 with S/W detection
	3 ==  BCH4 all in H/W
	4 ==  BCH8 with S/W detection
	5 ==  BCH8 all in H/W

> > +
> > +
> > + - elm_id:			Specifies elm device node. This is required to
> > +				support some BCH ECC schemes mentioned
> above.
> 
> Use dashes instead of underscores for properties. if all other properties are
> prepended with "ti,", then so should this.
> 
[Pekon]: Accepted. But again this is not new DT binding. It was added in
	Commit 97c794a1e37b1ca128ef38f17c069186bfa5fb1b
	Philip Avinash <avinashphilip@ti.com>
	gpmc: Add device tree documentation for elm handle

> What's an ELM device node? How is it specified? A phandle?
> 
[Pekon]: ELM is a DT node specified in soc.dtsi (because ELM is not
 present in  legacy devices). Example Please refer:
$KERNEL/arch/arm/boot/dts/am33xx.dtsi


> 
> 
> -Olof

[Pekon]: Thanks much for review.
I have accepted some feedbacks, and given reasons for others..
In-case you are convinced, I'll send another version of patch with
all feedbacks taken in. 
So, request you to re-review based on reasons mentioned.


with regards, pekon

Re: [PATCH v5 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

[Olof Johansson]

On Thu, Aug 22, 2013 at 07:56:57AM +0000, Gupta, Pekon wrote:
> > This change doesn't look like an improvement to me.
> > 
> [Pekon]: Accepted. I'll drop this. However, this is not a new binding.
> I was just elaborating & formatting the description because code allows only
> two values "16" or "8".

No need to prefix your replies with [Pekon]. Based on quotation level it's easy
to see who said what.

> > > - - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
> > >
> > > -		"sw"		Software method (default)
> > > -		"hw"		Hardware method
> > > -		"hw-romcode"	gpmc hamming mode method & romcode
> > layout
> > > -		"bch4"		4-bit BCH ecc code
> > > -		"bch8"		8-bit BCH ecc code
> > > + - ti,nand-ecc-opt:		Determines the ECC scheme used by driver.
> > > +				It can be any of the following strings:
> > 
> > The device tree should define the hardware, not configure the software.
> > 
> > Also, if it defines the scheme then you should probably call it something like
> > "ti,nand-ecc-scheme" instead.
> > 
> [Pekon]: Again, ti,nand-ecc-opt is not new DT binding, This was added in
> 	Commit bc6b1e7b86f5d8e4a6fc1c0189e64bba4077efe0
> 	Daniel Mack <zonque@gmail.com>
> 	ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
> I just expanded its options..

Yes, but it's not describing hardware.

If anything, the device entry should somehow describe the various ecc options
that the hardware implements (if you can't derive that from the compatible
value, which I think you can?).

> Also I'll try to explain how below ecc-scheme selection is linked to TI Hardware.
> TI SoC uses two separate H/W engines for calculating and correcting ECC.
> (a) GPMC: General Purpose Memory Controller which calculates ECC also.
> (b) ELM: Error Locator Module which just locates errors in BCHx code only.
> 
> *Reason-1*: All OMAP platforms have (a) GPMC h/w engine, but some 
> legacy OMAP  platforms do not have (b) ELM h/w engine. Such older 
> platforms use S/W lib/bch.c libraries for ECC error detection.

Ok, so then you just need a binary "elm-engine" property to indicate
that the hardware does have the engine.

> Therefore in-order to keep the driver consistent for all platforms we 
> needed to keep so many ECC options alive. Like below you would see
> two versions of BCH8 and BCH4
> - bch8_code_hw (supported on new devices with ELM h/w engine)
> - bch8_code_hw_detection_sw (kept for legacy devices)

All you need to specify is what ECC format is used. I.e. BCH8. Whether the
hardware or software will be used to calculate the checksums and detect/correct
errors is a driver decision, and not something that the device tree needs to
specify.

> *Reason-2*: Also H/W ECC schemes have different ECC layout, which is
> compatible to ROM code. Thus end-to-end NAND boot would work
> only with xx_code_hw schemes only.

So you should describe what the layout in use is. Wouldn't it be
possible to make the software handle the same layout as the hardware
engine if needed? I.e. the decision of using HW or SW is not a property
of the hardware and shouldn't be described in the device tree.

> Hence ECC selection is dependent on H/W, hence put as DT binding.
> But I agree going forward we should deprecate most of legacy options
> when there is common H/W platform for all devices.
> 
> 
> > > +
> > > +	"hamming_code_sw"		1-bit Hamming ECC
> > > +					- ECC calculation in software
> > > +					- Error detection in software
> > > +
> > > +	"hamming_code_hw"		1-bit Hamming ECC
> > > +					- ECC calculation in hardware
> > > +					- Error detection in software
> > 
> > Bzzt. Same here. It really doesn't matter to the hardware if the ECC is
> > calculated in HW or SW, and it doesn't belong in the device tree.
> 
> [Pekon]: Not a new bindings option just renamed, and kept for legacy
>  compatibility (please refer change diff below)

Same arguments as above.

> > > -		"sw"		Software method (default)
> 	renamed to "hamming_code_sw"
> 
> > > -		"hw"		Hardware method 
> 	renamed to "hamming_code_hw"
> 
> 
> > > +
> > > +	"hamming_code_hw_romcode"	1-bit Hamming ECC
> > > +					- ECC calculation in hardware
> > > +					- Error detection in software
> > > +					- ECC layout compatible to ROM code
> > > +
> > > +	"bch8_hw_code_detection_sw"	8-bit BCH ECC
> > > +					- ECC calculation in hardware
> > > +					- Error detection in software
> > > +					- depends on
> > CONFIG_MTD_NAND_ECC_BCH
> > 
> > Configuration options don't belong in here either.
> > 
> [Pekon]: As explained above, legacy platforms do not have ELM h/w.
> So they depend on lib/bch.c libraries for ECC detection. But that bloats
> the driver code-footprint a lot, just to maintain compatibility for legacy
> device platforms. Therefore I had to use KConfigs to keep driver
> code-footprint small.
>
> I can remove KConfig from documentation, but then it would confuse
> the users when they get NAND probing errors because:
> - bchx_hw_code requires ELM h/w engine, which is not present on 
> 	legacy devices.
> -  bchx_hw_code_detection_sw requires lib/bch.c which is only
> 	 enabled by CONFIG_MTD_NAND_ECC_BCH.
> 
> Will removing KConfig from DT Documentation be acceptable ?

Yes, please remove it. Bindings documentation should not document kernel config
options.

> > > +
> > > +	"bch8_code_hw"             	8-bit BCH ECC
> > > +					- ECC calculation in hardware
> > > +					- Error detection in hardware
> > > +					- depends on
> > CONFIG_MTD_NAND_OMAP_BCH
> > > +					- requires <elm_id> to be specified
> > 
> > Some of the above clearly shouldn't be described in the device tree at all, but
> > it's also not very convenient to describe them as strings. Since you have
> > a binding, it's probably easier to give them integer values and just define
> > what those mean.
> > 
> [Pekon]: Do you mean something like below ?
> ti,nand-ecc-scheme(n)  where 'n' means
> 	0 ==  1-bit Hamming in S/W
> 	1 ==  1-bit Hamming in H/W
> 	2 ==  BCH4 with S/W detection
> 	3 ==  BCH4 all in H/W
> 	4 ==  BCH8 with S/W detection
> 	5 ==  BCH8 all in H/W

Well, see above -- I think you can just describe the properties of the hardware
and make the driver pick appropriate setup based on it.

> > > + - elm_id:			Specifies elm device node. This is required to
> > > +				support some BCH ECC schemes mentioned
> > above.
> > 
> > Use dashes instead of underscores for properties. if all other properties are
> > prepended with "ti,", then so should this.
> > 
> [Pekon]: Accepted. But again this is not new DT binding. It was added in
> 	Commit 97c794a1e37b1ca128ef38f17c069186bfa5fb1b
> 	Philip Avinash <avinashphilip@ti.com>
> 	gpmc: Add device tree documentation for elm handle

Since you are revising the binding, this is the time to change it.

> > What's an ELM device node? How is it specified? A phandle?
> > 
> [Pekon]: ELM is a DT node specified in soc.dtsi (because ELM is not
>  present in  legacy devices). Example Please refer:
> $KERNEL/arch/arm/boot/dts/am33xx.dtsi

What I meant with a question like that is: Please document how it's specified.
It's not clear from reading it, since I had to ask.



-Olof

RE: [PATCH v5 1/4] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

[Gupta, Pekon]

> On Thu, Aug 22, 2013 at 07:56:57AM +0000, Gupta, Pekon wrote:

> If anything, the device entry should somehow describe the various ecc
> options
> that the hardware implements (if you can't derive that from the compatible
> value, which I think you can?).
> 
> > Also I'll try to explain how below ecc-scheme selection is linked to TI
> Hardware.
> > TI SoC uses two separate H/W engines for calculating and correcting ECC.
> > (a) GPMC: General Purpose Memory Controller which calculates ECC also.
> > (b) ELM: Error Locator Module which just locates errors in BCHx code only.
> >
> > *Reason-1*: All OMAP platforms have (a) GPMC h/w engine, but some
> > legacy OMAP  platforms do not have (b) ELM h/w engine. Such older
> > platforms use S/W lib/bch.c libraries for ECC error detection.
> 
> Ok, so then you just need a binary "elm-engine" property to indicate
> that the hardware does have the engine.
> 
> > Therefore in-order to keep the driver consistent for all platforms we
> > needed to keep so many ECC options alive. Like below you would see
> > two versions of BCH8 and BCH4
> > - bch8_code_hw (supported on new devices with ELM h/w engine)
> > - bch8_code_hw_detection_sw (kept for legacy devices)
> 
> All you need to specify is what ECC format is used. I.e. BCH8. Whether the
> hardware or software will be used to calculate the checksums and
> detect/correct
> errors is a driver decision, and not something that the device tree needs to
> specify.
> 
> > *Reason-2*: Also H/W ECC schemes have different ECC layout, which is
> > compatible to ROM code. Thus end-to-end NAND boot would work
> > only with xx_code_hw schemes only.
> 
> So you should describe what the layout in use is. Wouldn't it be
> possible to make the software handle the same layout as the hardware
> engine if needed? I.e. the decision of using HW or SW is not a property
> of the hardware and shouldn't be described in the device tree.
> 

Sorry I was caught up in some other priority work, therefore could not
update this. But I have recently posted another version with your feedbacks
incorporated. Please review them, whenever possible.
http://lists.infradead.org/pipermail/linux-mtd/2013-September/048611.html

Patch series at: 
http://lists.infradead.org/pipermail/linux-mtd/2013-September/048613.html


Thank you 
With regards, pekon

RE: [PATCH v5 0/4] mtd:nand:omap2: clean-up of supported ECC schemes

[Gupta, Pekon]

Hi,

> [...]
> 
> You might also consider a future patch for utilizing devm_* functions
> for the probe/remove routines in drivers/mtd/nand/omap2.c. That could
> improve some of the stuff I looked at in this series.
> 
Thanks for feedback.
I have not incorporated this particular update in v6, as I do not have much
knowledge about devm_* functions. But I'll keep it in my to-do(s) once
driver is complete feature wise. 

with regards, pekon