[PATCH v2 00/16] Fix probe functions error path

[PATCH v2 00/16] Fix probe functions error path

[Miquel Raynal]

Hello all,

In order to remove the limitation that prevents dynamic allocations
during the identification phase (nand_scan_ident()), we need to get rid
of the nand_scan_ident()/nand_scan_tail() calls from drivers and only
export nand_scan().

This series prepares the migration to nand_scan() by first fixing (and
enhancing some times) the probe function error path of the drivers that
do not use nand_scan() yet.

The three main problems addressed in the series are:
1/ a wrong error path that does not free/disable the resources
   correctly.
2/ nand_cleanup() is not called upon error after a successful
   nand_scan_tail().
3/ nand_release() is called instead of nand_cleanup(). nand_release()
   does call nand_cleanup() and also mtd_device_unregister(), which
   should not be called while mtd_device_register() as succeeded.

Thanks,
Miquèl

Changes since v1:
=================
 * Extracted only the patches fixing the error path from the first huge
   series (50+ patches) that also converted the drivers to nand_scan().
 * Changed wrong nand_release() calls to nand_cleanup().


Miquel Raynal (16):
  mtd: rawnand: brcmnand: fix probe function error path
  mtd: rawnand: cafe: fix probe function error path
  mtd: rawnand: davinci: fix probe function error path
  mtd: rawnand: denali: fix probe function error path
  mtd: rawnand: docg4: fix the probe function error path
  mtd: rawnand: fsl_elbc: fix probe function error path
  mtd: rawnand: fsl_ifc: fix probe function error path
  mtd: rawnand: fsmc: fix and enhance probe function error path
  mtd: rawnand: mxc: fix probe function error path
  mtd: rawnand: omap2: fix the probe function error path
  mtd: rawnand: sh_flctl: fix the probe function error path
  mtd: rawnand: tango: fix probe function error path
  mtd: rawnand: hisi504: fix and enhance the probe function error path
  mtd: rawnand: lpc32xx_mlc: fix and enhance the probe function error
    path
  mtd: rawnand: lpc32xx_slc: fix and enhance the probe function error
    path
  mtd: rawnand: s3c2410: enhance the probe function error path

 drivers/mtd/nand/raw/brcmnand/brcmnand.c |  6 ++++-
 drivers/mtd/nand/raw/cafe_nand.c         | 18 +++++++++------
 drivers/mtd/nand/raw/davinci_nand.c      |  6 +++--
 drivers/mtd/nand/raw/denali.c            |  4 +++-
 drivers/mtd/nand/raw/docg4.c             | 22 +++++++++---------
 drivers/mtd/nand/raw/fsl_elbc_nand.c     |  9 ++++++--
 drivers/mtd/nand/raw/fsl_ifc_nand.c      |  8 ++++++-
 drivers/mtd/nand/raw/fsmc_nand.c         | 27 +++++++++++++----------
 drivers/mtd/nand/raw/hisi504_nand.c      | 35 ++++++++++-------------------
 drivers/mtd/nand/raw/lpc32xx_mlc.c       | 38 +++++++++++++++++---------------
 drivers/mtd/nand/raw/lpc32xx_slc.c       | 26 ++++++++++++----------
 drivers/mtd/nand/raw/mxc_nand.c          | 11 +++++----
 drivers/mtd/nand/raw/omap2.c             |  5 ++++-
 drivers/mtd/nand/raw/s3c2410.c           | 24 ++++++++++----------
 drivers/mtd/nand/raw/sh_flctl.c          |  4 ++++
 drivers/mtd/nand/raw/tango_nand.c        |  4 +++-
 16 files changed, 140 insertions(+), 107 deletions(-)

-- 
2.14.1

[PATCH v2 01/16] mtd: rawnand: brcmnand: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index c28fd2bc1a84..1306aaa7a8bf 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2297,7 +2297,11 @@ static int brcmnand_init_cs(struct brcmnand_host *host, struct device_node *dn)
 	if (ret)
 		return ret;
 
-	return mtd_device_register(mtd, NULL, 0);
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret)
+		nand_cleanup(chip);
+
+	return ret;
 }
 
 static void brcmnand_save_restore_cs_config(struct brcmnand_host *host,
-- 
2.14.1

[PATCH v2 02/16] mtd: rawnand: cafe: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/cafe_nand.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
index 3c1b6a3786b2..de9682d87a56 100644
--- a/drivers/mtd/nand/raw/cafe_nand.c
+++ b/drivers/mtd/nand/raw/cafe_nand.c
@@ -774,21 +774,25 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	pci_set_drvdata(pdev, mtd);
 
 	mtd->name = "cafe_nand";
-	mtd_device_parse_register(mtd, part_probes, NULL, NULL, 0);
+	err = mtd_device_parse_register(mtd, part_probes, NULL, NULL, 0);
+	if (err)
+		goto out_cleanup_nand;
 
-	goto out;
+	return 0;
 
- out_free_dma:
+out_cleanup_nand:
+	nand_cleanup(&cafe->nand);
+out_free_dma:
 	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
- out_irq:
+out_irq:
 	/* Disable NAND IRQ in global IRQ mask register */
 	cafe_writel(cafe, ~1 & cafe_readl(cafe, GLOBAL_IRQ_MASK), GLOBAL_IRQ_MASK);
 	free_irq(pdev->irq, mtd);
- out_ior:
+out_ior:
 	pci_iounmap(pdev, cafe->mmio);
- out_free_mtd:
+out_free_mtd:
 	kfree(cafe);
- out:
+
 	return err;
 }
 
-- 
2.14.1

Re: [PATCH v2 02/16] mtd: rawnand: cafe: fix probe function error path

[Boris Brezillon]

On Wed, 21 Mar 2018 14:01:43 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> An error after nand_scan_tail() should trigger a nand_cleanup().
> The helper mtd_device_parse_register() returns an error code that should
> be checked and nand_cleanup() called accordingly.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/cafe_nand.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
> index 3c1b6a3786b2..de9682d87a56 100644
> --- a/drivers/mtd/nand/raw/cafe_nand.c
> +++ b/drivers/mtd/nand/raw/cafe_nand.c
> @@ -774,21 +774,25 @@ static int cafe_nand_probe(struct pci_dev *pdev,
>  	pci_set_drvdata(pdev, mtd);
>  
>  	mtd->name = "cafe_nand";
> -	mtd_device_parse_register(mtd, part_probes, NULL, NULL, 0);
> +	err = mtd_device_parse_register(mtd, part_probes, NULL, NULL, 0);
> +	if (err)
> +		goto out_cleanup_nand;
>  
> -	goto out;
> +	return 0;
>  
> - out_free_dma:
> +out_cleanup_nand:
> +	nand_cleanup(&cafe->nand);
> +out_free_dma:
>  	dma_free_coherent(&cafe->pdev->dev, 2112, cafe->dmabuf, cafe->dmaaddr);
> - out_irq:
> +out_irq:
>  	/* Disable NAND IRQ in global IRQ mask register */
>  	cafe_writel(cafe, ~1 & cafe_readl(cafe, GLOBAL_IRQ_MASK), GLOBAL_IRQ_MASK);
>  	free_irq(pdev->irq, mtd);
> - out_ior:
> +out_ior:
>  	pci_iounmap(pdev, cafe->mmio);
> - out_free_mtd:
> +out_free_mtd:
>  	kfree(cafe);
> - out:
> +
>  	return err;
>  }
>  

You're doing more than what's described in your commit message:
- fixing coding style issues
- reworking the success path

I'm not against those changes, but they should definitely be done in
separate patches.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 03/16] mtd: rawnand: davinci: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/davinci_nand.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index ccc8c43abcff..ccc51dc65f35 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -826,7 +826,7 @@ static int nand_davinci_probe(struct platform_device *pdev)
 	else
 		ret = mtd_device_register(mtd, NULL, 0);
 	if (ret < 0)
-		goto err;
+		goto cleanup_nand;
 
 	val = davinci_nand_readl(info, NRCSR_OFFSET);
 	dev_info(&pdev->dev, "controller rev. %d.%d\n",
@@ -834,14 +834,16 @@ static int nand_davinci_probe(struct platform_device *pdev)
 
 	return 0;
 
+cleanup_nand:
+	nand_cleanup(&info->chip);
 err:
 	clk_disable_unprepare(info->clk);
-
 err_clk_enable:
 	spin_lock_irq(&davinci_nand_lock);
 	if (info->chip.ecc.mode == NAND_ECC_HW_SYNDROME)
 		ecc4_busy = false;
 	spin_unlock_irq(&davinci_nand_lock);
+
 	return ret;
 }
 
-- 
2.14.1

[PATCH v2 04/16] mtd: rawnand: denali: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/denali.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 313c7f50621b..2a302a1d1430 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1384,10 +1384,12 @@ int denali_init(struct denali_nand_info *denali)
 	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret) {
 		dev_err(denali->dev, "Failed to register MTD: %d\n", ret);
-		goto free_buf;
+		goto cleanup_nand;
 	}
 	return 0;
 
+cleanup_nand:
+	nand_cleanup(chip);
 free_buf:
 	kfree(denali->buf);
 disable_irq:
-- 
2.14.1

[PATCH v2 05/16] mtd: rawnand: docg4: fix the probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup() call.
The helper mtd_device_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/docg4.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
index 1314aa99b9ab..bb96cb33cd6b 100644
--- a/drivers/mtd/nand/raw/docg4.c
+++ b/drivers/mtd/nand/raw/docg4.c
@@ -1341,7 +1341,7 @@ static int __init probe_docg4(struct platform_device *pdev)
 	nand = kzalloc(len, GFP_KERNEL);
 	if (nand == NULL) {
 		retval = -ENOMEM;
-		goto fail_unmap;
+		goto unmap;
 	}
 
 	mtd = nand_to_mtd(nand);
@@ -1357,7 +1357,7 @@ static int __init probe_docg4(struct platform_device *pdev)
 	doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
 	if (doc->bch == NULL) {
 		retval = -EINVAL;
-		goto fail;
+		goto free_nand;
 	}
 
 	platform_set_drvdata(pdev, doc);
@@ -1366,30 +1366,32 @@ static int __init probe_docg4(struct platform_device *pdev)
 	retval = read_id_reg(mtd);
 	if (retval == -ENODEV) {
 		dev_warn(dev, "No diskonchip G4 device found.\n");
-		goto fail;
+		goto free_bch;
 	}
 
 	retval = nand_scan_tail(mtd);
 	if (retval)
-		goto fail;
+		goto free_bch;
 
 	retval = read_factory_bbt(mtd);
 	if (retval)
-		goto fail;
+		goto cleanup_nand;
 
 	retval = mtd_device_parse_register(mtd, part_probes, NULL, NULL, 0);
 	if (retval)
-		goto fail;
+		goto cleanup_nand;
 
 	doc->mtd = mtd;
+
 	return 0;
 
-fail:
-	nand_release(mtd); /* deletes partitions and mtd devices */
+cleanup_nand:
+	nand_cleanup(nand);
+free_bch:
 	free_bch(doc->bch);
+free_nand:
 	kfree(nand);
-
-fail_unmap:
+unmap:
 	iounmap(virtadr);
 
 	return retval;
-- 
2.14.1

Re: [PATCH v2 05/16] mtd: rawnand: docg4: fix the probe function error path

[Boris Brezillon]

On Wed, 21 Mar 2018 14:01:46 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> An error after nand_scan_tail() should trigger a nand_cleanup() call.
> The helper mtd_device_register() returns an error code that should
> be checked and nand_cleanup() called accordingly.

The commit message is not really describing what you're fixing here
because nand_cleanup() is called as part of nand_release().

How about something like that:

"
nand_release() should not be called on an MTD device that has not been
registered. While it should work thanks to the checks done in
mtd_device_unregister() it's a bad practice to cleanup/release
something that has not previously been initialized/allocated.
    
Rework the error path to follow this rule.
"

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/docg4.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
> index 1314aa99b9ab..bb96cb33cd6b 100644
> --- a/drivers/mtd/nand/raw/docg4.c
> +++ b/drivers/mtd/nand/raw/docg4.c
> @@ -1341,7 +1341,7 @@ static int __init probe_docg4(struct platform_device *pdev)
>  	nand = kzalloc(len, GFP_KERNEL);
>  	if (nand == NULL) {
>  		retval = -ENOMEM;
> -		goto fail_unmap;
> +		goto unmap;
>  	}
>  
>  	mtd = nand_to_mtd(nand);
> @@ -1357,7 +1357,7 @@ static int __init probe_docg4(struct platform_device *pdev)
>  	doc->bch = init_bch(DOCG4_M, DOCG4_T, DOCG4_PRIMITIVE_POLY);
>  	if (doc->bch == NULL) {
>  		retval = -EINVAL;
> -		goto fail;
> +		goto free_nand;
>  	}
>  
>  	platform_set_drvdata(pdev, doc);
> @@ -1366,30 +1366,32 @@ static int __init probe_docg4(struct platform_device *pdev)
>  	retval = read_id_reg(mtd);
>  	if (retval == -ENODEV) {
>  		dev_warn(dev, "No diskonchip G4 device found.\n");
> -		goto fail;
> +		goto free_bch;
>  	}
>  
>  	retval = nand_scan_tail(mtd);
>  	if (retval)
> -		goto fail;
> +		goto free_bch;
>  
>  	retval = read_factory_bbt(mtd);
>  	if (retval)
> -		goto fail;
> +		goto cleanup_nand;
>  
>  	retval = mtd_device_parse_register(mtd, part_probes, NULL, NULL, 0);
>  	if (retval)
> -		goto fail;
> +		goto cleanup_nand;
>  
>  	doc->mtd = mtd;
> +
>  	return 0;
>  
> -fail:
> -	nand_release(mtd); /* deletes partitions and mtd devices */
> +cleanup_nand:
> +	nand_cleanup(nand);
> +free_bch:
>  	free_bch(doc->bch);
> +free_nand:
>  	kfree(nand);
> -
> -fail_unmap:
> +unmap:
>  	iounmap(virtadr);
>  
>  	return retval;



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 06/16] mtd: rawnand: fsl_elbc: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/fsl_elbc_nand.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c
index d28df991c73c..7a7cb60ec93c 100644
--- a/drivers/mtd/nand/raw/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c
@@ -926,15 +926,20 @@ static int fsl_elbc_nand_probe(struct platform_device *pdev)
 
 	/* First look for RedBoot table or partitions on the command
 	 * line, these take precedence over device tree information */
-	mtd_device_parse_register(mtd, part_probe_types, NULL,
-				  NULL, 0);
+	ret = mtd_device_parse_register(mtd, part_probe_types, NULL, NULL, 0);
+	if (ret)
+		goto cleanup_nand;
 
 	pr_info("eLBC NAND device at 0x%llx, bank %d\n",
 		(unsigned long long)res.start, priv->bank);
+
 	return 0;
 
+cleanup_nand:
+	nand_cleanup(&priv->chip);
 err:
 	fsl_elbc_chip_remove(priv);
+
 	return ret;
 }
 
-- 
2.14.1

Re: [PATCH v2 06/16] mtd: rawnand: fsl_elbc: fix probe function error path

[Boris Brezillon]

On Wed, 21 Mar 2018 14:01:47 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> An error after nand_scan_tail() should trigger a nand_cleanup().
> The helper mtd_device_parse_register() returns an error code that should
> be checked and nand_cleanup() called accordingly.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/fsl_elbc_nand.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c
> index d28df991c73c..7a7cb60ec93c 100644
> --- a/drivers/mtd/nand/raw/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c
> @@ -926,15 +926,20 @@ static int fsl_elbc_nand_probe(struct platform_device *pdev)
>  
>  	/* First look for RedBoot table or partitions on the command
>  	 * line, these take precedence over device tree information */
> -	mtd_device_parse_register(mtd, part_probe_types, NULL,
> -				  NULL, 0);
> +	ret = mtd_device_parse_register(mtd, part_probe_types, NULL, NULL, 0);
> +	if (ret)
> +		goto cleanup_nand;
>  
>  	pr_info("eLBC NAND device at 0x%llx, bank %d\n",
>  		(unsigned long long)res.start, priv->bank);
> +
>  	return 0;
>  
> +cleanup_nand:
> +	nand_cleanup(&priv->chip);
>  err:
>  	fsl_elbc_chip_remove(priv);

fsl_elbc_chip_remove() is already calling nand_release() (which
contains a call to nand_cleanup()), so we might experience double-free
issues if some of the fields are not re-initialized to NULL.

> +
>  	return ret;
>  }
>  



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 07/16] mtd: rawnand: fsl_ifc: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/fsl_ifc_nand.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index 7ca678f05ae3..6082528ec7f1 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -1065,14 +1065,20 @@ static int fsl_ifc_nand_probe(struct platform_device *dev)
 
 	/* First look for RedBoot table or partitions on the command
 	 * line, these take precedence over device tree information */
-	mtd_device_parse_register(mtd, part_probe_types, NULL, NULL, 0);
+	ret = mtd_device_parse_register(mtd, part_probe_types, NULL, NULL, 0);
+	if (ret)
+		goto cleanup_nand;
 
 	dev_info(priv->dev, "IFC NAND device at 0x%llx, bank %d\n",
 		 (unsigned long long)res.start, priv->bank);
+
 	return 0;
 
+cleanup_nand:
+	nand_cleanup(&priv->chip);
 err:
 	fsl_ifc_chip_remove(priv);
+
 	return ret;
 }
 
-- 
2.14.1

Re: [PATCH v2 07/16] mtd: rawnand: fsl_ifc: fix probe function error path

[Boris Brezillon]

On Wed, 21 Mar 2018 14:01:48 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> An error after nand_scan_tail() should trigger a nand_cleanup().
> The helper mtd_device_parse_register() returns an error code that should
> be checked and nand_cleanup() called accordingly.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/fsl_ifc_nand.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> index 7ca678f05ae3..6082528ec7f1 100644
> --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> @@ -1065,14 +1065,20 @@ static int fsl_ifc_nand_probe(struct platform_device *dev)
>  
>  	/* First look for RedBoot table or partitions on the command
>  	 * line, these take precedence over device tree information */
> -	mtd_device_parse_register(mtd, part_probe_types, NULL, NULL, 0);
> +	ret = mtd_device_parse_register(mtd, part_probe_types, NULL, NULL, 0);
> +	if (ret)
> +		goto cleanup_nand;
>  
>  	dev_info(priv->dev, "IFC NAND device at 0x%llx, bank %d\n",
>  		 (unsigned long long)res.start, priv->bank);
> +
>  	return 0;
>  
> +cleanup_nand:
> +	nand_cleanup(&priv->chip);
>  err:
>  	fsl_ifc_chip_remove(priv);
> +

Same as for the elbc patch: we might have double-free issues after this
change.

>  	return ret;
>  }
>  



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 08/16] mtd: rawnand: fsmc: fix and enhance probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().

Also, goto labels should indicate what the next cleanup is, not the
point from which they can be accessed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/fsmc_nand.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c
index 28c48dcc514e..f4a5a317d4ae 100644
--- a/drivers/mtd/nand/raw/fsmc_nand.c
+++ b/drivers/mtd/nand/raw/fsmc_nand.c
@@ -1022,12 +1022,12 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 		host->read_dma_chan = dma_request_channel(mask, filter, NULL);
 		if (!host->read_dma_chan) {
 			dev_err(&pdev->dev, "Unable to get read dma channel\n");
-			goto err_req_read_chnl;
+			goto disable_clk;
 		}
 		host->write_dma_chan = dma_request_channel(mask, filter, NULL);
 		if (!host->write_dma_chan) {
 			dev_err(&pdev->dev, "Unable to get write dma channel\n");
-			goto err_req_write_chnl;
+			goto release_dma_read_chan;
 		}
 	}
 
@@ -1050,7 +1050,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	ret = nand_scan_ident(mtd, 1, NULL);
 	if (ret) {
 		dev_err(&pdev->dev, "No NAND Device found!\n");
-		goto err_scan_ident;
+		goto release_dma_write_chan;
 	}
 
 	if (AMBA_REV_BITS(host->pid) >= 8) {
@@ -1065,7 +1065,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 			dev_warn(&pdev->dev, "No oob scheme defined for oobsize %d\n",
 				 mtd->oobsize);
 			ret = -EINVAL;
-			goto err_probe;
+			goto release_dma_write_chan;
 		}
 
 		mtd_set_ooblayout(mtd, &fsmc_ecc4_ooblayout_ops);
@@ -1090,7 +1090,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 
 		default:
 			dev_err(&pdev->dev, "Unsupported ECC mode!\n");
-			goto err_probe;
+			goto release_dma_write_chan;
 		}
 
 		/*
@@ -1110,7 +1110,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 					 "No oob scheme defined for oobsize %d\n",
 					 mtd->oobsize);
 				ret = -EINVAL;
-				goto err_probe;
+				goto release_dma_write_chan;
 			}
 		}
 	}
@@ -1118,26 +1118,29 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
 	/* Second stage of scan to fill MTD data-structures */
 	ret = nand_scan_tail(mtd);
 	if (ret)
-		goto err_probe;
+		goto release_dma_write_chan;
 
 	mtd->name = "nand";
 	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret)
-		goto err_probe;
+		goto cleanup_nand;
 
 	platform_set_drvdata(pdev, host);
 	dev_info(&pdev->dev, "FSMC NAND driver registration successful\n");
+
 	return 0;
 
-err_probe:
-err_scan_ident:
+cleanup_nand:
+	nand_cleanup(nand);
+release_dma_write_chan:
 	if (host->mode == USE_DMA_ACCESS)
 		dma_release_channel(host->write_dma_chan);
-err_req_write_chnl:
+release_dma_read_chan:
 	if (host->mode == USE_DMA_ACCESS)
 		dma_release_channel(host->read_dma_chan);
-err_req_read_chnl:
+disable_clk:
 	clk_disable_unprepare(host->clk);
+
 	return ret;
 }
 
-- 
2.14.1

Re: [PATCH v2 08/16] mtd: rawnand: fsmc: fix and enhance probe function error path

[Boris Brezillon]

On Wed, 21 Mar 2018 14:01:49 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> An error after nand_scan_tail() should trigger a nand_cleanup().
> 
> Also, goto labels should indicate what the next cleanup is, not the
> point from which they can be accessed.

Can you split that in 2 patches: one changing the error labels, and
another one adding the missing nand_cleanup()?

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/fsmc_nand.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c
> index 28c48dcc514e..f4a5a317d4ae 100644
> --- a/drivers/mtd/nand/raw/fsmc_nand.c
> +++ b/drivers/mtd/nand/raw/fsmc_nand.c
> @@ -1022,12 +1022,12 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  		host->read_dma_chan = dma_request_channel(mask, filter, NULL);
>  		if (!host->read_dma_chan) {
>  			dev_err(&pdev->dev, "Unable to get read dma channel\n");
> -			goto err_req_read_chnl;
> +			goto disable_clk;
>  		}
>  		host->write_dma_chan = dma_request_channel(mask, filter, NULL);
>  		if (!host->write_dma_chan) {
>  			dev_err(&pdev->dev, "Unable to get write dma channel\n");
> -			goto err_req_write_chnl;
> +			goto release_dma_read_chan;
>  		}
>  	}
>  
> @@ -1050,7 +1050,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  	ret = nand_scan_ident(mtd, 1, NULL);
>  	if (ret) {
>  		dev_err(&pdev->dev, "No NAND Device found!\n");
> -		goto err_scan_ident;
> +		goto release_dma_write_chan;
>  	}
>  
>  	if (AMBA_REV_BITS(host->pid) >= 8) {
> @@ -1065,7 +1065,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  			dev_warn(&pdev->dev, "No oob scheme defined for oobsize %d\n",
>  				 mtd->oobsize);
>  			ret = -EINVAL;
> -			goto err_probe;
> +			goto release_dma_write_chan;
>  		}
>  
>  		mtd_set_ooblayout(mtd, &fsmc_ecc4_ooblayout_ops);
> @@ -1090,7 +1090,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  
>  		default:
>  			dev_err(&pdev->dev, "Unsupported ECC mode!\n");
> -			goto err_probe;
> +			goto release_dma_write_chan;
>  		}
>  
>  		/*
> @@ -1110,7 +1110,7 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  					 "No oob scheme defined for oobsize %d\n",
>  					 mtd->oobsize);
>  				ret = -EINVAL;
> -				goto err_probe;
> +				goto release_dma_write_chan;
>  			}
>  		}
>  	}
> @@ -1118,26 +1118,29 @@ static int __init fsmc_nand_probe(struct platform_device *pdev)
>  	/* Second stage of scan to fill MTD data-structures */
>  	ret = nand_scan_tail(mtd);
>  	if (ret)
> -		goto err_probe;
> +		goto release_dma_write_chan;
>  
>  	mtd->name = "nand";
>  	ret = mtd_device_register(mtd, NULL, 0);
>  	if (ret)
> -		goto err_probe;
> +		goto cleanup_nand;
>  
>  	platform_set_drvdata(pdev, host);
>  	dev_info(&pdev->dev, "FSMC NAND driver registration successful\n");
> +
>  	return 0;
>  
> -err_probe:
> -err_scan_ident:
> +cleanup_nand:
> +	nand_cleanup(nand);
> +release_dma_write_chan:
>  	if (host->mode == USE_DMA_ACCESS)
>  		dma_release_channel(host->write_dma_chan);
> -err_req_write_chnl:
> +release_dma_read_chan:
>  	if (host->mode == USE_DMA_ACCESS)
>  		dma_release_channel(host->read_dma_chan);
> -err_req_read_chnl:
> +disable_clk:
>  	clk_disable_unprepare(host->clk);
> +
>  	return ret;
>  }
>  



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 09/16] mtd: rawnand: mxc: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/mxc_nand.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
index 944ecf03d3b1..45786e707b7b 100644
--- a/drivers/mtd/nand/raw/mxc_nand.c
+++ b/drivers/mtd/nand/raw/mxc_nand.c
@@ -1911,15 +1911,18 @@ static int mxcnd_probe(struct platform_device *pdev)
 		goto escan;
 
 	/* Register the partitions */
-	mtd_device_parse_register(mtd, part_probes,
-			NULL,
-			host->pdata.parts,
-			host->pdata.nr_parts);
+	err = mtd_device_parse_register(mtd, part_probes, NULL,
+					host->pdata.parts,
+					host->pdata.nr_parts);
+	if (err)
+		goto cleanup_nand;
 
 	platform_set_drvdata(pdev, host);
 
 	return 0;
 
+cleanup_nand:
+	nand_cleanup(this);
 escan:
 	if (host->clk_act)
 		clk_disable_unprepare(host->clk);
-- 
2.14.1

[PATCH v2 10/16] mtd: rawnand: omap2: fix the probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/omap2.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
index 8cdf7d3d8fa7..02dfd2e66938 100644
--- a/drivers/mtd/nand/raw/omap2.c
+++ b/drivers/mtd/nand/raw/omap2.c
@@ -2263,12 +2263,14 @@ static int omap_nand_probe(struct platform_device *pdev)
 
 	err = mtd_device_register(mtd, NULL, 0);
 	if (err)
-		goto return_error;
+		goto cleanup_nand;
 
 	platform_set_drvdata(pdev, mtd);
 
 	return 0;
 
+cleanup_nand:
+	nand_cleanup(nand_chip);
 return_error:
 	if (!IS_ERR_OR_NULL(info->dma))
 		dma_release_channel(info->dma);
@@ -2276,6 +2278,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_bch_free(nand_chip->ecc.priv);
 		nand_chip->ecc.priv = NULL;
 	}
+
 	return err;
 }
 
-- 
2.14.1

[PATCH v2 11/16] mtd: rawnand: sh_flctl: fix the probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/sh_flctl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/raw/sh_flctl.c b/drivers/mtd/nand/raw/sh_flctl.c
index 7a740d583866..c7abceffcc40 100644
--- a/drivers/mtd/nand/raw/sh_flctl.c
+++ b/drivers/mtd/nand/raw/sh_flctl.c
@@ -1214,9 +1214,13 @@ static int flctl_probe(struct platform_device *pdev)
 		goto err_chip;
 
 	ret = mtd_device_register(flctl_mtd, pdata->parts, pdata->nr_parts);
+	if (ret)
+		goto cleanup_nand;
 
 	return 0;
 
+cleanup_nand:
+	nand_cleanup(nand);
 err_chip:
 	flctl_release_dma(flctl);
 	pm_runtime_disable(&pdev->dev);
-- 
2.14.1

[PATCH v2 12/16] mtd: rawnand: tango: fix probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/tango_nand.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/tango_nand.c b/drivers/mtd/nand/raw/tango_nand.c
index c5bee00b7f5e..f54518ffb36a 100644
--- a/drivers/mtd/nand/raw/tango_nand.c
+++ b/drivers/mtd/nand/raw/tango_nand.c
@@ -591,8 +591,10 @@ static int chip_init(struct device *dev, struct device_node *np)
 	tchip->bb_cfg = BB_CFG(mtd->writesize, BBM_SIZE);
 
 	err = mtd_device_register(mtd, NULL, 0);
-	if (err)
+	if (err) {
+		nand_cleanup(chip);
 		return err;
+	}
 
 	nfc->chips[cs] = tchip;
 
-- 
2.14.1

[PATCH v2 13/16] mtd: rawnand: hisi504: fix and enhance the probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Also prepare the migration of the hisi504_nand driver to use nand_scan()
by cleaning the error path in the probe function.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/hisi504_nand.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/raw/hisi504_nand.c b/drivers/mtd/nand/raw/hisi504_nand.c
index 27558a67fa41..a1e009c8e556 100644
--- a/drivers/mtd/nand/raw/hisi504_nand.c
+++ b/drivers/mtd/nand/raw/hisi504_nand.c
@@ -731,23 +731,19 @@ static int hisi_nfc_probe(struct platform_device *pdev)
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		dev_err(dev, "no IRQ resource defined\n");
-		ret = -ENXIO;
-		goto err_res;
+		return -ENXIO;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	host->iobase = devm_ioremap_resource(dev, res);
-	if (IS_ERR(host->iobase)) {
-		ret = PTR_ERR(host->iobase);
-		goto err_res;
-	}
+	if (IS_ERR(host->iobase))
+		return PTR_ERR(host->iobase);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	host->mmio = devm_ioremap_resource(dev, res);
 	if (IS_ERR(host->mmio)) {
-		ret = PTR_ERR(host->mmio);
 		dev_err(dev, "devm_ioremap_resource[1] fail\n");
-		goto err_res;
+		return PTR_ERR(host->mmio);
 	}
 
 	mtd->name		= "hisi_nand";
@@ -770,19 +766,17 @@ static int hisi_nfc_probe(struct platform_device *pdev)
 	ret = devm_request_irq(dev, irq, hinfc_irq_handle, 0x0, "nandc", host);
 	if (ret) {
 		dev_err(dev, "failed to request IRQ\n");
-		goto err_res;
+		return ret;
 	}
 
 	ret = nand_scan_ident(mtd, max_chips, NULL);
 	if (ret)
-		goto err_res;
+		return ret;
 
 	host->buffer = dmam_alloc_coherent(dev, mtd->writesize + mtd->oobsize,
 		&host->dma_buffer, GFP_KERNEL);
-	if (!host->buffer) {
-		ret = -ENOMEM;
-		goto err_res;
-	}
+	if (!host->buffer)
+		return -ENOMEM;
 
 	host->dma_oob = host->dma_buffer + mtd->writesize;
 	memset(host->buffer, 0xff, mtd->writesize + mtd->oobsize);
@@ -798,8 +792,7 @@ static int hisi_nfc_probe(struct platform_device *pdev)
 	 */
 	default:
 		dev_err(dev, "NON-2KB page size nand flash\n");
-		ret = -EINVAL;
-		goto err_res;
+		return -EINVAL;
 	}
 	hinfc_write(host, flag, HINFC504_CON);
 
@@ -809,21 +802,17 @@ static int hisi_nfc_probe(struct platform_device *pdev)
 	ret = nand_scan_tail(mtd);
 	if (ret) {
 		dev_err(dev, "nand_scan_tail failed: %d\n", ret);
-		goto err_res;
+		return ret;
 	}
 
 	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret) {
 		dev_err(dev, "Err MTD partition=%d\n", ret);
-		goto err_mtd;
+		nand_cleanup(chip);
+		return ret;
 	}
 
 	return 0;
-
-err_mtd:
-	nand_release(mtd);
-err_res:
-	return ret;
 }
 
 static int hisi_nfc_remove(struct platform_device *pdev)
-- 
2.14.1

Re: [PATCH v2 13/16] mtd: rawnand: hisi504: fix and enhance the probe function error path

[Boris Brezillon]

On Wed, 21 Mar 2018 14:01:54 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> An error after nand_scan_tail() should trigger a nand_cleanup().
> The helper mtd_device_parse_register() returns an error code that should
> be checked and nand_cleanup() called accordingly.
> 
> Also prepare the migration of the hisi504_nand driver to use nand_scan()
> by cleaning the error path in the probe function.

Same here: please split that in 2 commits (the same goes for patch 14
and 15).

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/hisi504_nand.c | 35 ++++++++++++-----------------------
>  1 file changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/hisi504_nand.c b/drivers/mtd/nand/raw/hisi504_nand.c
> index 27558a67fa41..a1e009c8e556 100644
> --- a/drivers/mtd/nand/raw/hisi504_nand.c
> +++ b/drivers/mtd/nand/raw/hisi504_nand.c
> @@ -731,23 +731,19 @@ static int hisi_nfc_probe(struct platform_device *pdev)
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
>  		dev_err(dev, "no IRQ resource defined\n");
> -		ret = -ENXIO;
> -		goto err_res;
> +		return -ENXIO;
>  	}
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	host->iobase = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(host->iobase)) {
> -		ret = PTR_ERR(host->iobase);
> -		goto err_res;
> -	}
> +	if (IS_ERR(host->iobase))
> +		return PTR_ERR(host->iobase);
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	host->mmio = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(host->mmio)) {
> -		ret = PTR_ERR(host->mmio);
>  		dev_err(dev, "devm_ioremap_resource[1] fail\n");
> -		goto err_res;
> +		return PTR_ERR(host->mmio);
>  	}
>  
>  	mtd->name		= "hisi_nand";
> @@ -770,19 +766,17 @@ static int hisi_nfc_probe(struct platform_device *pdev)
>  	ret = devm_request_irq(dev, irq, hinfc_irq_handle, 0x0, "nandc", host);
>  	if (ret) {
>  		dev_err(dev, "failed to request IRQ\n");
> -		goto err_res;
> +		return ret;
>  	}
>  
>  	ret = nand_scan_ident(mtd, max_chips, NULL);
>  	if (ret)
> -		goto err_res;
> +		return ret;
>  
>  	host->buffer = dmam_alloc_coherent(dev, mtd->writesize + mtd->oobsize,
>  		&host->dma_buffer, GFP_KERNEL);
> -	if (!host->buffer) {
> -		ret = -ENOMEM;
> -		goto err_res;
> -	}
> +	if (!host->buffer)
> +		return -ENOMEM;
>  
>  	host->dma_oob = host->dma_buffer + mtd->writesize;
>  	memset(host->buffer, 0xff, mtd->writesize + mtd->oobsize);
> @@ -798,8 +792,7 @@ static int hisi_nfc_probe(struct platform_device *pdev)
>  	 */
>  	default:
>  		dev_err(dev, "NON-2KB page size nand flash\n");
> -		ret = -EINVAL;
> -		goto err_res;
> +		return -EINVAL;
>  	}
>  	hinfc_write(host, flag, HINFC504_CON);
>  
> @@ -809,21 +802,17 @@ static int hisi_nfc_probe(struct platform_device *pdev)
>  	ret = nand_scan_tail(mtd);
>  	if (ret) {
>  		dev_err(dev, "nand_scan_tail failed: %d\n", ret);
> -		goto err_res;
> +		return ret;
>  	}
>  
>  	ret = mtd_device_register(mtd, NULL, 0);
>  	if (ret) {
>  		dev_err(dev, "Err MTD partition=%d\n", ret);
> -		goto err_mtd;
> +		nand_cleanup(chip);
> +		return ret;
>  	}
>  
>  	return 0;
> -
> -err_mtd:
> -	nand_release(mtd);
> -err_res:
> -	return ret;
>  }
>  
>  static int hisi_nfc_remove(struct platform_device *pdev)



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 14/16] mtd: rawnand: lpc32xx_mlc: fix and enhance the probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Also prepare the migration of the lpc32xx_mlc driver to use nand_scan()
by cleaning the error path in the probe function.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/lpc32xx_mlc.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/nand/raw/lpc32xx_mlc.c b/drivers/mtd/nand/raw/lpc32xx_mlc.c
index e357948a7505..052d123a8304 100644
--- a/drivers/mtd/nand/raw/lpc32xx_mlc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_mlc.c
@@ -673,7 +673,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	host->io_base = devm_ioremap_resource(&pdev->dev, rc);
 	if (IS_ERR(host->io_base))
 		return PTR_ERR(host->io_base);
-	
+
 	host->io_base_phy = rc->start;
 
 	nand_chip = &host->nand_chip;
@@ -706,11 +706,11 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	if (IS_ERR(host->clk)) {
 		dev_err(&pdev->dev, "Clock initialization failure\n");
 		res = -ENOENT;
-		goto err_exit1;
+		goto free_gpio;
 	}
 	res = clk_prepare_enable(host->clk);
 	if (res)
-		goto err_put_clk;
+		goto put_clk;
 
 	nand_chip->cmd_ctrl = lpc32xx_nand_cmd_ctrl;
 	nand_chip->dev_ready = lpc32xx_nand_device_ready;
@@ -744,7 +744,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 		res = lpc32xx_dma_setup(host);
 		if (res) {
 			res = -EIO;
-			goto err_exit2;
+			goto unprepare_clk;
 		}
 	}
 
@@ -754,18 +754,18 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	 */
 	res = nand_scan_ident(mtd, 1, NULL);
 	if (res)
-		goto err_exit3;
+		goto release_dma_chan;
 
 	host->dma_buf = devm_kzalloc(&pdev->dev, mtd->writesize, GFP_KERNEL);
 	if (!host->dma_buf) {
 		res = -ENOMEM;
-		goto err_exit3;
+		goto release_dma_chan;
 	}
 
 	host->dummy_buf = devm_kzalloc(&pdev->dev, mtd->writesize, GFP_KERNEL);
 	if (!host->dummy_buf) {
 		res = -ENOMEM;
-		goto err_exit3;
+		goto release_dma_chan;
 	}
 
 	nand_chip->ecc.mode = NAND_ECC_HW;
@@ -783,14 +783,14 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	if (host->irq < 0) {
 		dev_err(&pdev->dev, "failed to get platform irq\n");
 		res = -EINVAL;
-		goto err_exit3;
+		goto release_dma_chan;
 	}
 
 	if (request_irq(host->irq, (irq_handler_t)&lpc3xxx_nand_irq,
 			IRQF_TRIGGER_HIGH, DRV_NAME, host)) {
 		dev_err(&pdev->dev, "Error requesting NAND IRQ\n");
 		res = -ENXIO;
-		goto err_exit3;
+		goto release_dma_chan;
 	}
 
 	/*
@@ -799,27 +799,29 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	 */
 	res = nand_scan_tail(mtd);
 	if (res)
-		goto err_exit4;
+		goto free_irq;
 
 	mtd->name = DRV_NAME;
 
 	res = mtd_device_register(mtd, host->ncfg->parts,
 				  host->ncfg->num_parts);
-	if (!res)
-		return res;
+	if (res)
+		goto cleanup_nand;
 
-	nand_release(mtd);
+	return 0;
 
-err_exit4:
+cleanup_nand:
+	nand_cleanup(nand_chip);
+free_irq:
 	free_irq(host->irq, host);
-err_exit3:
+release_dma_chan:
 	if (use_dma)
 		dma_release_channel(host->dma_chan);
-err_exit2:
+unprepare_clk:
 	clk_disable_unprepare(host->clk);
-err_put_clk:
+put_clk:
 	clk_put(host->clk);
-err_exit1:
+free_gpio:
 	lpc32xx_wp_enable(host);
 	gpio_free(host->ncfg->wp_gpio);
 
-- 
2.14.1

[PATCH v2 15/16] mtd: rawnand: lpc32xx_slc: fix and enhance the probe function error path

[Miquel Raynal]

An error after nand_scan_tail() should trigger a nand_cleanup().
The helper mtd_device_parse_register() returns an error code that should
be checked and nand_cleanup() called accordingly.

Also prepare the migration of the lpc32xx_slc driver to use nand_scan()
by cleaning the error path in the probe function.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/lpc32xx_slc.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/lpc32xx_slc.c b/drivers/mtd/nand/raw/lpc32xx_slc.c
index 5f7cc6da0a7f..143e1ec3a90a 100644
--- a/drivers/mtd/nand/raw/lpc32xx_slc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_slc.c
@@ -831,11 +831,11 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	if (IS_ERR(host->clk)) {
 		dev_err(&pdev->dev, "Clock failure\n");
 		res = -ENOENT;
-		goto err_exit1;
+		goto release_gpio;
 	}
 	res = clk_prepare_enable(host->clk);
 	if (res)
-		goto err_exit1;
+		goto release_gpio;
 
 	/* Set NAND IO addresses and command/ready functions */
 	chip->IO_ADDR_R = SLC_DATA(host->io_base);
@@ -874,19 +874,19 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 				      GFP_KERNEL);
 	if (host->data_buf == NULL) {
 		res = -ENOMEM;
-		goto err_exit2;
+		goto release_clk;
 	}
 
 	res = lpc32xx_nand_dma_setup(host);
 	if (res) {
 		res = -EIO;
-		goto err_exit2;
+		goto release_clk;
 	}
 
 	/* Find NAND device */
 	res = nand_scan_ident(mtd, 1, NULL);
 	if (res)
-		goto err_exit3;
+		goto release_dma;
 
 	/* OOB and ECC CPU and DMA work areas */
 	host->ecc_buf = (uint32_t *)(host->data_buf + LPC32XX_DMA_DATA_SIZE);
@@ -920,21 +920,23 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	 */
 	res = nand_scan_tail(mtd);
 	if (res)
-		goto err_exit3;
+		goto release_dma;
 
 	mtd->name = "nxp_lpc3220_slc";
 	res = mtd_device_register(mtd, host->ncfg->parts,
 				  host->ncfg->num_parts);
-	if (!res)
-		return res;
+	if (res)
+		goto cleanup_nand;
 
-	nand_release(mtd);
+	return 0;
 
-err_exit3:
+cleanup_nand:
+	nand_cleanup(chip);
+release_dma:
 	dma_release_channel(host->dma_chan);
-err_exit2:
+release_clk:
 	clk_disable_unprepare(host->clk);
-err_exit1:
+release_gpio:
 	lpc32xx_wp_enable(host);
 
 	return res;
-- 
2.14.1

[PATCH v2 16/16] mtd: rawnand: s3c2410: enhance the probe function error path

[Miquel Raynal]

Prepare the migration of the lpc32xx_slc driver to use nand_scan() by
cleaning the error path in the probe function.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/s3c2410.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c
index b5bc5f106c09..1bc0458063d8 100644
--- a/drivers/mtd/nand/raw/s3c2410.c
+++ b/drivers/mtd/nand/raw/s3c2410.c
@@ -124,13 +124,11 @@ struct s3c2410_nand_info;
  * @chip: The NAND chip information.
  * @set: The platform information supplied for this set of NAND chips.
  * @info: Link back to the hardware information.
- * @scan_res: The result from calling nand_scan_ident().
 */
 struct s3c2410_nand_mtd {
 	struct nand_chip		chip;
 	struct s3c2410_nand_set		*set;
 	struct s3c2410_nand_info	*info;
-	int				scan_res;
 };
 
 enum s3c_cpu_type {
@@ -1163,17 +1161,19 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
 		mtd->dev.parent = &pdev->dev;
 		s3c2410_nand_init_chip(info, nmtd, sets);
 
-		nmtd->scan_res = nand_scan_ident(mtd,
-						 (sets) ? sets->nr_chips : 1,
-						 NULL);
+		err = nand_scan_ident(mtd, (sets) ? sets->nr_chips : 1, NULL);
+		if (err)
+			goto exit_error;
 
-		if (nmtd->scan_res == 0) {
-			err = s3c2410_nand_update_chip(info, nmtd);
-			if (err < 0)
-				goto exit_error;
-			nand_scan_tail(mtd);
-			s3c2410_nand_add_partition(info, nmtd, sets);
-		}
+		err = s3c2410_nand_update_chip(info, nmtd);
+		if (err < 0)
+			goto exit_error;
+
+		err = nand_scan_tail(mtd);
+		if (err)
+			goto exit_error;
+
+		s3c2410_nand_add_partition(info, nmtd, sets);
 
 		if (sets != NULL)
 			sets++;
-- 
2.14.1

Re: [PATCH v2 16/16] mtd: rawnand: s3c2410: enhance the probe function error path

[Boris Brezillon]

On Wed, 21 Mar 2018 14:01:57 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Prepare the migration of the lpc32xx_slc driver to use nand_scan() by
> cleaning the error path in the probe function.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/s3c2410.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c
> index b5bc5f106c09..1bc0458063d8 100644
> --- a/drivers/mtd/nand/raw/s3c2410.c
> +++ b/drivers/mtd/nand/raw/s3c2410.c
> @@ -124,13 +124,11 @@ struct s3c2410_nand_info;
>   * @chip: The NAND chip information.
>   * @set: The platform information supplied for this set of NAND chips.
>   * @info: Link back to the hardware information.
> - * @scan_res: The result from calling nand_scan_ident().
>  */
>  struct s3c2410_nand_mtd {
>  	struct nand_chip		chip;
>  	struct s3c2410_nand_set		*set;
>  	struct s3c2410_nand_info	*info;
> -	int				scan_res;
>  };
>  
>  enum s3c_cpu_type {
> @@ -1163,17 +1161,19 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
>  		mtd->dev.parent = &pdev->dev;
>  		s3c2410_nand_init_chip(info, nmtd, sets);
>  
> -		nmtd->scan_res = nand_scan_ident(mtd,
> -						 (sets) ? sets->nr_chips : 1,
> -						 NULL);
> +		err = nand_scan_ident(mtd, (sets) ? sets->nr_chips : 1, NULL);
> +		if (err)
> +			goto exit_error;
>  
> -		if (nmtd->scan_res == 0) {
> -			err = s3c2410_nand_update_chip(info, nmtd);
> -			if (err < 0)
> -				goto exit_error;
> -			nand_scan_tail(mtd);
> -			s3c2410_nand_add_partition(info, nmtd, sets);
> -		}
> +		err = s3c2410_nand_update_chip(info, nmtd);
> +		if (err < 0)
> +			goto exit_error;
> +
> +		err = nand_scan_tail(mtd);
> +		if (err)
> +			goto exit_error;
> +
> +		s3c2410_nand_add_partition(info, nmtd, sets);

Not related to this patch, but it seems this we're ignoring the return
value of s3c2410_nand_add_partition(). Don't know if this is
intentional.

And there's something even weirder: s3c2410_nand_add_partition() only
registers the MTD device if sets != NULL, so what's the point of
scanning the bus if we know we'll not register the NAND chips we find
there? Looks like this probe path is overly complex for no real reason.

Anyway, I'll take your patch since it tends to improve a bit the code.

>  
>  		if (sets != NULL)
>  			sets++;



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 16/16] mtd: rawnand: s3c2410: enhance the probe function error path

[Miquel Raynal]

Hi Boris,

On Tue, 27 Mar 2018 11:06:50 +0200, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:

> On Wed, 21 Mar 2018 14:01:57 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Prepare the migration of the lpc32xx_slc driver to use nand_scan() by
> > cleaning the error path in the probe function.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/s3c2410.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c
> > index b5bc5f106c09..1bc0458063d8 100644
> > --- a/drivers/mtd/nand/raw/s3c2410.c
> > +++ b/drivers/mtd/nand/raw/s3c2410.c
> > @@ -124,13 +124,11 @@ struct s3c2410_nand_info;
> >   * @chip: The NAND chip information.
> >   * @set: The platform information supplied for this set of NAND chips.
> >   * @info: Link back to the hardware information.
> > - * @scan_res: The result from calling nand_scan_ident().
> >  */
> >  struct s3c2410_nand_mtd {
> >  	struct nand_chip		chip;
> >  	struct s3c2410_nand_set		*set;
> >  	struct s3c2410_nand_info	*info;
> > -	int				scan_res;
> >  };
> >  
> >  enum s3c_cpu_type {
> > @@ -1163,17 +1161,19 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
> >  		mtd->dev.parent = &pdev->dev;
> >  		s3c2410_nand_init_chip(info, nmtd, sets);
> >  
> > -		nmtd->scan_res = nand_scan_ident(mtd,
> > -						 (sets) ? sets->nr_chips : 1,
> > -						 NULL);
> > +		err = nand_scan_ident(mtd, (sets) ? sets->nr_chips : 1, NULL);
> > +		if (err)
> > +			goto exit_error;
> >  
> > -		if (nmtd->scan_res == 0) {
> > -			err = s3c2410_nand_update_chip(info, nmtd);
> > -			if (err < 0)
> > -				goto exit_error;
> > -			nand_scan_tail(mtd);
> > -			s3c2410_nand_add_partition(info, nmtd, sets);
> > -		}
> > +		err = s3c2410_nand_update_chip(info, nmtd);
> > +		if (err < 0)
> > +			goto exit_error;
> > +
> > +		err = nand_scan_tail(mtd);
> > +		if (err)
> > +			goto exit_error;
> > +
> > +		s3c2410_nand_add_partition(info, nmtd, sets);  
> 
> Not related to this patch, but it seems this we're ignoring the return
> value of s3c2410_nand_add_partition(). Don't know if this is
> intentional.

I known, and that is not the only one having very weird probe
functions, but because I cannot actually test the code and because my
main goal was absolutely not to fix every single NAND driver I chose to
not go into deeper details for now and focus on my target. This logic
should be fixed though.
 
> 
> And there's something even weirder: s3c2410_nand_add_partition() only
> registers the MTD device if sets != NULL, so what's the point of
> scanning the bus if we know we'll not register the NAND chips we find
> there? Looks like this probe path is overly complex for no real reason.
> 
> Anyway, I'll take your patch since it tends to improve a bit the code.

I addressed all your other comments and will send a new version of the
remaining patches.

Thank you,
Miquèl

-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 00/16] Fix probe functions error path

[Boris Brezillon]

On Wed, 21 Mar 2018 14:01:41 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hello all,
> 
> In order to remove the limitation that prevents dynamic allocations
> during the identification phase (nand_scan_ident()), we need to get rid
> of the nand_scan_ident()/nand_scan_tail() calls from drivers and only
> export nand_scan().
> 
> This series prepares the migration to nand_scan() by first fixing (and
> enhancing some times) the probe function error path of the drivers that
> do not use nand_scan() yet.
> 
> The three main problems addressed in the series are:
> 1/ a wrong error path that does not free/disable the resources
>    correctly.
> 2/ nand_cleanup() is not called upon error after a successful
>    nand_scan_tail().
> 3/ nand_release() is called instead of nand_cleanup(). nand_release()
>    does call nand_cleanup() and also mtd_device_unregister(), which
>    should not be called while mtd_device_register() as succeeded.
> 
> Thanks,
> Miquèl
> 
> Changes since v1:
> =================
>  * Extracted only the patches fixing the error path from the first huge
>    series (50+ patches) that also converted the drivers to nand_scan().
>  * Changed wrong nand_release() calls to nand_cleanup().
> 
> 
> Miquel Raynal (16):
>   mtd: rawnand: brcmnand: fix probe function error path
>   mtd: rawnand: cafe: fix probe function error path
>   mtd: rawnand: davinci: fix probe function error path
>   mtd: rawnand: denali: fix probe function error path

Applied patches 1 to 4...

>   mtd: rawnand: docg4: fix the probe function error path
>   mtd: rawnand: fsl_elbc: fix probe function error path
>   mtd: rawnand: fsl_ifc: fix probe function error path
>   mtd: rawnand: fsmc: fix and enhance probe function error path
>   mtd: rawnand: mxc: fix probe function error path
>   mtd: rawnand: omap2: fix the probe function error path
>   mtd: rawnand: sh_flctl: fix the probe function error path
>   mtd: rawnand: tango: fix probe function error path

... patches 9 to 12...

>   mtd: rawnand: hisi504: fix and enhance the probe function error path
>   mtd: rawnand: lpc32xx_mlc: fix and enhance the probe function error
>     path
>   mtd: rawnand: lpc32xx_slc: fix and enhance the probe function error
>     path
>   mtd: rawnand: s3c2410: enhance the probe function error path

... and patch 16.

> 
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c |  6 ++++-
>  drivers/mtd/nand/raw/cafe_nand.c         | 18 +++++++++------
>  drivers/mtd/nand/raw/davinci_nand.c      |  6 +++--
>  drivers/mtd/nand/raw/denali.c            |  4 +++-
>  drivers/mtd/nand/raw/docg4.c             | 22 +++++++++---------
>  drivers/mtd/nand/raw/fsl_elbc_nand.c     |  9 ++++++--
>  drivers/mtd/nand/raw/fsl_ifc_nand.c      |  8 ++++++-
>  drivers/mtd/nand/raw/fsmc_nand.c         | 27 +++++++++++++----------
>  drivers/mtd/nand/raw/hisi504_nand.c      | 35 ++++++++++-------------------
>  drivers/mtd/nand/raw/lpc32xx_mlc.c       | 38 +++++++++++++++++---------------
>  drivers/mtd/nand/raw/lpc32xx_slc.c       | 26 ++++++++++++----------
>  drivers/mtd/nand/raw/mxc_nand.c          | 11 +++++----
>  drivers/mtd/nand/raw/omap2.c             |  5 ++++-
>  drivers/mtd/nand/raw/s3c2410.c           | 24 ++++++++++----------
>  drivers/mtd/nand/raw/sh_flctl.c          |  4 ++++
>  drivers/mtd/nand/raw/tango_nand.c        |  4 +++-
>  16 files changed, 140 insertions(+), 107 deletions(-)
> 



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com