[PATCH 0/6] mmc: Fix error handling for mmc_of_parse()

[PATCH 0/6] mmc: Fix error handling for mmc_of_parse()

[Ulf Hansson]

Some mmc host drivers don't deal with errors from mmc_of_parse(). Patch 1 -> 5
fix these drivers.

Patch 6, make mmc_of_parse() to return errors at failures instead of ignoring
them. This patch indirectly also fixes a bug for the internal error handling in
mmc_of_parse().


Ulf Hansson (6):
  mmc: moxart: Handle error from mmc_of_parse()
  mmc: sdhci-bcm-kona: Handle error from mmc_of_parse()
  mmc: sdhci-of-esdhc: Handle error from mmc_of_parse()
  mmc: sdhci-s3c: Handle error from mmc_of_parse()
  mmc: sdhci-st: Do cleanup while mmc_of_parse() return an error
  mmc: core: Return error at failure of request CD/WP in mmc_of_parse()

 drivers/mmc/core/host.c           | 28 ++++++----------------------
 drivers/mmc/host/moxart-mmc.c     |  4 +++-
 drivers/mmc/host/sdhci-bcm-kona.c |  4 +++-
 drivers/mmc/host/sdhci-of-esdhc.c | 10 ++++++++--
 drivers/mmc/host/sdhci-s3c.c      |  4 +++-
 drivers/mmc/host/sdhci-st.c       |  4 ++--
 6 files changed, 25 insertions(+), 29 deletions(-)

-- 
1.9.1

[PATCH 1/6] mmc: moxart: Handle error from mmc_of_parse()

[Ulf Hansson]

Since mmc_of_parse() may fail, let's deal with it and thus do proper
error handling.

Cc: Jonas Jensen <jonas.jensen@gmail.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/moxart-mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index f3e18d0..d2a1ef6 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -599,7 +599,9 @@ static int moxart_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	mmc_of_parse(mmc);
+	ret = mmc_of_parse(mmc);
+	if (ret)
+		goto out;
 
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
-- 
1.9.1

[PATCH 2/6] mmc: sdhci-bcm-kona: Handle error from mmc_of_parse()

[Ulf Hansson]

Since mmc_of_parse() may fail, let's deal with it and thus do proper
error handling.

Cc: Christian Daudt <bcm@fixthebug.org>
Cc: Matt Porter <mporter@linaro.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/sdhci-bcm-kona.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
index e7e4fbd..34bb8f9 100644
--- a/drivers/mmc/host/sdhci-bcm-kona.c
+++ b/drivers/mmc/host/sdhci-bcm-kona.c
@@ -254,7 +254,9 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
 	kona_dev = sdhci_pltfm_priv(pltfm_priv);
 	mutex_init(&kona_dev->write_lock);
 
-	mmc_of_parse(host->mmc);
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto err_pltfm_free;
 
 	if (!host->mmc->f_max) {
 		dev_err(&pdev->dev, "Missing max-freq for SDHCI cfg\n");
-- 
1.9.1

Re: [PATCH 2/6] mmc: sdhci-bcm-kona: Handle error from mmc_of_parse()

[Florian Fainelli]

2014-12-18 1:41 GMT-08:00 Ulf Hansson <ulf.hansson@linaro.org>:
> Since mmc_of_parse() may fail, let's deal with it and thus do proper
> error handling.
>
> Cc: Christian Daudt <bcm@fixthebug.org>
> Cc: Matt Porter <mporter@linaro.org>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  drivers/mmc/host/sdhci-bcm-kona.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-bcm-kona.c b/drivers/mmc/host/sdhci-bcm-kona.c
> index e7e4fbd..34bb8f9 100644
> --- a/drivers/mmc/host/sdhci-bcm-kona.c
> +++ b/drivers/mmc/host/sdhci-bcm-kona.c
> @@ -254,7 +254,9 @@ static int sdhci_bcm_kona_probe(struct platform_device *pdev)
>         kona_dev = sdhci_pltfm_priv(pltfm_priv);
>         mutex_init(&kona_dev->write_lock);
>
> -       mmc_of_parse(host->mmc);
> +       ret = mmc_of_parse(host->mmc);
> +       if (ret)
> +               goto err_pltfm_free;
>
>         if (!host->mmc->f_max) {
>                 dev_err(&pdev->dev, "Missing max-freq for SDHCI cfg\n");
> --
> 1.9.1
>



-- 
Florian

[PATCH 3/6] mmc: sdhci-of-esdhc: Handle error from mmc_of_parse()

[Ulf Hansson]

Since mmc_of_parse() may fail, let's deal with it and thus do proper
error handling.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/sdhci-of-esdhc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 8872c85..429bf41 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -362,13 +362,19 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 	}
 
 	/* call to generic mmc_of_parse to support additional capabilities */
-	mmc_of_parse(host->mmc);
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto err;
+
 	mmc_of_parse_voltage(np, &host->ocr_mask);
 
 	ret = sdhci_add_host(host);
 	if (ret)
-		sdhci_pltfm_free(pdev);
+		goto err;
 
+	return 0;
+ err:
+	sdhci_pltfm_free(pdev);
 	return ret;
 }
 
-- 
1.9.1

[PATCH 4/6] mmc: sdhci-s3c: Handle error from mmc_of_parse()

[Ulf Hansson]

Since mmc_of_parse() may fail, let's deal with it and thus do proper
error handling.

Cc: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/sdhci-s3c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index c45b893..a282822 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -607,7 +607,9 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_suspend_ignore_children(&pdev->dev, 1);
 
-	mmc_of_parse(host->mmc);
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto err_req_regs;
 
 	ret = sdhci_add_host(host);
 	if (ret) {
-- 
1.9.1

Re: [PATCH 4/6] mmc: sdhci-s3c: Handle error from mmc_of_parse()

[Jaehoon Chung]

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

On 12/18/2014 06:41 PM, Ulf Hansson wrote:
> Since mmc_of_parse() may fail, let's deal with it and thus do proper
> error handling.
> 
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/host/sdhci-s3c.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index c45b893..a282822 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -607,7 +607,9 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  	pm_suspend_ignore_children(&pdev->dev, 1);
>  
> -	mmc_of_parse(host->mmc);
> +	ret = mmc_of_parse(host->mmc);
> +	if (ret)
> +		goto err_req_regs;
>  
>  	ret = sdhci_add_host(host);
>  	if (ret) {
> 

[PATCH 5/6] mmc: sdhci-st: Do cleanup while mmc_of_parse() return an error

[Ulf Hansson]

At errors, balance sdhci_pltfm_init() with sdhci_pltfm_free(), to make
sure all data is freed properly.

Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
Cc: Maxime Coquelin <maxime.coquelin@st.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/sdhci-st.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-st.c b/drivers/mmc/host/sdhci-st.c
index 328f348..882b07e 100644
--- a/drivers/mmc/host/sdhci-st.c
+++ b/drivers/mmc/host/sdhci-st.c
@@ -78,10 +78,9 @@ static int sdhci_st_probe(struct platform_device *pdev)
 	}
 
 	ret = mmc_of_parse(host->mmc);
-
 	if (ret) {
 		dev_err(&pdev->dev, "Failed mmc_of_parse\n");
-		return ret;
+		goto err_of;
 	}
 
 	clk_prepare_enable(clk);
@@ -108,6 +107,7 @@ static int sdhci_st_probe(struct platform_device *pdev)
 
 err_out:
 	clk_disable_unprepare(clk);
+err_of:
 	sdhci_pltfm_free(pdev);
 
 	return ret;
-- 
1.9.1

Re: [PATCH 5/6] mmc: sdhci-st: Do cleanup while mmc_of_parse() return an error

[Maxime Coquelin]

Hi Ulf,

On 12/18/2014 10:41 AM, Ulf Hansson wrote:
> At errors, balance sdhci_pltfm_init() with sdhci_pltfm_free(), to make
> sure all data is freed properly.
>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
> Cc: Maxime Coquelin <maxime.coquelin@st.com>
> Cc: Patrice Chotard <patrice.chotard@st.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>   drivers/mmc/host/sdhci-st.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
>

You can add my:
Acked-by: Maxime Coquelin <maxime.coquelin@st.com>

Thanks!
Maxime

[PATCH 6/6] mmc: core: Return error at failure of request CD/WP in mmc_of_parse()

[Ulf Hansson]

Instead of just printing an error when mmc_of_parse() fails to request
CD/WP GPIO pins, let's propagate all errors, except for -ENOENT.
Earlier only -EPROBE_DEFER was handled correctly.

As a side effect of this change and by relying on host drivers to
handle the errors during ->probe(), we don't need to free any data in
the error path.

This also means we are actually fixing a bug, since we remove the call
to mmc_gpio_free_cd() which wasn't the correct function to invoke to
handle cleanup. Instead that should have been mmc_gpiod_free_cd().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/host.c | 28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 270d58a..45c2dae 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -367,16 +367,10 @@ int mmc_of_parse(struct mmc_host *host)
 
 		ret = mmc_gpiod_request_cd(host, "cd", 0, true,
 					   0, &cd_gpio_invert);
-		if (ret) {
-			if (ret == -EPROBE_DEFER)
-				return ret;
-			if (ret != -ENOENT) {
-				dev_err(host->parent,
-					"Failed to request CD GPIO: %d\n",
-					ret);
-			}
-		} else
+		if (!ret)
 			dev_info(host->parent, "Got CD GPIO\n");
+		else if (ret != -ENOENT)
+			return ret;
 
 		/*
 		 * There are two ways to flag that the CD line is inverted:
@@ -397,16 +391,10 @@ int mmc_of_parse(struct mmc_host *host)
 	ro_cap_invert = of_property_read_bool(np, "wp-inverted");
 
 	ret = mmc_gpiod_request_ro(host, "wp", 0, false, 0, &ro_gpio_invert);
-	if (ret) {
-		if (ret == -EPROBE_DEFER)
-			goto out;
-		if (ret != -ENOENT) {
-			dev_err(host->parent,
-				"Failed to request WP GPIO: %d\n",
-				ret);
-		}
-	} else
+	if (!ret)
 		dev_info(host->parent, "Got WP GPIO\n");
+	else if (ret != -ENOENT)
+		return ret;
 
 	/* See the comment on CD inversion above */
 	if (ro_cap_invert ^ ro_gpio_invert)
@@ -458,10 +446,6 @@ int mmc_of_parse(struct mmc_host *host)
 	}
 
 	return 0;
-
-out:
-	mmc_gpio_free_cd(host);
-	return ret;
 }
 
 EXPORT_SYMBOL(mmc_of_parse);
-- 
1.9.1