[PATCH 1/3] mtd: fsl-quadspi: Fix the error paths

[PATCH 1/3] mtd: fsl-quadspi: Fix the error paths

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Jumping to 'map_failed' label is not correct at these points, as it misses to
disable the clocks that were previously enabled.

Jump to 'irq_failed' label instead that will correctly disable the clocks.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index a46bea3..a733bd2 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -890,24 +890,24 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 
 		ret = of_modalias_node(np, modalias, sizeof(modalias));
 		if (ret < 0)
-			goto map_failed;
+			goto irq_failed;
 
 		ret = of_property_read_u32(np, "spi-max-frequency",
 				&q->clk_rate);
 		if (ret < 0)
-			goto map_failed;
+			goto irq_failed;
 
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
 		ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
 		if (ret)
-			goto map_failed;
+			goto irq_failed;
 
 		ppdata.of_node = np;
 		ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
 		if (ret)
-			goto map_failed;
+			goto irq_failed;
 
 		/* Set the correct NOR size now. */
 		if (q->nor_size == 0) {
-- 
1.9.1

[PATCH 2/3] mtd: fsl-quadspi: Remove unneeded success/error messages

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

When the driver successfully probe we already have messages like:

[    1.140989] fsl-quadspi 21e4000.qspi: s25fl128s (16384 Kbytes)               
[    1.150902] fsl-quadspi 21e4000.qspi: s25fl128s (16384 Kbytes)

Or in case of error:

[    1.175920] fsl-quadspi: probe of 21e4000.qspi failed with error -12

, so remove the unneeded success/error messages.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index a733bd2..1c0572a 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -939,7 +939,6 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 
 	clk_disable(q->clk);
 	clk_disable(q->clk_en);
-	dev_info(dev, "QuadSPI SPI NOR flash driver\n");
 	return 0;
 
 last_init_failed:
@@ -954,7 +953,6 @@ irq_failed:
 clk_failed:
 	clk_disable_unprepare(q->clk_en);
 map_failed:
-	dev_err(dev, "Freescale QuadSPI probe failed\n");
 	return ret;
 }
 
-- 
1.9.1

[PATCH 3/3] mtd: fsl-quadspi: Remove unnecessary 'map_failed' label

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

There is no need to keep the 'map_failed' label. We can simply return the error
code directly and let the code shorter and cleaner.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 1c0572a..b1cc182 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -798,37 +798,30 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	/* find the resources */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "QuadSPI");
 	q->iobase = devm_ioremap_resource(dev, res);
-	if (IS_ERR(q->iobase)) {
-		ret = PTR_ERR(q->iobase);
-		goto map_failed;
-	}
+	if (IS_ERR(q->iobase))
+		return PTR_ERR(q->iobase);
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 					"QuadSPI-memory");
 	q->ahb_base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(q->ahb_base)) {
-		ret = PTR_ERR(q->ahb_base);
-		goto map_failed;
-	}
+	if (IS_ERR(q->ahb_base))
+		return PTR_ERR(q->ahb_base);
+
 	q->memmap_phy = res->start;
 
 	/* find the clocks */
 	q->clk_en = devm_clk_get(dev, "qspi_en");
-	if (IS_ERR(q->clk_en)) {
-		ret = PTR_ERR(q->clk_en);
-		goto map_failed;
-	}
+	if (IS_ERR(q->clk_en))
+		return PTR_ERR(q->clk_en);
 
 	q->clk = devm_clk_get(dev, "qspi");
-	if (IS_ERR(q->clk)) {
-		ret = PTR_ERR(q->clk);
-		goto map_failed;
-	}
+	if (IS_ERR(q->clk))
+		return PTR_ERR(q->clk);
 
 	ret = clk_prepare_enable(q->clk_en);
 	if (ret) {
 		dev_err(dev, "can not enable the qspi_en clock\n");
-		goto map_failed;
+		return ret;
 	}
 
 	ret = clk_prepare_enable(q->clk);
@@ -952,7 +945,6 @@ irq_failed:
 	clk_disable_unprepare(q->clk);
 clk_failed:
 	clk_disable_unprepare(q->clk_en);
-map_failed:
 	return ret;
 }
 
-- 
1.9.1

Re: [PATCH 1/3] mtd: fsl-quadspi: Fix the error paths

[Han Xu]

On Thu, Jan 22, 2015 at 6:43 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Jumping to 'map_failed' label is not correct at these points, as it misses to
> disable the clocks that were previously enabled.
>
> Jump to 'irq_failed' label instead that will correctly disable the clocks.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index a46bea3..a733bd2 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -890,24 +890,24 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>
>                 ret = of_modalias_node(np, modalias, sizeof(modalias));
>                 if (ret < 0)
> -                       goto map_failed;
> +                       goto irq_failed;
>
>                 ret = of_property_read_u32(np, "spi-max-frequency",
>                                 &q->clk_rate);
>                 if (ret < 0)
> -                       goto map_failed;
> +                       goto irq_failed;
>
>                 /* set the chip address for READID */
>                 fsl_qspi_set_base_addr(q, nor);
>
>                 ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
>                 if (ret)
> -                       goto map_failed;
> +                       goto irq_failed;
>
>                 ppdata.of_node = np;
>                 ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
>                 if (ret)
> -                       goto map_failed;
> +                       goto irq_failed;
>
>                 /* Set the correct NOR size now. */
>                 if (q->nor_size == 0) {
> --
> 1.9.1
Acked-by: Han Xu <han.xu@freescale.com>
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: fsl-quadspi: Remove unneeded success/error messages

[Han Xu]

On Thu, Jan 22, 2015 at 6:43 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> When the driver successfully probe we already have messages like:
>
> [    1.140989] fsl-quadspi 21e4000.qspi: s25fl128s (16384 Kbytes)
> [    1.150902] fsl-quadspi 21e4000.qspi: s25fl128s (16384 Kbytes)
>
> Or in case of error:
>
> [    1.175920] fsl-quadspi: probe of 21e4000.qspi failed with error -12
>
> , so remove the unneeded success/error messages.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Acked-by: Han Xu <han.xu@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index a733bd2..1c0572a 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -939,7 +939,6 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>
>         clk_disable(q->clk);
>         clk_disable(q->clk_en);
> -       dev_info(dev, "QuadSPI SPI NOR flash driver\n");
>         return 0;
>
>  last_init_failed:
> @@ -954,7 +953,6 @@ irq_failed:
>  clk_failed:
>         clk_disable_unprepare(q->clk_en);
>  map_failed:
> -       dev_err(dev, "Freescale QuadSPI probe failed\n");
>         return ret;
>  }
>
> --
> 1.9.1
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/3] mtd: fsl-quadspi: Remove unnecessary 'map_failed' label

[Han Xu]

On Thu, Jan 22, 2015 at 6:43 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> There is no need to keep the 'map_failed' label. We can simply return the error
> code directly and let the code shorter and cleaner.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Acked-by: Han Xu <han.xu@freescale.com>

> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 1c0572a..b1cc182 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -798,37 +798,30 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>         /* find the resources */
>         res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "QuadSPI");
>         q->iobase = devm_ioremap_resource(dev, res);
> -       if (IS_ERR(q->iobase)) {
> -               ret = PTR_ERR(q->iobase);
> -               goto map_failed;
> -       }
> +       if (IS_ERR(q->iobase))
> +               return PTR_ERR(q->iobase);
>
>         res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>                                         "QuadSPI-memory");
>         q->ahb_base = devm_ioremap_resource(dev, res);
> -       if (IS_ERR(q->ahb_base)) {
> -               ret = PTR_ERR(q->ahb_base);
> -               goto map_failed;
> -       }
> +       if (IS_ERR(q->ahb_base))
> +               return PTR_ERR(q->ahb_base);
> +
>         q->memmap_phy = res->start;
>
>         /* find the clocks */
>         q->clk_en = devm_clk_get(dev, "qspi_en");
> -       if (IS_ERR(q->clk_en)) {
> -               ret = PTR_ERR(q->clk_en);
> -               goto map_failed;
> -       }
> +       if (IS_ERR(q->clk_en))
> +               return PTR_ERR(q->clk_en);
>
>         q->clk = devm_clk_get(dev, "qspi");
> -       if (IS_ERR(q->clk)) {
> -               ret = PTR_ERR(q->clk);
> -               goto map_failed;
> -       }
> +       if (IS_ERR(q->clk))
> +               return PTR_ERR(q->clk);
>
>         ret = clk_prepare_enable(q->clk_en);
>         if (ret) {
>                 dev_err(dev, "can not enable the qspi_en clock\n");
> -               goto map_failed;
> +               return ret;
>         }
>
>         ret = clk_prepare_enable(q->clk);
> @@ -952,7 +945,6 @@ irq_failed:
>         clk_disable_unprepare(q->clk);
>  clk_failed:
>         clk_disable_unprepare(q->clk_en);
> -map_failed:
>         return ret;
>  }
>
> --
> 1.9.1
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/3] mtd: fsl-quadspi: Fix the error paths

[Brian Norris]

On Thu, Jan 22, 2015 at 10:43:05PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Jumping to 'map_failed' label is not correct at these points, as it misses to
> disable the clocks that were previously enabled.
> 
> Jump to 'irq_failed' label instead that will correctly disable the clocks.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Applied all 3 to l2-mtd.git, thanks.

> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index a46bea3..a733bd2 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -890,24 +890,24 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>  
>  		ret = of_modalias_node(np, modalias, sizeof(modalias));
>  		if (ret < 0)
> -			goto map_failed;
> +			goto irq_failed;
>  
>  		ret = of_property_read_u32(np, "spi-max-frequency",
>  				&q->clk_rate);
>  		if (ret < 0)
> -			goto map_failed;
> +			goto irq_failed;
>  
>  		/* set the chip address for READID */
>  		fsl_qspi_set_base_addr(q, nor);
>  
>  		ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
>  		if (ret)
> -			goto map_failed;
> +			goto irq_failed;
>  
>  		ppdata.of_node = np;
>  		ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
>  		if (ret)
> -			goto map_failed;
> +			goto irq_failed;
>  
>  		/* Set the correct NOR size now. */
>  		if (q->nor_size == 0) {

FYI, the error handling is still wrong here (it's just less wrong). If
the second device fails to scan for some reason, you aren't
unregistering the first. This will leave an MTD around that has no
driver backing it.

Brian