mmc: sdhci-pxav3: fix error handling of sdhci_add_host

PATCH In review Xiang Wang 4 participants 6 messages Jul 16, 2014 – Sep 10, 2014

Subsystems (2)

MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver

lore

Share & tools

lore

Xiang Wang

author

2014-07-16 07:50 UTC #0 lore raw

PATCH 1/1 mmc: sdhci-pxav3: fix error handling of sdhci_add_host

From: Xiang Wang <[email protected]>

Commit 0dcaa2499b7d111bd70da5b0976c34210c850fb3 improved error handling of sdhci_add_host. However, "err_of_parse" and "err_cd_req" should be placed after "pm_runtime_disable(&pdev->dev)".

Signed-off-by Xiang Wang <[email protected]>

---
 drivers/mmc/host/sdhci-pxav3.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/mmc/host/sdhci-pxav3.c +2 -2

index 2fd73b3..0644655 100644
@@ -380,11 +380,11 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_of_parse:
-err_cd_req:
 err_add_host:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+err_of_parse:
+err_cd_req:
 	clk_disable_unprepare(clk);
 	clk_put(clk);
 err_clk_get:

Signature · 1 line

1.7.5.4

Laurent Pinchart

Acked-by on v1 1/1

2014-07-16 09:53 UTC #1 lore raw

Hi Xiang,

Thank you for the patch.

Quoted 7 lines · On Wednesday 16 July 2014 15:50:09 Xiang Wang wrote:

From: Xiang Wang <[email protected]>
Commit 0dcaa2499b7d111bd70da5b0976c34210c850fb3 improved error handling of sdhci_add_host. However, "err_of_parse" and "err_cd_req" should be placed after "pm_runtime_disable(&pdev->dev)".
Signed-off-by Xiang Wang <[email protected]>

Acked-by Laurent Pinchart <[email protected]>

While we're touching the probe error path, how about converting the driver to the devm_* helpers ? I'll send a patch shortly.

Quoted 23 lines

---
 drivers/mmc/host/sdhci-pxav3.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/mmc/host/sdhci-pxav3.c +2 -2

index 2fd73b3..0644655 100644
@@ -380,11 +380,11 @@ static int sdhci_pxav3_probe(struct platform_device
 *pdev)
 
 	return 0;
 
-err_of_parse:
-err_cd_req:
 err_add_host:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+err_of_parse:
+err_cd_req:
 	clk_disable_unprepare(clk);
 	clk_put(clk);

err_clk_get:

Signature · 3 lines

Regards,

Laurent Pinchart

Laurent Pinchart

2014-09-10 13:24 UTC #2 lore raw

Quoted 10 lines · On Wednesday 16 July 2014 11:53:21 Laurent Pinchart wrote:

Quoted 7 lines · On Wednesday 16 July 2014 15:50:09 Xiang Wang wrote:
From: Xiang Wang <[email protected]>
Commit 0dcaa2499b7d111bd70da5b0976c34210c850fb3 improved error handling of sdhci_add_host. However, "err_of_parse" and "err_cd_req" should be placed after "pm_runtime_disable(&pdev->dev)".
Signed-off-by Xiang Wang <[email protected]>
Acked-by Laurent Pinchart <[email protected]>

I can't find the patch in mainline, has it been lost somewhere ?

Quoted 28 lines

---

 drivers/mmc/host/sdhci-pxav3.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/drivers/mmc/host/sdhci-pxav3.c index 2fd73b3..0644655 100644

drivers/mmc/host/sdhci-pxav3.c +2 -2

@@ -380,11 +380,11 @@ static int sdhci_pxav3_probe(struct platform_device
 *pdev)
 
 	return 0;
 
-err_of_parse:
 
-err_cd_req:
 err_add_host:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+err_of_parse:
 
+err_cd_req:
 	clk_disable_unprepare(clk);
 	clk_put(clk);
 
 err_clk_get:

Signature · 3 lines

Regards,

Laurent Pinchart

Ulf Hansson

2014-09-10 13:56 UTC #3 lore raw

Quoted 8 lines · Laurent Pinchart

Quoted 7 lines · On Wednesday 16 July 2014 11:53:21 Laurent Pinchart wrote:
Quoted 6 lines · On Wednesday 16 July 2014 15:50:09 Xiang Wang wrote:
From: Xiang Wang <[email protected]>
Commit 0dcaa2499b7d111bd70da5b0976c34210c850fb3 improved error handling of sdhci_add_host. However, "err_of_parse" and "err_cd_req" should be placed after "pm_runtime_disable(&pdev->dev)".

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
Signed-off-by Xiang Wang <[email protected]>
Acked-by Laurent Pinchart <[email protected]>
I can't find the patch in mainline, has it been lost somewhere ?

It was lost, but now found, resolved a conflict for it and queued for 3.18.

Thanks for the reminder!

Kind regards
Uffe

Laurent Pinchart

2014-07-16 09:53 UTC #4 lore raw

PATCH 1/1 mmc: sdhci-pxav3: Use devm_* managed helpers

In reply to Xiang Wang

This simplifies probe error and remove code paths.

Signed-off-by Laurent Pinchart <[email protected]>

---
 drivers/mmc/host/sdhci-pxav3.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

This patch applies on top of Xiang Wang's " mmc: sdhci-pxav3: fix error handling of sdhci_add_host" patch.

drivers/mmc/host/sdhci-pxav3.c +3 -9

index 19b7c8a..e90a188 100644
@@ -288,15 +288,13 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
 	int ret;
 	struct clk *clk;
 
-	pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
+	pxa = devm_kzalloc(&pdev->dev, sizeof(struct sdhci_pxa), GFP_KERNEL);
 	if (!pxa)
 		return -ENOMEM;
 
 	host = sdhci_pltfm_init(pdev, &sdhci_pxav3_pdata, 0);
-	if (IS_ERR(host)) {
-		kfree(pxa);
+	if (IS_ERR(host))
 		return PTR_ERR(host);
-	}
 
 	if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
 		ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());

@@ -308,7 +306,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
 	pltfm_host = sdhci_priv(host);
 	pltfm_host->priv = pxa;
 
-	clk = clk_get(dev, NULL);
+	clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(dev, "failed to get io clock\n");
 		ret = PTR_ERR(clk);

@@ -389,11 +387,9 @@ err_add_host:
 err_of_parse:
 err_cd_req:
 	clk_disable_unprepare(clk);
-	clk_put(clk);
 err_clk_get:
 err_mbus_win:
 	sdhci_pltfm_free(pdev);
-	kfree(pxa);
 	return ret;
 }

@@ -408,10 +404,8 @@ static int sdhci_pxav3_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 
 	clk_disable_unprepare(pltfm_host->clk);
-	clk_put(pltfm_host->clk);
 
 	sdhci_pltfm_free(pdev);
-	kfree(pxa);
 
 	return 0;
 }

Signature · 3 lines

Regards,

Laurent Pinchart

Ulf Hansson

Applied

2014-07-26 09:47 UTC #5 lore raw

Re: [PATCH] mmc: sdhci-pxav3: Use devm_* managed helpers

Quoted 3 lines · Laurent Pinchart

This simplifies probe error and remove code paths.
Signed-off-by Laurent Pinchart <[email protected]>

Thanks! Applied for next.

Kind regards
Uffe

Quoted 70 lines

---
 drivers/mmc/host/sdhci-pxav3.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

This patch applies on top of Xiang Wang's " mmc: sdhci-pxav3: fix error handling of sdhci_add_host" patch.

drivers/mmc/host/sdhci-pxav3.c +3 ~~-10~~

index 19b7c8a..e90a188 100644
@@ -288,15 +288,13 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
        int ret;
        struct clk *clk;
 
-       pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
+       pxa = devm_kzalloc(&pdev->dev, sizeof(struct sdhci_pxa), GFP_KERNEL);
        if (!pxa)
                return -ENOMEM;
 
        host = sdhci_pltfm_init(pdev, &sdhci_pxav3_pdata, 0);
-       if (IS_ERR(host)) {
-               kfree(pxa);
+       if (IS_ERR(host))
                return PTR_ERR(host);
-       }
 
        if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
                ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());

@@ -308,7 +306,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
        pltfm_host = sdhci_priv(host);
        pltfm_host->priv = pxa;
 
-       clk = clk_get(dev, NULL);
+       clk = devm_clk_get(dev, NULL);
        if (IS_ERR(clk)) {
                dev_err(dev, "failed to get io clock\n");
                ret = PTR_ERR(clk);

@@ -389,11 +387,9 @@ err_add_host:
 err_of_parse:
 err_cd_req:
        clk_disable_unprepare(clk);
-       clk_put(clk);
 err_clk_get:
 err_mbus_win:
        sdhci_pltfm_free(pdev);
-       kfree(pxa);
        return ret;
 }

@@ -408,10 +404,8 @@ static int sdhci_pxav3_remove(struct platform_device *pdev)
        pm_runtime_disable(&pdev->dev);
 
        clk_disable_unprepare(pltfm_host->clk);
-       clk_put(pltfm_host->clk);
 
        sdhci_pltfm_free(pdev);
-       kfree(pxa);
 
        return 0;
 }
--

Regards,

Laurent Pinchart

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