Question about WM8904 regcache

Question about WM8904 regcache

[Bo Shen]

Hi Mark,
   I am sorry to disturb you. On our EK board, it use wm8904 codec. When 
I enable the audio, and use alsa utils aplay to test, I meet the 
following issue.
   The audio can not playback, it show the following error.
---8>---
#aplay 44.1k_16bit_stereo.wav
Playing WAVE '44.1k_16bit_stereo.wav' : Signed 16 bit Little Endian, 
Rate 44100 Hz, Stereo
aplay: pcm_write:1737: write error: Input/output error
---<8---

   After check the register, they are not be configured correctly. When 
read from debug file system, found the cache_dirty always Y before 
playback and also failed playback.
   I try to add the following two line in wm8904_set_bias_level into 
"case SND_SOC_BIAS_ON", it doesn't work.
---8>---
@@ -1832,9 +1834,12 @@ static int wm8904_set_bias_level(struct 
snd_soc_codec *codec,

         switch (level) {
         case SND_SOC_BIAS_ON:
+               regcache_cache_only(wm8904->regmap, false);
+               regcache_sync(wm8904->regmap);
                 break;
---<8---

   However, If I comment the regcache_cache_only(wm8904->regmap, true) 
as following, then it play OK.
---8>---
@@ -2232,7 +2239,7 @@ static int wm8904_i2c_probe(struct i2c_client *i2c,
                             WM8904_POBCTRL, 0);

         /* Can leave the device powered off until we need it */
-       regcache_cache_only(wm8904->regmap, true);
+       // regcache_cache_only(wm8904->regmap, true);
---<8---

   So, any clue for this issue?

   Thanks.

Best Regards,
Bo Shen

Re: Question about WM8904 regcache

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1498 bytes --]

On Fri, Jul 12, 2013 at 03:00:26PM +0800, Bo Shen wrote:

>   I try to add the following two line in wm8904_set_bias_level into
> "case SND_SOC_BIAS_ON", it doesn't work.
> ---8>---
> @@ -1832,9 +1834,12 @@ static int wm8904_set_bias_level(struct
> snd_soc_codec *codec,
> 
>         switch (level) {
>         case SND_SOC_BIAS_ON:
> +               regcache_cache_only(wm8904->regmap, false);
> +               regcache_sync(wm8904->regmap);
>                 break;
> ---<8---

That's far too late to be doing that, the device is already supposed to
be powered and running - you can see the device is doing this already in
the transition from _OFF to _STANDBY which is where it should happen.

>   However, If I comment the regcache_cache_only(wm8904->regmap,
> true) as following, then it play OK.
> ---8>---
> @@ -2232,7 +2239,7 @@ static int wm8904_i2c_probe(struct i2c_client *i2c,
>                             WM8904_POBCTRL, 0);
> 
>         /* Can leave the device powered off until we need it */
> -       regcache_cache_only(wm8904->regmap, true);
> +       // regcache_cache_only(wm8904->regmap, true);
> ---<8---

>   So, any clue for this issue?

The device is supposed to be idle_bias_off so sitting in _BIAS_OFF when
not in use, this should mean a transition to _STANDBY as part of the
power up sequence which should reenable interaction with the device.
Check to see why that is not happening - _OFF should be the default bias
level as well.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Question about WM8904 regcache

[Bo Shen]

Hi Mark,

On 07/12/2013 04:42 PM, Mark Brown wrote:
> On Fri, Jul 12, 2013 at 03:00:26PM +0800, Bo Shen wrote:
>
>>    I try to add the following two line in wm8904_set_bias_level into
>> "case SND_SOC_BIAS_ON", it doesn't work.
>> ---8>---
>> @@ -1832,9 +1834,12 @@ static int wm8904_set_bias_level(struct
>> snd_soc_codec *codec,
>>
>>          switch (level) {
>>          case SND_SOC_BIAS_ON:
>> +               regcache_cache_only(wm8904->regmap, false);
>> +               regcache_sync(wm8904->regmap);
>>                  break;
>> ---<8---
>
> That's far too late to be doing that, the device is already supposed to
> be powered and running - you can see the device is doing this already in
> the transition from _OFF to _STANDBY which is where it should happen.
>
>>    However, If I comment the regcache_cache_only(wm8904->regmap,
>> true) as following, then it play OK.
>> ---8>---
>> @@ -2232,7 +2239,7 @@ static int wm8904_i2c_probe(struct i2c_client *i2c,
>>                              WM8904_POBCTRL, 0);
>>
>>          /* Can leave the device powered off until we need it */
>> -       regcache_cache_only(wm8904->regmap, true);
>> +       // regcache_cache_only(wm8904->regmap, true);
>> ---<8---
>
>>    So, any clue for this issue?
>
> The device is supposed to be idle_bias_off so sitting in _BIAS_OFF when
> not in use, this should mean a transition to _STANDBY as part of the
> power up sequence which should reenable interaction with the device.
> Check to see why that is not happening - _OFF should be the default bias
> level as well.
>

Do you mean the system boot up the device should be in _BIAS_OFF state?

I add debug info to set_bias_level function, and don't see this is 
called when system boot up. When I run aplay command, it call from 
_STANDBY to _ON, not _OFF, after show the error message, it call from 
_PREPARE to _OFF.

---8>---
# aplay 44.1k_16bit_stereo.wav
Playing WAVE '44.1k_16bit_stereo.wav' : Signed 16 bit Little Endian, 
Rate 44100 Hz, Stereo
 >>>>>> leve is: 1
 >>>>>> leve is: 2
 >>>>>> leve is: 3
aplay: pcm_write:1737: write error: Input/output error
# >>>>>> leve is: 2
 >>>>>> leve is: 1
 >>>>>> leve is: 0
---<8---

Best Regards,
Bo Shen

Re: Question about WM8904 regcache

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 685 bytes --]

On Fri, Jul 12, 2013 at 06:03:16PM +0800, Bo Shen wrote:

> Do you mean the system boot up the device should be in _BIAS_OFF state?

Yes.

> I add debug info to set_bias_level function, and don't see this is
> called when system boot up. When I run aplay command, it call from
> _STANDBY to _ON, not _OFF, after show the error message, it call
> from _PREPARE to _OFF.

It's not called on boot, it's just the default state - the probe()
function is intended to leave things in _OFF.  What you describe sounds
like it's sitting in _STANDBY, you need to debug why this is happening.
I can't see anything in current code - snd_soc_register_codec() sets the
bias level to _OFF by default.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]