tlv320aic3x audio driver -- possible bug
In the C source file "tlv320aic3x" in the function "aic3x_set_power", when the argument for power is 0, the hardware registers are saved to the Linux computer as follows:
regcache_cache_only(aic3x->regmap, true);
And then later when "aic3x_set_power" is called with power set to 1, the previous values are then restored back to their hardware registers:
regcache_cache_only(aic3x->regmap, false); regcache_sync(aic3x->regmap);
When the device is powered back on, the previous setting for microphone bias (MICBIAS) is lost. If it was previously set to 2 volts or 2.5 volts, then when it's powered back on it becomes Off.
I'm not very familiar with audio codec chips and how they work, but I think maybe the order of function calls in the "else" branch might be incorrect. Right now the code looks like this:
/* * Do soft reset to this codec instance in order to clear * possible VDD leakage currents in case the supply regulators * remain on */ snd_soc_component_write(component, AIC3X_RESET, SOFT_RESET); regcache_mark_dirty(aic3x->regmap); aic3x->power = 0; /* HW writes are needless when bias is off */ regcache_cache_only(aic3x->regmap, true); ret = regulator_bulk_disable(ARRAY_SIZE(aic3x->supplies), aic3x->supplies);
However I think that the call to "regcache_cache_only" should come before the soft reset takeS place. The setting for MICBIAS is lost at this line:
snd_soc_component_write(component, AIC3X_RESET, SOFT_RESET);
And so I think that the call to "regcache_cache_only" should be moved up above "snd_soc_component_write", as follows:
/* * Do soft reset to this codec instance in order to clear * possible VDD leakage currents in case the supply regulators * remain on */ regcache_cache_only(aic3x->regmap, true); snd_soc_component_write(component, AIC3X_RESET, SOFT_RESET); regcache_mark_dirty(aic3x->regmap); aic3x->power = 0; ret = regulator_bulk_disable(ARRAY_SIZE(aic3x->supplies), aic3x->supplies);
On Fri, Jul 23, 2021 at 02:29:51PM +0100, Frederick Gotham wrote:
When the device is powered back on, the previous setting for microphone bias (MICBIAS) is lost. If it was previously set to 2 volts or 2.5 volts, then when it's powered back on it becomes Off.
If this is configured in a volatile register (I don't know off hand) then it needs to be specifically saved and restored separately, I know this is an issue with the accessory detection - I've got a patch from talking to someone else about an issue they were having there.
However I think that the call to "regcache_cache_only" should come before the soft reset takeS place. The setting for MICBIAS is lost at this line:
snd_soc_component_write(component, AIC3X_RESET, SOFT_RESET);
And so I think that the call to "regcache_cache_only" should be moved up above "snd_soc_component_write", as follows:
/* * Do soft reset to this codec instance in order to clear * possible VDD leakage currents in case the supply regulators * remain on */ regcache_cache_only(aic3x->regmap, true); snd_soc_component_write(component, AIC3X_RESET, SOFT_RESET);
I don't understand how you think this will fix things - we're still powering off the regulators (if they're controllable or if that happens over suspend) so we'll still loose all the register state and exactly the same thing will happen when we try to restore, at best this will mean the reset that's being done there doesn't actually happens so we've got the risk of having higher power consumption over suspend if the supplies are maintained over suspend.
participants (2)
-
Frederick Gotham
-
Mark Brown