[1/2] ASoC: wm8960: correct the min gain value of some PGA
diff mbox

Message ID 22ae1784eb84bf45b23bb7095b7dea8a116c0c2c.1441798038.git.zidan.wang@freescale.com
State Accepted
Commit 3758ff5f3dab57cd768d54279962a2f6bbc17188
Headers show

Commit Message

Zidan Wang Sept. 9, 2015, 11:29 a.m. UTC
The min gain is the corresponding gain value when the register value is 0
instead of 1, just correct it.

Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
---
 sound/soc/codecs/wm8960.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Charles Keepax Sept. 9, 2015, 11:49 a.m. UTC | #1
On Wed, Sep 09, 2015 at 07:29:10PM +0800, Zidan Wang wrote:
> The min gain is the corresponding gain value when the register value is 0
> instead of 1, just correct it.
> 
> Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
> ---
>  sound/soc/codecs/wm8960.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
> index e3b7d0c..6163474 100644
> --- a/sound/soc/codecs/wm8960.c
> +++ b/sound/soc/codecs/wm8960.c
> @@ -211,11 +211,11 @@ static int wm8960_put_deemph(struct snd_kcontrol *kcontrol,
>  	return wm8960_set_deemph(codec);
>  }
>  
> -static const DECLARE_TLV_DB_SCALE(adc_tlv, -9700, 50, 0);
> -static const DECLARE_TLV_DB_SCALE(dac_tlv, -12700, 50, 1);
> +static const DECLARE_TLV_DB_SCALE(adc_tlv, -9750, 50, 1);
> +static const DECLARE_TLV_DB_SCALE(dac_tlv, -12750, 50, 1);

The value zero is used for digital mute here. So I don't think it
is really appropriate to extend the TLV to show it as -97.5dB or
-127.5dB.

>  static const DECLARE_TLV_DB_SCALE(bypass_tlv, -2100, 300, 0);
>  static const DECLARE_TLV_DB_SCALE(out_tlv, -12100, 100, 1);
> -static const DECLARE_TLV_DB_SCALE(boost_tlv, -1200, 300, 1);
> +static const DECLARE_TLV_DB_SCALE(boost_tlv, -1500, 300, 1);

Same here.

>  
>  static const struct snd_kcontrol_new wm8960_snd_controls[] = {
>  SOC_DOUBLE_R_TLV("Capture Volume", WM8960_LINVOL, WM8960_RINVOL,
> -- 
> 1.9.1

Thanks,
Charles
Zidan Wang Sept. 10, 2015, 1:15 a.m. UTC | #2
On Wed, Sep 09, 2015 at 12:49:54PM +0100, Charles Keepax wrote:
> On Wed, Sep 09, 2015 at 07:29:10PM +0800, Zidan Wang wrote:
> > The min gain is the corresponding gain value when the register value is 0
> > instead of 1, just correct it.
> > 
> > Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
> > ---
> >  sound/soc/codecs/wm8960.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
> > index e3b7d0c..6163474 100644
> > --- a/sound/soc/codecs/wm8960.c
> > +++ b/sound/soc/codecs/wm8960.c
> > @@ -211,11 +211,11 @@ static int wm8960_put_deemph(struct snd_kcontrol *kcontrol,
> >  	return wm8960_set_deemph(codec);
> >  }
> >  
> > -static const DECLARE_TLV_DB_SCALE(adc_tlv, -9700, 50, 0);
> > -static const DECLARE_TLV_DB_SCALE(dac_tlv, -12700, 50, 1);
> > +static const DECLARE_TLV_DB_SCALE(adc_tlv, -9750, 50, 1);
> > +static const DECLARE_TLV_DB_SCALE(dac_tlv, -12750, 50, 1);
> 
> The value zero is used for digital mute here. So I don't think it
> is really appropriate to extend the TLV to show it as -97.5dB or
> -127.5dB.

I think the min register value will corresponding to the min gain.
So value 0 will also have a gain value, although it's for digital mute.

Refer to wm8962 codec driver, beep gain range is -90db~-6db, step in 6db,
but it define the beep gain like below:
static const DECLARE_TLV_DB_SCALE(beep_tlv, -9600, 600, 1);

Best Regards,
Zidan Wang
> 
> >  static const DECLARE_TLV_DB_SCALE(bypass_tlv, -2100, 300, 0);
> >  static const DECLARE_TLV_DB_SCALE(out_tlv, -12100, 100, 1);
> > -static const DECLARE_TLV_DB_SCALE(boost_tlv, -1200, 300, 1);
> > +static const DECLARE_TLV_DB_SCALE(boost_tlv, -1500, 300, 1);
> 
> Same here.
> 
> >  
> >  static const struct snd_kcontrol_new wm8960_snd_controls[] = {
> >  SOC_DOUBLE_R_TLV("Capture Volume", WM8960_LINVOL, WM8960_RINVOL,
> > -- 
> > 1.9.1
> 
> Thanks,
> Charles
Charles Keepax Sept. 10, 2015, 8:24 a.m. UTC | #3
On Thu, Sep 10, 2015 at 09:15:32AM +0800, Zidan Wang wrote:
> On Wed, Sep 09, 2015 at 12:49:54PM +0100, Charles Keepax wrote:
> > On Wed, Sep 09, 2015 at 07:29:10PM +0800, Zidan Wang wrote:
> > > The min gain is the corresponding gain value when the register value is 0
> > > instead of 1, just correct it.
> > > 
> > > Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
> > > ---
> > >  sound/soc/codecs/wm8960.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
> > > index e3b7d0c..6163474 100644
> > > --- a/sound/soc/codecs/wm8960.c
> > > +++ b/sound/soc/codecs/wm8960.c
> > > @@ -211,11 +211,11 @@ static int wm8960_put_deemph(struct snd_kcontrol *kcontrol,
> > >  	return wm8960_set_deemph(codec);
> > >  }
> > >  
> > > -static const DECLARE_TLV_DB_SCALE(adc_tlv, -9700, 50, 0);
> > > -static const DECLARE_TLV_DB_SCALE(dac_tlv, -12700, 50, 1);
> > > +static const DECLARE_TLV_DB_SCALE(adc_tlv, -9750, 50, 1);
> > > +static const DECLARE_TLV_DB_SCALE(dac_tlv, -12750, 50, 1);
> > 
> > The value zero is used for digital mute here. So I don't think it
> > is really appropriate to extend the TLV to show it as -97.5dB or
> > -127.5dB.
> 
> I think the min register value will corresponding to the min gain.
> So value 0 will also have a gain value, although it's for digital mute.
> 
> Refer to wm8962 codec driver, beep gain range is -90db~-6db, step in 6db,
> but it define the beep gain like below:
> static const DECLARE_TLV_DB_SCALE(beep_tlv, -9600, 600, 1);
> 

I might have to defer to Mark on this one then, if it is normal
to just use an additional gain value for mute in this type of
situation then it is ok with me. But it doesn't seem like that
would be a sensible thing, as you are asking for -97.5dB but
that is not what you are getting which doesn't seem like the
nicest interface.

Thanks,
Charles
Mark Brown Sept. 10, 2015, 10:58 a.m. UTC | #4
On Thu, Sep 10, 2015 at 09:24:20AM +0100, Charles Keepax wrote:
> On Thu, Sep 10, 2015 at 09:15:32AM +0800, Zidan Wang wrote:

> > > > +static const DECLARE_TLV_DB_SCALE(adc_tlv, -9750, 50, 1);
> > > > +static const DECLARE_TLV_DB_SCALE(dac_tlv, -12750, 50, 1);

> > I think the min register value will corresponding to the min gain.
> > So value 0 will also have a gain value, although it's for digital mute.

> > Refer to wm8962 codec driver, beep gain range is -90db~-6db, step in 6db,
> > but it define the beep gain like below:
> > static const DECLARE_TLV_DB_SCALE(beep_tlv, -9600, 600, 1);

> I might have to defer to Mark on this one then, if it is normal
> to just use an additional gain value for mute in this type of
> situation then it is ok with me. But it doesn't seem like that
> would be a sensible thing, as you are asking for -97.5dB but
> that is not what you are getting which doesn't seem like the
> nicest interface.

The last value in the scale macro is a flag saying if the minimum value
is mute - if it's set to 1 userspace should know this is what's going
on.
Charles Keepax Sept. 10, 2015, 11:35 a.m. UTC | #5
On Thu, Sep 10, 2015 at 11:58:21AM +0100, Mark Brown wrote:
> On Thu, Sep 10, 2015 at 09:24:20AM +0100, Charles Keepax wrote:
> > On Thu, Sep 10, 2015 at 09:15:32AM +0800, Zidan Wang wrote:
> 
> > > > > +static const DECLARE_TLV_DB_SCALE(adc_tlv, -9750, 50, 1);
> > > > > +static const DECLARE_TLV_DB_SCALE(dac_tlv, -12750, 50, 1);
> 
> > > I think the min register value will corresponding to the min gain.
> > > So value 0 will also have a gain value, although it's for digital mute.
> 
> > > Refer to wm8962 codec driver, beep gain range is -90db~-6db, step in 6db,
> > > but it define the beep gain like below:
> > > static const DECLARE_TLV_DB_SCALE(beep_tlv, -9600, 600, 1);
> 
> > I might have to defer to Mark on this one then, if it is normal
> > to just use an additional gain value for mute in this type of
> > situation then it is ok with me. But it doesn't seem like that
> > would be a sensible thing, as you are asking for -97.5dB but
> > that is not what you are getting which doesn't seem like the
> > nicest interface.
> 
> The last value in the scale macro is a flag saying if the minimum value
> is mute - if it's set to 1 userspace should know this is what's going
> on.

Ah sorry my bad, thanks.

In that case:

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

Patch
diff mbox

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index e3b7d0c..6163474 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -211,11 +211,11 @@  static int wm8960_put_deemph(struct snd_kcontrol *kcontrol,
 	return wm8960_set_deemph(codec);
 }
 
-static const DECLARE_TLV_DB_SCALE(adc_tlv, -9700, 50, 0);
-static const DECLARE_TLV_DB_SCALE(dac_tlv, -12700, 50, 1);
+static const DECLARE_TLV_DB_SCALE(adc_tlv, -9750, 50, 1);
+static const DECLARE_TLV_DB_SCALE(dac_tlv, -12750, 50, 1);
 static const DECLARE_TLV_DB_SCALE(bypass_tlv, -2100, 300, 0);
 static const DECLARE_TLV_DB_SCALE(out_tlv, -12100, 100, 1);
-static const DECLARE_TLV_DB_SCALE(boost_tlv, -1200, 300, 1);
+static const DECLARE_TLV_DB_SCALE(boost_tlv, -1500, 300, 1);
 
 static const struct snd_kcontrol_new wm8960_snd_controls[] = {
 SOC_DOUBLE_R_TLV("Capture Volume", WM8960_LINVOL, WM8960_RINVOL,