From patchwork Mon Jun 30 09:58:18 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King X-Patchwork-Id: 4447051 Return-Path: X-Original-To: patchwork-alsa-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 73D31BEEAA for ; Mon, 30 Jun 2014 09:58:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9676920364 for ; Mon, 30 Jun 2014 09:58:54 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id 1AAAE20374 for ; Mon, 30 Jun 2014 09:58:53 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id E20C32654F1; Mon, 30 Jun 2014 11:58:46 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-1.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=no version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id A9D632610C7; Mon, 30 Jun 2014 11:58:36 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 21B5D2610C7; Mon, 30 Jun 2014 11:58:36 +0200 (CEST) Received: from pandora.arm.linux.org.uk (gw-1.arm.linux.org.uk [78.32.30.217]) by alsa0.perex.cz (Postfix) with ESMTP id 22BC72610CE for ; Mon, 30 Jun 2014 11:58:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora; h=Date:Sender:Message-Id:Subject:Cc:To:From; bh=ojcfvx3n21uxEF6Dz35FNk2+e5/PRk16HfT516+PtF8=; b=NLKNy9XozM1I7U0g2mtzCj7yXEAgA/PfcfehfuTUnSQayK0RwPjMwfVf9Mxu1bdjVYXq/1Sq1bmm/jLRTMy2+Hq1N7zo8x328918tQJHHyX/3JqKV3ybTvLpH2MKIraL9RcDXJm9JmejPrg6WmYdjsKS/DvZvLySeMfuzYc2brs=; Received: from e0022681537dd.dyn.arm.linux.org.uk ([2002:4e20:1eda:1:222:68ff:fe15:37dd]:53680 helo=rmk-PC.arm.linux.org.uk) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76) (envelope-from ) id 1X1YLr-0008Nd-1u; Mon, 30 Jun 2014 10:58:19 +0100 Received: from rmk by rmk-PC.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1X1YLq-0004E1-MT; Mon, 30 Jun 2014 10:58:18 +0100 From: Russell King To: Mark Brown Message-Id: Date: Mon, 30 Jun 2014 10:58:18 +0100 Cc: Takashi Iwai , alsa-devel@alsa-project.org, Liam Girdwood Subject: [alsa-devel] [PATCH] ASoC: sgtl5000: fix devres handling X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP The sgtl5000 uses a two-stage initialisation process. The first stage is when the platform driver is probed, where some resources are found and initialised. The second stage is via the codec driver's probe function, where regulators are found and initialised using the managed resource support. The problem here is that this works fine until the codec driver is unbound. When this occurs, sgtl5000_remove() is called which is a no-op as far as the managed resource code is concerned. The regulators remain allocated, and their pointers in sgtl5000_priv remain valid. If the codec is now re-probed, it will again try and find the regulators, which will now be busy. This will fail. That's not the only problem - if using the LDO regulator, the regulator is unregistered from the regulator core code, but it still has a user. When the user is cleaned up (eg, by removing the module) it hits the free'd regulator, and this can oops the kernel. This bug was originally introduced by 63e54cd9caa3ce ("ASoC: sgtl5000: Use devm_regulator_bulk_get()"). Signed-off-by: Russell King --- You may wish to give some consideration to whether ASoC core code should be doing this instead, this is probably a very common source of brokenness where two-stage driver initialisation exists. These kinds of issues are why I consider (as a general rule) that two-stage driver initialisation is really bad. sound/soc/codecs/sgtl5000.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c index 96ce061c1f10..3bbd89065313 100644 --- a/sound/soc/codecs/sgtl5000.c +++ b/sound/soc/codecs/sgtl5000.c @@ -1305,6 +1305,9 @@ static int sgtl5000_probe(struct snd_soc_codec *codec) int ret; struct sgtl5000_priv *sgtl5000 = snd_soc_codec_get_drvdata(codec); + if (!devres_open_group(codec->dev, NULL, GFP_KERNEL)) + return -ENOMEM; + ret = sgtl5000_enable_regulators(codec); if (ret) return ret; @@ -1362,6 +1365,9 @@ static int sgtl5000_probe(struct snd_soc_codec *codec) err: regulator_bulk_disable(ARRAY_SIZE(sgtl5000->supplies), sgtl5000->supplies); + + devres_release_group(codec->dev, NULL); + ldo_regulator_remove(codec); return ret; @@ -1375,6 +1381,9 @@ static int sgtl5000_remove(struct snd_soc_codec *codec) regulator_bulk_disable(ARRAY_SIZE(sgtl5000->supplies), sgtl5000->supplies); + + devres_release_group(codec->dev, NULL); + ldo_regulator_remove(codec); return 0;