[PATCH v4 0/5] nand: OMAP collected fixes

[PATCH v4 0/5] nand: OMAP collected fixes

[Ezequiel Garcia]

I've collected all the changes recently discussed in a whole patchset.
If you are just reading this for the first time please take a look at:

[1] http://www.spinics.net/lists/linux-omap/msg110965.html
[2] http://www.spinics.net/lists/linux-omap/msg110958.html 

As requested by Roger, the omap_elm is a 'bool' driver now. The plan
is to merge it inside omap2_nand, but my queue is really filled, so
if anyone feels like doing, please be my guest.

Although I'm hoping this to be the last round, feedback and testing
is welcome.

Brian: I wonder if it's too late for v3.18?

Thanks!

Ezequiel Garcia (5):
  nand: omap2: Remove horrible ifdefs to fix module probe
  nand: omap2: Replace pr_err with dev_err
  mtd: nand: Move ELM driver and rename as omap_elm
  mtd: nand: Rename OMAP NAND driver
  mtd: nand: Constrain omap_elm to built-in

 drivers/mtd/devices/Makefile                   |   1 -
 drivers/mtd/nand/Kconfig                       |   2 +-
 drivers/mtd/nand/Makefile                      |   3 +-
 drivers/mtd/nand/{omap2.c => omap2_nand.c}     | 160 ++++++++++++++-----------
 drivers/mtd/{devices/elm.c => nand/omap_elm.c} |   0
 include/linux/platform_data/elm.h              |  16 +++
 6 files changed, 107 insertions(+), 75 deletions(-)
 rename drivers/mtd/nand/{omap2.c => omap2_nand.c} (95%)
 rename drivers/mtd/{devices/elm.c => nand/omap_elm.c} (100%)

-- 
2.1.0

[PATCH v4 1/5] nand: omap2: Remove horrible ifdefs to fix module probe

[Ezequiel Garcia]

The current code abuses ifdefs to determine if the selected ECC scheme
is supported by the running kernel. As a result the code is hard to read,
and it also fails to load as a module.

This commit removes all the ifdefs and instead introduces a function
omap2_nand_ecc_check() to check if the ECC is supported by using
IS_ENABLED(CONFIG_xxx).

Since IS_ENABLED() is true when a config is =y or =m, this change fixes the
module so it can be loaded with no issues.

Acked-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/omap2.c          | 130 +++++++++++++++++++++-----------------
 include/linux/platform_data/elm.h |  16 +++++
 2 files changed, 87 insertions(+), 59 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index e1a9b31..f97a4ff 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -136,7 +136,6 @@
 
 #define BADBLOCK_MARKER_LENGTH		2
 
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
 static u_char bch16_vector[] = {0xf5, 0x24, 0x1c, 0xd0, 0x61, 0xb3, 0xf1, 0x55,
 				0x2e, 0x2c, 0x86, 0xa3, 0xed, 0x36, 0x1b, 0x78,
 				0x48, 0x76, 0xa9, 0x3b, 0x97, 0xd1, 0x7a, 0x93,
@@ -144,7 +143,6 @@ static u_char bch16_vector[] = {0xf5, 0x24, 0x1c, 0xd0, 0x61, 0xb3, 0xf1, 0x55,
 static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
 	0xac, 0x6b, 0xff, 0x99, 0x7b};
 static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10};
-#endif
 
 /* oob info generated runtime depending on ecc algorithm and layout selected */
 static struct nand_ecclayout omap_oobinfo;
@@ -1292,7 +1290,6 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
 	return 0;
 }
 
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
 /**
  * erased_sector_bitflips - count bit flips
  * @data:	data sector buffer
@@ -1593,33 +1590,71 @@ static int omap_read_page_bch(struct mtd_info *mtd, struct nand_chip *chip,
 /**
  * is_elm_present - checks for presence of ELM module by scanning DT nodes
  * @omap_nand_info: NAND device structure containing platform data
- * @bch_type: 0x0=BCH4, 0x1=BCH8, 0x2=BCH16
  */
-static int is_elm_present(struct omap_nand_info *info,
-			struct device_node *elm_node, enum bch_ecc bch_type)
+static bool is_elm_present(struct omap_nand_info *info,
+			   struct device_node *elm_node)
 {
 	struct platform_device *pdev;
-	struct nand_ecc_ctrl *ecc = &info->nand.ecc;
-	int err;
+
 	/* check whether elm-id is passed via DT */
 	if (!elm_node) {
 		pr_err("nand: error: ELM DT node not found\n");
-		return -ENODEV;
+		return false;
 	}
 	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;
+		return false;
 	}
 	/* ELM module available, now configure it */
 	info->elm_dev = &pdev->dev;
-	err = elm_config(info->elm_dev, bch_type,
-		(info->mtd.writesize / ecc->size), ecc->size, ecc->bytes);
+	return true;
+}
 
-	return err;
+static bool omap2_nand_ecc_check(struct omap_nand_info *info,
+				 struct omap_nand_platform_data	*pdata)
+{
+	bool ecc_needs_bch, ecc_needs_omap_bch, ecc_needs_elm;
+
+	switch (info->ecc_opt) {
+	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
+	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
+		ecc_needs_omap_bch = false;
+		ecc_needs_bch = true;
+		ecc_needs_elm = false;
+		break;
+	case OMAP_ECC_BCH4_CODE_HW:
+	case OMAP_ECC_BCH8_CODE_HW:
+	case OMAP_ECC_BCH16_CODE_HW:
+		ecc_needs_omap_bch = true;
+		ecc_needs_bch = false;
+		ecc_needs_elm = true;
+		break;
+	default:
+		ecc_needs_omap_bch = false;
+		ecc_needs_bch = false;
+		ecc_needs_elm = false;
+		break;
+	}
+
+	if (ecc_needs_bch && !IS_ENABLED(CONFIG_MTD_NAND_ECC_BCH)) {
+		dev_err(&info->pdev->dev,
+			"CONFIG_MTD_NAND_ECC_BCH not enabled\n");
+		return false;
+	}
+	if (ecc_needs_omap_bch && !IS_ENABLED(CONFIG_MTD_NAND_OMAP_BCH)) {
+		dev_err(&info->pdev->dev,
+			"CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
+		return false;
+	}
+	if (ecc_needs_elm && !is_elm_present(info, pdata->elm_of_node)) {
+		dev_err(&info->pdev->dev, "ELM not available\n");
+		return false;
+	}
+
+	return true;
 }
-#endif /* CONFIG_MTD_NAND_ECC_BCH */
 
 static int omap_nand_probe(struct platform_device *pdev)
 {
@@ -1797,6 +1832,11 @@ static int omap_nand_probe(struct platform_device *pdev)
 		goto return_error;
 	}
 
+	if (!omap2_nand_ecc_check(info, pdata)) {
+		err = -EINVAL;
+		goto return_error;
+	}
+
 	/* populate MTD interface based on ECC scheme */
 	ecclayout		= &omap_oobinfo;
 	switch (info->ecc_opt) {
@@ -1829,7 +1869,6 @@ static int omap_nand_probe(struct platform_device *pdev)
 		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;
@@ -1861,14 +1900,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 			err = -EINVAL;
 		}
 		break;
-#else
-		pr_err("nand: error: CONFIG_MTD_NAND_ECC_BCH not enabled\n");
-		err = -EINVAL;
-		goto return_error;
-#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;
@@ -1890,21 +1923,15 @@ static int omap_nand_probe(struct platform_device *pdev)
 		/* reserved marker already included in ecclayout->eccbytes */
 		ecclayout->oobfree->offset	=
 				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
-		/* 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;
+
+		err = elm_config(info->elm_dev, BCH4_ECC,
+				 info->mtd.writesize / nand_chip->ecc.size,
+				 nand_chip->ecc.size, nand_chip->ecc.bytes);
+		if (err < 0)
 			goto return_error;
-		}
 		break;
-#else
-		pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
-		err = -EINVAL;
-		goto return_error;
-#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;
@@ -1937,14 +1964,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 			goto return_error;
 		}
 		break;
-#else
-		pr_err("nand: error: CONFIG_MTD_NAND_ECC_BCH not enabled\n");
-		err = -EINVAL;
-		goto return_error;
-#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;
@@ -1956,12 +1977,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.calculate	= omap_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 */
-		err = is_elm_present(info, pdata->elm_of_node, BCH8_ECC);
-		if (err < 0) {
-			pr_err("nand: error: could not initialize ELM\n");
+
+		err = elm_config(info->elm_dev, BCH8_ECC,
+				 info->mtd.writesize / nand_chip->ecc.size,
+				 nand_chip->ecc.size, nand_chip->ecc.bytes);
+		if (err < 0)
 			goto return_error;
-		}
+
 		/* define ECC layout */
 		ecclayout->eccbytes		= nand_chip->ecc.bytes *
 							(mtd->writesize /
@@ -1973,14 +1995,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 		ecclayout->oobfree->offset	=
 				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
 		break;
-#else
-		pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
-		err = -EINVAL;
-		goto return_error;
-#endif
 
 	case OMAP_ECC_BCH16_CODE_HW:
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
 		pr_info("using OMAP_ECC_BCH16_CODE_HW ECC scheme\n");
 		nand_chip->ecc.mode		= NAND_ECC_HW;
 		nand_chip->ecc.size		= 512;
@@ -1991,12 +2007,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.calculate	= omap_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 */
-		err = is_elm_present(info, pdata->elm_of_node, BCH16_ECC);
-		if (err < 0) {
-			pr_err("ELM is required for this ECC scheme\n");
+
+		err = elm_config(info->elm_dev, BCH16_ECC,
+				 info->mtd.writesize / nand_chip->ecc.size,
+				 nand_chip->ecc.size, nand_chip->ecc.bytes);
+		if (err < 0)
 			goto return_error;
-		}
+
 		/* define ECC layout */
 		ecclayout->eccbytes		= nand_chip->ecc.bytes *
 							(mtd->writesize /
@@ -2008,11 +2025,6 @@ static int omap_nand_probe(struct platform_device *pdev)
 		ecclayout->oobfree->offset	=
 				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
 		break;
-#else
-		pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
-		err = -EINVAL;
-		goto return_error;
-#endif
 	default:
 		pr_err("nand: error: invalid or unsupported ECC scheme\n");
 		err = -EINVAL;
diff --git a/include/linux/platform_data/elm.h b/include/linux/platform_data/elm.h
index 780d1e9..b8686c0 100644
--- a/include/linux/platform_data/elm.h
+++ b/include/linux/platform_data/elm.h
@@ -42,8 +42,24 @@ struct elm_errorvec {
 	int error_loc[16];
 };
 
+#if IS_ENABLED(CONFIG_MTD_NAND_OMAP_BCH)
 void elm_decode_bch_error_page(struct device *dev, u8 *ecc_calc,
 		struct elm_errorvec *err_vec);
 int elm_config(struct device *dev, enum bch_ecc bch_type,
 	int ecc_steps, int ecc_step_size, int ecc_syndrome_size);
+#else
+static inline void
+elm_decode_bch_error_page(struct device *dev, u8 *ecc_calc,
+			  struct elm_errorvec *err_vec)
+{
+}
+
+static inline int elm_config(struct device *dev, enum bch_ecc bch_type,
+			     int ecc_steps, int ecc_step_size,
+			     int ecc_syndrome_size)
+{
+	return -ENOSYS;
+}
+#endif /* CONFIG_MTD_NAND_ECC_BCH */
+
 #endif /* __ELM_H */
-- 
2.1.0

[PATCH v4 2/5] nand: omap2: Replace pr_err with dev_err

[Ezequiel Garcia]

Usage of pr_err is frowned upon, so replace it with dev_err.

Acked-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/omap2.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f97a4ff..3b357e9 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1375,7 +1375,7 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
 		erased_ecc_vec = bch16_vector;
 		break;
 	default:
-		pr_err("invalid driver configuration\n");
+		dev_err(&info->pdev->dev, "invalid driver configuration\n");
 		return -EINVAL;
 	}
 
@@ -1446,7 +1446,8 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
 	err = 0;
 	for (i = 0; i < eccsteps; i++) {
 		if (err_vec[i].error_uncorrectable) {
-			pr_err("nand: uncorrectable bit-flips found\n");
+			dev_err(&info->pdev->dev,
+				"uncorrectable bit-flips found\n");
 			err = -EBADMSG;
 		} else if (err_vec[i].error_reported) {
 			for (j = 0; j < err_vec[i].error_count; j++) {
@@ -1483,8 +1484,9 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
 							1 << bit_pos;
 					}
 				} else {
-					pr_err("invalid bit-flip @ %d:%d\n",
-							 byte_pos, bit_pos);
+					dev_err(&info->pdev->dev,
+						"invalid bit-flip @ %d:%d\n",
+						byte_pos, bit_pos);
 					err = -EBADMSG;
 				}
 			}
@@ -1598,13 +1600,13 @@ static bool is_elm_present(struct omap_nand_info *info,
 
 	/* check whether elm-id is passed via DT */
 	if (!elm_node) {
-		pr_err("nand: error: ELM DT node not found\n");
+		dev_err(&info->pdev->dev, "ELM devicetree node not found\n");
 		return false;
 	}
 	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");
+		dev_err(&info->pdev->dev, "ELM device not found\n");
 		return false;
 	}
 	/* ELM module available, now configure it */
@@ -1734,14 +1736,14 @@ static int omap_nand_probe(struct platform_device *pdev)
 	/* scan NAND device connected to chip controller */
 	nand_chip->options |= pdata->devsize & NAND_BUSWIDTH_16;
 	if (nand_scan_ident(mtd, 1, NULL)) {
-		pr_err("nand device scan failed, may be bus-width mismatch\n");
+		dev_err(&info->pdev->dev, "scan failed, may be bus-width mismatch\n");
 		err = -ENXIO;
 		goto return_error;
 	}
 
 	/* 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");
+		dev_err(&info->pdev->dev, "small page devices are not supported\n");
 		err = -EINVAL;
 		goto return_error;
 	}
@@ -1896,8 +1898,9 @@ static int omap_nand_probe(struct platform_device *pdev)
 							nand_chip->ecc.bytes,
 							&ecclayout);
 		if (!nand_chip->ecc.priv) {
-			pr_err("nand: error: unable to use s/w BCH library\n");
+			dev_err(&info->pdev->dev, "unable to use BCH library\n");
 			err = -EINVAL;
+			goto return_error;
 		}
 		break;
 
@@ -1959,7 +1962,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 							nand_chip->ecc.bytes,
 							&ecclayout);
 		if (!nand_chip->ecc.priv) {
-			pr_err("nand: error: unable to use s/w BCH library\n");
+			dev_err(&info->pdev->dev, "unable to use BCH library\n");
 			err = -EINVAL;
 			goto return_error;
 		}
@@ -2026,7 +2029,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
 		break;
 	default:
-		pr_err("nand: error: invalid or unsupported ECC scheme\n");
+		dev_err(&info->pdev->dev, "invalid or unsupported ECC scheme\n");
 		err = -EINVAL;
 		goto return_error;
 	}
@@ -2038,8 +2041,9 @@ static int omap_nand_probe(struct platform_device *pdev)
 	ecclayout->oobfree->length = mtd->oobsize - ecclayout->oobfree->offset;
 	/* 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);
+		dev_err(&info->pdev->dev,
+			"not enough OOB bytes required = %d, available=%d\n",
+			ecclayout->eccbytes, mtd->oobsize);
 		err = -EINVAL;
 		goto return_error;
 	}
-- 
2.1.0

[PATCH v4 3/5] mtd: nand: Move ELM driver and rename as omap_elm

[Ezequiel Garcia]

The ELM driver is only used by the OMAP NAND driver, so let's move it
to the nand/ directory. Additionally, let's rename it to a less confusing
name, so the module is built with a meaningful name, instead of the previous
'elm.ko'.

Acked-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/devices/Makefile                   | 1 -
 drivers/mtd/nand/Makefile                      | 1 +
 drivers/mtd/{devices/elm.c => nand/omap_elm.c} | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/mtd/{devices/elm.c => nand/omap_elm.c} (100%)

diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile
index c68868f..f0b0e61 100644
--- a/drivers/mtd/devices/Makefile
+++ b/drivers/mtd/devices/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_MTD_LART)		+= lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)	+= block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)	+= mtd_dataflash.o
 obj-$(CONFIG_MTD_M25P80)	+= m25p80.o
-obj-$(CONFIG_MTD_NAND_OMAP_BCH)	+= elm.o
 obj-$(CONFIG_MTD_SPEAR_SMI)	+= spear_smi.o
 obj-$(CONFIG_MTD_SST25L)	+= sst25l.o
 obj-$(CONFIG_MTD_BCM47XXSFLASH)	+= bcm47xxsflash.o
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index a035e7c..b3237b7 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_MTD_NAND_NDFC)		+= ndfc.o
 obj-$(CONFIG_MTD_NAND_ATMEL)		+= atmel_nand.o
 obj-$(CONFIG_MTD_NAND_GPIO)		+= gpio.o
 obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2.o
+obj-$(CONFIG_MTD_NAND_OMAP_BCH)		+= omap_elm.o
 obj-$(CONFIG_MTD_NAND_CM_X270)		+= cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_PXA3xx)		+= pxa3xx_nand.o
 obj-$(CONFIG_MTD_NAND_TMIO)		+= tmio_nand.o
diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/nand/omap_elm.c
similarity index 100%
rename from drivers/mtd/devices/elm.c
rename to drivers/mtd/nand/omap_elm.c
-- 
2.1.0

[PATCH v4 4/5] mtd: nand: Rename OMAP NAND driver

[Ezequiel Garcia]

Rename it to a less generic name, so the module is built with a meaningful
name instead of the previous 'omap2.ko'.

Acked-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/Makefile                  | 2 +-
 drivers/mtd/nand/{omap2.c => omap2_nand.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/mtd/nand/{omap2.c => omap2_nand.c} (100%)

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index b3237b7..4bcdeb0 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -26,7 +26,7 @@ obj-$(CONFIG_MTD_NAND_CS553X)		+= cs553x_nand.o
 obj-$(CONFIG_MTD_NAND_NDFC)		+= ndfc.o
 obj-$(CONFIG_MTD_NAND_ATMEL)		+= atmel_nand.o
 obj-$(CONFIG_MTD_NAND_GPIO)		+= gpio.o
-obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2.o
+obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2_nand.o
 obj-$(CONFIG_MTD_NAND_OMAP_BCH)		+= omap_elm.o
 obj-$(CONFIG_MTD_NAND_CM_X270)		+= cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_PXA3xx)		+= pxa3xx_nand.o
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2_nand.c
similarity index 100%
rename from drivers/mtd/nand/omap2.c
rename to drivers/mtd/nand/omap2_nand.c
-- 
2.1.0

Re: [PATCH v4 4/5] mtd: nand: Rename OMAP NAND driver

[Brian Norris]

On Sat, Sep 20, 2014 at 05:53:15PM +0100, Ezequiel Garcia wrote:
> Rename it to a less generic name, so the module is built with a meaningful
> name instead of the previous 'omap2.ko'.

You mention renaming only so that you can change the module name, but
you also (partly at my prompting) argued for future merging of omap_nand
and omap_elm into a single module. So it seems like you might have to do
several renamings, in that case, right?

omap2.c => omap_nand.c (this patch)

omap_nand.c => <somethingelse>.c (later)
 ...so you can link somethingelse.o and omap_elm.o into omap_nand.{o,ko}

If I'm right here, then maybe (rather than multiple renames), you just
want this patch for now? (untested)

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index b3237b742eb5..1a4d5e9db75b 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -26,7 +26,7 @@ obj-$(CONFIG_MTD_NAND_CS553X)		+= cs553x_nand.o
 obj-$(CONFIG_MTD_NAND_NDFC)		+= ndfc.o
 obj-$(CONFIG_MTD_NAND_ATMEL)		+= atmel_nand.o
 obj-$(CONFIG_MTD_NAND_GPIO)		+= gpio.o
-obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2.o
+obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap_nand.o
 obj-$(CONFIG_MTD_NAND_OMAP_BCH)		+= omap_elm.o
 obj-$(CONFIG_MTD_NAND_CM_X270)		+= cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_PXA3xx)		+= pxa3xx_nand.o
@@ -52,3 +52,4 @@ obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
 obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o
+omap_nand-objs := omap2.o

> Acked-by: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/mtd/nand/Makefile                  | 2 +-
>  drivers/mtd/nand/{omap2.c => omap2_nand.c} | 0
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename drivers/mtd/nand/{omap2.c => omap2_nand.c} (100%)

[snip]

Brian

[PATCH v4 5/5] mtd: nand: Constrain omap_elm to built-in

[Ezequiel Garcia]

This fixes the following build error when omap2_nand is chosen built-in,
and omap_elm is chosen as a module:

  drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config'
  drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config'
  drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config'
  drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page'

Fix this by making omap_elm a 'bool' driver.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f1cf503..549c0cb 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -96,7 +96,7 @@ config MTD_NAND_OMAP2
 
 config MTD_NAND_OMAP_BCH
 	depends on MTD_NAND_OMAP2
-	tristate "Support hardware based BCH error correction"
+	bool "Support hardware based BCH error correction"
 	default n
 	select BCH
 	help
-- 
2.1.0

Re: [PATCH v4 5/5] mtd: nand: Constrain omap_elm to built-in

[Roger Quadros]

On 09/20/2014 07:53 PM, Ezequiel Garcia wrote:
> This fixes the following build error when omap2_nand is chosen built-in,
> and omap_elm is chosen as a module:
> 
>   drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config'
>   drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config'
>   drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config'
>   drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page'
> 
> Fix this by making omap_elm a 'bool' driver.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

Acked-by: <rogerq@ti.com>

--
cheers,
-roger

> ---
>  drivers/mtd/nand/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index f1cf503..549c0cb 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -96,7 +96,7 @@ config MTD_NAND_OMAP2
>  
>  config MTD_NAND_OMAP_BCH
>  	depends on MTD_NAND_OMAP2
> -	tristate "Support hardware based BCH error correction"
> +	bool "Support hardware based BCH error correction"
>  	default n
>  	select BCH
>  	help
> 

Re: [PATCH v4 5/5] mtd: nand: Constrain omap_elm to built-in

[Brian Norris]

On Sat, Sep 20, 2014 at 05:53:16PM +0100, Ezequiel Garcia wrote:
> This fixes the following build error when omap2_nand is chosen built-in,
> and omap_elm is chosen as a module:
> 
>   drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config'
>   drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config'
>   drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config'
>   drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page'
> 
> Fix this by making omap_elm a 'bool' driver.

Hmm, so we're sacrificing a "feature" (that omap2 + omap_elm can both be
built as modules) because we haven't solved one particular developer UI
inconsistency (that we don't prevent the config combination of
MTD_NAND_OMAP2=y and MTD_NAND_OMAP_BCH=m)? I wouldn't tend to sacrifice
the feature for the sake of a bad config.

Do you have any guesstimate as to whether the linker failure affects
many people? Are people just hitting this with randconfig?

Do you know if anyone ships loadable modules for this driver? e.g., any
embedded distros?

I could go either way on this, but I just wasn't sure if these sorts of
questions had been asked/answered.

> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/mtd/nand/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index f1cf503..549c0cb 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -96,7 +96,7 @@ config MTD_NAND_OMAP2
>  
>  config MTD_NAND_OMAP_BCH
>  	depends on MTD_NAND_OMAP2
> -	tristate "Support hardware based BCH error correction"
> +	bool "Support hardware based BCH error correction"
>  	default n
>  	select BCH
>  	help

Brian

Re: [PATCH v4 5/5] mtd: nand: Constrain omap_elm to built-in

[Ezequiel Garcia]

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



On 09/22/2014 03:58 PM, Brian Norris wrote:
> On Sat, Sep 20, 2014 at 05:53:16PM +0100, Ezequiel Garcia wrote:
>> This fixes the following build error when omap2_nand is chosen built-in,
>> and omap_elm is chosen as a module:
>>
>>   drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config'
>>   drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config'
>>   drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config'
>>   drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page'
>>
>> Fix this by making omap_elm a 'bool' driver.
> 
> Hmm, so we're sacrificing a "feature" (that omap2 + omap_elm can both be
> built as modules) because we haven't solved one particular developer UI
> inconsistency (that we don't prevent the config combination of
> MTD_NAND_OMAP2=y and MTD_NAND_OMAP_BCH=m)? I wouldn't tend to sacrifice
> the feature for the sake of a bad config.
> 

FWIW, I agree with you, as already said to Roger.
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 5/5] mtd: nand: Constrain omap_elm to built-in

[Roger Quadros]

On 09/22/2014 09:58 PM, Brian Norris wrote:
> On Sat, Sep 20, 2014 at 05:53:16PM +0100, Ezequiel Garcia wrote:
>> This fixes the following build error when omap2_nand is chosen built-in,
>> and omap_elm is chosen as a module:
>>
>>   drivers/mtd/nand/omap2.c:2010: undefined reference to `elm_config'
>>   drivers/mtd/nand/omap2.c:1980: undefined reference to `elm_config'
>>   drivers/mtd/nand/omap2.c:1927: undefined reference to `elm_config'
>>   drivers/mtd/nand/omap2.c:1444: undefined reference to `elm_decode_bch_error_page'
>>
>> Fix this by making omap_elm a 'bool' driver.
> 
> Hmm, so we're sacrificing a "feature" (that omap2 + omap_elm can both be
> built as modules) because we haven't solved one particular developer UI
> inconsistency (that we don't prevent the config combination of
> MTD_NAND_OMAP2=y and MTD_NAND_OMAP_BCH=m)? I wouldn't tend to sacrifice
> the feature for the sake of a bad config.
> 
> Do you have any guesstimate as to whether the linker failure affects
> many people? Are people just hitting this with randconfig?

I think this issue was hit only during a manual menuconfig setting.

> 
> Do you know if anyone ships loadable modules for this driver? e.g., any
> embedded distros?

Not sure about other distros but TI releases always set both
OMAP_NAND and OMAP_BCH as built-in.


cheers,
-roger

> 
> I could go either way on this, but I just wasn't sure if these sorts of
> questions had been asked/answered.
> 
>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>> ---
>>  drivers/mtd/nand/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>> index f1cf503..549c0cb 100644
>> --- a/drivers/mtd/nand/Kconfig
>> +++ b/drivers/mtd/nand/Kconfig
>> @@ -96,7 +96,7 @@ config MTD_NAND_OMAP2
>>  
>>  config MTD_NAND_OMAP_BCH
>>  	depends on MTD_NAND_OMAP2
>> -	tristate "Support hardware based BCH error correction"
>> +	bool "Support hardware based BCH error correction"
>>  	default n
>>  	select BCH
>>  	help
> 
> Brian
> 

Re: [PATCH v4 0/5] nand: OMAP collected fixes

[Roger Quadros]

Hi Ezequiel,

On 09/20/2014 07:53 PM, Ezequiel Garcia wrote:
> I've collected all the changes recently discussed in a whole patchset.
> If you are just reading this for the first time please take a look at:
> 
> [1] http://www.spinics.net/lists/linux-omap/msg110965.html
> [2] http://www.spinics.net/lists/linux-omap/msg110958.html 
> 
> As requested by Roger, the omap_elm is a 'bool' driver now. The plan
> is to merge it inside omap2_nand, but my queue is really filled, so
> if anyone feels like doing, please be my guest.
> 
> Although I'm hoping this to be the last round, feedback and testing
> is welcome.

Patches 3 and 4 still have checkpatch issues.

cheers,
-roger

> 
> Brian: I wonder if it's too late for v3.18?
> 
> Thanks!
> 
> Ezequiel Garcia (5):
>   nand: omap2: Remove horrible ifdefs to fix module probe
>   nand: omap2: Replace pr_err with dev_err
>   mtd: nand: Move ELM driver and rename as omap_elm
>   mtd: nand: Rename OMAP NAND driver
>   mtd: nand: Constrain omap_elm to built-in
> 
>  drivers/mtd/devices/Makefile                   |   1 -
>  drivers/mtd/nand/Kconfig                       |   2 +-
>  drivers/mtd/nand/Makefile                      |   3 +-
>  drivers/mtd/nand/{omap2.c => omap2_nand.c}     | 160 ++++++++++++++-----------
>  drivers/mtd/{devices/elm.c => nand/omap_elm.c} |   0
>  include/linux/platform_data/elm.h              |  16 +++
>  6 files changed, 107 insertions(+), 75 deletions(-)
>  rename drivers/mtd/nand/{omap2.c => omap2_nand.c} (95%)
>  rename drivers/mtd/{devices/elm.c => nand/omap_elm.c} (100%)
> 

Re: [PATCH v4 0/5] nand: OMAP collected fixes

[Ezequiel Garcia]

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



On 09/22/2014 09:22 AM, Roger Quadros wrote:
> Hi Ezequiel,
> 
> On 09/20/2014 07:53 PM, Ezequiel Garcia wrote:
>> I've collected all the changes recently discussed in a whole patchset.
>> If you are just reading this for the first time please take a look at:
>>
>> [1] http://www.spinics.net/lists/linux-omap/msg110965.html
>> [2] http://www.spinics.net/lists/linux-omap/msg110958.html 
>>
>> As requested by Roger, the omap_elm is a 'bool' driver now. The plan
>> is to merge it inside omap2_nand, but my queue is really filled, so
>> if anyone feels like doing, please be my guest.
>>
>> Although I'm hoping this to be the last round, feedback and testing
>> is welcome.
> 
> Patches 3 and 4 still have checkpatch issues.
> 

I think you didn't look at them in detail. Unless I'm missing something,
it's just warning about MAINTAINERS update:

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#16:
 drivers/mtd/{devices/elm.c => nand/omap_elm.c} | 0

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#13:
 drivers/mtd/nand/{omap2.c => omap2_nand.c} | 0

This driver is not in MAINTAINERS. If you want to start maintaining it,
it'd be good to add you to MAINTAINERS so people Cc you properly on patches.

-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 0/5] nand: OMAP collected fixes

[Roger Quadros]

On 09/22/2014 03:51 PM, Ezequiel Garcia wrote:
> 
> 
> On 09/22/2014 09:22 AM, Roger Quadros wrote:
>> Hi Ezequiel,
>>
>> On 09/20/2014 07:53 PM, Ezequiel Garcia wrote:
>>> I've collected all the changes recently discussed in a whole patchset.
>>> If you are just reading this for the first time please take a look at:
>>>
>>> [1] http://www.spinics.net/lists/linux-omap/msg110965.html
>>> [2] http://www.spinics.net/lists/linux-omap/msg110958.html 
>>>
>>> As requested by Roger, the omap_elm is a 'bool' driver now. The plan
>>> is to merge it inside omap2_nand, but my queue is really filled, so
>>> if anyone feels like doing, please be my guest.
>>>
>>> Although I'm hoping this to be the last round, feedback and testing
>>> is welcome.
>>
>> Patches 3 and 4 still have checkpatch issues.
>>
> 
> I think you didn't look at them in detail. Unless I'm missing something,
> it's just warning about MAINTAINERS update:
> 
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #16:
>  drivers/mtd/{devices/elm.c => nand/omap_elm.c} | 0
> 
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #13:
>  drivers/mtd/nand/{omap2.c => omap2_nand.c} | 0
> 
> This driver is not in MAINTAINERS. If you want to start maintaining it,
> it'd be good to add you to MAINTAINERS so people Cc you properly on patches.
> 

Sorry for the noise. I was re-generating the patch without the rename detection flag
causing errors which are present in the original file itself.

cheers,
-roger

Re: [PATCH v4 0/5] nand: OMAP collected fixes

[Brian Norris]

On Sat, Sep 20, 2014 at 05:53:11PM +0100, Ezequiel Garcia wrote:
> I've collected all the changes recently discussed in a whole patchset.
> If you are just reading this for the first time please take a look at:
> 
> [1] http://www.spinics.net/lists/linux-omap/msg110965.html
> [2] http://www.spinics.net/lists/linux-omap/msg110958.html 
> 
> As requested by Roger, the omap_elm is a 'bool' driver now. The plan
> is to merge it inside omap2_nand, but my queue is really filled, so
> if anyone feels like doing, please be my guest.
> 
> Although I'm hoping this to be the last round, feedback and testing
> is welcome.
> 
> Brian: I wonder if it's too late for v3.18?
> 
> Thanks!
> 
> Ezequiel Garcia (5):
>   nand: omap2: Remove horrible ifdefs to fix module probe
>   nand: omap2: Replace pr_err with dev_err
>   mtd: nand: Move ELM driver and rename as omap_elm

Pushed the first 3 to l2-mtd.git. Thanks!

>   mtd: nand: Rename OMAP NAND driver

One comment on this one, which I'll poast on its patch.

>   mtd: nand: Constrain omap_elm to built-in
> 
>  drivers/mtd/devices/Makefile                   |   1 -
>  drivers/mtd/nand/Kconfig                       |   2 +-
>  drivers/mtd/nand/Makefile                      |   3 +-
>  drivers/mtd/nand/{omap2.c => omap2_nand.c}     | 160 ++++++++++++++-----------
>  drivers/mtd/{devices/elm.c => nand/omap_elm.c} |   0
>  include/linux/platform_data/elm.h              |  16 +++
>  6 files changed, 107 insertions(+), 75 deletions(-)
>  rename drivers/mtd/nand/{omap2.c => omap2_nand.c} (95%)
>  rename drivers/mtd/{devices/elm.c => nand/omap_elm.c} (100%)
> 

Brian