[PATCH v2 0/3] nand: omap2: Two and a half improvements

[PATCH v2 0/3] nand: omap2: Two and a half improvements

[Ezequiel Garcia]

Changes from v1:

  * Rebased on v3.14-rc2.

  * Removed a few s/pr_err/dev_err change from patch two, and added it
    to patch three. This was some git-rebase leftover.

Pekon's attempt to add flash BBT support for this driver made me realise
the addition made sense and there were good reasons for it. The first patch
adds support for enabling a flash BBT either from legacy board files or
from devicetree.

While testing this, I noticed how the driver relied on a whole bunch of
horrible #ifdefs, which prevented me from loading the driver as a module.
The second patch attempts to fix that.

The third patch is just a dummy cleanup replacing pr_errs with dev_errs.
This driver is abusing from user messages, but I'm not sure fixing them
worths the trouble.

Ezequiel Garcia (3):
  nand: omap2: Add support for flash-based bad block table
  nand: omap2: Remove horrible ifdefs to fix module probe
  nand: omap2: Replace pr_err with dev_err

 arch/arm/mach-omap2/gpmc.c                   |   2 +
 drivers/mtd/nand/omap2.c                     | 162 +++++++++++++++------------
 include/linux/platform_data/elm.h            |  14 +++
 include/linux/platform_data/mtd-nand-omap2.h |   1 +
 4 files changed, 107 insertions(+), 72 deletions(-)

-- 
2.0.1

[PATCH v2 1/3] nand: omap2: Add support for flash-based bad block table

[Ezequiel Garcia]

This commit adds a new platform-data boolean property that enables use
of a flash-based bad block table. This can also be enabled by setting
the 'nand-on-flash-bbt' devicetree property.

If the flash BBT is not enabled, the driver falls back to use OOB
bad block markers only, as before. If the flash BBT is enabled the
kernel will keep track of bad blocks using a BBT, in addition to
the OOB markers.

As explained by Brian Norris the reasons for using a BBT are:

""
The primary reason would be that NAND datasheets specify it these days.
A better argument is that nobody guarantees that you can write a
bad block marker to a worn out block; you may just get program failures.

This has been acknowledged by several developers over the last several
years.

Additionally, you get a boot-time performance improvement if you only
have to read a few pages, instead of a page or two from every block on
the flash.
""

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 arch/arm/mach-omap2/gpmc.c                   | 2 ++
 drivers/mtd/nand/omap2.c                     | 6 +++++-
 include/linux/platform_data/mtd-nand-omap2.h | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 2f97228..b55a225 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1440,6 +1440,8 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
 				break;
 			}
 
+	gpmc_nand_data->flash_bbt = of_get_nand_on_flash_bbt(child);
+
 	val = of_get_nand_bus_width(child);
 	if (val == 16)
 		gpmc_nand_data->devsize = NAND_BUSWIDTH_16;
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 5967b38..e1a9b31 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1663,7 +1663,6 @@ static int omap_nand_probe(struct platform_device *pdev)
 	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);
 	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
@@ -1692,6 +1691,11 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->chip_delay = 50;
 	}
 
+	if (pdata->flash_bbt)
+		nand_chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
+	else
+		nand_chip->options |= NAND_SKIP_BBTSCAN;
+
 	/* scan NAND device connected to chip controller */
 	nand_chip->options |= pdata->devsize & NAND_BUSWIDTH_16;
 	if (nand_scan_ident(mtd, 1, NULL)) {
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 16ec262..090bbab 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -71,6 +71,7 @@ struct omap_nand_platform_data {
 	struct mtd_partition	*parts;
 	int			nr_parts;
 	bool			dev_ready;
+	bool			flash_bbt;
 	enum nand_io		xfer_type;
 	int			devsize;
 	enum omap_ecc           ecc_opt;
-- 
2.0.1

Re: [PATCH v2 1/3] nand: omap2: Add support for flash-based bad block table

[Roger Quadros]

Ezequiel,

On 09/08/2014 02:27 PM, Ezequiel Garcia wrote:
> This commit adds a new platform-data boolean property that enables use
> of a flash-based bad block table. This can also be enabled by setting
> the 'nand-on-flash-bbt' devicetree property.

I'm not much aware of how on-flash-BBT works internally, but will it break things if
we keep on-flash-BBT "enabled" as the default option and add a DT property only to
explicitly disable the on-flash-BBT?

> 
> If the flash BBT is not enabled, the driver falls back to use OOB
> bad block markers only, as before. If the flash BBT is enabled the
> kernel will keep track of bad blocks using a BBT, in addition to
> the OOB markers.
> 
> As explained by Brian Norris the reasons for using a BBT are:
> 
> ""
> The primary reason would be that NAND datasheets specify it these days.
> A better argument is that nobody guarantees that you can write a
> bad block marker to a worn out block; you may just get program failures.
> 
> This has been acknowledged by several developers over the last several
> years.
> 
> Additionally, you get a boot-time performance improvement if you only
> have to read a few pages, instead of a page or two from every block on
> the flash.
> ""

cheers,
-roger

> 
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  arch/arm/mach-omap2/gpmc.c                   | 2 ++
>  drivers/mtd/nand/omap2.c                     | 6 +++++-
>  include/linux/platform_data/mtd-nand-omap2.h | 1 +
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 2f97228..b55a225 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -1440,6 +1440,8 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>  				break;
>  			}
>  
> +	gpmc_nand_data->flash_bbt = of_get_nand_on_flash_bbt(child);
> +
>  	val = of_get_nand_bus_width(child);
>  	if (val == 16)
>  		gpmc_nand_data->devsize = NAND_BUSWIDTH_16;
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 5967b38..e1a9b31 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1663,7 +1663,6 @@ static int omap_nand_probe(struct platform_device *pdev)
>  	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);
>  	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
> @@ -1692,6 +1691,11 @@ static int omap_nand_probe(struct platform_device *pdev)
>  		nand_chip->chip_delay = 50;
>  	}
>  
> +	if (pdata->flash_bbt)
> +		nand_chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
> +	else
> +		nand_chip->options |= NAND_SKIP_BBTSCAN;
> +
>  	/* scan NAND device connected to chip controller */
>  	nand_chip->options |= pdata->devsize & NAND_BUSWIDTH_16;
>  	if (nand_scan_ident(mtd, 1, NULL)) {
> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
> index 16ec262..090bbab 100644
> --- a/include/linux/platform_data/mtd-nand-omap2.h
> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> @@ -71,6 +71,7 @@ struct omap_nand_platform_data {
>  	struct mtd_partition	*parts;
>  	int			nr_parts;
>  	bool			dev_ready;
> +	bool			flash_bbt;
>  	enum nand_io		xfer_type;
>  	int			devsize;
>  	enum omap_ecc           ecc_opt;
> 

Re: [PATCH v2 1/3] nand: omap2: Add support for flash-based bad block table

[Ezequiel Garcia]

On 09 Sep 11:35 AM, Roger Quadros wrote:
> Ezequiel,
> 
> On 09/08/2014 02:27 PM, Ezequiel Garcia wrote:
> > This commit adds a new platform-data boolean property that enables use
> > of a flash-based bad block table. This can also be enabled by setting
> > the 'nand-on-flash-bbt' devicetree property.
> 
> I'm not much aware of how on-flash-BBT works internally, but will it break things if
> we keep on-flash-BBT "enabled" as the default option and add a DT property only to
> explicitly disable the on-flash-BBT?
> 

No, that wouldn't work because the DT property already exists and it works to
enable the flash BBT when it's present.

Documentation/devicetree/bindings/mtd/nand.txt

Of course, we can add a new no-nand-on-flash-bbt, but I really don't see the
point. Users can just put the property in all the devicetree board files where
it's needed.

And moreover, I don't want to change the default behavior of the driver; it's
better to allow to *add* a new feature, if such feature is desired.
Otherwise, users with some data in a flash's last blocks would be wiped and
replaced with the BBT.
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH v2 1/3] nand: omap2: Add support for flash-based bad block table

[Roger Quadros]

On 09/09/2014 04:27 PM, Ezequiel Garcia wrote:
> On 09 Sep 11:35 AM, Roger Quadros wrote:
>> Ezequiel,
>>
>> On 09/08/2014 02:27 PM, Ezequiel Garcia wrote:
>>> This commit adds a new platform-data boolean property that enables use
>>> of a flash-based bad block table. This can also be enabled by setting
>>> the 'nand-on-flash-bbt' devicetree property.
>>
>> I'm not much aware of how on-flash-BBT works internally, but will it break things if
>> we keep on-flash-BBT "enabled" as the default option and add a DT property only to
>> explicitly disable the on-flash-BBT?
>>
> 
> No, that wouldn't work because the DT property already exists and it works to
> enable the flash BBT when it's present.
> 
> Documentation/devicetree/bindings/mtd/nand.txt
> 
> Of course, we can add a new no-nand-on-flash-bbt, but I really don't see the
> point. Users can just put the property in all the devicetree board files where
> it's needed.
> 
> And moreover, I don't want to change the default behavior of the driver; it's
> better to allow to *add* a new feature, if such feature is desired.
> Otherwise, users with some data in a flash's last blocks would be wiped and
> replaced with the BBT.
> 

OK. Let's stick with your patch then.

cheers,
-roger

[PATCH v2 2/3] 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.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/omap2.c          | 130 +++++++++++++++++++++-----------------
 include/linux/platform_data/elm.h |  14 ++++
 2 files changed, 85 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..25d1bca 100644
--- a/include/linux/platform_data/elm.h
+++ b/include/linux/platform_data/elm.h
@@ -42,8 +42,22 @@ 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
+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)
+{
+	return -ENOSYS;
+}
+#endif /* CONFIG_MTD_NAND_ECC_BCH */
+
 #endif /* __ELM_H */
-- 
2.0.1

[PATCH v2 3/3] nand: omap2: Replace pr_err with dev_err

[Ezequiel Garcia]

Usage of pr_err is frowned upon, so replace it with dev_err.
Aditionally, the message on nand_bch_init() error is redundant,
since proper error is showed if the function fails.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/omap2.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f97a4ff..eb5e898 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;
 	}
@@ -1959,7 +1961,6 @@ 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");
 			err = -EINVAL;
 			goto return_error;
 		}
@@ -2026,7 +2027,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 +2039,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.0.1

Re: [PATCH v2 3/3] nand: omap2: Replace pr_err with dev_err

[Roger Quadros]

Hi Ezequiel,

On 09/08/2014 02:27 PM, Ezequiel Garcia wrote:
> Usage of pr_err is frowned upon, so replace it with dev_err.
> Aditionally, the message on nand_bch_init() error is redundant,
> since proper error is showed if the function fails.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/mtd/nand/omap2.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index f97a4ff..eb5e898 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;
>  	}
> @@ -1959,7 +1961,6 @@ 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");

Where would the error be printed in this case?

I also noticed a similar message in the OMAP_ECC_BCH4_CODE_HW_DETECTION_SW case.
And it seems to be missing a "goto return_error" there.

>  			err = -EINVAL;
>  			goto return_error;
>  		}
> @@ -2026,7 +2027,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 +2039,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;
>  	}
> 

cheers,
-roger

Re: [PATCH v2 3/3] nand: omap2: Replace pr_err with dev_err

[Ezequiel Garcia]

On 08 Sep 02:57 PM, Roger Quadros wrote:
> Hi Ezequiel,
> 
> On 09/08/2014 02:27 PM, Ezequiel Garcia wrote:
> > Usage of pr_err is frowned upon, so replace it with dev_err.
> > Aditionally, the message on nand_bch_init() error is redundant,
> > since proper error is showed if the function fails.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> > ---
> >  drivers/mtd/nand/omap2.c | 26 ++++++++++++++------------
> >  1 file changed, 14 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> > index f97a4ff..eb5e898 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;
> >  	}
> > @@ -1959,7 +1961,6 @@ 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");
> 
> Where would the error be printed in this case?
> 

Well, nand_bch_init() has a lot of messages for the errors, but I just noticed
some cases where there's no message printed. Would you prefer to leave these
untouched?

> I also noticed a similar message in the OMAP_ECC_BCH4_CODE_HW_DETECTION_SW case.
> And it seems to be missing a "goto return_error" there.
> 

Argh, right. I'll have to resend this one.
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH v2 3/3] nand: omap2: Replace pr_err with dev_err

[Roger Quadros]

On 09/08/2014 05:35 PM, Ezequiel Garcia wrote:
> On 08 Sep 02:57 PM, Roger Quadros wrote:
>> Hi Ezequiel,
>>
>> On 09/08/2014 02:27 PM, Ezequiel Garcia wrote:
>>> Usage of pr_err is frowned upon, so replace it with dev_err.
>>> Aditionally, the message on nand_bch_init() error is redundant,
>>> since proper error is showed if the function fails.
>>>
>>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>>> ---
>>>  drivers/mtd/nand/omap2.c | 26 ++++++++++++++------------
>>>  1 file changed, 14 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>>> index f97a4ff..eb5e898 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;
>>>  	}
>>> @@ -1959,7 +1961,6 @@ 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");
>>
>> Where would the error be printed in this case?
>>
> 
> Well, nand_bch_init() has a lot of messages for the errors, but I just noticed
> some cases where there's no message printed. Would you prefer to leave these
> untouched?

Yes, let's leave these messages here to tackle the cases where error messages aren't printed
in nand_bch_init().

> 
>> I also noticed a similar message in the OMAP_ECC_BCH4_CODE_HW_DETECTION_SW case.
>> And it seems to be missing a "goto return_error" there.
>>
> 
> Argh, right. I'll have to resend this one.
> 

cheers,
-roger