linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] MMC: fix sdhci-dove removal
@ 2012-10-15  9:43 Russell King - ARM Linux
  2012-10-15 10:37 ` Russell King - ARM Linux
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2012-10-15  9:43 UTC (permalink / raw)
  To: linux-arm-kernel

1. Unregister the device _BEFORE_ taking away any resources it may
   be using.
2. Don't check clks against NULL.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
I noticed this while merging v3.6 into Rabeeh's cubox kernel.

 drivers/mmc/host/sdhci-dove.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index 90140eb..c11db64 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -117,14 +117,13 @@ static int __devexit sdhci_dove_remove(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_dove_priv *priv = pltfm_host->priv;
 
-	if (priv->clk) {
-		if (!IS_ERR(priv->clk)) {
-			clk_disable_unprepare(priv->clk);
-			clk_put(priv->clk);
-		}
-		devm_kfree(&pdev->dev, priv->clk);
+	sdhci_pltfm_unregister(pdev);
+
+	if (!IS_ERR(priv->clk)) {
+		clk_disable_unprepare(priv->clk);
+		clk_put(priv->clk);
 	}
-	return sdhci_pltfm_unregister(pdev);
+	return 0;
 }
 
 static const struct of_device_id sdhci_dove_of_match_table[] __devinitdata = {

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-15  9:43 [PATCH] MMC: fix sdhci-dove removal Russell King - ARM Linux
@ 2012-10-15 10:37 ` Russell King - ARM Linux
  2012-10-15 10:44   ` Russell King - ARM Linux
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2012-10-15 10:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 15, 2012 at 10:43:48AM +0100, Russell King - ARM Linux wrote:
> 1. Unregister the device _BEFORE_ taking away any resources it may
>    be using.
> 2. Don't check clks against NULL.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Looking at this driver some more, who the hell came up with the sdhci
registration interface?  It violates one of the most fundamental
principles of kernel driver programming.  You do _NOT_ publish your
driver interfaces _UNTIL_ you have finished setting your device up.
Otherwise, in a preemptible or SMP kernel, your driver can be used
before the initialization has completed.

As this driver calls sdhci_pltfm_register() before it has obtained the
clock for the interface, and this function does:
	sdhci_pltfm_init
	sdhci_add_host
		mmc_add_host
			mmc_start_host
				mmc_power_up
					mmc_set_ios
						sdhci_set_ios

See, we're trying to power up and clock the card _before_ the dove
sdhci driver has even claimed the clock let alone enabled it.  This
is total bollocks.  The sdhci platform interface is total crap for
creating this broken design in the first place.  This is why MMC has
the init + add interfaces, they're there to allow drivers to do stuff
the Right(tm) way and avoid shit like the above.

This should have been picked up at review time before the driver went
into mainline.  In any case, it needs to be fixed.

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-15 10:37 ` Russell King - ARM Linux
@ 2012-10-15 10:44   ` Russell King - ARM Linux
  2012-10-15 14:58     ` Jean-Christophe PLAGNIOL-VILLARD
  2012-10-29 21:10     ` Chris Ball
  0 siblings, 2 replies; 10+ messages in thread
From: Russell King - ARM Linux @ 2012-10-15 10:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 15, 2012 at 11:37:25AM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 15, 2012 at 10:43:48AM +0100, Russell King - ARM Linux wrote:
> > 1. Unregister the device _BEFORE_ taking away any resources it may
> >    be using.
> > 2. Don't check clks against NULL.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> Looking at this driver some more, who the hell came up with the sdhci
> registration interface?  It violates one of the most fundamental
> principles of kernel driver programming.  You do _NOT_ publish your
> driver interfaces _UNTIL_ you have finished setting your device up.
> Otherwise, in a preemptible or SMP kernel, your driver can be used
> before the initialization has completed.
> 
> As this driver calls sdhci_pltfm_register() before it has obtained the
> clock for the interface, and this function does:
> 	sdhci_pltfm_init
> 	sdhci_add_host
> 		mmc_add_host
> 			mmc_start_host
> 				mmc_power_up
> 					mmc_set_ios
> 						sdhci_set_ios
> 
> See, we're trying to power up and clock the card _before_ the dove
> sdhci driver has even claimed the clock let alone enabled it.  This
> is total bollocks.  The sdhci platform interface is total crap for
> creating this broken design in the first place.  This is why MMC has
> the init + add interfaces, they're there to allow drivers to do stuff
> the Right(tm) way and avoid shit like the above.
> 
> This should have been picked up at review time before the driver went
> into mainline.  In any case, it needs to be fixed.

Here's an updated patch which just about fixes the sdhci-dove driver.
I would not be surprised given the idiotic sdhci-pltfm API if many
other drivers suffered the same bug.

8<====
From: Russell King <rmk+kernel@arm.linux.org.uk>
Subject: [PATCH] MMC: fix sdhci-dove probe/removal

1. Never ever publish a device in the system before it has been setup
   to a usable state.
2. Unregister the device _BEFORE_ taking away any resources it may be
   using.
3. Don't check clks against NULL.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/mmc/host/sdhci-dove.c |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index a6e53a1..7d3a4e4 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -83,30 +83,31 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev)
 	struct sdhci_dove_priv *priv;
 	int ret;
 
-	ret = sdhci_pltfm_register(pdev, &sdhci_dove_pdata);
-	if (ret)
-		goto sdhci_dove_register_fail;
-
 	priv = devm_kzalloc(&pdev->dev, sizeof(struct sdhci_dove_priv),
 			    GFP_KERNEL);
 	if (!priv) {
 		dev_err(&pdev->dev, "unable to allocate private data");
-		ret = -ENOMEM;
-		goto sdhci_dove_allocate_fail;
+		return -ENOMEM;
 	}
 
+	priv->clk = clk_get(&pdev->dev, NULL);
+	if (!IS_ERR(priv->clk))
+		clk_prepare_enable(priv->clk);
+
+	ret = sdhci_pltfm_register(pdev, &sdhci_dove_pdata);
+	if (ret)
+		goto sdhci_dove_register_fail;
+
 	host = platform_get_drvdata(pdev);
 	pltfm_host = sdhci_priv(host);
 	pltfm_host->priv = priv;
 
-	priv->clk = clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(priv->clk))
-		clk_prepare_enable(priv->clk);
 	return 0;
 
-sdhci_dove_allocate_fail:
-	sdhci_pltfm_unregister(pdev);
 sdhci_dove_register_fail:
+	clk_unprepare_disable(priv->clk);
+	clk_put(priv->clk);
+sdhci_dove_allocate_fail:
 	return ret;
 }
 
@@ -116,14 +117,13 @@ static int __devexit sdhci_dove_remove(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_dove_priv *priv = pltfm_host->priv;
 
-	if (priv->clk) {
-		if (!IS_ERR(priv->clk)) {
-			clk_disable_unprepare(priv->clk);
-			clk_put(priv->clk);
-		}
-		devm_kfree(&pdev->dev, priv->clk);
+	sdhci_pltfm_unregister(pdev);
+
+	if (!IS_ERR(priv->clk)) {
+		clk_disable_unprepare(priv->clk);
+		clk_put(priv->clk);
 	}
-	return sdhci_pltfm_unregister(pdev);
+	return 0;
 }
 
 static struct platform_driver sdhci_dove_driver = {
-- 
1.7.4.4

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-15 10:44   ` Russell King - ARM Linux
@ 2012-10-15 14:58     ` Jean-Christophe PLAGNIOL-VILLARD
  2012-10-15 15:07       ` Russell King - ARM Linux
  2012-10-29 21:10     ` Chris Ball
  1 sibling, 1 reply; 10+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-15 14:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 11:44 Mon 15 Oct     , Russell King - ARM Linux wrote:
> On Mon, Oct 15, 2012 at 11:37:25AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Oct 15, 2012 at 10:43:48AM +0100, Russell King - ARM Linux wrote:
> > > 1. Unregister the device _BEFORE_ taking away any resources it may
> > >    be using.
> > > 2. Don't check clks against NULL.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > 
> > Looking at this driver some more, who the hell came up with the sdhci
> > registration interface?  It violates one of the most fundamental
> > principles of kernel driver programming.  You do _NOT_ publish your
> > driver interfaces _UNTIL_ you have finished setting your device up.
> > Otherwise, in a preemptible or SMP kernel, your driver can be used
> > before the initialization has completed.
> > 
> > As this driver calls sdhci_pltfm_register() before it has obtained the
> > clock for the interface, and this function does:
> > 	sdhci_pltfm_init
> > 	sdhci_add_host
> > 		mmc_add_host
> > 			mmc_start_host
> > 				mmc_power_up
> > 					mmc_set_ios
> > 						sdhci_set_ios
> > 
> > See, we're trying to power up and clock the card _before_ the dove
> > sdhci driver has even claimed the clock let alone enabled it.  This
> > is total bollocks.  The sdhci platform interface is total crap for
> > creating this broken design in the first place.  This is why MMC has
> > the init + add interfaces, they're there to allow drivers to do stuff
> > the Right(tm) way and avoid shit like the above.
> > 
> > This should have been picked up at review time before the driver went
> > into mainline.  In any case, it needs to be fixed.
> 
> Here's an updated patch which just about fixes the sdhci-dove driver.
> I would not be surprised given the idiotic sdhci-pltfm API if many
> other drivers suffered the same bug.
> 
> 8<====
> From: Russell King <rmk+kernel@arm.linux.org.uk>
> Subject: [PATCH] MMC: fix sdhci-dove probe/removal
> 
> 1. Never ever publish a device in the system before it has been setup
>    to a usable state.
> 2. Unregister the device _BEFORE_ taking away any resources it may be
>    using.
> 3. Don't check clks against NULL.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  drivers/mmc/host/sdhci-dove.c |   36 ++++++++++++++++++------------------
>  1 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
> index a6e53a1..7d3a4e4 100644
> --- a/drivers/mmc/host/sdhci-dove.c
> +++ b/drivers/mmc/host/sdhci-dove.c
> @@ -83,30 +83,31 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev)
>  	struct sdhci_dove_priv *priv;
>  	int ret;
>  
> -	ret = sdhci_pltfm_register(pdev, &sdhci_dove_pdata);
> -	if (ret)
> -		goto sdhci_dove_register_fail;
> -
>  	priv = devm_kzalloc(&pdev->dev, sizeof(struct sdhci_dove_priv),
>  			    GFP_KERNEL);
>  	if (!priv) {
>  		dev_err(&pdev->dev, "unable to allocate private data");
> -		ret = -ENOMEM;
> -		goto sdhci_dove_allocate_fail;
> +		return -ENOMEM;
>  	}
>  
> +	priv->clk = clk_get(&pdev->dev, NULL);
you have devm_clk_get too

maybe you could use it here too

Best Regards,
J.

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-15 14:58     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-10-15 15:07       ` Russell King - ARM Linux
  0 siblings, 0 replies; 10+ messages in thread
From: Russell King - ARM Linux @ 2012-10-15 15:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 15, 2012 at 04:58:55PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:44 Mon 15 Oct     , Russell King - ARM Linux wrote:
> > On Mon, Oct 15, 2012 at 11:37:25AM +0100, Russell King - ARM Linux wrote:
> > > On Mon, Oct 15, 2012 at 10:43:48AM +0100, Russell King - ARM Linux wrote:
> > > > 1. Unregister the device _BEFORE_ taking away any resources it may
> > > >    be using.
> > > > 2. Don't check clks against NULL.
> > > > 
> > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > 
> > > Looking at this driver some more, who the hell came up with the sdhci
> > > registration interface?  It violates one of the most fundamental
> > > principles of kernel driver programming.  You do _NOT_ publish your
> > > driver interfaces _UNTIL_ you have finished setting your device up.
> > > Otherwise, in a preemptible or SMP kernel, your driver can be used
> > > before the initialization has completed.
> > > 
> > > As this driver calls sdhci_pltfm_register() before it has obtained the
> > > clock for the interface, and this function does:
> > > 	sdhci_pltfm_init
> > > 	sdhci_add_host
> > > 		mmc_add_host
> > > 			mmc_start_host
> > > 				mmc_power_up
> > > 					mmc_set_ios
> > > 						sdhci_set_ios
> > > 
> > > See, we're trying to power up and clock the card _before_ the dove
> > > sdhci driver has even claimed the clock let alone enabled it.  This
> > > is total bollocks.  The sdhci platform interface is total crap for
> > > creating this broken design in the first place.  This is why MMC has
> > > the init + add interfaces, they're there to allow drivers to do stuff
> > > the Right(tm) way and avoid shit like the above.
> > > 
> > > This should have been picked up at review time before the driver went
> > > into mainline.  In any case, it needs to be fixed.
> > 
> > Here's an updated patch which just about fixes the sdhci-dove driver.
> > I would not be surprised given the idiotic sdhci-pltfm API if many
> > other drivers suffered the same bug.
> > 
> > 8<====
> > From: Russell King <rmk+kernel@arm.linux.org.uk>
> > Subject: [PATCH] MMC: fix sdhci-dove probe/removal
> > 
> > 1. Never ever publish a device in the system before it has been setup
> >    to a usable state.
> > 2. Unregister the device _BEFORE_ taking away any resources it may be
> >    using.
> > 3. Don't check clks against NULL.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> >  drivers/mmc/host/sdhci-dove.c |   36 ++++++++++++++++++------------------
> >  1 files changed, 18 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
> > index a6e53a1..7d3a4e4 100644
> > --- a/drivers/mmc/host/sdhci-dove.c
> > +++ b/drivers/mmc/host/sdhci-dove.c
> > @@ -83,30 +83,31 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev)
> >  	struct sdhci_dove_priv *priv;
> >  	int ret;
> >  
> > -	ret = sdhci_pltfm_register(pdev, &sdhci_dove_pdata);
> > -	if (ret)
> > -		goto sdhci_dove_register_fail;
> > -
> >  	priv = devm_kzalloc(&pdev->dev, sizeof(struct sdhci_dove_priv),
> >  			    GFP_KERNEL);
> >  	if (!priv) {
> >  		dev_err(&pdev->dev, "unable to allocate private data");
> > -		ret = -ENOMEM;
> > -		goto sdhci_dove_allocate_fail;
> > +		return -ENOMEM;
> >  	}
> >  
> > +	priv->clk = clk_get(&pdev->dev, NULL);
> you have devm_clk_get too
> 
> maybe you could use it here too

This isn't a cleanup patch, this is a patch just to fix some stupid bugs
in this code.  If someone wants to convert that afterwards, it should be
an entirely separate patch.

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-15 10:44   ` Russell King - ARM Linux
  2012-10-15 14:58     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-10-29 21:10     ` Chris Ball
  2012-10-29 21:43       ` Russell King - ARM Linux
  1 sibling, 1 reply; 10+ messages in thread
From: Chris Ball @ 2012-10-29 21:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

On Mon, Oct 15 2012, Russell King - ARM Linux wrote:
> Here's an updated patch which just about fixes the sdhci-dove driver.
> I would not be surprised given the idiotic sdhci-pltfm API if many
> other drivers suffered the same bug.
>
> 8<====
> From: Russell King <rmk+kernel@arm.linux.org.uk>
> Subject: [PATCH] MMC: fix sdhci-dove probe/removal
>
> 1. Never ever publish a device in the system before it has been setup
>    to a usable state.
> 2. Unregister the device _BEFORE_ taking away any resources it may be
>    using.
> 3. Don't check clks against NULL.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

This version of the patch doesn't apply cleanly or compile -- maybe you
have a later revision?

/home/cjb/git/mmc/drivers/mmc/host/sdhci-dove.c: In function ?sdhci_dove_probe?:
/home/cjb/git/mmc/drivers/mmc/host/sdhci-dove.c:109:2: error: implicit declaration of function ?clk_unprepare_disable? [-Werror=implicit-function-declaration]
/home/cjb/git/mmc/drivers/mmc/host/sdhci-dove.c:111:1: warning: label ?sdhci_dove_allocate_fail? defined but not used [-Wunused-label]

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-29 21:10     ` Chris Ball
@ 2012-10-29 21:43       ` Russell King - ARM Linux
  2012-10-29 21:49         ` Chris Ball
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2012-10-29 21:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 29, 2012 at 05:10:37PM -0400, Chris Ball wrote:
> Hi Russell,
> 
> On Mon, Oct 15 2012, Russell King - ARM Linux wrote:
> > Here's an updated patch which just about fixes the sdhci-dove driver.
> > I would not be surprised given the idiotic sdhci-pltfm API if many
> > other drivers suffered the same bug.
> >
> > 8<====
> > From: Russell King <rmk+kernel@arm.linux.org.uk>
> > Subject: [PATCH] MMC: fix sdhci-dove probe/removal
> >
> > 1. Never ever publish a device in the system before it has been setup
> >    to a usable state.
> > 2. Unregister the device _BEFORE_ taking away any resources it may be
> >    using.
> > 3. Don't check clks against NULL.
> >
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> This version of the patch doesn't apply cleanly or compile -- maybe you
> have a later revision?

Well, I'm only going to send a patch against 3.6 because that's the
kernel I'm running on the cubox - which is the platform I'm finding
these bugs on.  As it's a platform which I want to keep stable for
other work, it's not getting -rc kernels on it.

It's also been through several revisions across conflict resolutions
which is why the above errors occurred (thanks to having to fix them
up multiple times.)  And to top it off, there's additional patches
below this one too, so separating out just the fix is virtually
impossible to do safely.

Anyway, here's a replacement patch, updated against my cubox tree but
lacking the cubox gpio changes for it.  Un-tested through: (a) I can't
test this on its own on the cubox, (b) I don't have a separate dove
tree to test it against and my disk space is all but out at the moment
for yet another build tree...

It would helpful of course to have _more_ of the cubox stuff in mainline
but unfortunately it's all non-DT so wouldn't be accepted.

 drivers/mmc/host/sdhci-dove.c |   38 ++++++++++++++++++++------------------
 1 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c
index a6e53a1..bb3cefe 100644
--- a/drivers/mmc/host/sdhci-dove.c
+++ b/drivers/mmc/host/sdhci-dove.c
@@ -19,6 +19,7 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#include <linux/err.h>
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/err.h>
@@ -83,30 +84,32 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev)
 	struct sdhci_dove_priv *priv;
 	int ret;
 
-	ret = sdhci_pltfm_register(pdev, &sdhci_dove_pdata);
-	if (ret)
-		goto sdhci_dove_register_fail;
-
 	priv = devm_kzalloc(&pdev->dev, sizeof(struct sdhci_dove_priv),
 			    GFP_KERNEL);
 	if (!priv) {
 		dev_err(&pdev->dev, "unable to allocate private data");
-		ret = -ENOMEM;
-		goto sdhci_dove_allocate_fail;
+		return -ENOMEM;
 	}
 
+	priv->clk = clk_get(&pdev->dev, NULL);
+	if (!IS_ERR(priv->clk))
+		clk_prepare_enable(priv->clk);
+
+	ret = sdhci_pltfm_register(pdev, &sdhci_dove_pdata);
+	if (ret)
+		goto sdhci_dove_register_fail;
+
 	host = platform_get_drvdata(pdev);
 	pltfm_host = sdhci_priv(host);
 	pltfm_host->priv = priv;
 
-	priv->clk = clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(priv->clk))
-		clk_prepare_enable(priv->clk);
 	return 0;
 
-sdhci_dove_allocate_fail:
-	sdhci_pltfm_unregister(pdev);
 sdhci_dove_register_fail:
+	if (!IS_ERR(priv->clk)) {
+		clk_disable_unprepare(priv->clk);
+		clk_put(priv->clk);
+	}
 	return ret;
 }
 
@@ -116,14 +119,13 @@ static int __devexit sdhci_dove_remove(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_dove_priv *priv = pltfm_host->priv;
 
-	if (priv->clk) {
-		if (!IS_ERR(priv->clk)) {
-			clk_disable_unprepare(priv->clk);
-			clk_put(priv->clk);
-		}
-		devm_kfree(&pdev->dev, priv->clk);
+	sdhci_pltfm_unregister(pdev);
+
+	if (!IS_ERR(priv->clk)) {
+		clk_disable_unprepare(priv->clk);
+		clk_put(priv->clk);
 	}
-	return sdhci_pltfm_unregister(pdev);
+	return 0;
 }
 
 static struct platform_driver sdhci_dove_driver = {

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-29 21:43       ` Russell King - ARM Linux
@ 2012-10-29 21:49         ` Chris Ball
  2012-10-29 22:16           ` Sebastian Hesselbarth
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Ball @ 2012-10-29 21:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Oct 29 2012, Russell King - ARM Linux wrote:
> Anyway, here's a replacement patch, updated against my cubox tree but
> lacking the cubox gpio changes for it.  Un-tested through: (a) I can't
> test this on its own on the cubox, (b) I don't have a separate dove
> tree to test it against and my disk space is all but out at the moment
> for yet another build tree...

Compile-tested and pushed to mmc-next, and I'll submit it for 3.7 if
I get a Tested-by from someone like Sebastian (CC'd), or for 3.8 if I
don't.  (I'll add sdhci-pltfm API rework to my TODO list, too.)

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-29 21:49         ` Chris Ball
@ 2012-10-29 22:16           ` Sebastian Hesselbarth
  2012-10-29 22:20             ` Chris Ball
  0 siblings, 1 reply; 10+ messages in thread
From: Sebastian Hesselbarth @ 2012-10-29 22:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/29/2012 10:49 PM, Chris Ball wrote:
> On Mon, Oct 29 2012, Russell King - ARM Linux wrote:
>> Anyway, here's a replacement patch, updated against my cubox tree but
>> lacking the cubox gpio changes for it.  Un-tested through: (a) I can't
>> test this on its own on the cubox, (b) I don't have a separate dove
>> tree to test it against and my disk space is all but out at the moment
>> for yet another build tree...
>
> Compile-tested and pushed to mmc-next, and I'll submit it for 3.7 if
> I get a Tested-by from someone like Sebastian (CC'd), or for 3.8 if I
> don't.  (I'll add sdhci-pltfm API rework to my TODO list, too.)

Chris, Russell,

obviously I am not the one to ask for a Tested-by because it was my
patch that originally caused all the trouble. My apologies for this!

Anyway, I did test the patch on 3.7-rc3 and except that you need a
3-way git am because of a one-line offset, sdhci-dove runs just fine.

Tested-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

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

* [PATCH] MMC: fix sdhci-dove removal
  2012-10-29 22:16           ` Sebastian Hesselbarth
@ 2012-10-29 22:20             ` Chris Ball
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Ball @ 2012-10-29 22:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Mon, Oct 29 2012, Sebastian Hesselbarth wrote:
> On 10/29/2012 10:49 PM, Chris Ball wrote:
>> On Mon, Oct 29 2012, Russell King - ARM Linux wrote:
>>> Anyway, here's a replacement patch, updated against my cubox tree but
>>> lacking the cubox gpio changes for it.  Un-tested through: (a) I can't
>>> test this on its own on the cubox, (b) I don't have a separate dove
>>> tree to test it against and my disk space is all but out at the moment
>>> for yet another build tree...
>>
>> Compile-tested and pushed to mmc-next, and I'll submit it for 3.7 if
>> I get a Tested-by from someone like Sebastian (CC'd), or for 3.8 if I
>> don't.  (I'll add sdhci-pltfm API rework to my TODO list, too.)
>
> Chris, Russell,
>
> obviously I am not the one to ask for a Tested-by because it was my
> patch that originally caused all the trouble. My apologies for this!
>
> Anyway, I did test the patch on 3.7-rc3 and except that you need a
> 3-way git am because of a one-line offset, sdhci-dove runs just fine.
>
> Tested-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Great, thanks -- will push to 3.7.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

end of thread, other threads:[~2012-10-29 22:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-15  9:43 [PATCH] MMC: fix sdhci-dove removal Russell King - ARM Linux
2012-10-15 10:37 ` Russell King - ARM Linux
2012-10-15 10:44   ` Russell King - ARM Linux
2012-10-15 14:58     ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-15 15:07       ` Russell King - ARM Linux
2012-10-29 21:10     ` Chris Ball
2012-10-29 21:43       ` Russell King - ARM Linux
2012-10-29 21:49         ` Chris Ball
2012-10-29 22:16           ` Sebastian Hesselbarth
2012-10-29 22:20             ` Chris Ball

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).