[v2] mmc: sdhci-esdhc-imx: Check the return value from clk_prepare_enable()

Commit Message

Fabio Estevam Oct. 9, 2013, 8:37 p.m. UTC

From: Fabio Estevam <[e-mail]>

clk_prepare_enable() may fail, so let's check its return value and propagate it
in the case of error.

Also, fix the sequence for disabling the clock in the probe error path and
also in the remove funct

Signed-off-by: Fabio Estevam <[e-mail]>
---
Changes since v1:
- Fix the naming scheme of goto labels

 drivers/mmc/host/sdhci-esdhc-imx.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

Comments

Shawn Guo Oct. 12, 2013, 7:38 a.m. UTC | #1

On Wed, Oct 09, 2013 at 05:37:39PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <[e-mail]>
> 
> clk_prepare_enable() may fail, so let's check its return value and propagate it
> in the case of error.
> 
> Also, fix the sequence for disabling the clock in the probe error path and
> also in the remove funct
> 
> Signed-off-by: Fabio Estevam <[e-mail]>

Acked-by: Shawn Guo <[e-mail]>

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Fabio Estevam Oct. 26, 2013, 6:11 p.m. UTC | #2

Chris,

On Sat, Oct 12, 2013 at 4:38 AM, Shawn Guo <[e-mail]> wrote:
> On Wed, Oct 09, 2013 at 05:37:39PM -0300, Fabio Estevam wrote:
>> From: Fabio Estevam <[e-mail]>
>>
>> clk_prepare_enable() may fail, so let's check its return value and propagate it
>> in the case of error.
>>
>> Also, fix the sequence for disabling the clock in the probe error path and
>> also in the remove funct
>>
>> Signed-off-by: Fabio Estevam <[e-mail]>
>
> Acked-by: Shawn Guo <[e-mail]>

Any comments on this one?

Regards,

Fabio Estevam
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Chris Ball Dec. 12, 2013, 1:24 a.m. UTC | #3

Hi Fabio,

On Sat, Oct 26 2013, Fabio Estevam wrote:
> Chris,
>
> On Sat, Oct 12, 2013 at 4:38 AM, Shawn Guo <[e-mail]> wrote:
>> On Wed, Oct 09, 2013 at 05:37:39PM -0300, Fabio Estevam wrote:
>>> From: Fabio Estevam <[e-mail]>
>>>
>>> clk_prepare_enable() may fail, so let's check its return value and
>>> propagate it
>>> in the case of error.
>>>
>>> Also, fix the sequence for disabling the clock in the probe error path and
>>> also in the remove funct
>>>
>>> Signed-off-by: Fabio Estevam <[e-mail]>
>>
>> Acked-by: Shawn Guo <[e-mail]>
>
> Any comments on this one?

Looks like this one doesn't apply cleanly anymore, sorry for leaving
it so long -- please could you submit a new version?  Thanks.

- Chris.

Patch

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index b9899e9..32b0586 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -877,14 +877,22 @@  static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 
 	pltfm_host->clk = imx_data->clk_per;
 
-	clk_prepare_enable(imx_data->clk_per);
-	clk_prepare_enable(imx_data->clk_ipg);
-	clk_prepare_enable(imx_data->clk_ahb);
+	err = clk_prepare_enable(imx_data->clk_per);
+	if (err)
+		goto free_sdhci;
+
+	err = clk_prepare_enable(imx_data->clk_ipg);
+	if (err)
+		goto disable_clk_per;
+
+	err = clk_prepare_enable(imx_data->clk_ahb);
+	if (err)
+		goto disable_clk_ipg;
 
 	imx_data->pinctrl = devm_pinctrl_get(&pdev->dev);
 	if (IS_ERR(imx_data->pinctrl)) {
 		err = PTR_ERR(imx_data->pinctrl);
-		goto disable_clk;
+		goto disable_clk_ahb;
 	}
 
 	imx_data->pins_default = pinctrl_lookup_state(imx_data->pinctrl,
@@ -892,7 +900,7 @@  static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (IS_ERR(imx_data->pins_default)) {
 		err = PTR_ERR(imx_data->pins_default);
 		dev_err(mmc_dev(host->mmc), "could not get default state\n");
-		goto disable_clk;
+		goto disable_clk_ahb;
 	}
 
 	host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
@@ -917,7 +925,7 @@  static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		if (!host->mmc->parent->platform_data) {
 			dev_err(mmc_dev(host->mmc), "no board data!\n");
 			err = -EINVAL;
-			goto disable_clk;
+			goto disable_clk_ahb;
 		}
 		imx_data->boarddata = *((struct esdhc_platform_data *)
 					host->mmc->parent->platform_data);
@@ -929,7 +937,7 @@  static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		if (err) {
 			dev_err(mmc_dev(host->mmc),
 				"failed to request write-protect gpio!\n");
-			goto disable_clk;
+			goto disable_clk_ahb;
 		}
 		host->mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
 	}
@@ -941,7 +949,7 @@  static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		if (err) {
 			dev_err(mmc_dev(host->mmc),
 				"failed to request card-detect gpio!\n");
-			goto disable_clk;
+			goto disable_clk_ahb;
 		}
 		/* fall through */
 
@@ -990,14 +998,16 @@  static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 
 	err = sdhci_add_host(host);
 	if (err)
-		goto disable_clk;
+		goto disable_clk_ahb;
 
 	return 0;
 
-disable_clk:
-	clk_disable_unprepare(imx_data->clk_per);
-	clk_disable_unprepare(imx_data->clk_ipg);
+disable_clk_ahb:
 	clk_disable_unprepare(imx_data->clk_ahb);
+disable_clk_ipg:
+	clk_disable_unprepare(imx_data->clk_ipg);
+disable_clk_per:
+	clk_disable_unprepare(imx_data->clk_per);
 free_sdhci:
 	sdhci_pltfm_free(pdev);
 	return err;
@@ -1012,9 +1022,9 @@  static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
 
 	sdhci_remove_host(host, dead);
 
-	clk_disable_unprepare(imx_data->clk_per);
-	clk_disable_unprepare(imx_data->clk_ipg);
 	clk_disable_unprepare(imx_data->clk_ahb);
+	clk_disable_unprepare(imx_data->clk_ipg);
+	clk_disable_unprepare(imx_data->clk_per);
 
 	sdhci_pltfm_free(pdev);