[alsa-devel] [PATCH] ASoC: WM8962: Enable start-up and normal bias after reset in runtime resume
The analogue circuits in WM8962 require a mid-rail analogue reference voltage, VMID, which generator is enabled by STARTUP_BIAS_ENA bit, while VMID_BUF_ENA bit can enable buffered VMID to avoid audible pops.
Besides, these circuits also need a bias current enabled by BIAS_ENA.
Then set VMID_SEL to 2x5k divider, the fast start-up mode, to power it up. And let set_bias_level() handle its 2x250k for low power standby and 2x50k for normal operation.
This patch might be kinda reverting operation against commit 52c0eee3 but after testing with WM8962 CODEC, these configurations during power up should be essential and soc-core would not do any of them. So the codec drvier need to handle it.
Signed-off-by: Nicolin Chen b42378@freescale.com --- sound/soc/codecs/wm8962.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c index 7a7a056..c63b58e 100644 --- a/sound/soc/codecs/wm8962.c +++ b/sound/soc/codecs/wm8962.c @@ -3773,6 +3773,17 @@ static int wm8962_runtime_resume(struct device *dev)
regcache_sync(wm8962->regmap);
+ regmap_update_bits(wm8962->regmap, WM8962_ANTI_POP, + WM8962_STARTUP_BIAS_ENA | WM8962_VMID_BUF_ENA, + WM8962_STARTUP_BIAS_ENA | WM8962_VMID_BUF_ENA); + + /* Bias enable at 2*5k (fast start-up) */ + regmap_update_bits(wm8962->regmap, WM8962_PWR_MGMT_1, + WM8962_BIAS_ENA | WM8962_VMID_SEL_MASK, + WM8962_BIAS_ENA | 0x180); + + msleep(5); + return 0; }
On Fri, Jun 14, 2013 at 04:52:20PM +0800, Nicolin Chen wrote:
The analogue circuits in WM8962 require a mid-rail analogue reference voltage, VMID, which generator is enabled by STARTUP_BIAS_ENA bit, while VMID_BUF_ENA bit can enable buffered VMID to avoid audible pops.
Besides, these circuits also need a bias current enabled by BIAS_ENA.
Then set VMID_SEL to 2x5k divider, the fast start-up mode, to power it up. And let set_bias_level() handle its 2x250k for low power standby and 2x50k for normal operation.
This patch might be kinda reverting operation against commit 52c0eee3 but after testing with WM8962 CODEC, these configurations during power up should be essential and soc-core would not do any of them. So the codec drvier need to handle it.
The biases should be being managed using set_bias_level() which will get called automatically when audio is being started and stopped.
Hi,
On Fri, Jun 14, 2013 at 10:59:17AM +0100, Mark Brown wrote:
The biases should be being managed using set_bias_level() which will get called automatically when audio is being started and stopped.
I understand your point because I also found other wm89xx drivers do this way. But the truth is, originally this 8962 codec driver did put biases settings into set_bias_level() as the others. But after commit d23031a4d they're put into pm_resume(). So I can also see the point that it's reasonable to handle these settings in pm_resume() since DAPM would call pm_runtime_get_sync(), so as snd_soc_open() would though, pm_resume() would then finish the settings before the bias level being set to STANDBY, according to dapm_pre_sequence_async() func.
As far as I can understand, I think these two approaches should be running into a same consequence, while I'm not sure which one's better.
Sir, do you still think it's better to put them back into set_bias_level()? Could you please help me understand the reason?
Thank you and looking forward to your reply. Nicolin Chen
On Fri, Jun 14, 2013 at 06:08:39PM +0800, Nicolin Chen wrote:
I understand your point because I also found other wm89xx drivers do this way. But the truth is, originally this 8962 codec driver did put biases settings into set_bias_level() as the others. But after commit d23031a4d they're put into pm_resume().
Don't just quote commit IDs, use the subject lines as well so humans can read your e-mails. The commit you're referring to moves some code out of set_bias_level() into runtime PM.
As far as I can understand, I think these two approaches should be running into a same consequence, while I'm not sure which one's better.
Sir, do you still think it's better to put them back into set_bias_level()? Could you please help me understand the reason?
So, if that's the case why are you changing this... it may be that what you need to do is to write a better commit message so it's clear why you're making the change, your message just described what the code change does but not why it's being done in that way.
That said the general concept is that runtime PM would be powering on the device for digital operation (eg, for the GPIOs) and then set_bias_level() powering up the analogue portions of the device.
On Fri, Jun 14, 2013 at 11:51:18AM +0100, Mark Brown wrote:
Don't just quote commit IDs, use the subject lines as well so humans can read your e-mails. The commit you're referring to moves some code out of set_bias_level() into runtime PM.
I'm sorry, I'll obey the rule from now on.
So, if that's the case why are you changing this... it may be that what you need to do is to write a better commit message so it's clear why you're making the change, your message just described what the code change does but not why it's being done in that way.
You're right. I'll revise it.
That said the general concept is that runtime PM would be powering on the device for digital operation (eg, for the GPIOs) and then set_bias_level() powering up the analogue portions of the device.
Understood. But I just tested the set_bias_level() way to handle BIAS, it prompts "wm8962 0-001a: DC servo timed out" when I start to play a wav file. There should be still some parts of code not right. So I'm gonna just revise the comment to continue the resume() methods. It should be appropriate since pm_suspend() is handling bias-off in the current code.
If you still can't accept it, I will then trace the code next week. It's Friday night here, and I'm starving now :(
Thank you, Nicolin Chen
On Fri, Jun 14, 2013 at 07:07:43PM +0800, Nicolin Chen wrote:
On Fri, Jun 14, 2013 at 11:51:18AM +0100, Mark Brown wrote:
That said the general concept is that runtime PM would be powering on the device for digital operation (eg, for the GPIOs) and then set_bias_level() powering up the analogue portions of the device.
Understood. But I just tested the set_bias_level() way to handle BIAS, it prompts "wm8962 0-001a: DC servo timed out" when I start to play a wav file. There should be still some parts of code not right. So I'm gonna just revise the comment to continue the resume() methods. It should be appropriate since pm_suspend() is handling bias-off in the current code.
If the DC servo is timing out that'll be a problem with the clocking... the FLL is partially analogue so if you're using the FLL that makes sense. Looking at the code we're also bringing the bias down in the suspend path so it needs to be powered up in the resume path for symmetry if nothing else.
On Fri, Jun 14, 2013 at 12:21:10PM +0100, Mark Brown wrote:
If the DC servo is timing out that'll be a problem with the clocking... the FLL is partially analogue so if you're using the FLL that makes sense. Looking at the code we're also bringing the bias down in the suspend path so it needs to be powered up in the resume path for symmetry if nothing else.
Thank you for helping me understand the case. And just sent the v2.
Hope you can apply it. Nicolin Chen
participants (2)
-
Mark Brown -
Nicolin Chen