[alsa-devel] [PATCH] ASoC: sgtl5000: drop sgtl5000_set_bias_level() call from sgtl5000_remove()

[Mark Brown]

On Tue, Dec 03, 2013 at 09:26:51AM +0800, Shawn Guo wrote:

> Looking at function sgtl5000_set_bias_level(), we can find it
> essentially does the following 3 things in case of SND_SOC_BIAS_OFF.

>  1. Set regmap into cache only mode with regcache_cache_only() call
>  2. Disable supplies by calling regulator_bulk_disable()
>  3. Set codec->dapm.bias_level as SND_SOC_BIAS_OFF

> For 1, it's clearly incorrect and that's why we see the failure here.
> For 2, it's unnecessary since sgtl5000_remove() already calls
> regulator_bulk_disable() to disable supplies.  For 3, it does not make
> any sense either, since the bias level will be initialized to
> SND_SOC_BIAS_STANDBY in the next probe anyway.
> 
> Let's drop the sgtl5000_set_bias_level() call from sgtl5000_remove() to
> fix the kernel Oops below.

It's not clear to me that this is the best fix, all this code looks
confused.  Given that (as is idiomatic) the regulators are being enabled
in set_bias_level() what's happening in _OFF makes sense, what I'd
expect to see happen is that the duplicate disables and frees are
deleted from the remove path and the probe remembers to remove the cache
only setting.  Alternatively all the regulator and cache handling could
be removed from set_bias_level().  The end result should be that either
set_bias_level() or the probe and remove are handling this but not both.

The regulator get and remove probably ought to move out to the I2C
level, as should much of what happens in the probe function.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20131203/69d5d706/attachment.sig>