From patchwork Mon Oct 29 21:43:07 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 1666851 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id C28D63FDDA for ; Mon, 29 Oct 2012 21:50:13 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TSxC6-0004FO-OT; Mon, 29 Oct 2012 21:48:27 +0000 Received: from caramon.arm.linux.org.uk ([78.32.30.218]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TSxC3-0004Dr-47 for linux-arm-kernel@lists.infradead.org; Mon, 29 Oct 2012 21:48:24 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=caramon; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=vrpImRld8t1egQr8KsuOZKGlQOZz12hVwRw4iXhyUZc=; b=HTXAZudeyJRz+z8d4QyLl7u71+Qpet7r2o65FfOdLGoEMN4BHB6+GTJj2POaMbbXkzy25lCKNnF0jVwa7ze6/zBifBOgXRYjhSvT1z9FxiG0xl6WrWb1kXNyc73NZtjQ4yUtBBVq2V/GqJNrlFiDyAuxyJQfO6IDIfw4NaQS5n0=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:46443) by caramon.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76) (envelope-from ) id 1TSx6y-0004vF-Qx; Mon, 29 Oct 2012 21:43:09 +0000 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1TSx6x-0007Wr-NO; Mon, 29 Oct 2012 21:43:07 +0000 Date: Mon, 29 Oct 2012 21:43:07 +0000 From: Russell King - ARM Linux To: Chris Ball Subject: Re: [PATCH] MMC: fix sdhci-dove removal Message-ID: <20121029214307.GH21164@n2100.arm.linux.org.uk> References: <20121015094348.GP21164@n2100.arm.linux.org.uk> <20121015103725.GQ21164@n2100.arm.linux.org.uk> <20121015104456.GR21164@n2100.arm.linux.org.uk> <87txtdq90y.fsf@octavius.laptop.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87txtdq90y.fsf@octavius.laptop.org> User-Agent: Mutt/1.5.19 (2009-01-05) X-Spam-Note: CRM114 invocation failed X-Spam-Score: -5.0 (-----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-5.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [78.32.30.218 listed in list.dnswl.org] -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org 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 > > 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 > > 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(-) Tested-by: Sebastian Hesselbarth 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 #include #include #include @@ -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 = {