2013-10-08 13:52:18

by Fabio Estevam

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

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 <[email protected]>
---
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

2013-10-09 01:41:56

by Shawn Guo

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

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 <[email protected]>
> ---
> 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
>
>