Re: [PATCH v2 2/2] ASoC: codecs: pcm186x: Fix energysense SLEEP bit

2019-02-20 Thread Mark Brown
On Tue, Feb 19, 2019 at 11:06:48AM -0600, Andrew F. Davis wrote:

> Did this work for you before? Strange it would if reversed, I wonder if
> the SLEEP bit is really doing anything here. Can investigate later, for
> this patch:

Thinking about it some more I'm wondering if it gets overridden by
something else (ie, the CODEC actually being asleep requires this being
set and some other thing being turned off).


signature.asc
Description: PGP signature


Re: [PATCH v2 2/2] ASoC: codecs: pcm186x: Fix energysense SLEEP bit

2019-02-20 Thread Mark Brown
On Tue, Feb 19, 2019 at 04:29:28PM +, codrin.ciubota...@microchip.com wrote:

> The ADCs are sleeping when the SLEEP bit is set and running when it's
> cleared, so the bit should be inverted.
> Tested on pcm1863.

This definitely seems to be the case from the datasheet but...

> @@ -184,8 +184,8 @@ static const struct snd_soc_dapm_widget 
> pcm1865_dapm_widgets[] = {
>* Put the codec into SLEEP mode when not in use, allowing the
>* Energysense mechanism to operate.
>*/
> - SND_SOC_DAPM_ADC("ADC1", "HiFi Capture 1", PCM186X_POWER_CTRL, 1,  0),
> - SND_SOC_DAPM_ADC("ADC2", "HiFi Capture 2", PCM186X_POWER_CTRL, 1,  0),
> + SND_SOC_DAPM_ADC("ADC1", "HiFi Capture 1", PCM186X_POWER_CTRL, 1,  1),
> + SND_SOC_DAPM_ADC("ADC2", "HiFi Capture 2", PCM186X_POWER_CTRL, 1,  1),
>  };

...we still have a bug on pcm1865 as we've got the same register bit for
both channels which isn't something that's supposed to happen.  Looking
at the register description this feels like it'd be better handled in
set_bias_level() or as a DAPM supply widget.  However that's already an
issue without this patch.  I am a bit curious how this ever worked for
anyone though.


signature.asc
Description: PGP signature


Re: [PATCH v2 2/2] ASoC: codecs: pcm186x: Fix energysense SLEEP bit

2019-02-20 Thread Codrin.Ciubotariu
On 19.02.2019 19:06, Andrew F. Davis wrote:
> On 2/19/19 10:29 AM, codrin.ciubota...@microchip.com wrote:
>> From: Codrin Ciubotariu 
>>
>> The ADCs are sleeping when the SLEEP bit is set and running when it's
>> cleared, so the bit should be inverted.
>> Tested on pcm1863.
>>
> 
> Did this work for you before? Strange it would if reversed, I wonder if
> the SLEEP bit is really doing anything here. Can investigate later, for
> this patch:

The SLEEP bit seems to work. At least STATE from register 114 says the 
IP is sleeping. Without this change, if pcm1863 is used as master, I can 
see BCK and LRCK generated while no sound is played. Once I start an 
audio stream, the clocks stop.

Thank you for your ACK. I would really appreciate if you could also test 
it on your side.

Best regards,
Codrin

> 
> Acked-by: Andrew F. Davis 
> 
>> Signed-off-by: Codrin Ciubotariu 
>> ---
>>
>> Changes in v2:
>>   - none;
>>
>>   sound/soc/codecs/pcm186x.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/sound/soc/codecs/pcm186x.c b/sound/soc/codecs/pcm186x.c
>> index c36a391fec8a..c5fcc632f670 100644
>> --- a/sound/soc/codecs/pcm186x.c
>> +++ b/sound/soc/codecs/pcm186x.c
>> @@ -158,7 +158,7 @@ static const struct snd_soc_dapm_widget 
>> pcm1863_dapm_widgets[] = {
>>   * Put the codec into SLEEP mode when not in use, allowing the
>>   * Energysense mechanism to operate.
>>   */
>> -SND_SOC_DAPM_ADC("ADC", "HiFi Capture", PCM186X_POWER_CTRL, 1,  0),
>> +SND_SOC_DAPM_ADC("ADC", "HiFi Capture", PCM186X_POWER_CTRL, 1,  1),
>>   };
>>   
>>   static const struct snd_soc_dapm_widget pcm1865_dapm_widgets[] = {
>> @@ -184,8 +184,8 @@ static const struct snd_soc_dapm_widget 
>> pcm1865_dapm_widgets[] = {
>>   * Put the codec into SLEEP mode when not in use, allowing the
>>   * Energysense mechanism to operate.
>>   */
>> -SND_SOC_DAPM_ADC("ADC1", "HiFi Capture 1", PCM186X_POWER_CTRL, 1,  0),
>> -SND_SOC_DAPM_ADC("ADC2", "HiFi Capture 2", PCM186X_POWER_CTRL, 1,  0),
>> +SND_SOC_DAPM_ADC("ADC1", "HiFi Capture 1", PCM186X_POWER_CTRL, 1,  1),
>> +SND_SOC_DAPM_ADC("ADC2", "HiFi Capture 2", PCM186X_POWER_CTRL, 1,  1),
>>   };
>>   
>>   static const struct snd_soc_dapm_route pcm1863_dapm_routes[] = {
>>



Re: [PATCH v2 2/2] ASoC: codecs: pcm186x: Fix energysense SLEEP bit

2019-02-19 Thread Andrew F. Davis
On 2/19/19 10:29 AM, codrin.ciubota...@microchip.com wrote:
> From: Codrin Ciubotariu 
> 
> The ADCs are sleeping when the SLEEP bit is set and running when it's
> cleared, so the bit should be inverted.
> Tested on pcm1863.
> 

Did this work for you before? Strange it would if reversed, I wonder if
the SLEEP bit is really doing anything here. Can investigate later, for
this patch:

Acked-by: Andrew F. Davis 

> Signed-off-by: Codrin Ciubotariu 
> ---
> 
> Changes in v2:
>  - none;
> 
>  sound/soc/codecs/pcm186x.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/codecs/pcm186x.c b/sound/soc/codecs/pcm186x.c
> index c36a391fec8a..c5fcc632f670 100644
> --- a/sound/soc/codecs/pcm186x.c
> +++ b/sound/soc/codecs/pcm186x.c
> @@ -158,7 +158,7 @@ static const struct snd_soc_dapm_widget 
> pcm1863_dapm_widgets[] = {
>* Put the codec into SLEEP mode when not in use, allowing the
>* Energysense mechanism to operate.
>*/
> - SND_SOC_DAPM_ADC("ADC", "HiFi Capture", PCM186X_POWER_CTRL, 1,  0),
> + SND_SOC_DAPM_ADC("ADC", "HiFi Capture", PCM186X_POWER_CTRL, 1,  1),
>  };
>  
>  static const struct snd_soc_dapm_widget pcm1865_dapm_widgets[] = {
> @@ -184,8 +184,8 @@ static const struct snd_soc_dapm_widget 
> pcm1865_dapm_widgets[] = {
>* Put the codec into SLEEP mode when not in use, allowing the
>* Energysense mechanism to operate.
>*/
> - SND_SOC_DAPM_ADC("ADC1", "HiFi Capture 1", PCM186X_POWER_CTRL, 1,  0),
> - SND_SOC_DAPM_ADC("ADC2", "HiFi Capture 2", PCM186X_POWER_CTRL, 1,  0),
> + SND_SOC_DAPM_ADC("ADC1", "HiFi Capture 1", PCM186X_POWER_CTRL, 1,  1),
> + SND_SOC_DAPM_ADC("ADC2", "HiFi Capture 2", PCM186X_POWER_CTRL, 1,  1),
>  };
>  
>  static const struct snd_soc_dapm_route pcm1863_dapm_routes[] = {
>