[alsa-devel] [PATCH] ASoC: cs4271: free reset gpio in cs4271_remove()
The reset GPIO line is acquired in cs4271_probe(), so it should be freed from cs4271_remove() as well. This way, the driver can unloaded and reloaded more than once.
Signed-off-by: Daniel Mack zonque@gmail.com --- sound/soc/codecs/cs4271.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs4271.c b/sound/soc/codecs/cs4271.c index ce05fd9..aeaa5c6 100644 --- a/sound/soc/codecs/cs4271.c +++ b/sound/soc/codecs/cs4271.c @@ -604,9 +604,11 @@ static int cs4271_remove(struct snd_soc_codec *codec) { struct cs4271_private *cs4271 = snd_soc_codec_get_drvdata(codec);
- if (gpio_is_valid(cs4271->gpio_nreset)) + if (gpio_is_valid(cs4271->gpio_nreset)) { /* Set codec to the reset state */ gpio_set_value(cs4271->gpio_nreset, 0); + devm_gpio_free(codec->dev, cs4271->gpio_nreset); + }
return 0; };
On Wed, Feb 19, 2014 at 12:07:12PM +0100, Daniel Mack wrote:
- if (gpio_is_valid(cs4271->gpio_nreset)) { /* Set codec to the reset state */ gpio_set_value(cs4271->gpio_nreset, 0);
devm_gpio_free(codec->dev, cs4271->gpio_nreset);- }
If it's being requested as a managed resource shouldn't it be being freed automatically?
On 02/19/2014 12:15 PM, Mark Brown wrote:
On Wed, Feb 19, 2014 at 12:07:12PM +0100, Daniel Mack wrote:
- if (gpio_is_valid(cs4271->gpio_nreset)) { /* Set codec to the reset state */ gpio_set_value(cs4271->gpio_nreset, 0);
devm_gpio_free(codec->dev, cs4271->gpio_nreset);- }
If it's being requested as a managed resource shouldn't it be being freed automatically?
Nope, as the module itself will not go away necessarily. I hit the bug when unloading and reloading the machine driver. cs4271_probe(codec) will fail to acquire and drive the reset line, and the codec stays in reset.
Daniel
On Wed, Feb 19, 2014 at 12:24:23PM +0100, Daniel Mack wrote:
On 02/19/2014 12:15 PM, Mark Brown wrote:
On Wed, Feb 19, 2014 at 12:07:12PM +0100, Daniel Mack wrote:
- if (gpio_is_valid(cs4271->gpio_nreset)) { /* Set codec to the reset state */ gpio_set_value(cs4271->gpio_nreset, 0);
devm_gpio_free(codec->dev, cs4271->gpio_nreset);- }
If it's being requested as a managed resource shouldn't it be being freed automatically?
Nope, as the module itself will not go away necessarily. I hit the bug when unloading and reloading the machine driver. cs4271_probe(codec) will fail to acquire and drive the reset line, and the codec stays in reset.
The fix here is to move the resource acquisition to the bus level probe instead of the ASoC card startup.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/19/2014 12:43 PM, Mark Brown wrote:
On Wed, Feb 19, 2014 at 12:24:23PM +0100, Daniel Mack wrote:
On 02/19/2014 12:15 PM, Mark Brown wrote:
On Wed, Feb 19, 2014 at 12:07:12PM +0100, Daniel Mack wrote:
- if (gpio_is_valid(cs4271->gpio_nreset)) { /* Set codec to
the reset state */ gpio_set_value(cs4271->gpio_nreset, 0); + devm_gpio_free(codec->dev, cs4271->gpio_nreset); + }
If it's being requested as a managed resource shouldn't it be being freed automatically?
Nope, as the module itself will not go away necessarily. I hit the bug when unloading and reloading the machine driver. cs4271_probe(codec) will fail to acquire and drive the reset line, and the codec stays in reset.
The fix here is to move the resource acquisition to the bus level probe instead of the ASoC card startup.
Alright. I thought there was a verdict once that resource claiming should be done right before usage, but you're right, it really rather belongs into the bus probe functions.
I'll send another version.
Thanks! Daniel
On Wed, Feb 19, 2014 at 01:09:27PM +0100, Daniel Mack wrote:
Alright. I thought there was a verdict once that resource claiming should be done right before usage, but you're right, it really rather belongs into the bus probe functions.
No, that's not ever been the case as far as I can recall. There were some things it used to not be possible to do due to the ASoC level I/O only working after the card comes up but that's a different issue.
On 02/19/2014 12:24 PM, Daniel Mack wrote:
On 02/19/2014 12:15 PM, Mark Brown wrote:
On Wed, Feb 19, 2014 at 12:07:12PM +0100, Daniel Mack wrote:
- if (gpio_is_valid(cs4271->gpio_nreset)) { /* Set codec to the reset state */ gpio_set_value(cs4271->gpio_nreset, 0);
devm_gpio_free(codec->dev, cs4271->gpio_nreset);- }
If it's being requested as a managed resource shouldn't it be being freed automatically?
Nope, as the module itself will not go away necessarily. I hit the bug when unloading and reloading the machine driver. cs4271_probe(codec) will fail to acquire and drive the reset line, and the codec stays in reset.
In my opinion a better solution is to move the requesting of the GPIO from the CODEC probe() function to the SPI/I2C probe() functions. Actually it looks as if most of the code (if not all) in the CODEC probe() function should be done when the driver is probed.
- Lars
participants (4)
-
Daniel Mack -
Daniel Mack -
Lars-Peter Clausen -
Mark Brown