All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King <rmk+kernel@arm.linux.org.uk>
To: Mark Brown <broonie@kernel.org>
Cc: Takashi Iwai <tiwai@suse.de>,
	alsa-devel@alsa-project.org, Liam Girdwood <lgirdwood@gmail.com>
Subject: [PATCH] ASoC: sgtl5000: fix devres handling
Date: Mon, 30 Jun 2014 10:58:18 +0100	[thread overview]
Message-ID: <E1X1YLq-0004E1-MT@rmk-PC.arm.linux.org.uk> (raw)

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 <rmk+kernel@arm.linux.org.uk>
---
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;
-- 
1.8.3.1

             reply	other threads:[~2014-06-30  9:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-30  9:58 Russell King [this message]
2014-06-30 14:44 ` [PATCH] ASoC: sgtl5000: fix devres handling Mark Brown
2014-06-30 14:47   ` Russell King - ARM Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1X1YLq-0004E1-MT@rmk-PC.arm.linux.org.uk \
    --to=rmk+kernel@arm.linux.org.uk \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.