public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: sdhci-esdhc-imx: Check the return value from clk_prepare_enable()
@ 2013-10-08 13:47 Fabio Estevam
  2013-10-09  1:42 ` Shawn Guo
  0 siblings, 1 reply; 4+ messages in thread
From: Fabio Estevam @ 2013-10-08 13:47 UTC (permalink / raw)
  To: cjb; +Cc: shawn.guo, festevam, linux-mmc, linux-kernel, Fabio Estevam

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 function.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index b9899e9..abc9836 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -877,9 +877,17 @@ 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 err_ipg;
+
+	err = clk_prepare_enable(imx_data->clk_ahb);
+	if (err)
+		goto err_ahb;
 
 	imx_data->pinctrl = devm_pinctrl_get(&pdev->dev);
 	if (IS_ERR(imx_data->pinctrl)) {
@@ -995,9 +1003,11 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	return 0;
 
 disable_clk:
-	clk_disable_unprepare(imx_data->clk_per);
-	clk_disable_unprepare(imx_data->clk_ipg);
 	clk_disable_unprepare(imx_data->clk_ahb);
+err_ahb:
+	clk_disable_unprepare(imx_data->clk_ipg);
+err_ipg:
+	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);
 
-- 
1.8.1.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mmc: sdhci-esdhc-imx: Check the return value from clk_prepare_enable()
  2013-10-08 13:47 Fabio Estevam
@ 2013-10-09  1:42 ` Shawn Guo
  0 siblings, 0 replies; 4+ messages in thread
From: Shawn Guo @ 2013-10-09  1:42 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: cjb, festevam, linux-mmc, linux-kernel

On Tue, Oct 08, 2013 at 10:47:28AM -0300, Fabio Estevam wrote:
> 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 function.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index b9899e9..abc9836 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -877,9 +877,17 @@ 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 err_ipg;
> +
> +	err = clk_prepare_enable(imx_data->clk_ahb);
> +	if (err)
> +		goto err_ahb;
>  
>  	imx_data->pinctrl = devm_pinctrl_get(&pdev->dev);
>  	if (IS_ERR(imx_data->pinctrl)) {
> @@ -995,9 +1003,11 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	return 0;
>  
>  disable_clk:
> -	clk_disable_unprepare(imx_data->clk_per);
> -	clk_disable_unprepare(imx_data->clk_ipg);
>  	clk_disable_unprepare(imx_data->clk_ahb);
> +err_ahb:

Naming schema of the existing labels seems to be 'acting' rather than
'reasoning', so I guess 'disable_ipg:' might fit here better?

Shawn

> +	clk_disable_unprepare(imx_data->clk_ipg);
> +err_ipg:
> +	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);
>  
> -- 
> 1.8.1.2
> 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] mmc: sdhci-esdhc-imx: Check the return value from clk_prepare_enable()
@ 2017-04-28 20:00 Fabio Estevam
  2017-05-22 13:21 ` Fabio Estevam
  0 siblings, 1 reply; 4+ messages in thread
From: Fabio Estevam @ 2017-04-28 20:00 UTC (permalink / raw)
  To: ulf.hansson; +Cc: adrian.hunter, dongas86, linux-mmc, Fabio Estevam

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

clk_prepare_enable() may fail, so we should better check its return value
and propagate it in the case of error.

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 54 +++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 23d8b8a..b5c7241 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1250,14 +1250,20 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 
 	pltfm_host->clk = imx_data->clk_per;
 	pltfm_host->clock = clk_get_rate(pltfm_host->clk);
-	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_per_clk;
+	err = clk_prepare_enable(imx_data->clk_ahb);
+	if (err)
+		goto disable_ipg_clk;
 
 	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_ahb_clk;
 	}
 
 	imx_data->pins_default = pinctrl_lookup_state(imx_data->pinctrl,
@@ -1297,13 +1303,13 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	else
 		err = sdhci_esdhc_imx_probe_nondt(pdev, host, imx_data);
 	if (err)
-		goto disable_clk;
+		goto disable_ahb_clk;
 
 	sdhci_esdhc_imx_hwinit(host);
 
 	err = sdhci_add_host(host);
 	if (err)
-		goto disable_clk;
+		goto disable_ahb_clk;
 
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
@@ -1313,10 +1319,12 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 
 	return 0;
 
-disable_clk:
-	clk_disable_unprepare(imx_data->clk_per);
-	clk_disable_unprepare(imx_data->clk_ipg);
+disable_ahb_clk:
 	clk_disable_unprepare(imx_data->clk_ahb);
+disable_ipg_clk:
+	clk_disable_unprepare(imx_data->clk_ipg);
+disable_per_clk:
+	clk_disable_unprepare(imx_data->clk_per);
 free_sdhci:
 	sdhci_pltfm_free(pdev);
 	return err;
@@ -1393,14 +1401,34 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
+	int err;
 
 	if (!sdhci_sdio_irq_enabled(host)) {
-		clk_prepare_enable(imx_data->clk_per);
-		clk_prepare_enable(imx_data->clk_ipg);
+		err = clk_prepare_enable(imx_data->clk_per);
+		if (err)
+			return err;
+		err = clk_prepare_enable(imx_data->clk_ipg);
+		if (err)
+			goto disable_per_clk;
 	}
-	clk_prepare_enable(imx_data->clk_ahb);
+	err = clk_prepare_enable(imx_data->clk_ahb);
+	if (err)
+		goto disable_ipg_clk;
+	err = sdhci_runtime_resume_host(host);
+	if (err)
+		goto disable_ahb_clk;
+
+	return 0;
 
-	return sdhci_runtime_resume_host(host);
+disable_ahb_clk:
+	clk_disable_unprepare(imx_data->clk_ahb);
+disable_ipg_clk:
+	if (!sdhci_sdio_irq_enabled(host))
+		clk_disable_unprepare(imx_data->clk_ipg);
+disable_per_clk:
+	if (!sdhci_sdio_irq_enabled(host))
+		clk_disable_unprepare(imx_data->clk_per);
+	return err;
 }
 #endif
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mmc: sdhci-esdhc-imx: Check the return value from clk_prepare_enable()
  2017-04-28 20:00 [PATCH] mmc: sdhci-esdhc-imx: Check the return value from clk_prepare_enable() Fabio Estevam
@ 2017-05-22 13:21 ` Fabio Estevam
  0 siblings, 0 replies; 4+ messages in thread
From: Fabio Estevam @ 2017-05-22 13:21 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Adrian Hunter, Dong Aisheng, linux-mmc@vger.kernel.org,
	Fabio Estevam

Hi Ulf,

On Fri, Apr 28, 2017 at 5:00 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> clk_prepare_enable() may fail, so we should better check its return value
> and propagate it in the case of error.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>

A gentle ping.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-05-22 13:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-28 20:00 [PATCH] mmc: sdhci-esdhc-imx: Check the return value from clk_prepare_enable() Fabio Estevam
2017-05-22 13:21 ` Fabio Estevam
  -- strict thread matches above, loose matches on Subject: below --
2013-10-08 13:47 Fabio Estevam
2013-10-09  1:42 ` Shawn Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox