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

[Shawn Guo]

On Tue, Dec 03, 2013 at 04:03:54PM +0000, Mark Brown wrote:
> 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.

Yea, I'm working on a series to clean up and fix sgtl5000 driver, and
this issue should be removed by the series along the way.  So please
disregard this patch.

Shawn

> 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.