[PATCH v10 00/10] mtd:nand:omap2: clean-up of supported ECC schemes

[PATCH v10 00/10] mtd:nand:omap2: clean-up of supported ECC schemes

[Pekon Gupta]

*changes v9 -> v10*
[PATCH 1/10], [PATCH 2/10]
  swapped [PATCH v9 1/9] and [PATCH v9 2/9] so that DT parsing updates
  (with backward compatibility) happen before the deprecation of DT values.
  This way DTB does not break functionally between the patches.
[PATCH 3/10] <no update>
[PATCH 4/10] 
  dropped [PATCH v9 4/9] introducing NAND_BUSWIDTH_AUTO, instead
  using DT 'nand-bus-width' for device bus-width
[PATCH 5/10] <no update>
[PATCH 6/10] <no update>
[PATCH 7/10] 
  separated out drivers/mtd/nand/Kconfig updates into separate [PATCH v10 10/10]
  cleanup: s/info->nand\./nand_chip->
[PATCH 8/10] <no update>
[PATCH 9/10] cleanup: s/out_release_mem_region/return_error
[PATCH 10/10] <new> spawned from [PATCH v9 8/9] for Kconfig updates


*changes v8 -> v9*
[PATCH 1/9] <no update from [PATCH v8 1/6]>
[PATCH 2/9] <only commit log updated from [PATCH v8 2/6]>
 As per feedbacks from Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> previous
 revision [PATCH v8 3/6] and [PATCH 4/6] are split into following sub-patches:
- [PATCH 3/9] <new> replaces local reference with generic names (mtd, nand_chip)
- [PATCH 4/9] <new> enables auto-detection of bus-width
- [PATCH 5/9] <new> removes omap3_init_bch: populates ecc-scheme data
- [PATCH 6/9] <new> removes omap3_init_bch_tail: populates ecc-layout
- [PATCH 7/9] <new> replaces lib/bch.c with nand_bch.c wrapper
[PATCH 8/9] <no update same as [PATCH v8 5/6]
[PATCH 9/9] removed devm_free_xx functions


*Changes v7 -> v8*
[PATCH 1/6] <no updates>
[PATCH 2/6]
	- updated DT parsing of "ti,nand-ecc-opts" so that its "ham1" remains
		compatible to "sw","hw","hw-romcode"
	- updated DT parsing of "ti,elm-id" to retain compatibility to "elm_id"
	- using of_parse_phandle() to get ELM device pointer from DT
[PATCH 3..6/6] <commit log updates>


*Changes v6 -> v7*
[PATCH 1/6] <NEW> split from [PATCH v6 2/4] as per feedbacks from Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[PATCH 2/6] incorporated feedbacks from DT maintainers
[PATCH 3/6] cleaned and incorporated feedbacks from Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[PATCH 4/6] rebasing changes and cleanup
[PATCH 5/6] updated omap3430-sdp.dts
[PATCH 6/6] <NEW> updated for devm_xx


*Changes v5 -> v6*
[PATCH 1/4]: 
	- updated DT binding for gpmc-nand based on 'Olof Johansson's feedbacks
	http://lists.infradead.org/pipermail/linux-mtd/2013-August/048394.html
	- detection of ELM device via ti,elm-id DT node, moved to gpmc.c driver
[PATCH 2/4]
	- removed: support for following obselete ECC schemes
	OMAP_ECC_HAMMING_CODE_DEFAULT (S/W based 1-bit Hamming ECC)
	OMAP_ECC_HAMMING_CODE_HW_ROMCODE (H/W based 1-bit Hamming ECC scheme)
	- updated: using omap_oobinfo as chip->ecc.layout for all ecc-schemes
	- clean: error messages
[PATCH 3/4] cleaned to include changes for OMAP_ECC_BCH8_CODE_HW only
[PATCH 4/4] updated to include DT property changes


*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-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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]


After this patch series, omap2-nand driver will supports following ECC schemes:
+---------------------------------------+---------------+---------------+
| ECC scheme                            |ECC calculation|Error detection|
+---------------------------------------+---------------+---------------+
|OMAP_ECC_HAM1_CODE_HW                  |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH4_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W (lib/bch.c)|
| (needs CONFIG_MTD_NAND_ECC_BCH)       |               |               |
|                                       |               |               |
|OMAP_ECC_BCH4_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
| (needs CONFIG_MTD_NAND_OMAP_BCH &&    |               |               |
|        ti,elm-id)                     |               |               |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W (lib/bch.c)|
| (needs CONFIG_MTD_NAND_ECC_BCH)       |               |               |
|                                       |               |               |
|OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
| (needs CONFIG_MTD_NAND_OMAP_BCH &&    |               |               |
|        ti,elm-id)                     |               |               |
+---------------------------------------+---------------+---------------+


Pekon Gupta (10):
  ARM: OMAP2+: cleaned-up DT support of various ECC schemes
  mtd: nand: omap: combine different flavours of 1-bit hamming ecc
    schemes
  mtd: nand: omap: cleanup: replace local references with generic
    framework names
  mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET,
    CMD_CMD_PARAM use only x8 bus
  mtd:nand:omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in
    device_probe
  mtd: nand: omap: clean-up ecc layout for BCH ecc schemes
  mtd: nand: omap: use drivers/mtd/nand/nand_bch.c wrapper for BCH ECC
    instead of lib/bch.c
  ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-opt
  mtd: nand: omap: updated devm_xx for all resource allocation and free
    calls
  mtd: nand: omap: remove selection of BCH ecc-scheme via KConfig

 .../devicetree/bindings/mtd/gpmc-nand.txt          |  16 +-
 arch/arm/boot/dts/am335x-evm.dts                   |   3 +-
 arch/arm/boot/dts/omap3430-sdp.dts                 |   2 +-
 arch/arm/mach-omap2/board-flash.c                  |   2 +-
 arch/arm/mach-omap2/gpmc.c                         |  48 +-
 drivers/mtd/nand/Kconfig                           |  40 +-
 drivers/mtd/nand/omap2.c                           | 665 ++++++++++-----------
 include/linux/platform_data/mtd-nand-omap2.h       |  18 +-
 8 files changed, 372 insertions(+), 422 deletions(-)

-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 01/10] ARM: OMAP2+: cleaned-up DT support of various ECC schemes

[Pekon Gupta]

OMAP NAND driver support multiple ECC scheme, which can used in different
flavours, depending on in-build Hardware engines present on SoC.

This patch updates following in DT bindings related to sectionion of ecc-schemes
- ti,elm-id: replaces elm_id (maintains backward compatibility)
- ti,nand-ecc-opts: selection of h/w or s/w implementation of an ecc-scheme
	depends on ti,elm-id. (supported values ham1, bch4, and bch8)
- maintain backward compatibility to deprecated DT bindings (sw, hw, hw-romcode)

Below table shows different flavours of ecc-schemes supported by OMAP devices
+---------------------------------------+---------------+---------------+
| ECC scheme                            |ECC calculation|Error detection|
+---------------------------------------+---------------+---------------+
|OMAP_ECC_HAM1_CODE_HW                  |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W            |
|(requires CONFIG_MTD_NAND_ECC_BCH)     |               |               |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
|(requires CONFIG_MTD_NAND_OMAP_BCH &&  |               |               |
| ti,elm-id in DT)                      |               |               |
+---------------------------------------+---------------+---------------+

To optimize footprint of omap2-nand driver, selection of some ECC schemes
also require enabling following Kconfigs, in addition to setting appropriate
DT bindings
- Kconfig:CONFIG_MTD_NAND_ECC_BCH        error detection done in software
- Kconfig:CONFIG_MTD_NAND_OMAP_BCH       error detection done by h/w engine

Signed-off-by: Pekon Gupta <pekon@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  8 +++-
 arch/arm/mach-omap2/gpmc.c                         | 48 +++++++++++++++-------
 include/linux/platform_data/mtd-nand-omap2.h       | 13 +++++-
 3 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
index df338cb..bfe07e1 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -36,8 +36,12 @@ 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.
+ - elm_id:	<deprecated> use "ti,elm-id" instead
+ - ti,elm-id:	Specifies phandle of the ELM devicetree node.
+		ELM is an on-chip hardware engine on TI SoC which is used for
+		locating ECC errors for BCHx algorithms. SoC devices which have
+		ELM hardware engines should specify this device node in .dtsi
+		Using ELM for ECC error correction frees some CPU cycles.
 
 For inline partiton table parsing (optional):
 
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 579697a..c877129 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1341,14 +1341,6 @@ 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",
-};
-
 static const char * const nand_xfer_types[] = {
 	[NAND_OMAP_PREFETCH_POLLED]		= "prefetch-polled",
 	[NAND_OMAP_POLLED]			= "polled",
@@ -1378,13 +1370,41 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
 	gpmc_nand_data->cs = val;
 	gpmc_nand_data->of_node = child;
 
-	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])) {
-				gpmc_nand_data->ecc_opt = val;
-				break;
-			}
+	/* Detect availability of ELM module */
+	gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
+	if (gpmc_nand_data->elm_of_node == NULL)
+		gpmc_nand_data->elm_of_node =
+					of_parse_phandle(child, "elm_id", 0);
+	if (gpmc_nand_data->elm_of_node == NULL)
+		pr_warn("%s: ti,elm-id property not found\n", __func__);
+
+	/* select ecc-scheme for NAND */
+	if (of_property_read_string(child, "ti,nand-ecc-opt", &s)) {
+		pr_err("%s: ti,nand-ecc-opt not found\n", __func__);
+		return -ENODEV;
+	}
+	if (!strcmp(s, "ham1") || !strcmp(s, "sw") ||
+		!strcmp(s, "hw") || !strcmp(s, "hw-romcode"))
+		gpmc_nand_data->ecc_opt =
+				OMAP_ECC_HAM1_CODE_HW;
+	else if (!strcmp(s, "bch4"))
+		if (gpmc_nand_data->elm_of_node)
+			gpmc_nand_data->ecc_opt =
+				OMAP_ECC_BCH4_CODE_HW;
+		else
+			gpmc_nand_data->ecc_opt =
+				OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
+	else if (!strcmp(s, "bch8"))
+		if (gpmc_nand_data->elm_of_node)
+			gpmc_nand_data->ecc_opt =
+				OMAP_ECC_BCH8_CODE_HW;
+		else
+			gpmc_nand_data->ecc_opt =
+				OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
+	else
+		pr_err("%s: ti,nand-ecc-opt invalid value\n", __func__);
 
+	/* select data transfer mode for NAND controller */
 	if (!of_property_read_string(child, "ti,nand-xfer-type", &s))
 		for (val = 0; val < ARRAY_SIZE(nand_xfer_types); val++)
 			if (!strcasecmp(s, nand_xfer_types[val])) {
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 6bf9ef4..e4128f1 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -28,8 +28,16 @@ enum omap_ecc {
 	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 GPMC, Error detection by Software */
+	OMAP_ECC_HAM1_CODE_HW,
+	/* 4-bit  ECC calculation by GPMC, Error detection by Software */
+	OMAP_ECC_BCH4_CODE_HW_DETECTION_SW,
+	/* 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,
+	/* 8-bit  ECC calculation by GPMC, Error detection by ELM */
+	OMAP_ECC_BCH8_CODE_HW,
 };
 
 struct gpmc_nand_regs {
@@ -63,5 +71,6 @@ struct omap_nand_platform_data {
 
 	/* for passing the partitions */
 	struct device_node	*of_node;
+	struct device_node	*elm_of_node;
 };
 #endif
-- 
1.8.1

[PATCH v10 02/10] mtd: nand: omap: combine different flavours of 1-bit hamming ecc schemes

[Pekon Gupta]

OMAP NAND driver currently supports multiple flavours of 1-bit Hamming
ecc-scheme, like:
- OMAP_ECC_HAMMING_CODE_DEFAULT
	1-bit hamming ecc code using software library
- OMAP_ECC_HAMMING_CODE_HW
	1-bit hamming ecc-code using GPMC h/w engine
- OMAP_ECC_HAMMING_CODE_HW_ROMCODE
	1-bit hamming ecc-code using GPMC h/w engin with ecc-layout compatible
	to ROM code.

This patch combines above multiple ecc-schemes into single implementation:
- OMAP_ECC_HAM1_CODE_HW
	1-bit hamming ecc-code using GPMC h/w engine with ROM-code compatible
	ecc-layout.

Signed-off-by: Pekon Gupta <pekon-l0cyMroinI0@public.gmane.org>
Reviewed-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/mtd/gpmc-nand.txt | 8 ++++----
 arch/arm/mach-omap2/board-flash.c                   | 2 +-
 drivers/mtd/nand/omap2.c                            | 9 +++------
 include/linux/platform_data/mtd-nand-omap2.h        | 7 +------
 4 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
index bfe07e1..5e1f31b 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -22,10 +22,10 @@ Optional properties:
 				width of 8 is assumed.
 
  - 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
+		"sw"		<deprecated> use "ham1" instead
+		"hw"		<deprecated> use "ham1" instead
+		"hw-romcode"	<deprecated> use "ham1" instead
+		"ham1"		1-bit Hamming ecc code
 		"bch4"		4-bit BCH ecc code
 		"bch8"		8-bit BCH ecc code
 
diff --git a/arch/arm/mach-omap2/board-flash.c b/arch/arm/mach-omap2/board-flash.c
index fc20a61..ac82512 100644
--- a/arch/arm/mach-omap2/board-flash.c
+++ b/arch/arm/mach-omap2/board-flash.c
@@ -142,7 +142,7 @@ __init board_nand_init(struct mtd_partition *nand_parts, u8 nr_parts, u8 cs,
 	board_nand_data.nr_parts	= nr_parts;
 	board_nand_data.devsize		= nand_type;
 
-	board_nand_data.ecc_opt = OMAP_ECC_HAMMING_CODE_DEFAULT;
+	board_nand_data.ecc_opt = OMAP_ECC_BCH8_CODE_HW;
 	gpmc_nand_init(&board_nand_data, gpmc_t);
 }
 #endif /* CONFIG_MTD_NAND_OMAP2 || CONFIG_MTD_NAND_OMAP2_MODULE */
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 4ecf0e5..8d521aa 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1993,10 +1993,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 	}
 
 	/* 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)) {
+	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
 		info->nand.ecc.bytes            = 3;
 		info->nand.ecc.size             = 512;
 		info->nand.ecc.strength         = 1;
@@ -2025,7 +2022,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 	}
 
 	/* rom code layout */
-	if (pdata->ecc_opt == OMAP_ECC_HAMMING_CODE_HW_ROMCODE) {
+	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
 
 		if (info->nand.options & NAND_BUSWIDTH_16)
 			offset = 2;
@@ -2033,7 +2030,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 			offset = 1;
 			info->nand.badblock_pattern = &bb_descrip_flashbased;
 		}
-		omap_oobinfo.eccbytes = 3 * (info->mtd.oobsize/16);
+		omap_oobinfo.eccbytes = 3 * (info->mtd.writesize / 512);
 		for (i = 0; i < omap_oobinfo.eccbytes; i++)
 			omap_oobinfo.eccpos[i] = i+offset;
 
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index e4128f1..4da5bfa 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -23,13 +23,8 @@ 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 */
 	/* 1-bit  ECC calculation by GPMC, Error detection by Software */
-	OMAP_ECC_HAM1_CODE_HW,
+	OMAP_ECC_HAM1_CODE_HW = 0,
 	/* 4-bit  ECC calculation by GPMC, Error detection by Software */
 	OMAP_ECC_BCH4_CODE_HW_DETECTION_SW,
 	/* 4-bit  ECC calculation by GPMC, Error detection by ELM */
-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 03/10] mtd: nand: omap: cleanup: replace local references with generic framework names

[Pekon Gupta]

This patch updates following in omap_nand_probe() and omap_nand_remove()
- replaces "info->nand" with "nand_chip" (struct nand_chip *nand_chip)
- replaces "info->mtd" with "mtd" (struct mtd_info *mtd)
- white-space and formatting cleanup

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

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 8d521aa..5596368 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1824,10 +1824,12 @@ 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		*nand_chip;
 	int				err;
 	int				i, offset;
-	dma_cap_mask_t mask;
-	unsigned sig;
+	dma_cap_mask_t			mask;
+	unsigned			sig;
 	struct resource			*res;
 	struct mtd_part_parser_data	ppdata = {};
 
@@ -1846,17 +1848,16 @@ static int omap_nand_probe(struct platform_device *pdev)
 	spin_lock_init(&info->controller.lock);
 	init_waitqueue_head(&info->controller.wq);
 
-	info->pdev = pdev;
-
+	info->pdev		= pdev;
 	info->gpmc_cs		= pdata->cs;
 	info->reg		= pdata->reg;
-
-	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_SKIP_BBTSCAN;
+	mtd			= &info->mtd;
+	mtd->priv		= &info->nand;
+	mtd->name		= dev_name(&pdev->dev);
+	mtd->owner		= THIS_MODULE;
+	nand_chip		= &info->nand;
+	nand_chip->options	= pdata->devsize;
+	nand_chip->options	|= NAND_SKIP_BBTSCAN;
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 	info->of_node		= pdata->of_node;
 #endif
@@ -1877,16 +1878,16 @@ static int omap_nand_probe(struct platform_device *pdev)
 		goto out_free_info;
 	}
 
-	info->nand.IO_ADDR_R = ioremap(info->phys_base, info->mem_size);
-	if (!info->nand.IO_ADDR_R) {
+	nand_chip->IO_ADDR_R = ioremap(info->phys_base, info->mem_size);
+	if (!nand_chip->IO_ADDR_R) {
 		err = -ENOMEM;
 		goto out_release_mem_region;
 	}
 
-	info->nand.controller = &info->controller;
+	nand_chip->controller = &info->controller;
 
-	info->nand.IO_ADDR_W = info->nand.IO_ADDR_R;
-	info->nand.cmd_ctrl  = omap_hwcontrol;
+	nand_chip->IO_ADDR_W = nand_chip->IO_ADDR_R;
+	nand_chip->cmd_ctrl  = omap_hwcontrol;
 
 	/*
 	 * If RDY/BSY line is connected to OMAP then use the omap ready
@@ -1896,26 +1897,26 @@ static int omap_nand_probe(struct platform_device *pdev)
 	 * device and read status register until you get a failure or success
 	 */
 	if (pdata->dev_ready) {
-		info->nand.dev_ready = omap_dev_ready;
-		info->nand.chip_delay = 0;
+		nand_chip->dev_ready = omap_dev_ready;
+		nand_chip->chip_delay = 0;
 	} else {
-		info->nand.waitfunc = omap_wait;
-		info->nand.chip_delay = 50;
+		nand_chip->waitfunc = omap_wait;
+		nand_chip->chip_delay = 50;
 	}
 
 	switch (pdata->xfer_type) {
 	case NAND_OMAP_PREFETCH_POLLED:
-		info->nand.read_buf   = omap_read_buf_pref;
-		info->nand.write_buf  = omap_write_buf_pref;
+		nand_chip->read_buf   = omap_read_buf_pref;
+		nand_chip->write_buf  = omap_write_buf_pref;
 		break;
 
 	case NAND_OMAP_POLLED:
-		if (info->nand.options & NAND_BUSWIDTH_16) {
-			info->nand.read_buf   = omap_read_buf16;
-			info->nand.write_buf  = omap_write_buf16;
+		if (nand_chip->options & NAND_BUSWIDTH_16) {
+			nand_chip->read_buf   = omap_read_buf16;
+			nand_chip->write_buf  = omap_write_buf16;
 		} else {
-			info->nand.read_buf   = omap_read_buf8;
-			info->nand.write_buf  = omap_write_buf8;
+			nand_chip->read_buf   = omap_read_buf8;
+			nand_chip->write_buf  = omap_write_buf8;
 		}
 		break;
 
@@ -1944,8 +1945,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 					err);
 				goto out_release_mem_region;
 			}
-			info->nand.read_buf   = omap_read_buf_dma_pref;
-			info->nand.write_buf  = omap_write_buf_dma_pref;
+			nand_chip->read_buf   = omap_read_buf_dma_pref;
+			nand_chip->write_buf  = omap_write_buf_dma_pref;
 		}
 		break;
 
@@ -1980,8 +1981,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 			goto out_release_mem_region;
 		}
 
-		info->nand.read_buf  = omap_read_buf_irq_pref;
-		info->nand.write_buf = omap_write_buf_irq_pref;
+		nand_chip->read_buf  = omap_read_buf_irq_pref;
+		nand_chip->write_buf = omap_write_buf_irq_pref;
 
 		break;
 
@@ -1994,16 +1995,16 @@ static int omap_nand_probe(struct platform_device *pdev)
 
 	/* select the ecc type */
 	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_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;
-		info->nand.ecc.mode             = NAND_ECC_HW;
+		nand_chip->ecc.bytes            = 3;
+		nand_chip->ecc.size             = 512;
+		nand_chip->ecc.strength         = 1;
+		nand_chip->ecc.calculate        = omap_calculate_ecc;
+		nand_chip->ecc.hwctl            = omap_enable_hwecc;
+		nand_chip->ecc.correct          = omap_correct_data;
+		nand_chip->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);
+		err = omap3_init_bch(mtd, pdata->ecc_opt);
 		if (err) {
 			err = -EINVAL;
 			goto out_release_mem_region;
@@ -2013,9 +2014,9 @@ static int omap_nand_probe(struct platform_device *pdev)
 	/* 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)) {
+	if (nand_scan_ident(mtd, 1, NULL)) {
+		nand_chip->options ^= NAND_BUSWIDTH_16;
+		if (nand_scan_ident(mtd, 1, NULL)) {
 			err = -ENXIO;
 			goto out_release_mem_region;
 		}
@@ -2024,25 +2025,25 @@ static int omap_nand_probe(struct platform_device *pdev)
 	/* rom code layout */
 	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
 
-		if (info->nand.options & NAND_BUSWIDTH_16)
+		if (nand_chip->options & NAND_BUSWIDTH_16) {
 			offset = 2;
-		else {
+		} else {
 			offset = 1;
-			info->nand.badblock_pattern = &bb_descrip_flashbased;
+			nand_chip->badblock_pattern = &bb_descrip_flashbased;
 		}
-		omap_oobinfo.eccbytes = 3 * (info->mtd.writesize / 512);
+		omap_oobinfo.eccbytes = 3 * (mtd->writesize / 512);
 		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 -
+		omap_oobinfo.oobfree->length = mtd->oobsize -
 					(offset + omap_oobinfo.eccbytes);
 
-		info->nand.ecc.layout = &omap_oobinfo;
+		nand_chip->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);
+		err = omap3_init_bch_tail(mtd);
 		if (err) {
 			err = -EINVAL;
 			goto out_release_mem_region;
@@ -2050,16 +2051,16 @@ static int omap_nand_probe(struct platform_device *pdev)
 	}
 
 	/* second phase scan */
-	if (nand_scan_tail(&info->mtd)) {
+	if (nand_scan_tail(mtd)) {
 		err = -ENXIO;
 		goto out_release_mem_region;
 	}
 
 	ppdata.of_node = pdata->of_node;
-	mtd_device_parse_register(&info->mtd, NULL, &ppdata, pdata->parts,
+	mtd_device_parse_register(mtd, NULL, &ppdata, pdata->parts,
 				  pdata->nr_parts);
 
-	platform_set_drvdata(pdev, &info->mtd);
+	platform_set_drvdata(pdev, mtd);
 
 	return 0;
 
@@ -2080,9 +2081,10 @@ out_free_info:
 static int omap_nand_remove(struct platform_device *pdev)
 {
 	struct mtd_info *mtd = platform_get_drvdata(pdev);
+	struct nand_chip *nand_chip = mtd->priv;
 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
 							mtd);
-	omap3_free_bch(&info->mtd);
+	omap3_free_bch(mtd);
 
 	if (info->dma)
 		dma_release_channel(info->dma);
@@ -2093,8 +2095,8 @@ static int omap_nand_remove(struct platform_device *pdev)
 		free_irq(info->gpmc_irq_fifo, info);
 
 	/* Release NAND device, its internal structures and partitions */
-	nand_release(&info->mtd);
-	iounmap(info->nand.IO_ADDR_R);
+	nand_release(mtd);
+	iounmap(nand_chip->IO_ADDR_R);
 	release_mem_region(info->phys_base, info->mem_size);
 	kfree(info);
 	return 0;
-- 
1.8.1

[PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Pekon Gupta]

As per comments below, NAND_CMD_RESET, NAND_CMD_READID, and NAND_CMD_PARAM would
work only in x8 mode.
    commit 64b37b2a63eb2f80b65c7185f0013f8ffc637ae3
    Author:     Matthieu CASTET <matthieu.castet@parrot.com>
    AuthorDate: 2012-11-06
    Note that nand_scan_ident send command (NAND_CMD_RESET, NAND_CMD_READID, NAND_CMD_PARAM), address and read data
    The ONFI specificication is not very clear for x16 device if high byte of address should be driven to 0,
    but according to [1] it should be ok to not drive it during autodetection.

    [1]
    3.3.2. Target Initialization

    [...]
    The Read ID and Read Parameter Page commands only use the lower 8-bits of the data bus.
    The host shall not issue commands that use a word data width on x16 devices until the host
    determines the device supports a 16-bit data bus width in the parameter page.

Thus this patch run nand_scan_ident() with driver configured as x8 device.
Once the NAND device is detected, and its ONFI params are read, the driver
is re-configured based on device-width as passed by DT bindinig 'nand-bus-width'

In-case there is a mis-match between the DT binding 'nand-bus-width' and actual
device-width detected during nand_get_flash_type() then probe returns failure.

All other low-level callback updates happen after the device detection.

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

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 5596368..d29edda 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1856,7 +1856,6 @@ static int omap_nand_probe(struct platform_device *pdev)
 	mtd->name		= dev_name(&pdev->dev);
 	mtd->owner		= THIS_MODULE;
 	nand_chip		= &info->nand;
-	nand_chip->options	= pdata->devsize;
 	nand_chip->options	|= NAND_SKIP_BBTSCAN;
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 	info->of_node		= pdata->of_node;
@@ -1904,6 +1903,39 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->chip_delay = 50;
 	}
 
+	/* scan NAND device connected to chip controller */
+	/* configure driver in x8 mode to read ONFI parameter page, as
+	 * NAND_CMD_READID & NAND_CMD_PARAM may not work in x16 mode */
+	nand_chip->options &= ~NAND_BUSWIDTH_16;
+	if (nand_scan_ident(mtd, 1, NULL)) {
+		/* nand_scan_ident failed */
+		if (pdata->devsize) {
+			/* may be because of mis-match of device-width,
+			 * platform data (DT binding) also says its x16 device
+			 * So re-scan with proper device-width */
+			nand_chip->options |= pdata->devsize;
+			if (nand_scan_ident(mtd, 1, NULL)) {
+				err = -ENXIO;
+				goto out_release_mem_region;
+			}
+		} else {
+			/* some genuine failure, because even platform-data
+			 * (DT binding) says that bus-width is x8 */
+			err = -ENXIO;
+			goto out_release_mem_region;
+		}
+	} else {
+		/* nand_scan_ident passed with x8 mode */
+		if (pdata->devsize) {
+			/* but platform-data (DT binding) say its x16 device */
+			pr_err("%s: incorrect bus-width config\n", DRIVER_NAME);
+			err = -EINVAL;
+			err = -ENXIO;
+			goto out_release_mem_region;
+		}
+	}
+
+	/* re-populate low-level callbacks based on xfer modes */
 	switch (pdata->xfer_type) {
 	case NAND_OMAP_PREFETCH_POLLED:
 		nand_chip->read_buf   = omap_read_buf_pref;
@@ -2011,17 +2043,6 @@ static int omap_nand_probe(struct platform_device *pdev)
 		}
 	}
 
-	/* 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(mtd, 1, NULL)) {
-		nand_chip->options ^= NAND_BUSWIDTH_16;
-		if (nand_scan_ident(mtd, 1, NULL)) {
-			err = -ENXIO;
-			goto out_release_mem_region;
-		}
-	}
-
 	/* rom code layout */
 	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
 
-- 
1.8.1

Re: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Brian Norris]

Hi Pekon,

On Sat, Oct 19, 2013 at 02:14:08PM +0530, Pekon Gupta wrote:
> As per comments below, NAND_CMD_RESET, NAND_CMD_READID, and NAND_CMD_PARAM would
> work only in x8 mode.
>     commit 64b37b2a63eb2f80b65c7185f0013f8ffc637ae3
>     Author:     Matthieu CASTET <matthieu.castet@parrot.com>
>     AuthorDate: 2012-11-06
>     Note that nand_scan_ident send command (NAND_CMD_RESET, NAND_CMD_READID, NAND_CMD_PARAM), address and read data
>     The ONFI specificication is not very clear for x16 device if high byte of address should be driven to 0,
>     but according to [1] it should be ok to not drive it during autodetection.
> 
>     [1]
>     3.3.2. Target Initialization
> 
>     [...]
>     The Read ID and Read Parameter Page commands only use the lower 8-bits of the data bus.
>     The host shall not issue commands that use a word data width on x16 devices until the host
>     determines the device supports a 16-bit data bus width in the parameter page.
> 
> Thus this patch run nand_scan_ident() with driver configured as x8 device.

So are you saying that the driver currently doesn't work if you started
in x16 buswidth? Are you having problems with a particular setup? What
sort of devices are you testing?

Running nand_scan_ident() with x8 when the device is actualy x16 will
just cause nand_scan_ident() to abort with an error. It doesn't help you
with the fact that RESET/READID/PARAM need special 8-bit handling on x16
devices, so you're not solving the problem alluded to by Matthieu.

> Once the NAND device is detected, and its ONFI params are read, the driver
> is re-configured based on device-width as passed by DT bindinig 'nand-bus-width'
> 
> In-case there is a mis-match between the DT binding 'nand-bus-width' and actual
> device-width detected during nand_get_flash_type() then probe returns failure.
> 
> All other low-level callback updates happen after the device detection.
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>

What is your patch trying to solve? It seems like it's just a distortion
of the same code that existed previously. It tries running
nand_scan_ident() in one buswidth configuration, and then it tries the
other if it failed. You still aren't doing either of the options we
talked about previously. I'll repeat them:

(1) You specify the buswidth given by device-tree/platform-data; if this
    is incorrect, you fail

(2) You auto-configure using NAND_BUSWIDTH_AUTO, and you use that to
    validate the platform data; you just warn if the buswidth provided
    by device-tree/platform-data was incorrect

Am I sensing that you are having some implementation problem with one of
these? You really shouldn't need to run nand_scan_ident() twice.

Or perhaps the "solution" in this patch is just that you're moving
around the reassignment of the callback functions? If so, this is not
obvious... see below.

> ---
>  drivers/mtd/nand/omap2.c | 45 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 33 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 5596368..d29edda 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1856,7 +1856,6 @@ static int omap_nand_probe(struct platform_device *pdev)
>  	mtd->name		= dev_name(&pdev->dev);
>  	mtd->owner		= THIS_MODULE;
>  	nand_chip		= &info->nand;
> -	nand_chip->options	= pdata->devsize;
>  	nand_chip->options	|= NAND_SKIP_BBTSCAN;
>  #ifdef CONFIG_MTD_NAND_OMAP_BCH
>  	info->of_node		= pdata->of_node;
> @@ -1904,6 +1903,39 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		nand_chip->chip_delay = 50;
>  	}
>  
> +	/* scan NAND device connected to chip controller */

Why is this comment separated from the comment below it? Either give a
newline between them (if they're really separate) or make it a single
comment block.

> +	/* configure driver in x8 mode to read ONFI parameter page, as
> +	 * NAND_CMD_READID & NAND_CMD_PARAM may not work in x16 mode */
> +	nand_chip->options &= ~NAND_BUSWIDTH_16;
> +	if (nand_scan_ident(mtd, 1, NULL)) {
> +		/* nand_scan_ident failed */
> +		if (pdata->devsize) {
> +			/* may be because of mis-match of device-width,
> +			 * platform data (DT binding) also says its x16 device
> +			 * So re-scan with proper device-width */
> +			nand_chip->options |= pdata->devsize;
> +			if (nand_scan_ident(mtd, 1, NULL)) {
> +				err = -ENXIO;
> +				goto out_release_mem_region;
> +			}
> +		} else {
> +			/* some genuine failure, because even platform-data
> +			 * (DT binding) says that bus-width is x8 */
> +			err = -ENXIO;
> +			goto out_release_mem_region;
> +		}
> +	} else {
> +		/* nand_scan_ident passed with x8 mode */
> +		if (pdata->devsize) {
> +			/* but platform-data (DT binding) say its x16 device */
> +			pr_err("%s: incorrect bus-width config\n", DRIVER_NAME);

You only print this message in one case (detect x8, but DT said x16) and
not the other (detect x16, but DT said x8). This is a result of your
unnecessarily complicated logic here.

> +			err = -EINVAL;
> +			err = -ENXIO;
> +			goto out_release_mem_region;
> +		}
> +	}
> +
> +	/* re-populate low-level callbacks based on xfer modes */

So am I to understand that the main purpose of this patch is not about
the detection of the buswidth type, but about the (re)assignment of the
callback functions? If so, you aren't making it very clear. (I wasn't
paying too much attention to the fact that this code block is being
moved across all the callback assignments.) The above code is just a
more verbose way of doing the same thing as the code you're replacing,
with an extra check to compare with the device-tree/platform-data.

>  	switch (pdata->xfer_type) {
>  	case NAND_OMAP_PREFETCH_POLLED:
>  		nand_chip->read_buf   = omap_read_buf_pref;
> @@ -2011,17 +2043,6 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	/* 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(mtd, 1, NULL)) {
> -		nand_chip->options ^= NAND_BUSWIDTH_16;
> -		if (nand_scan_ident(mtd, 1, NULL)) {
> -			err = -ENXIO;
> -			goto out_release_mem_region;
> -		}
> -	}
> -
>  	/* rom code layout */
>  	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
>  

Anyway, I'm just going to have to flat out NAK this patch for now.
Please rework the series without this patch and we can continue
discussion of this patch independently (for one, this thread does not
need to CC all of the device-tree folks).

Brian

RE: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Gupta, Pekon]

Hi Brian,

> From: Brian Norris [mailto:computersforpeace@gmail.com]
> On Sat, Oct 19, 2013 at 02:14:08PM +0530, Pekon Gupta wrote:
[...]

> >
> > Thus this patch run nand_scan_ident() with driver configured as x8 device.
> 
> So are you saying that the driver currently doesn't work if you started
> in x16 buswidth? Are you having problems with a particular setup? What
> sort of devices are you testing?
> 
No, I'm saying that you cannot read ONFI params in x16 mode.
And, that is what was pointed out in following commit log also ..
(Reference to 3.3.2. Target Initialization: given above)
So, if I run nand_scan_ident() in x16 mode, my ONFI detection would 
fail for sure ..


> Running nand_scan_ident() with x8 when the device is actualy x16 will
> just cause nand_scan_ident() to abort with an error. It doesn't help you
> with the fact that RESET/READID/PARAM need special 8-bit handling on x16
> devices, so you're not solving the problem alluded to by Matthieu.
> 
Yes, absolutely agree.. 
The original code was calling nand_scan_ident() twice, without taking
into consideration whether the first nand_scan_ident() failed because
of bus-width mismatch or something else.

I'm also calling nand_scan_ident() twice, but only when the failure may
be due to bus-width mis-match. I'm just avoiding an extra call to
nand_scan_ident() if the failure was genuine.

If the device actually was x8 then nand_scan_ident() should not fail
for the first-time (in my patch), but if it still fails, then it's a genuine failure
_not_ because of bus-width mismatch.
I'm avoiding the call to nand_scan_ident() again .. 
(same is in my comments below..)

[...]

> What is your patch trying to solve? It seems like it's just a distortion
> of the same code that existed previously. It tries running
> nand_scan_ident() in one buswidth configuration, and then it tries the
> other if it failed. You still aren't doing either of the options we
> talked about previously. I'll repeat them:
> 
Absolutely.. probably you missed my replies in [PATCH v9 4/9]...


> (1) You specify the buswidth given by device-tree/platform-data; if this
>     is incorrect, you fail
> 
Absolutely this is what I'm doing.
But tell me how would you know the actual device-width if
nand_scan_ident()  fails 
(a) to probe ONFI params and 
- your device is  not in nand_ids[]
So get the actual device width, I call the first nand_scan_ident() in x8 mode.
so that ONFI params are read.
Now, if it nand_scan_ident() fails then it means actual device *may* be x16
So, I re-invoke nand_scan_ident() with chip->option |= pdata->devsize;


> (2) You auto-configure using NAND_BUSWIDTH_AUTO, and you use that to
>     validate the platform data; you just warn if the buswidth provided
>     by device-tree/platform-data was incorrect
> 
I have removed NAND_BUSWIDTH_AUTO implementation, as per feedbacks
This is <new> patch. May be you are confusing it with earlier version...
*please re-review*


> Am I sensing that you are having some implementation problem with one of
> these? You really shouldn't need to run nand_scan_ident() twice.
> 
> Or perhaps the "solution" in this patch is just that you're moving
> around the reassignment of the callback functions? If so, this is not
> obvious... see below.
> 
I'm repost my replies from [PATCH v9 4/9] in-case you missed...

------------------------------------
The GPMC driver will be touched by NAND_BUSWIDTH_AUTO change.
There are two set of configurations in GPMC controller..
(a) device based configurations:
  These are static configurations derived on DT bindings for each
  chip-select (like NAND signal timings, etc). These done on GPMC probe.
(b) ecc-scheme based configurations:
  These are dynamic configurations done in NAND driver in
  chip->ecc.hwctl() and are refreshed at each NAND accesss.

However, 'nand-bus-width' configuration is part of both (1) and (2).
Thus, both 'OMAP NAND driver' and 'GPMC driver' need to be
consistent in programming  'nand-bus-width' otherwise ecc-engine
would not work.

Thus, I'm proceeding with (1) DT based parsing of 'nand-bus-width'.
I have re-posted [PATCH v10 4/10] with this updates. However, please
take into account that some limitation of dual programming require
such probe. New patch also call nand_scan_ident() twice, but only
on certain pre-determined conditions, not in all failure.
Once I convert to NAND_BUSWIDTH_AUTO these should get clean,
 as I would update both GPMC and NAND driver for this.
(Till then this was most optimal solution I could think of)..
------------------------------------





> > ---
> >  drivers/mtd/nand/omap2.c | 45
> +++++++++++++++++++++++++++++++++------------
> >  1 file changed, 33 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> > index 5596368..d29edda 100644
> > --- a/drivers/mtd/nand/omap2.c
> > +++ b/drivers/mtd/nand/omap2.c
> > @@ -1856,7 +1856,6 @@ static int omap_nand_probe(struct
> platform_device *pdev)
> >  	mtd->name		= dev_name(&pdev->dev);
> >  	mtd->owner		= THIS_MODULE;
> >  	nand_chip		= &info->nand;
> > -	nand_chip->options	= pdata->devsize;
> >  	nand_chip->options	|= NAND_SKIP_BBTSCAN;

See there is no NAND_BUSWIDTH_AUTO here ....



> > +	/* re-populate low-level callbacks based on xfer modes */
> >  	switch (pdata->xfer_type) {
> >  	case NAND_OMAP_PREFETCH_POLLED:
> >  		nand_chip->read_buf   = omap_read_buf_pref;
> 
> So am I to understand that the main purpose of this patch is not about
> the detection of the buswidth type, but about the (re)assignment of the
> callback functions? If so, you aren't making it very clear. (I wasn't
> paying too much attention to the fact that this code block is being
> moved across all the callback assignments.) The above code is just a
> more verbose way of doing the same thing as the code you're replacing,
> with an extra check to compare with the device-tree/platform-data.
> 
Nope, this is just the comment clean-up.. Please drop it if it confuses you.
One main request please review this patch by locally including it.
May be then you can understand that it has *nothing* to do with 
(re)assignment of callbacks.. rather there is no re-assignment at all
in this patch..
See there is no change in the lines below this comment change
Probably this comment clean-up confused you..


> > @@ -2011,17 +2043,6 @@ static int omap_nand_probe(struct
> platform_device *pdev)
> >  		}
> >  	}
> >
> > -	/* 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(mtd, 1, NULL)) {
> > -		nand_chip->options ^= NAND_BUSWIDTH_16;
> > -		if (nand_scan_ident(mtd, 1, NULL)) {
> > -			err = -ENXIO;
> > -			goto out_release_mem_region;
> > -		}
> > -	}
> > -
> >  	/* rom code layout */
> >  	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
> >
> 
> Anyway, I'm just going to have to flat out NAK this patch for now.
> Please rework the series without this patch and we can continue
> discussion of this patch independently (for one, this thread does not
> need to CC all of the device-tree folks).
> 

I think there was some confusion here..
So, I hv explained my patch above. Request you to please re-review this. 

I'll take NAND_BUSWIDTH_AUTO implementation as a separate patch,
because it would require changes in GPMC driver as stated above.
And so it would be all together independent patch-set.

I would wait for your reply.. 


with regards, pekon

Re: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Brian Norris]

On 10/22/2013 10:07 PM, Gupta, Pekon wrote:
>> From: Brian Norris [mailto:computersforpeace@gmail.com]
>> On Sat, Oct 19, 2013 at 02:14:08PM +0530, Pekon Gupta wrote:
> [...]
> 
>>>
>>> Thus this patch run nand_scan_ident() with driver configured as x8 device.
>>
>> So are you saying that the driver currently doesn't work if you started
>> in x16 buswidth? Are you having problems with a particular setup? What
>> sort of devices are you testing?
>>
> No, I'm saying that you cannot read ONFI params in x16 mode.
> And, that is what was pointed out in following commit log also ..
> (Reference to 3.3.2. Target Initialization: given above)
> So, if I run nand_scan_ident() in x16 mode, my ONFI detection would 
> fail for sure ..

But you cannot just run nand_scan_ident() with !(chip->options &
NAND_BUSWIDTH_16) when your devices is x16. You need to solve the ONFI
detection problem while correctly specifying NAND_BUSWIDTH_16.

Since you didn't answer the other 2 questions there: are you testing any
x16 devices?

>> Running nand_scan_ident() with x8 when the device is actualy x16 will
>> just cause nand_scan_ident() to abort with an error. It doesn't help you
>> with the fact that RESET/READID/PARAM need special 8-bit handling on x16
>> devices, so you're not solving the problem alluded to by Matthieu.
>>
> Yes, absolutely agree.. 
> The original code was calling nand_scan_ident() twice, without taking
> into consideration whether the first nand_scan_ident() failed because
> of bus-width mismatch or something else.
> 
> I'm also calling nand_scan_ident() twice, but only when the failure may
> be due to bus-width mis-match. I'm just avoiding an extra call to
> nand_scan_ident() if the failure was genuine.

You NEVER need to call nand_scan_ident() twice for the same chip.
Period. I will reject any patch that retains this pattern. It is wrong,
and I seriously doubt the code does what you think it does when you do this.

I think nand_scan_ident() may have a weak point where it won't support
ONFI properly for x16 devices. I believe NAND_BUSWIDTH_AUTO was added to
help with this fact. (I don't have any x16 devices to test it.) But if
this is a problem for you, fix it. Don't work around it.

>> What is your patch trying to solve? It seems like it's just a distortion
>> of the same code that existed previously. It tries running
>> nand_scan_ident() in one buswidth configuration, and then it tries the
>> other if it failed. You still aren't doing either of the options we
>> talked about previously. I'll repeat them:
>>
> Absolutely.. probably you missed my replies in [PATCH v9 4/9]...

No, I did not. I just don't see how you think that your code matches the
options (1) or (2) that I described. Perhaps it's a failure in
communication. I will try to be absolutely clear.

>> (1) You specify the buswidth given by device-tree/platform-data; if this
>>     is incorrect, you fail
>>
> Absolutely this is what I'm doing.

No it isn't. You are ignoring the provided buswidth information and
UNCONDITIONALLY trying x8. If/when that fails, you then error out or
retry in x16 (depending on the DT/platform-data).

This is plain wrong.

nand_base is designed (and it's documented in the comments) that the
driver must set the buswidth correctly BEFORE calling nand_scan_ident().
You may not use nand_scan_ident() as a trial-and-error identification
function.

So, to properly do (1), you should only have something like this, just
like all the other NAND drivers:

  nand_chip->options = pdata->devsize & NAND_BUSWIDTH_16;

  ret = nand_scan_ident(...);
  if (ret) {
      // exit with error code...
  }

If there is a problem with this, then you have to fix your driver or
nand_scan_ident(). Perhaps you have to adjust your readbuf() or
cmdfunc() callbacks to do this. But do not add complicated workaround
logic in your driver.

> But tell me how would you know the actual device-width if
> nand_scan_ident()  fails 

If you are not using NAND_BUSWIDTH_AUTO, then you MUST know the correct
buswidth before calling nand_scan_ident(). If your
device-tree/platform-data is wrong, then fix that.

> (a) to probe ONFI params and 
> - your device is  not in nand_ids[]
> So get the actual device width, I call the first nand_scan_ident() in x8 mode.
> so that ONFI params are read.

You don't call nand_scan_ident() with !(chip->options &
NAND_BUSWIDTH_16) when you have an x16 device.

Now, if this causes NAND_CMD_PARAM to fail, then you have a *different*
problem to solve. But you are not solving this problem.

[snipping the rest]

I read your patch, and I gave you my review. I will not accept this
patch, nor any patch that works around nand_scan_ident() by calling it
twice. Fix the framework if the framework is giving you problems.

I believe that this patch is not integral to the rest of the series, so
I will repeat: separate this patch out so I can take the rest of this
series without it.

Brian

RE: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Gupta, Pekon]

Hi Brian,

> From: Brian Norris [mailto:computersforpeace@gmail.com]
> > On 10/22/2013 10:07 PM, Gupta, Pekon wrote:
> >> From: Brian Norris [mailto:computersforpeace@gmail.com]
> >> On Sat, Oct 19, 2013 at 02:14:08PM +0530, Pekon Gupta wrote:

[...]

>> So are you saying that the driver currently doesn't work if you started
>> in x16 buswidth? Are you having problems with a particular setup? What
>> sort of devices are you testing?
>>
...
> Since you didn't answer the other 2 questions there: are you testing any
> x16 devices?
> 

Ans-1: Yes, I'm testing on following boards:
  (a) AM335x-EVM which has x8 Micron device.
  http://www.ti.com/tool/tmdxevm3358
  (b) beaglebone with 'NAND cape', which has x16 Micron device.
  http://beagleboardtoys.info/index.php?title=BeagleBone_4Gb_16-Bit_NAND_Module
  (c) AM437x board (which has 4K page NAND from Macronix).
  (d) Also would be testing on DRA7xx again having Micron Device.

Ans-2:
Its not the setup but rather constrain in nand_base.c which prevents
reading of ONFI params in x16 mode. Please read below..

Ans-3:
Mostly are either x8 or x16 SLC NAND device.


[...]

> You NEVER need to call nand_scan_ident() twice for the same chip.
> Period. I will reject any patch that retains this pattern. It is wrong,
> and I seriously doubt the code does what you think it does when you do this.
> 
> I think nand_scan_ident() may have a weak point where it won't support
> ONFI properly for x16 devices. I believe NAND_BUSWIDTH_AUTO was added
> to
> help with this fact. (I don't have any x16 devices to test it.) But if
> this is a problem for you, fix it. Don't work around it.
> 
So, here comes the conflict.. 
If I'm _not_ using NAND_BUSWIDTH_AUTO, how should I read the ONFI 
params on x16 device ? I don't think there is any way because of following
code in generic nand_base.c
@@ nand_flash_onfi_detect()
/*
 * ONFI must be probed in 8-bit mode or with NAND_BUSWIDTH_AUTO, not
 * with NAND_BUSWIDTH_16
 */
if (chip->options & NAND_BUSWIDTH_16) {
	pr_err("ONFI cannot be probed in 16-bit mode; aborting\n");
	return 0;
}

Introduced in commit..
commit 0ce82b7f7b7373b16ecf7b5725e21e2975204500
Author:     Matthieu CASTET <matthieu.castet@parrot.com>
AuthorDate: 2013-01-16

And, I think this commit has implicitly made NAND_BUSWIDTH_AUTO
*mandatory* to be supported by every driver for interfacing with
x16 NAND devices. Isn't it ?
(unless you add every x16 device present in market to nand_flash_id[])

 [...]

> nand_base is designed (and it's documented in the comments) that the
> driver must set the buswidth correctly BEFORE calling nand_scan_ident().
> You may not use nand_scan_ident() as a trial-and-error identification
> function.
> 
> So, to properly do (1), you should only have something like this, just
> like all the other NAND drivers:
> 
>   nand_chip->options = pdata->devsize & NAND_BUSWIDTH_16;
> 
>   ret = nand_scan_ident(...);
>   if (ret) {
>       // exit with error code...
>   }
> 
For a x16 device.. This would *always* fail for x16 device, unless device
is listed in nand_flash_id[]. isn't  it ?
because you cannot read ONFI params in x16 mode, and your device is
not listed in nand_flash_ids[], so your device would not get identified.

And I sincerely don't know how other NAND drivers are probing
x16 NAND devices _without_ enabling NAND_BUSWIDTH_AUTO.
(may be by adding every supported NAND device to nand_flash_id[]
look-up table, which is not recommended).


> If there is a problem with this, then you have to fix your driver or
> nand_scan_ident(). Perhaps you have to adjust your readbuf() or
> cmdfunc() callbacks to do this. But do not add complicated workaround
> logic in your driver.
> 
chip->cmdfunc() and chip->readbufs() are all fine. Rather I let the
generic driver set them for nand_scan_ident().
So, all this calling nand_scan_ident() twice was done because of
previously mentioned reasons..


> [snipping the rest]
> 
> I read your patch, and I gave you my review. I will not accept this
> patch, nor any patch that works around nand_scan_ident() by calling it
> twice. Fix the framework if the framework is giving you problems.
> 
It's a chicken and egg problem, 
I have no solution but all I can say is the above commit, which converted
WARNING into ERROR, until all drivers adapt to NAND_BUSWIDTH_AUTO.

Anyway I have to do nand_scan_ident() somewhere before populating
the chip->ecc.layout and other fields.. so can't drop the patch..
But I'll put your suggestion, instead of my mine.

> I believe that this patch is not integral to the rest of the series, so
> I will repeat: separate this patch out so I can take the rest of this
> series without it.
> 
I'll replace the patch with your suggestions,
So, that you have both versions. Please pick whichever you like :-)


with regards, pekon

Re: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Ezequiel Garcia]

Brian,

On Tue, Oct 22, 2013 at 11:13:32PM -0700, Brian Norris wrote:
> On 10/22/2013 10:07 PM, Gupta, Pekon wrote:
> >> From: Brian Norris [mailto:computersforpeace@gmail.com]
> >> On Sat, Oct 19, 2013 at 02:14:08PM +0530, Pekon Gupta wrote:
> > [...]
> > 
> >>>
> >>> Thus this patch run nand_scan_ident() with driver configured as x8 device.
> >>
> >> So are you saying that the driver currently doesn't work if you started
> >> in x16 buswidth? Are you having problems with a particular setup? What
> >> sort of devices are you testing?
> >>
> > No, I'm saying that you cannot read ONFI params in x16 mode.
> > And, that is what was pointed out in following commit log also ..
> > (Reference to 3.3.2. Target Initialization: given above)
> > So, if I run nand_scan_ident() in x16 mode, my ONFI detection would 
> > fail for sure ..
> 
> But you cannot just run nand_scan_ident() with !(chip->options &
> NAND_BUSWIDTH_16) when your devices is x16. You need to solve the ONFI
> detection problem while correctly specifying NAND_BUSWIDTH_16.
> 
> Since you didn't answer the other 2 questions there: are you testing any
> x16 devices?
> 

I'm jumping on this thread without having read all the discussion, sorry
about that.

FWIW, I have a Beaglebone with a 16-bit bus NAND attached to it.

Coincidentally, yesterday I was doing some tests as I'm ramping up the
NAND and I found that weird double nand_scan_ident() call.
The whole thing looks buggy to me, so I'm happy to help, review, test
and patches to take care of this.

I'm using some TI SDK with some ancient v3.2.x (with no git history!),
but from this discussion it seems the issue is still present in
mainline.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Gupta, Pekon]

Hi,

> From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
[...]
> FWIW, I have a Beaglebone with a 16-bit bus NAND attached to it.
> 
> Coincidentally, yesterday I was doing some tests as I'm ramping up the
> NAND and I found that weird double nand_scan_ident() call.
> The whole thing looks buggy to me, so I'm happy to help, review, test
> and patches to take care of this.
> 
Yes, thanks .. that would be of great help.. 
And may be your experience of Atmel drivers would help me here..

*Correct, should not be double calls to nand_scan_ident()..*
But there is a constrain in nand_base.c, that it does not allow ONFI
page reading in x16 mode.. So how to overcome that..

I see the similar implementation in your ATMEL driver, it does not use
NAND_BUSWIDTH_AUTO so how do you perform ONFI read
for x16 devices ?
drivers/mtd/nand/atmel_nand.c @@atmel_nand_probe()
/* here you move to x16 mode based on your DT or platform data */
	if (host->board.bus_width_16)	/* 16-bit bus width */
		nand_chip->options |= NAND_BUSWIDTH_16;
/* And then you call nand_scan_ident */
/* first scan to find the device and get the page size */
	if (nand_scan_ident(mtd, 1, NULL)) {
		res = -ENXIO;
		goto err_scan_ident;
	}

Wouldn't this fail, _unless_ your device is listed in nand_flash_id[] ?
because it would not be able to read ONFI params.. 
Refer below commit.. 
commit 0ce82b7f7b7373b16ecf7b5725e21e2975204500
Author:     Matthieu CASTET <matthieu.castet@parrot.com>
AuthorDate: 2013-01-16

Thanks for pitching in, this would help me to understand this better.



> I'm using some TI SDK with some ancient v3.2.x (with no git history!),
> but from this discussion it seems the issue is still present in
> mainline.
> 
Aah sorry, then you might have some problem here in rebasing the
patches. But still if you can, thanks much ..


with regards, pekon
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Ezequiel Garcia]

Hi Gupta,

On Wed, Oct 23, 2013 at 01:15:20PM +0000, Gupta, Pekon wrote:
> Hi,
> 
> > From: Ezequiel Garcia [mailto:ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org]
> [...]
> > FWIW, I have a Beaglebone with a 16-bit bus NAND attached to it.
> > 
> > Coincidentally, yesterday I was doing some tests as I'm ramping up the
> > NAND and I found that weird double nand_scan_ident() call.
> > The whole thing looks buggy to me, so I'm happy to help, review, test
> > and patches to take care of this.
> > 
> Yes, thanks .. that would be of great help.. 
> And may be your experience of Atmel drivers would help me here..
> 

It's not Atmel, but Marvell :-)

> *Correct, should not be double calls to nand_scan_ident()..*
> But there is a constrain in nand_base.c, that it does not allow ONFI
> page reading in x16 mode.. So how to overcome that..
> 
> I see the similar implementation in your ATMEL driver, it does not use
> NAND_BUSWIDTH_AUTO so how do you perform ONFI read
> for x16 devices ?
> drivers/mtd/nand/atmel_nand.c @@atmel_nand_probe()
> /* here you move to x16 mode based on your DT or platform data */
> 	if (host->board.bus_width_16)	/* 16-bit bus width */
> 		nand_chip->options |= NAND_BUSWIDTH_16;
> /* And then you call nand_scan_ident */
> /* first scan to find the device and get the page size */
> 	if (nand_scan_ident(mtd, 1, NULL)) {
> 		res = -ENXIO;
> 		goto err_scan_ident;
> 	}
> 
> Wouldn't this fail, _unless_ your device is listed in nand_flash_id[] ?
> because it would not be able to read ONFI params.. 
> Refer below commit.. 
> commit 0ce82b7f7b7373b16ecf7b5725e21e2975204500
> Author:     Matthieu CASTET <matthieu.castet-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
> AuthorDate: 2013-01-16
> 
> 

Not my driver, but I'm taking a look at it now. Not sure if I'll get
into something here.

> 
> 
> > I'm using some TI SDK with some ancient v3.2.x (with no git history!),
> > but from this discussion it seems the issue is still present in
> > mainline.
> > 
> Aah sorry, then you might have some problem here in rebasing the
> patches. But still if you can, thanks much ..
> 

I'm currently trying mainline (just for this issue not for my product).
I just need some time to prepare the bootargs and write a DT node for
the NAND cape.

Again, not sure if I'll make some progress, but I'll give it a shot :-)
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Ezequiel Garcia]

Pekon,

On Wed, Oct 23, 2013 at 10:24:57AM -0300, Ezequiel Garcia wrote:
[..]
> 
> I'm currently trying mainline (just for this issue not for my product).
> I just need some time to prepare the bootargs and write a DT node for
> the NAND cape.
> 
> Again, not sure if I'll make some progress, but I'll give it a shot :-)

I won't be able to make too much progress without some help or without
squeezing my brains out :P

Care to push some git branch on some random repo with DT support for
the NAND cape in the Beaglebone?

Please base it in either some recent l2-mtd or some tag from Linus.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Gupta, Pekon]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

Hi Ezequiel,

> From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
> I won't be able to make too much progress without some help or without
> squeezing my brains out :P
> 
> Care to push some git branch on some random repo with DT support for
> the NAND cape in the Beaglebone?
> 
Apologies for delay, I was testing and preparing a newer version of patch-set..

Please find the beagle-bone DTS attached. However, consider this as RFC
only, as I'm waiting for current series which has some DT-binding updates
to get accepted first.

Commit log in the patch would also guide you for some board tweaks
for enabling NAND cape on beagle-bone (LT/white)..
(I have recently sent v11 of the patch series incorporating last few
 comments from Brian about nand_scan_ident().)


Thanks.. 
with regards, pekon

[-- Attachment #2: 0001-ARM-DTS-am335x-bone-enable-support-for-NAND-cape.patch --]
[-- Type: application/octet-stream, Size: 5291 bytes --]

From 88b9f815b8dca74322491b7d07cb14754435cd45 Mon Sep 17 00:00:00 2001
From: Pekon Gupta <pekon@ti.com>
Date: Wed, 23 Oct 2013 20:26:51 +0530
Subject: [PATCH 1/1] ARM: DTS: am335x-bone: enable support for NAND cape

beaglebone board can be connected to expansion boards to add devices to them.
These expansion boards are called 'capes'. This patch updates pin-mux for
'NAND' cape which can be used with beaglebone LT (white).
Further information and datasheets of this NAND cape can be found at:
- http://beagleboardtoys.info/index.php?title=BeagleBone_Memory_Expansion
- http://beagleboardtoys.info/index.php?title=BeagleBone_4Gb_16-Bit_NAND_Module

*Specifics for Beaglebone Cape*
(1) On Memory-Expander Cape boot modes can be optionally be selected via
    DIP switch(SW2) present on cape board.
	SW2[SWITCH_BOOT] == 0: boot mode selected via DIP switch(SW2)
	SW2[SWITCH_BOOT] == 1: follow default boot order
				MMC-> SPI -> UART -> USB .. 24MHz

(2) As BOOTSEL values are sampled only at POR, so after changing any setting
    on SW2 (DIP switch), disconnect and reconnect all board power supply
   (including mini-USB) to POR the beaglebone.

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 arch/arm/boot/dts/am335x-bone-common.dtsi | 28 ++++++++++++
 arch/arm/boot/dts/am335x-bone.dts         | 75 +++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
index 2f66ded..73dcc58 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -107,6 +107,34 @@
 				0x14c (PIN_INPUT_PULLDOWN | MUX_MODE7)
 			>;
 		};
+
+		nandflash_pins_s0: nandflash_pins_s0 {
+			pinctrl-single,pins = <
+				0x0 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad0.gpmc_ad0 */
+				0x4 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad1.gpmc_ad1 */
+				0x8 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad2.gpmc_ad2 */
+				0xc (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad3.gpmc_ad3 */
+				0x10 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad4.gpmc_ad4 */
+				0x14 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad5.gpmc_ad5 */
+				0x18 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad6.gpmc_ad6 */
+				0x1c (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad7.gpmc_ad7 */
+				0x20 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad8.gpmc_ad8 */
+				0x24 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad9.gpmc_ad9 */
+				0x28 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad10.gpmc_ad10 */
+				0x2c (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad11.gpmc_ad11 */
+				0x30 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad12.gpmc_ad12 */
+				0x34 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad13.gpmc_ad13 */
+				0x38 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad14.gpmc_ad14 */
+				0x3c (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_ad15.gpmc_ad15 */
+				0x70 (PIN_INPUT_PULLUP | MUX_MODE0)	/* gpmc_wait0.gpmc_wait0 */
+				0x74 (PIN_INPUT_PULLUP | MUX_MODE7)	/* gpmc_wpn.gpio0_30 */
+				0x7c (PIN_OUTPUT | MUX_MODE0)		/* gpmc_csn0.gpmc_csn0  */
+				0x90 (PIN_OUTPUT | MUX_MODE0)		/* gpmc_advn_ale.gpmc_advn_ale */
+				0x94 (PIN_OUTPUT | MUX_MODE0)		/* gpmc_oen_ren.gpmc_oen_ren */
+				0x98 (PIN_OUTPUT | MUX_MODE0)		/* gpmc_wen.gpmc_wen */
+				0x9c (PIN_OUTPUT | MUX_MODE0)		/* gpmc_be0n_cle.gpmc_be0n_cle */
+			>;
+		};
 	};
 
 	ocp {
diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index 7993c48..d9172c7 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -9,3 +9,78 @@
 
 #include "am33xx.dtsi"
 #include "am335x-bone-common.dtsi"
+
+&elm {
+	status = "okay";
+};
+
+&gpmc {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&nandflash_pins_s0>;
+	ranges = <0 0 0x08000000 0x10000000>;	/* CS0: NAND */
+	nand@0,0 {
+		reg = <0 0 0>; /* CS0, offset 0 */
+		nand-bus-width = <16>;
+		ti,nand-ecc-opt = "bch8";
+		gpmc,device-width = <2>;
+		gpmc,sync-clk-ps = <0>;
+		gpmc,cs-on-ns = <0>;
+		gpmc,cs-rd-off-ns = <740>;
+		gpmc,cs-wr-off-ns = <740>;
+		gpmc,adv-on-ns = <0>;
+		gpmc,adv-rd-off-ns = <740>;
+		gpmc,adv-wr-off-ns = <740>;
+		gpmc,we-on-ns = <90>;
+		gpmc,we-off-ns = <600>;
+		gpmc,oe-on-ns = <150>;
+		gpmc,oe-off-ns = <650>;
+		gpmc,access-ns = <560>;
+		gpmc,rd-cycle-ns = <740>;
+		gpmc,wr-cycle-ns = <740>;
+		gpmc,wait-on-read = "true";
+		gpmc,wait-on-write = "true";
+		gpmc,bus-turnaround-ns = <0>;
+		gpmc,cycle2cycle-delay-ns = <0>;
+		gpmc,clk-activation-ns = <0>;
+		gpmc,wait-monitoring-ns = <0>;
+		gpmc,wr-access-ns = <40>;
+		gpmc,wr-data-mux-bus-ns = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ti,elm-id = <&elm>;
+		/* MTD partition table */
+		partition@0 {
+			label = "SPL1";
+			reg = <0x00000000 0x000020000>;
+		};
+		partition@1 {
+			label = "SPL2";
+			reg = <0x00020000 0x00020000>;
+		};
+		partition@2 {
+			label = "SPL3";
+			reg = <0x00040000 0x00020000>;
+		};
+		partition@3 {
+			label = "SPL4";
+			reg = <0x00060000 0x00020000>;
+		};
+		partition@4 {
+			label = "U-boot";
+			reg = <0x00080000 0x001e0000>;
+		};
+		partition@5 {
+			label = "environment";
+			reg = <0x00260000 0x00020000>;
+		};
+		partition@6 {
+			label = "Kernel";
+			reg = <0x00280000 0x00500000>;
+		};
+		partition@7 {
+			label = "File-System";
+			reg = <0x00780000 0x0F880000>;
+		};
+	};
+};
-- 
1.8.1


[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v10 04/10] mtd: nand: omap: fix device scan: NAND_CMD_READID, NAND_CMD_RESET, CMD_CMD_PARAM use only x8 bus

[Ezequiel Garcia]

Pekon,

On Thu, Oct 24, 2013 at 12:59:26PM +0000, Gupta, Pekon wrote:
> Hi Ezequiel,
> 
> > From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
> > I won't be able to make too much progress without some help or without
> > squeezing my brains out :P
> > 
> > Care to push some git branch on some random repo with DT support for
> > the NAND cape in the Beaglebone?
> > 
> Apologies for delay, I was testing and preparing a newer version of patch-set..
> 
> Please find the beagle-bone DTS attached. However, consider this as RFC
> only, as I'm waiting for current series which has some DT-binding updates
> to get accepted first.
> 
> Commit log in the patch would also guide you for some board tweaks
> for enabling NAND cape on beagle-bone (LT/white)..
> (I have recently sent v11 of the patch series incorporating last few
>  comments from Brian about nand_scan_ident().)
> 

Thanks! I'll give this a try.

Anyway, for future reference, it's really easier for testers and
reviewers to just push the code in some public git repo (github? gitorious?).

This way I don't have to apply the patches from my mailbox,
but just checkout a branch...
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 05/10] mtd:nand:omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in device_probe

[Pekon Gupta]

current implementation in omap3_init_bch() has some redundant code like:
(1) omap3_init_bch() re-probes the DT-binding to detect presence of ELM h/w
    engine on SoC. And based on that it selects implemetation of ecc-scheme.
    However, this is already done as part of GPMC DT parsing.
(2) As omap3_init_bch() serves as common function for configuring all types of
    BCHx ecc-schemes, so there are multiple levels of redudant if..then..else
    checks while populating nand_chip->ecc.

This patch make following changes to OMAP NAND driver:
(1) removes omap3_init_bch(): each ecc-scheme is individually configured in
    omap_nand_probe() there by removing redundant if..then..else checks.
(2) adds is_elm_present(): re-probing of ELM device via DT is not required as
    it's done in GPMC driver probe. Thus is_elm_present() just initializes ELM
    driver with NAND probe data, when ecc-scheme with h/w based error-detection
    is used.
(3) separates out configuration of different flavours of "BCH4" and "BCH8"
    ecc-schemes as given in below table
(4) conditionally compiles callbacks implementations of ecc.hwctl(),
    ecc.calculate(), ecc.correct() to avoid warning of un-used functions.

+---------------------------------------+---------------+---------------+
| ECC scheme                            |ECC calculation|Error detection|
+---------------------------------------+---------------+---------------+
|OMAP_ECC_HAM1_CODE_HW                  |H/W (GPMC)     |S/W            |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH4_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W (lib/bch.c)|
| (needs CONFIG_MTD_NAND_ECC_BCH)       |               |               |
|                                       |               |               |
|OMAP_ECC_BCH4_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
| (needs CONFIG_MTD_NAND_OMAP_BCH &&    |               |               |
|        ti,elm-id)                     |               |               |
+---------------------------------------+---------------+---------------+
|OMAP_ECC_BCH8_CODE_HW_DETECTION_SW     |H/W (GPMC)     |S/W (lib/bch.c)|
| (needs CONFIG_MTD_NAND_ECC_BCH)       |               |               |
|                                       |               |               |
|OMAP_ECC_BCH8_CODE_HW                  |H/W (GPMC)     |H/W (ELM)      |
| (needs CONFIG_MTD_NAND_OMAP_BCH &&    |               |               |
|        ti,elm-id)                     |               |               |
+---------------------------------------+---------------+---------------+

- 'CONFIG_MTD_NAND_ECC_BCH' is generic KConfig required to build lib/bch.c
    which is required for ECC error detection done in software.
    (mainly used for legacy platforms which do not have on-chip ELM engine)

- 'CONFIG_MTD_NAND_OMAP_BCH' is OMAP specific Kconfig to detemine presence
    on ELM h/w engine on SoC.

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

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index d29edda..978240b 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -25,10 +25,8 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
 #include <linux/bch.h>
 #include <linux/platform_data/elm.h>
-#endif
 
 #include <linux/platform_data/mtd-nand-omap2.h>
 
@@ -141,6 +139,8 @@
 #define BCH_ECC_SIZE0		0x0	/* ecc_size0 = 0, no oob protection */
 #define BCH_ECC_SIZE1		0x20	/* ecc_size1 = 32 */
 
+#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 +182,12 @@ struct omap_nand_info {
 	u_char				*buf;
 	int					buf_len;
 	struct gpmc_nand_regs		reg;
-
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
+	/* fields specific for BCHx_HW ECC scheme */
 	struct bch_control             *bch;
 	struct nand_ecclayout           ecclayout;
 	bool				is_elm_used;
 	struct device			*elm_dev;
 	struct device_node		*of_node;
-#endif
 };
 
 /**
@@ -1058,8 +1056,7 @@ static int omap_dev_ready(struct mtd_info *mtd)
 	}
 }
 
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
-
+#if defined(CONFIG_MTD_NAND_ECC_BCH) || defined(CONFIG_MTD_NAND_OMAP_BCH)
 /**
  * omap3_enable_hwecc_bch - Program OMAP3 GPMC to perform BCH ECC correction
  * @mtd: MTD device structure
@@ -1140,7 +1137,9 @@ static void omap3_enable_hwecc_bch(struct mtd_info *mtd, int mode)
 	/* Clear ecc and enable bits */
 	writel(ECCCLEAR | ECC1, info->reg.gpmc_ecc_control);
 }
+#endif
 
+#ifdef CONFIG_MTD_NAND_ECC_BCH
 /**
  * omap3_calculate_ecc_bch4 - Generate 7 bytes of ECC bytes
  * @mtd: MTD device structure
@@ -1225,7 +1224,9 @@ static int omap3_calculate_ecc_bch8(struct mtd_info *mtd, const u_char *dat,
 
 	return 0;
 }
+#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
@@ -1517,7 +1518,9 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
 
 	return stat;
 }
+#endif /* CONFIG_MTD_NAND_OMAP_BCH */
 
+#ifdef CONFIG_MTD_NAND_ECC_BCH
 /**
  * omap3_correct_data_bch - Decode received data and correct errors
  * @mtd: MTD device structure
@@ -1549,7 +1552,9 @@ static int omap3_correct_data_bch(struct mtd_info *mtd, u_char *data,
 	}
 	return count;
 }
+#endif /* CONFIG_MTD_NAND_ECC_BCH */
 
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
 /**
  * omap_write_page_bch - BCH ecc based write page function for entire page
  * @mtd:		mtd info structure
@@ -1637,118 +1642,48 @@ 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
+ * 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 void omap3_free_bch(struct mtd_info *mtd)
+static int is_elm_present(struct omap_nand_info *info,
+			struct device_node *elm_node, enum bch_ecc bch_type)
 {
-	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
-						   mtd);
-	if (info->bch) {
-		free_bch(info->bch);
-		info->bch = NULL;
+	struct platform_device *pdev;
+	info->is_elm_used = false;
+	/* check whether elm-id is passed via DT */
+	if (!elm_node) {
+		pr_err("nand: error: ELM DT node not found\n");
+		return -ENODEV;
+	}
+	pdev = of_find_device_by_node(elm_node);
+	/* check whether ELM device is registered */
+	if (!pdev) {
+		pr_err("nand: error: ELM device not found\n");
+		return -ENODEV;
 	}
+	/* ELM module available, now configure it */
+	info->elm_dev = &pdev->dev;
+	if (elm_config(info->elm_dev, bch_type))
+		return -ENODEV;
+	info->is_elm_used = true;
+	return 0;
 }
+#endif /* CONFIG_MTD_NAND_ECC_BCH */
 
+#ifdef CONFIG_MTD_NAND_ECC_BCH
 /**
- * omap3_init_bch - Initialize BCH ECC
+ * omap3_free_bch - Release BCH ecc resources
  * @mtd: MTD device structure
- * @ecc_opt: OMAP ECC mode (OMAP_ECC_BCH4_CODE_HW or OMAP_ECC_BCH8_CODE_HW)
  */
-static int omap3_init_bch(struct mtd_info *mtd, int ecc_opt)
+static void omap3_free_bch(struct mtd_info *mtd)
 {
-	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;
-
-	/* 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;
-		}
+	if (info->bch) {
+		free_bch(info->bch);
+		info->bch = NULL;
 	}
-
-	pr_info("enabling NAND BCH ecc with %d-bit correction\n", max_errors);
-	return 0;
-fail:
-	omap3_free_bch(mtd);
-	return -1;
 }
 
 /**
@@ -1806,11 +1741,6 @@ fail:
 }
 
 #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;
@@ -1818,7 +1748,7 @@ static int omap3_init_bch_tail(struct mtd_info *mtd)
 static void omap3_free_bch(struct mtd_info *mtd)
 {
 }
-#endif /* CONFIG_MTD_NAND_OMAP_BCH */
+#endif /* CONFIG_MTD_NAND_ECC_BCH */
 
 static int omap_nand_probe(struct platform_device *pdev)
 {
@@ -1851,15 +1781,14 @@ 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->of_node		= pdata->of_node;
 	mtd			= &info->mtd;
 	mtd->priv		= &info->nand;
 	mtd->name		= dev_name(&pdev->dev);
 	mtd->owner		= THIS_MODULE;
 	nand_chip		= &info->nand;
 	nand_chip->options	|= NAND_SKIP_BBTSCAN;
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
-	info->of_node		= pdata->of_node;
-#endif
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL) {
@@ -2025,22 +1954,125 @@ static int omap_nand_probe(struct platform_device *pdev)
 		goto out_release_mem_region;
 	}
 
-	/* select the ecc type */
-	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
+	/* populate MTD interface based on ECC scheme */
+	switch (pdata->ecc_opt) {
+	case OMAP_ECC_HAM1_CODE_HW:
+		pr_info("nand: using OMAP_ECC_HAM1_CODE_HW\n");
+		nand_chip->ecc.mode             = NAND_ECC_HW;
 		nand_chip->ecc.bytes            = 3;
 		nand_chip->ecc.size             = 512;
 		nand_chip->ecc.strength         = 1;
 		nand_chip->ecc.calculate        = omap_calculate_ecc;
 		nand_chip->ecc.hwctl            = omap_enable_hwecc;
 		nand_chip->ecc.correct          = omap_correct_data;
-		nand_chip->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(mtd, pdata->ecc_opt);
-		if (err) {
+		break;
+
+	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
+#ifdef CONFIG_MTD_NAND_ECC_BCH
+		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW_DETECTION_SW\n");
+		nand_chip->ecc.mode		= NAND_ECC_HW;
+		nand_chip->ecc.size		= 512;
+		nand_chip->ecc.bytes		= 7;
+		nand_chip->ecc.strength		= 4;
+		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
+		nand_chip->ecc.correct		= omap3_correct_data_bch;
+		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch4;
+		/* software bch library is used for locating errors */
+		info->bch = init_bch(nand_chip->ecc.bytes,
+					nand_chip->ecc.strength,
+					OMAP_ECC_BCH8_POLYNOMIAL);
+		if (!info->bch) {
+			pr_err("nand: error: unable to use s/w BCH library\n");
 			err = -EINVAL;
+		}
+		break;
+#else
+		pr_err("nand: error: CONFIG_MTD_NAND_ECC_BCH not enabled\n");
+		err = -EINVAL;
+		goto out_release_mem_region;
+#endif
+
+	case OMAP_ECC_BCH4_CODE_HW:
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+		pr_info("nand: using OMAP_ECC_BCH4_CODE_HW ECC scheme\n");
+		nand_chip->ecc.mode		= NAND_ECC_HW;
+		nand_chip->ecc.size		= 512;
+		/* 14th bit is kept reserved for ROM-code compatibility */
+		nand_chip->ecc.bytes		= 7 + 1;
+		nand_chip->ecc.strength		= 4;
+		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
+		nand_chip->ecc.correct		= omap_elm_correct_data;
+		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch;
+		nand_chip->ecc.read_page	= omap_read_page_bch;
+		nand_chip->ecc.write_page	= omap_write_page_bch;
+		/* This ECC scheme requires ELM H/W block */
+		if (is_elm_present(info, pdata->elm_of_node, BCH4_ECC) < 0) {
+			pr_err("nand: error: could not initialize ELM\n");
+			err = -ENODEV;
 			goto out_release_mem_region;
 		}
+		break;
+#else
+		pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
+		err = -EINVAL;
+		goto out_release_mem_region;
+#endif
+
+	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
+#ifdef CONFIG_MTD_NAND_ECC_BCH
+		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW_DETECTION_SW\n");
+		nand_chip->ecc.mode		= NAND_ECC_HW;
+		nand_chip->ecc.size		= 512;
+		nand_chip->ecc.bytes		= 13;
+		nand_chip->ecc.strength		= 8;
+		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
+		nand_chip->ecc.correct		= omap3_correct_data_bch;
+		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch8;
+		/* software bch library is used for locating errors */
+		info->bch = init_bch(nand_chip->ecc.bytes,
+					nand_chip->ecc.strength,
+					OMAP_ECC_BCH8_POLYNOMIAL);
+		if (!info->bch) {
+			pr_err("nand: error: unable to use s/w BCH library\n");
+			err = -EINVAL;
+			goto out_release_mem_region;
+		}
+		break;
+#else
+		pr_err("nand: error: CONFIG_MTD_NAND_ECC_BCH not enabled\n");
+		err = -EINVAL;
+		goto out_release_mem_region;
+#endif
+
+	case OMAP_ECC_BCH8_CODE_HW:
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
+		pr_info("nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme\n");
+		nand_chip->ecc.mode		= NAND_ECC_HW;
+		nand_chip->ecc.size		= 512;
+		/* 14th bit is kept reserved for ROM-code compatibility */
+		nand_chip->ecc.bytes		= 13 + 1;
+		nand_chip->ecc.strength		= 8;
+		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
+		nand_chip->ecc.correct		= omap_elm_correct_data;
+		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch;
+		nand_chip->ecc.read_page	= omap_read_page_bch;
+		nand_chip->ecc.write_page	= omap_write_page_bch;
+		/* This ECC scheme requires ELM H/W block */
+		if (is_elm_present(info, pdata->elm_of_node, BCH8_ECC) < 0) {
+			pr_err("nand: error: could not initialize ELM\n");
+			goto out_release_mem_region;
+		}
+		break;
+#else
+		pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
+		err = -EINVAL;
+		goto out_release_mem_region;
+#endif
+
+	default:
+		pr_err("nand: error: invalid or unsupported ECC scheme\n");
+		err = -EINVAL;
+		goto out_release_mem_region;
 	}
 
 	/* rom code layout */
@@ -2062,6 +2094,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 
 		nand_chip->ecc.layout = &omap_oobinfo;
 	} else if ((pdata->ecc_opt == OMAP_ECC_BCH4_CODE_HW) ||
+		   (pdata->ecc_opt == OMAP_ECC_BCH4_CODE_HW_DETECTION_SW) ||
+		   (pdata->ecc_opt == OMAP_ECC_BCH8_CODE_HW_DETECTION_SW) ||
 		   (pdata->ecc_opt == OMAP_ECC_BCH8_CODE_HW)) {
 		/* build OOB layout for BCH ECC correction */
 		err = omap3_init_bch_tail(mtd);
@@ -2094,6 +2128,7 @@ 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(mtd);
 	kfree(info);
 
 	return err;
-- 
1.8.1

[PATCH v10 06/10] mtd: nand: omap: clean-up ecc layout for BCH ecc schemes

[Pekon Gupta]

In current implementation omap3_init_bch_tail() is a common function to
define ecc layout for different BCHx ecc schemes.This patch:
(1) removes omap3_init_bch_tail() and defines ecc layout for individual
    ecc-schemes along with populating their nand_chip->ecc data in
    omap_nand_probe(). This improves the readability and scalability of
    code for add new ecc schemes in future.
(2) removes 'struct nand_bbt_descr bb_descrip_flashbased' because default
    nand_bbt_descr in nand_bbt.c matches the same (.len=1 for x8 devices).
(3) add the check to see if NAND device has enough OOB/Spare bytes to
    store ECC signature of whole page, as defined by ecc-scheme.

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

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 978240b..c93655e 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -139,6 +139,7 @@
 #define BCH_ECC_SIZE0		0x0	/* ecc_size0 = 0, no oob protection */
 #define BCH_ECC_SIZE1		0x20	/* ecc_size1 = 32 */
 
+#define BADBLOCK_MARKER_LENGTH		2
 #define OMAP_ECC_BCH8_POLYNOMIAL	0x201b
 
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
@@ -149,17 +150,6 @@ static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10};
 
 /* oob info generated runtime depending on ecc algorithm and layout selected */
 static struct nand_ecclayout omap_oobinfo;
-/* Define some generic bad / good block scan pattern which are used
- * while scanning a device for factory marked good / bad blocks
- */
-static uint8_t scan_ff_pattern[] = { 0xff };
-static struct nand_bbt_descr bb_descrip_flashbased = {
-	.options = NAND_BBT_SCANALLPAGES,
-	.offs = 0,
-	.len = 1,
-	.pattern = scan_ff_pattern,
-};
-
 
 struct omap_nand_info {
 	struct nand_hw_control		controller;
@@ -184,7 +174,6 @@ struct omap_nand_info {
 	struct gpmc_nand_regs		reg;
 	/* fields specific for BCHx_HW ECC scheme */
 	struct bch_control             *bch;
-	struct nand_ecclayout           ecclayout;
 	bool				is_elm_used;
 	struct device			*elm_dev;
 	struct device_node		*of_node;
@@ -1686,65 +1675,8 @@ static void omap3_free_bch(struct mtd_info *mtd)
 	}
 }
 
-/**
- * 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;
-	}
-
-	/* 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_tail(struct mtd_info *mtd)
-{
-	return -1;
-}
+
 static void omap3_free_bch(struct mtd_info *mtd)
 {
 }
@@ -1756,8 +1688,9 @@ static int omap_nand_probe(struct platform_device *pdev)
 	struct omap_nand_platform_data	*pdata;
 	struct mtd_info			*mtd;
 	struct nand_chip		*nand_chip;
+	struct nand_ecclayout		*ecclayout;
 	int				err;
-	int				i, offset;
+	int				i;
 	dma_cap_mask_t			mask;
 	unsigned			sig;
 	struct resource			*res;
@@ -1864,6 +1797,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* check for small page devices */
+	if ((mtd->oobsize < 64) && (pdata->ecc_opt != OMAP_ECC_HAM1_CODE_HW)) {
+		pr_err("small page devices are not supported\n");
+		err = -EINVAL;
+		goto out_release_mem_region;
+	}
+
 	/* re-populate low-level callbacks based on xfer modes */
 	switch (pdata->xfer_type) {
 	case NAND_OMAP_PREFETCH_POLLED:
@@ -1955,6 +1895,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 	}
 
 	/* populate MTD interface based on ECC scheme */
+	nand_chip->ecc.layout	= &omap_oobinfo;
+	ecclayout		= &omap_oobinfo;
 	switch (pdata->ecc_opt) {
 	case OMAP_ECC_HAM1_CODE_HW:
 		pr_info("nand: using OMAP_ECC_HAM1_CODE_HW\n");
@@ -1965,6 +1907,16 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.calculate        = omap_calculate_ecc;
 		nand_chip->ecc.hwctl            = omap_enable_hwecc;
 		nand_chip->ecc.correct          = omap_correct_data;
+		/* define ECC layout */
+		ecclayout->eccbytes		= nand_chip->ecc.bytes *
+							(mtd->writesize /
+							nand_chip->ecc.size);
+		if (nand_chip->options & NAND_BUSWIDTH_16)
+			ecclayout->eccpos[0]	= BADBLOCK_MARKER_LENGTH;
+		else
+			ecclayout->eccpos[0]	= 1;
+		ecclayout->oobfree->offset	= ecclayout->eccpos[0] +
+							ecclayout->eccbytes;
 		break;
 
 	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -1977,6 +1929,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
 		nand_chip->ecc.correct		= omap3_correct_data_bch;
 		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch4;
+		/* define ECC layout */
+		ecclayout->eccbytes		= nand_chip->ecc.bytes *
+							(mtd->writesize /
+							nand_chip->ecc.size);
+		ecclayout->eccpos[0]		= BADBLOCK_MARKER_LENGTH;
+		ecclayout->oobfree->offset	= ecclayout->eccpos[0] +
+							ecclayout->eccbytes;
 		/* software bch library is used for locating errors */
 		info->bch = init_bch(nand_chip->ecc.bytes,
 					nand_chip->ecc.strength,
@@ -2005,6 +1964,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch;
 		nand_chip->ecc.read_page	= omap_read_page_bch;
 		nand_chip->ecc.write_page	= omap_write_page_bch;
+		/* define ECC layout */
+		ecclayout->eccbytes		= nand_chip->ecc.bytes *
+							(mtd->writesize /
+							nand_chip->ecc.size);
+		ecclayout->eccpos[0]		= BADBLOCK_MARKER_LENGTH;
+		ecclayout->oobfree->offset	= ecclayout->eccpos[0] +
+							ecclayout->eccbytes;
 		/* This ECC scheme requires ELM H/W block */
 		if (is_elm_present(info, pdata->elm_of_node, BCH4_ECC) < 0) {
 			pr_err("nand: error: could not initialize ELM\n");
@@ -2028,6 +1994,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
 		nand_chip->ecc.correct		= omap3_correct_data_bch;
 		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch8;
+		/* define ECC layout */
+		ecclayout->eccbytes		= nand_chip->ecc.bytes *
+							(mtd->writesize /
+							nand_chip->ecc.size);
+		ecclayout->eccpos[0]		= BADBLOCK_MARKER_LENGTH;
+		ecclayout->oobfree->offset	= ecclayout->eccpos[0] +
+							ecclayout->eccbytes;
 		/* software bch library is used for locating errors */
 		info->bch = init_bch(nand_chip->ecc.bytes,
 					nand_chip->ecc.strength,
@@ -2062,6 +2035,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 			pr_err("nand: error: could not initialize ELM\n");
 			goto out_release_mem_region;
 		}
+		/* define ECC layout */
+		ecclayout->eccbytes		= nand_chip->ecc.bytes *
+							(mtd->writesize /
+							nand_chip->ecc.size);
+		ecclayout->eccpos[0]		= BADBLOCK_MARKER_LENGTH;
+		ecclayout->oobfree->offset	= ecclayout->eccpos[0] +
+							ecclayout->eccbytes;
 		break;
 #else
 		pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
@@ -2075,34 +2055,17 @@ static int omap_nand_probe(struct platform_device *pdev)
 		goto out_release_mem_region;
 	}
 
-	/* rom code layout */
-	if (pdata->ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
-
-		if (nand_chip->options & NAND_BUSWIDTH_16) {
-			offset = 2;
-		} else {
-			offset = 1;
-			nand_chip->badblock_pattern = &bb_descrip_flashbased;
-		}
-		omap_oobinfo.eccbytes = 3 * (mtd->writesize / 512);
-		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 = mtd->oobsize -
-					(offset + omap_oobinfo.eccbytes);
-
-		nand_chip->ecc.layout = &omap_oobinfo;
-	} else if ((pdata->ecc_opt == OMAP_ECC_BCH4_CODE_HW) ||
-		   (pdata->ecc_opt == OMAP_ECC_BCH4_CODE_HW_DETECTION_SW) ||
-		   (pdata->ecc_opt == OMAP_ECC_BCH8_CODE_HW_DETECTION_SW) ||
-		   (pdata->ecc_opt == OMAP_ECC_BCH8_CODE_HW)) {
-		/* build OOB layout for BCH ECC correction */
-		err = omap3_init_bch_tail(mtd);
-		if (err) {
-			err = -EINVAL;
-			goto out_release_mem_region;
-		}
+	/* populate remaining ECC layout data */
+	ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH +
+							ecclayout->eccbytes);
+	for (i = 1; i < ecclayout->eccbytes; i++)
+		ecclayout->eccpos[i] = ecclayout->eccpos[0] + i;
+	/* check if NAND device's OOB is enough to store ECC signatures */
+	if (mtd->oobsize < (ecclayout->eccbytes + BADBLOCK_MARKER_LENGTH)) {
+		pr_err("not enough OOB bytes required = %d, available=%d\n",
+					   ecclayout->eccbytes, mtd->oobsize);
+		err = -EINVAL;
+		goto out_release_mem_region;
 	}
 
 	/* second phase scan */
-- 
1.8.1

[PATCH v10 07/10] mtd: nand: omap: use drivers/mtd/nand/nand_bch.c wrapper for BCH ECC instead of lib/bch.c

[Pekon Gupta]

generic frame-work in mtd/nand/nand_bch.c is a wrapper above lib/bch.h which
encapsulates all control information specific to BCH ecc algorithm in software.
Thus this patch:
(1) replace omap specific implementations with equivalent wrapper in nand_bch.c
    so that generic code from nand_bch.c is re-used. like;
        omap3_correct_data_bch() -> nand_bch_correct_data()
        omap3_free_bch() -> nand_bch_free()
(2) replace direct calls to lib/bch.c with wrapper functions defined in nand_bch.c
	init_bch() -> nand_bch_init()

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

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index c93655e..4f8efe0 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -25,7 +25,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 
-#include <linux/bch.h>
+#include <linux/mtd/nand_bch.h>
 #include <linux/platform_data/elm.h>
 
 #include <linux/platform_data/mtd-nand-omap2.h>
@@ -140,7 +140,6 @@
 #define BCH_ECC_SIZE1		0x20	/* ecc_size1 = 32 */
 
 #define BADBLOCK_MARKER_LENGTH		2
-#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,
@@ -173,7 +172,6 @@ struct omap_nand_info {
 	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;
@@ -1507,43 +1505,7 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
 
 	return stat;
 }
-#endif /* CONFIG_MTD_NAND_OMAP_BCH */
 
-#ifdef CONFIG_MTD_NAND_ECC_BCH
-/**
- * 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;
-}
-#endif /* CONFIG_MTD_NAND_ECC_BCH */
-
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
 /**
  * omap_write_page_bch - BCH ecc based write page function for entire page
  * @mtd:		mtd info structure
@@ -1660,28 +1622,6 @@ static int is_elm_present(struct omap_nand_info *info,
 }
 #endif /* CONFIG_MTD_NAND_ECC_BCH */
 
-#ifdef CONFIG_MTD_NAND_ECC_BCH
-/**
- * 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 */
-
 static int omap_nand_probe(struct platform_device *pdev)
 {
 	struct omap_nand_info		*info;
@@ -1714,13 +1654,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->of_node		= pdata->of_node;
 	mtd			= &info->mtd;
 	mtd->priv		= &info->nand;
 	mtd->name		= dev_name(&pdev->dev);
 	mtd->owner		= THIS_MODULE;
 	nand_chip		= &info->nand;
+	nand_chip->ecc.priv	= NULL;
 	nand_chip->options	|= NAND_SKIP_BBTSCAN;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1927,7 +1867,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.bytes		= 7;
 		nand_chip->ecc.strength		= 4;
 		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
-		nand_chip->ecc.correct		= omap3_correct_data_bch;
+		nand_chip->ecc.correct		= nand_bch_correct_data;
 		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch4;
 		/* define ECC layout */
 		ecclayout->eccbytes		= nand_chip->ecc.bytes *
@@ -1937,10 +1877,11 @@ static int omap_nand_probe(struct platform_device *pdev)
 		ecclayout->oobfree->offset	= ecclayout->eccpos[0] +
 							ecclayout->eccbytes;
 		/* software bch library is used for locating errors */
-		info->bch = init_bch(nand_chip->ecc.bytes,
-					nand_chip->ecc.strength,
-					OMAP_ECC_BCH8_POLYNOMIAL);
-		if (!info->bch) {
+		nand_chip->ecc.priv		= nand_bch_init(mtd,
+							nand_chip->ecc.size,
+							nand_chip->ecc.bytes,
+							&nand_chip->ecc.layout);
+		if (!nand_chip->ecc.priv) {
 			pr_err("nand: error: unable to use s/w BCH library\n");
 			err = -EINVAL;
 		}
@@ -1992,7 +1933,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.bytes		= 13;
 		nand_chip->ecc.strength		= 8;
 		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
-		nand_chip->ecc.correct		= omap3_correct_data_bch;
+		nand_chip->ecc.correct		= nand_bch_correct_data;
 		nand_chip->ecc.calculate	= omap3_calculate_ecc_bch8;
 		/* define ECC layout */
 		ecclayout->eccbytes		= nand_chip->ecc.bytes *
@@ -2002,10 +1943,11 @@ static int omap_nand_probe(struct platform_device *pdev)
 		ecclayout->oobfree->offset	= ecclayout->eccpos[0] +
 							ecclayout->eccbytes;
 		/* software bch library is used for locating errors */
-		info->bch = init_bch(nand_chip->ecc.bytes,
-					nand_chip->ecc.strength,
-					OMAP_ECC_BCH8_POLYNOMIAL);
-		if (!info->bch) {
+		nand_chip->ecc.priv		= nand_bch_init(mtd,
+							nand_chip->ecc.size,
+							nand_chip->ecc.bytes,
+							&nand_chip->ecc.layout);
+		if (!nand_chip->ecc.priv) {
 			pr_err("nand: error: unable to use s/w BCH library\n");
 			err = -EINVAL;
 			goto out_release_mem_region;
@@ -2091,7 +2033,10 @@ 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(mtd);
+	if (nand_chip->ecc.priv) {
+		nand_bch_free(nand_chip->ecc.priv);
+		nand_chip->ecc.priv = NULL;
+	}
 	kfree(info);
 
 	return err;
@@ -2103,7 +2048,10 @@ static int omap_nand_remove(struct platform_device *pdev)
 	struct nand_chip *nand_chip = mtd->priv;
 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
 							mtd);
-	omap3_free_bch(mtd);
+	if (nand_chip->ecc.priv) {
+		nand_bch_free(nand_chip->ecc.priv);
+		nand_chip->ecc.priv = NULL;
+	}
 
 	if (info->dma)
 		dma_release_channel(info->dma);
-- 
1.8.1

[PATCH v10 08/10] ARM: dts: AM33xx: updated default ECC scheme in nand-ecc-opt

[Pekon Gupta]

Updated DTS to replace deprecated binding with newer values
Refer: Documentation/devicetree/bindings/mtd/gpmc-nand.txt

Signed-off-by: Pekon Gupta <pekon@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/boot/dts/am335x-evm.dts   | 3 +--
 arch/arm/boot/dts/omap3430-sdp.dts | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index e8ec875..1aee6ac 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -269,7 +269,6 @@
 				reg = <0 0 0>; /* CS0, offset 0 */
 				nand-bus-width = <8>;
 				ti,nand-ecc-opt = "bch8";
-				gpmc,device-nand = "true";
 				gpmc,device-width = <1>;
 				gpmc,sync-clk-ps = <0>;
 				gpmc,cs-on-ns = <0>;
@@ -296,7 +295,7 @@
 
 				#address-cells = <1>;
 				#size-cells = <1>;
-				elm_id = <&elm>;
+				ti,elm-id = <&elm>;
 
 				/* MTD partition table */
 				partition@0 {
diff --git a/arch/arm/boot/dts/omap3430-sdp.dts b/arch/arm/boot/dts/omap3430-sdp.dts
index e2249bc..501f863 100644
--- a/arch/arm/boot/dts/omap3430-sdp.dts
+++ b/arch/arm/boot/dts/omap3430-sdp.dts
@@ -105,7 +105,7 @@
 		reg = <1 0 0x08000000>;
 		nand-bus-width = <8>;
 
-		ti,nand-ecc-opt = "sw";
+		ti,nand-ecc-opt = "ham1";
 		gpmc,cs-on-ns = <0>;
 		gpmc,cs-rd-off-ns = <36>;
 		gpmc,cs-wr-off-ns = <36>;
-- 
1.8.1

[PATCH v10 09/10] mtd: nand: omap: updated devm_xx for all resource allocation and free calls

[Pekon Gupta]

"Managed Device Resource" or devm_xx calls takes care of automatic freeing
of the resource in case of:
- failure during driver probe
- failure during resource allocation
- detaching or unloading of driver module (rmmod)
Reference: Documentation/driver-model/devres.txt

Though OMAP NAND driver handles freeing of resource allocation in most of
the cases, but using devm_xx provides more clean and effortless approach
to handle all such cases.

- simplifies label for exiting probe during error
  s/out_release_mem_region/return_error

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

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 4f8efe0..45433f4 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1642,7 +1642,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	info = kzalloc(sizeof(struct omap_nand_info), GFP_KERNEL);
+	info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
+				GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
@@ -1667,22 +1668,23 @@ static int omap_nand_probe(struct platform_device *pdev)
 	if (res == NULL) {
 		err = -EINVAL;
 		dev_err(&pdev->dev, "error getting memory resource\n");
-		goto out_free_info;
+		goto return_error;
 	}
 
 	info->phys_base = res->start;
 	info->mem_size = resource_size(res);
 
-	if (!request_mem_region(info->phys_base, info->mem_size,
-				pdev->dev.driver->name)) {
+	if (!devm_request_mem_region(&pdev->dev, info->phys_base,
+				info->mem_size,	pdev->dev.driver->name)) {
 		err = -EBUSY;
-		goto out_free_info;
+		goto return_error;
 	}
 
-	nand_chip->IO_ADDR_R = ioremap(info->phys_base, info->mem_size);
+	nand_chip->IO_ADDR_R = devm_ioremap(&pdev->dev, info->phys_base,
+						info->mem_size);
 	if (!nand_chip->IO_ADDR_R) {
 		err = -ENOMEM;
-		goto out_release_mem_region;
+		goto return_error;
 	}
 
 	nand_chip->controller = &info->controller;
@@ -1718,13 +1720,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 			nand_chip->options |= pdata->devsize;
 			if (nand_scan_ident(mtd, 1, NULL)) {
 				err = -ENXIO;
-				goto out_release_mem_region;
+				goto return_error;
 			}
 		} else {
 			/* some genuine failure, because even platform-data
 			 * (DT binding) says that bus-width is x8 */
 			err = -ENXIO;
-			goto out_release_mem_region;
+			goto return_error;
 		}
 	} else {
 		/* nand_scan_ident passed with x8 mode */
@@ -1733,7 +1735,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 			pr_err("%s: incorrect bus-width config\n", DRIVER_NAME);
 			err = -EINVAL;
 			err = -ENXIO;
-			goto out_release_mem_region;
+			goto return_error;
 		}
 	}
 
@@ -1741,7 +1743,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 	if ((mtd->oobsize < 64) && (pdata->ecc_opt != OMAP_ECC_HAM1_CODE_HW)) {
 		pr_err("small page devices are not supported\n");
 		err = -EINVAL;
-		goto out_release_mem_region;
+		goto return_error;
 	}
 
 	/* re-populate low-level callbacks based on xfer modes */
@@ -1769,7 +1771,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 		if (!info->dma) {
 			dev_err(&pdev->dev, "DMA engine request failed\n");
 			err = -ENXIO;
-			goto out_release_mem_region;
+			goto return_error;
 		} else {
 			struct dma_slave_config cfg;
 
@@ -1784,7 +1786,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 			if (err) {
 				dev_err(&pdev->dev, "DMA engine slave config failed: %d\n",
 					err);
-				goto out_release_mem_region;
+				goto return_error;
 			}
 			nand_chip->read_buf   = omap_read_buf_dma_pref;
 			nand_chip->write_buf  = omap_write_buf_dma_pref;
@@ -1796,30 +1798,32 @@ static int omap_nand_probe(struct platform_device *pdev)
 		if (info->gpmc_irq_fifo <= 0) {
 			dev_err(&pdev->dev, "error getting fifo irq\n");
 			err = -ENODEV;
-			goto out_release_mem_region;
+			goto return_error;
 		}
-		err = request_irq(info->gpmc_irq_fifo,	omap_nand_irq,
-					IRQF_SHARED, "gpmc-nand-fifo", info);
+		err = devm_request_irq(&pdev->dev, info->gpmc_irq_fifo,
+					omap_nand_irq, IRQF_SHARED,
+					"gpmc-nand-fifo", info);
 		if (err) {
 			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
 						info->gpmc_irq_fifo, err);
 			info->gpmc_irq_fifo = 0;
-			goto out_release_mem_region;
+			goto return_error;
 		}
 
 		info->gpmc_irq_count = platform_get_irq(pdev, 1);
 		if (info->gpmc_irq_count <= 0) {
 			dev_err(&pdev->dev, "error getting count irq\n");
 			err = -ENODEV;
-			goto out_release_mem_region;
+			goto return_error;
 		}
-		err = request_irq(info->gpmc_irq_count,	omap_nand_irq,
-					IRQF_SHARED, "gpmc-nand-count", info);
+		err = devm_request_irq(&pdev->dev, info->gpmc_irq_count,
+					omap_nand_irq, IRQF_SHARED,
+					"gpmc-nand-count", info);
 		if (err) {
 			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
 						info->gpmc_irq_count, err);
 			info->gpmc_irq_count = 0;
-			goto out_release_mem_region;
+			goto return_error;
 		}
 
 		nand_chip->read_buf  = omap_read_buf_irq_pref;
@@ -1831,7 +1835,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev,
 			"xfer_type(%d) not supported!\n", pdata->xfer_type);
 		err = -EINVAL;
-		goto out_release_mem_region;
+		goto return_error;
 	}
 
 	/* populate MTD interface based on ECC scheme */
@@ -1889,7 +1893,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 #else
 		pr_err("nand: error: CONFIG_MTD_NAND_ECC_BCH not enabled\n");
 		err = -EINVAL;
-		goto out_release_mem_region;
+		goto return_error;
 #endif
 
 	case OMAP_ECC_BCH4_CODE_HW:
@@ -1916,13 +1920,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		if (is_elm_present(info, pdata->elm_of_node, BCH4_ECC) < 0) {
 			pr_err("nand: error: could not initialize ELM\n");
 			err = -ENODEV;
-			goto out_release_mem_region;
+			goto return_error;
 		}
 		break;
 #else
 		pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
 		err = -EINVAL;
-		goto out_release_mem_region;
+		goto return_error;
 #endif
 
 	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
@@ -1950,13 +1954,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		if (!nand_chip->ecc.priv) {
 			pr_err("nand: error: unable to use s/w BCH library\n");
 			err = -EINVAL;
-			goto out_release_mem_region;
+			goto return_error;
 		}
 		break;
 #else
 		pr_err("nand: error: CONFIG_MTD_NAND_ECC_BCH not enabled\n");
 		err = -EINVAL;
-		goto out_release_mem_region;
+		goto return_error;
 #endif
 
 	case OMAP_ECC_BCH8_CODE_HW:
@@ -1975,7 +1979,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 		/* This ECC scheme requires ELM H/W block */
 		if (is_elm_present(info, pdata->elm_of_node, BCH8_ECC) < 0) {
 			pr_err("nand: error: could not initialize ELM\n");
-			goto out_release_mem_region;
+			goto return_error;
 		}
 		/* define ECC layout */
 		ecclayout->eccbytes		= nand_chip->ecc.bytes *
@@ -1988,13 +1992,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 #else
 		pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
 		err = -EINVAL;
-		goto out_release_mem_region;
+		goto return_error;
 #endif
 
 	default:
 		pr_err("nand: error: invalid or unsupported ECC scheme\n");
 		err = -EINVAL;
-		goto out_release_mem_region;
+		goto return_error;
 	}
 
 	/* populate remaining ECC layout data */
@@ -2007,13 +2011,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		pr_err("not enough OOB bytes required = %d, available=%d\n",
 					   ecclayout->eccbytes, mtd->oobsize);
 		err = -EINVAL;
-		goto out_release_mem_region;
+		goto return_error;
 	}
 
 	/* second phase scan */
 	if (nand_scan_tail(mtd)) {
 		err = -ENXIO;
-		goto out_release_mem_region;
+		goto return_error;
 	}
 
 	ppdata.of_node = pdata->of_node;
@@ -2024,21 +2028,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 
 	return 0;
 
-out_release_mem_region:
+return_error:
 	if (info->dma)
 		dma_release_channel(info->dma);
-	if (info->gpmc_irq_count > 0)
-		free_irq(info->gpmc_irq_count, info);
-	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:
 	if (nand_chip->ecc.priv) {
 		nand_bch_free(nand_chip->ecc.priv);
 		nand_chip->ecc.priv = NULL;
 	}
-	kfree(info);
-
 	return err;
 }
 
@@ -2052,20 +2048,9 @@ static int omap_nand_remove(struct platform_device *pdev)
 		nand_bch_free(nand_chip->ecc.priv);
 		nand_chip->ecc.priv = NULL;
 	}
-
 	if (info->dma)
 		dma_release_channel(info->dma);
-
-	if (info->gpmc_irq_count > 0)
-		free_irq(info->gpmc_irq_count, info);
-	if (info->gpmc_irq_fifo > 0)
-		free_irq(info->gpmc_irq_fifo, info);
-
-	/* Release NAND device, its internal structures and partitions */
 	nand_release(mtd);
-	iounmap(nand_chip->IO_ADDR_R);
-	release_mem_region(info->phys_base, info->mem_size);
-	kfree(info);
 	return 0;
 }
 
-- 
1.8.1

[PATCH v10 10/10] mtd: nand: omap: remove selection of BCH ecc-scheme via KConfig

[Pekon Gupta]

With OMAP NAND driver updates, selection of ecc-scheme:
*DT enabled kernel*
 	depends on ti,nand-ecc-opt and ti,elm-id DT bindings.
*Non DT enabled kernel*
	depends on elm_dev and ecc-scheme passed along with platform-data
	from board file.

So, selection of ecc-scheme (BCH8 or BCH4) from KConfig can be removed

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/Kconfig | 40 ++++++----------------------------------
 1 file changed, 6 insertions(+), 34 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index d885298..93ae6a6 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -96,43 +96,15 @@ 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
-
-if MTD_NAND_OMAP_BCH
-config BCH_CONST_M
-	default 13
-config BCH_CONST_T
-	default 4 if MTD_NAND_OMAP_BCH4
-	default 8 if MTD_NAND_OMAP_BCH8
-endif
+	  This config enables the ELM hardware engine, which can be used to
+	  locate and correct errors when using BCH ECC scheme. This offloads
+	  the cpu from doing ECC error searching and correction. However some
+	  legacy OMAP families like OMAP2xxx, OMAP3xxx do not have ELM engine
+	  so they should not enable this config symbol.
 
 config MTD_NAND_IDS
 	tristate
-- 
1.8.1

Re: [PATCH v10 10/10] mtd: nand: omap: remove selection of BCH ecc-scheme via KConfig

[Ezequiel Garcia]

Gupta,

I already have a question :-)

On Sat, Oct 19, 2013 at 02:14:14PM +0530, Pekon Gupta wrote:
> With OMAP NAND driver updates, selection of ecc-scheme:
> *DT enabled kernel*
>  	depends on ti,nand-ecc-opt and ti,elm-id DT bindings.
> *Non DT enabled kernel*
> 	depends on elm_dev and ecc-scheme passed along with platform-data
> 	from board file.
> 
> So, selection of ecc-scheme (BCH8 or BCH4) from KConfig can be removed
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  drivers/mtd/nand/Kconfig | 40 ++++++----------------------------------
>  1 file changed, 6 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index d885298..93ae6a6 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -96,43 +96,15 @@ config MTD_NAND_OMAP2
>  
>  config MTD_NAND_OMAP_BCH
>  	depends on MTD_NAND && MTD_NAND_OMAP2 && ARCH_OMAP3

I'm wondering how are you testing this in your SOC_AM33XX board (which
is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?

IMO, no need to depend on any ARCH_xxx, just make it depend on the driver
option itself, which already depends on ARCH_OMAP2PLUS (which is the bigger
SoC family).

Care to send a one-patch fix for that, so this *independent* bug can be
taken by Brian? Please add my Reported-by when you do so.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v10 10/10] mtd: nand: omap: remove selection of BCH ecc-scheme via KConfig

[Gupta, Pekon]

> From: Ezequiel Garcia [mailto:ezequiel.garcia@free-electrons.com]
> 
> Gupta,
> 
Please call me 'Pekon' :-)

[...]

> 
> I'm wondering how are you testing this in your SOC_AM33XX board (which
> is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?
> 
Yes, omap2plus_defconfig is a super set..
arch/arm/configs/omap2plus_defconfig automatically enables ARCH_OMAP3.

> IMO, no need to depend on any ARCH_xxx, just make it depend on the
> driver 
> option itself, which already depends on ARCH_OMAP2PLUS (which is the
> bigger SoC family).
> 
> Care to send a one-patch fix for that, so this *independent* bug can be
> taken by Brian? Please add my Reported-by when you do so.
> 
Thanks. It's good to clean this dependency..
Yes, surely I'll send a independent patch with your reported-by..


with regards, pekon
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v10 10/10] mtd: nand: omap: remove selection of BCH ecc-scheme via KConfig

[Ezequiel Garcia]

Pekon,

On Wed, Oct 23, 2013 at 01:55:58PM +0000, Gupta, Pekon wrote:
> > 
> > I'm wondering how are you testing this in your SOC_AM33XX board (which
> > is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?
> > 
> Yes, omap2plus_defconfig is a super set..
> arch/arm/configs/omap2plus_defconfig automatically enables ARCH_OMAP3.
> 

Yes, but I always remove what I won't use to reduce build time.

And  now that you bring this issue. IMHO, the AM33xx family is going
to be more and more widely used, so maybe introducing an am33xx_defconfig
makes sense.

Or is the trend to have the least possible amount of defconfigs?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 10/10] mtd: nand: omap: remove selection of BCH ecc-scheme via KConfig

[Javier Martinez Canillas]

Hi Ezequiel

On Wed, Oct 23, 2013 at 4:13 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Pekon,
>
> On Wed, Oct 23, 2013 at 01:55:58PM +0000, Gupta, Pekon wrote:
>> >
>> > I'm wondering how are you testing this in your SOC_AM33XX board (which
>> > is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?
>> >
>> Yes, omap2plus_defconfig is a super set..
>> arch/arm/configs/omap2plus_defconfig automatically enables ARCH_OMAP3.
>>
>
> Yes, but I always remove what I won't use to reduce build time.
>
> And  now that you bring this issue. IMHO, the AM33xx family is going
> to be more and more widely used, so maybe introducing an am33xx_defconfig
> makes sense.
>
> Or is the trend to have the least possible amount of defconfigs?

I'm not familiar with AM33xx so I don't know how similar is to OMAP3
but we used to have different defconfigs for each OMAP board before
and consolidated everything in only two defconfigs: omap1_defconfig to
be used for all OMAP1 devices and omap2plus_defconfig to be used for
all OMAP2+ (i.e: OMAP{2,3,4,5}) devices.

Those have all the common kconfig options and board vendors can
customize to fit their needs and have a delta with something like:

$ ./scripts/kconfig/merge_config.sh
arch/arm/configs/omap2plus_defconfig /foo/bar/igep_defconfig

Again I'm not familiar with AM33xx but what I do know is that we
should keep defconfigs to a minimum.

> --
> Ezequiel García, Free Electrons
> Embedded Linux, Kernel and Android Engineering
> http://free-electrons.com
> --

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 10/10] mtd: nand: omap: remove selection of BCH ecc-scheme via KConfig

[Ezequiel Garcia]

Javier,

On Thu, Oct 24, 2013 at 09:49:09PM +0200, Javier Martinez Canillas wrote:
> Hi Ezequiel
> 
> On Wed, Oct 23, 2013 at 4:13 PM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > Pekon,
> >
> > On Wed, Oct 23, 2013 at 01:55:58PM +0000, Gupta, Pekon wrote:
> >> >
> >> > I'm wondering how are you testing this in your SOC_AM33XX board (which
> >> > is not ARCH_OMAP3). You probably have ARCH_OMAP3 always selected?
> >> >
> >> Yes, omap2plus_defconfig is a super set..
> >> arch/arm/configs/omap2plus_defconfig automatically enables ARCH_OMAP3.
> >>
> >
> > Yes, but I always remove what I won't use to reduce build time.
> >
> > And  now that you bring this issue. IMHO, the AM33xx family is going
> > to be more and more widely used, so maybe introducing an am33xx_defconfig
> > makes sense.
> >
> > Or is the trend to have the least possible amount of defconfigs?
> 
> I'm not familiar with AM33xx so I don't know how similar is to OMAP3
> but we used to have different defconfigs for each OMAP board before
> and consolidated everything in only two defconfigs: omap1_defconfig to
> be used for all OMAP1 devices and omap2plus_defconfig to be used for
> all OMAP2+ (i.e: OMAP{2,3,4,5}) devices.
> 
> Those have all the common kconfig options and board vendors can
> customize to fit their needs and have a delta with something like:
> 
> $ ./scripts/kconfig/merge_config.sh
> arch/arm/configs/omap2plus_defconfig /foo/bar/igep_defconfig
> 
> Again I'm not familiar with AM33xx but what I do know is that we
> should keep defconfigs to a minimum.
> 

Fair enough.

It's just a bit annoying to have a bigger-than-minimal kernel,
and I get tired of stripping the options.

But I can't ask the kernel to hold my favorite picks either:
we have enough churn already ;-)
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 00/10] mtd:nand:omap2: clean-up of supported ECC schemes

[Brian Norris]

On Sat, Oct 19, 2013 at 02:14:04PM +0530, Pekon Gupta wrote:
> 
> *changes v9 -> v10*
> [PATCH 1/10], [PATCH 2/10]
>   swapped [PATCH v9 1/9] and [PATCH v9 2/9] so that DT parsing updates
>   (with backward compatibility) happen before the deprecation of DT values.
>   This way DTB does not break functionally between the patches.
> [PATCH 3/10] <no update>
> [PATCH 4/10] 
>   dropped [PATCH v9 4/9] introducing NAND_BUSWIDTH_AUTO, instead
>   using DT 'nand-bus-width' for device bus-width

As mentioned in reply to the patch, I don't think this patch belongs in
this series now, and it is still not adequate (the original code there
is wrong in some ways, but your changes are not clearly better).

> [PATCH 5/10] <no update>
> [PATCH 6/10] <no update>
> [PATCH 7/10] 
>   separated out drivers/mtd/nand/Kconfig updates into separate [PATCH v10 10/10]
>   cleanup: s/info->nand\./nand_chip->
> [PATCH 8/10] <no update>
> [PATCH 9/10] cleanup: s/out_release_mem_region/return_error
> [PATCH 10/10] <new> spawned from [PATCH v9 8/9] for Kconfig updates

Otherwise, the series looks good.

Thanks,
Brian

RE: [PATCH v10 00/10] mtd:nand:omap2: clean-up of supported ECC schemes

[Gupta, Pekon]

Hi Brian,

> From: Brian Norris [mailto:computersforpeace@gmail.com]
> > [PATCH 4/10]
> >   dropped [PATCH v9 4/9] introducing NAND_BUSWIDTH_AUTO, instead
> >   using DT 'nand-bus-width' for device bus-width
> 
> As mentioned in reply to the patch, I don't think this patch belongs in
> this series now, and it is still not adequate (the original code there
> is wrong in some ways, but your changes are not clearly better).
> 
Probably there was some confusion in review this patch.. 
I have added my comments and replies to you.
Request you to please re-review..

And as I suppose this is the only remaining piece in this series, so
If that's ok, request you to please pull-in this series..
So, I can re-work on next pending series.. 


with regards, pekon