ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Timur Karaldin]

Hi, colleagues!

I'm newbie with ALSA devel. I need to get extra functionality from codec 
through ALSA mixer.
Task: 1. making possibility to set-up gain for each input 
(XXX_2_LADCCTRL and XXX_2_RADCCTRL registers)
2. making possibility to set-up debounce for button and headset 
detection ( HEADSET_DETECT_CTRL_A register)
3. making possibility to enable high-power AC-coupled output 
(HEADSET_DETECT_CTRL_A register)
4. making possibility to enable stereo pseudo differential output 
(HEADSET_DETECT_CTRL_B register)
5. enable headset detection (HEADSET_DETECT_CTRL_A)
6. getting headset detect status (HEADSET_DETECT_CTRL_A)

Well, my patch works in generally, but I don't like it very much, because:
1. All controls for input amplifiers' gain are shown in 
Alsamixer-Playback section. But not only mine, capture controls from 
base codec's version are shown in playback section too, for example 
(Right or Left) PGA Mixer Line1(L or R)/2(L or R)/MIC3(L or R), which is 
definitely capture settings. Should I worried about it? because, as I 
understand, it could not be setted correctly during changing 
playback/capture mode for power-saving reason.
2.  "Right PGA Mixer Line1R" control (and similar) using the same bits 
in LINE1R_2_RADCCTRL register as my "Line1R RADC Gain" control (and 
similar) and put there b"1111" when user switch disconnect it from ADC. 
So my control is not shown when user disconnect it from ADC.
3. When I start/stop arecord some of my options were reset (debounces, 
headset enable, high-power AC-coupled, pseudo differential output). When 
I set up it in shell and then start arecord, it's reset again. Is it the 
reason because of displaying controls in playback mixer section?

I will be very pleased for any advise, thank you!


here is my patch for tlv320aic3x.c
---
  sound/soc/codecs/tlv320aic3x.c | 55 
+++++++++++++++++++++++++++++++++++++++---
  1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index d7349bc..27d031e 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -122,11 +122,24 @@ static const struct reg_default aic3x_reg[] = {
         { 108, 0x00 }, { 109, 0x00 },
  };

+static bool aic3106_volatile(struct device *dev, unsigned int reg)
+{
+    switch(reg)
+    {
+        case AIC3X_HEADSET_DETECT_CTRL_A:
+        case AIC3X_HEADSET_DETECT_CTRL_B:
+            return true;
+        default:
+            return false;
+    }
+}
+
  static const struct regmap_config aic3x_regmap = {
         .reg_bits = 8,
         .val_bits = 8,

         .max_register = DAC_ICC_ADJ,
+    .volatile_reg = aic3106_volatile,
         .reg_defaults = aic3x_reg,
         .num_reg_defaults = ARRAY_SIZE(aic3x_reg),
         .cache_type = REGCACHE_RBTREE,
@@ -196,7 +209,7 @@ static int mic_bias_event(struct snd_soc_dapm_widget *w,
         struct snd_kcontrol *kcontrol, int event)
  {
         struct snd_soc_codec *codec = w->codec;
-       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+    struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);

         switch (event) {
         case SND_SOC_DAPM_POST_PMU:
@@ -270,6 +283,26 @@ static const struct soc_enum aic3x_agc_decay_enum[] = {
         SOC_ENUM_SINGLE(RAGC_CTRL_A, 0, 4, aic3x_agc_decay),
  };

+static const char *aic3x_adc_gain[] = { "0dB", "-1.5dB", "-3dB", 
"-4.5dB", "-6dB", "-7.5dB", "-9dB", "-10.5dB", "-12dB" };
+static const struct soc_enum aic3x_adc_gain_enum[] = {
+       SOC_ENUM_SINGLE(MIC3LR_2_LADC_CTRL, 4, 9, aic3x_adc_gain),
+       SOC_ENUM_SINGLE(MIC3LR_2_RADC_CTRL, 0, 9, aic3x_adc_gain),
+       SOC_ENUM_SINGLE(LINE1L_2_LADC_CTRL, 3, 9, aic3x_adc_gain),
+       SOC_ENUM_SINGLE(LINE1R_2_RADC_CTRL, 3, 9, aic3x_adc_gain),
+       SOC_ENUM_SINGLE(LINE2L_2_LADC_CTRL, 3, 9, aic3x_adc_gain),
+       SOC_ENUM_SINGLE(LINE2R_2_RADC_CTRL, 3, 9, aic3x_adc_gain),
+};
+static const char *aic3x_headset_debounce[] = { "16ms", "32ms", "64ms", 
"128ms", "256ms", "512ms" };
+static const struct soc_enum aic3x_headset_debounce_enum[] = {
+    SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 2, 6, 
aic3x_headset_debounce),
+};
+static const char *aic3x_button_debounce[] = { "0ms", "8ms", "16ms", 
"32ms" };
+static const struct soc_enum aic3x_button_debounce_enum[] = {
+    SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 0, 4, 
aic3x_button_debounce),
+};
+
+
+
  /*
   * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps
   */
@@ -399,6 +432,19 @@ static const struct snd_kcontrol_new 
aic3x_snd_controls[] = {
         SOC_DOUBLE_R("PGA Capture Switch", LADC_VOL, RADC_VOL, 7, 0x01, 1),

         SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]),
+    /* Additional controls */
+       SOC_ENUM("Mic3L LADC Gain", aic3x_adc_gain_enum[0]),
+       SOC_ENUM("Mic3R RADC Gain", aic3x_adc_gain_enum[1]),
+       SOC_ENUM("Line1L LADC Gain", aic3x_adc_gain_enum[2]),
+       SOC_ENUM("Line1R RADC Gain", aic3x_adc_gain_enum[3]),
+       SOC_ENUM("Line2L LADC Gain", aic3x_adc_gain_enum[4]),
+       SOC_ENUM("Line2R RADC Gain", aic3x_adc_gain_enum[5]),
+    SOC_ENUM("Headset Debounce", aic3x_headset_debounce_enum),
+    SOC_ENUM("Button Debounce", aic3x_button_debounce_enum),
+    SOC_SINGLE("Headset Detected", AIC3X_HEADSET_DETECT_CTRL_A, 
AIC3X_HEADSET_DETECT_SHIFT, AIC3X_HEADSET_DETECT_MASK, 0),
+    SOC_SINGLE("Headset Detect Enable", AIC3X_HEADSET_DETECT_CTRL_A, 7, 
1, 0),
+    SOC_SINGLE("High power output Ac-coupled", 
AIC3X_HEADSET_DETECT_CTRL_B, 7, 1, 0),
+    SOC_SINGLE("Stereo pseudodifferential output", 
AIC3X_HEADSET_DETECT_CTRL_B, 3, 1, 0),
  };

  static const struct snd_kcontrol_new aic3x_mono_controls[] = {
@@ -614,7 +660,7 @@ static const struct snd_soc_dapm_widget 
aic3x_dapm_widgets[] = {
         SND_SOC_DAPM_REG(snd_soc_dapm_micbias, "DMic Rate 32",
                          AIC3X_ASD_INTF_CTRLA, 0, 3, 3, 0),

-       /* Mic Bias */
+    /* Mic Bias */
         SND_SOC_DAPM_SUPPLY("Mic Bias", MICBIAS_CTRL, 6, 0,
                          mic_bias_event,
                          SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
@@ -692,7 +738,7 @@ static const struct snd_soc_dapm_route intercon[] = {
         {"Left Line2L Mux", "single-ended", "LINE2L"},
         {"Left Line2L Mux", "differential", "LINE2L"},

-       {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"},
+    {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"},
         {"Left PGA Mixer", "Line1R Switch", "Left Line1R Mux"},
         {"Left PGA Mixer", "Line2L Switch", "Left Line2L Mux"},
         {"Left PGA Mixer", "Mic3L Switch", "MIC3L"},
@@ -814,6 +860,7 @@ static const struct snd_soc_dapm_route intercon[] = {
         {"Right HPCOM Mux", "external feedback", "Right HPCOM Mixer"},
         {"Right HP Com", NULL, "Right HPCOM Mux"},
         {"HPRCOM", NULL, "Right HP Com"},
+
  };

  static const struct snd_soc_dapm_route intercon_mono[] = {
@@ -918,7 +965,9 @@ static int aic3x_hw_params(struct snd_pcm_substream 
*substream,
         data = (LDAC2LCH | RDAC2RCH);
         data |= (fsref == 44100) ? FSREF_44100 : FSREF_48000;
         if (params_rate(params) >= 64000)
+       {
                 data |= DUAL_RATE_MODE;
+       }
         snd_soc_write(codec, AIC3X_CODEC_DATAPATH_REG, data);

         /* codec sample rate select */
--
and tlv320aic3x.h
---
  sound/soc/codecs/tlv320aic3x.h | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
index e521ac3..8ff42d2 100644
--- a/sound/soc/codecs/tlv320aic3x.h
+++ b/sound/soc/codecs/tlv320aic3x.h
@@ -271,6 +271,12 @@ enum {
         AIC3X_BUTTON_DEBOUNCE_32MS      = 3
  };

+typedef struct {
+    struct platform_device* pdev;
+    struct proc_dir_entry *proc_value;
+    struct snd_soc_codec *codec;
+} aic3106_detect_t;
+
  #define AIC3X_HEADSET_DETECT_ENABLED   0x80
  #define AIC3X_HEADSET_DETECT_SHIFT     5
  #define AIC3X_HEADSET_DETECT_MASK      3
--





-- 
С уважением,
Руководитель проектов
Морские комплексы и системы плюс
Каралдин Т.М.
----------------------------------------------------------------------------------
Best regards,
Projects director
Marine complexes and systems plus
Timur Karaldin

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Peter Ujfalusi]

Hi Timur,

On 03/14/16 13:06, Timur Karaldin wrote:
> Hi, colleagues!
> 
> I'm newbie with ALSA devel. I need to get extra functionality from codec
> through ALSA mixer.
> Task: 1. making possibility to set-up gain for each input (XXX_2_LADCCTRL and
> XXX_2_RADCCTRL registers)

This part is a bit tricky as in the registers:
value 0..8 is gain from 0dB to -12dB in steps of -1.5dB and value 0xf means
basically mute/disconnect.

> 2. making possibility to set-up debounce for button and headset detection (
> HEADSET_DETECT_CTRL_A register)
> 3. making possibility to enable high-power AC-coupled output
> (HEADSET_DETECT_CTRL_A register)
> 4. making possibility to enable stereo pseudo differential output
> (HEADSET_DETECT_CTRL_B register)
> 5. enable headset detection (HEADSET_DETECT_CTRL_A)
> 6. getting headset detect status (HEADSET_DETECT_CTRL_A)
> 
> Well, my patch works in generally, but I don't like it very much, because:
> 1. All controls for input amplifiers' gain are shown in Alsamixer-Playback
> section. But not only mine, capture controls from base codec's version are
> shown in playback section too, for example (Right or Left) PGA Mixer Line1(L
> or R)/2(L or R)/MIC3(L or R), which is definitely capture settings. Should I
> worried about it? because, as I understand, it could not be setted correctly
> during changing playback/capture mode for power-saving reason.
> 2.  "Right PGA Mixer Line1R" control (and similar) using the same bits in
> LINE1R_2_RADCCTRL register as my "Line1R RADC Gain" control (and similar) and
> put there b"1111" when user switch disconnect it from ADC. So my control is
> not shown when user disconnect it from ADC.
> 3. When I start/stop arecord some of my options were reset (debounces, headset
> enable, high-power AC-coupled, pseudo differential output). When I set up it
> in shell and then start arecord, it's reset again. Is it the reason because of
> displaying controls in playback mixer section?
> 
> I will be very pleased for any advise, thank you!
> 
> 
> here is my patch for tlv320aic3x.c
> ---
>  sound/soc/codecs/tlv320aic3x.c | 55 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 52 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
> index d7349bc..27d031e 100644
> --- a/sound/soc/codecs/tlv320aic3x.c
> +++ b/sound/soc/codecs/tlv320aic3x.c
> @@ -122,11 +122,24 @@ static const struct reg_default aic3x_reg[] = {
>         { 108, 0x00 }, { 109, 0x00 },
>  };
> 
> +static bool aic3106_volatile(struct device *dev, unsigned int reg)
> +{
> +    switch(reg)
> +    {
> +        case AIC3X_HEADSET_DETECT_CTRL_A:
> +        case AIC3X_HEADSET_DETECT_CTRL_B:
> +            return true;
> +        default:
> +            return false;
> +    }
> +}
> +
>  static const struct regmap_config aic3x_regmap = {
>         .reg_bits = 8,
>         .val_bits = 8,
> 
>         .max_register = DAC_ICC_ADJ,
> +    .volatile_reg = aic3106_volatile,
>         .reg_defaults = aic3x_reg,
>         .num_reg_defaults = ARRAY_SIZE(aic3x_reg),
>         .cache_type = REGCACHE_RBTREE,
> @@ -196,7 +209,7 @@ static int mic_bias_event(struct snd_soc_dapm_widget *w,
>         struct snd_kcontrol *kcontrol, int event)
>  {
>         struct snd_soc_codec *codec = w->codec;
> -       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
> +    struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);

You have couple of these nop changes in the patch...

> 
>         switch (event) {
>         case SND_SOC_DAPM_POST_PMU:
> @@ -270,6 +283,26 @@ static const struct soc_enum aic3x_agc_decay_enum[] = {
>         SOC_ENUM_SINGLE(RAGC_CTRL_A, 0, 4, aic3x_agc_decay),
>  };
> 
> +static const char *aic3x_adc_gain[] = { "0dB", "-1.5dB", "-3dB", "-4.5dB",
> "-6dB", "-7.5dB", "-9dB", "-10.5dB", "-12dB" };
> +static const struct soc_enum aic3x_adc_gain_enum[] = {
> +       SOC_ENUM_SINGLE(MIC3LR_2_LADC_CTRL, 4, 9, aic3x_adc_gain),
> +       SOC_ENUM_SINGLE(MIC3LR_2_RADC_CTRL, 0, 9, aic3x_adc_gain),
> +       SOC_ENUM_SINGLE(LINE1L_2_LADC_CTRL, 3, 9, aic3x_adc_gain),
> +       SOC_ENUM_SINGLE(LINE1R_2_RADC_CTRL, 3, 9, aic3x_adc_gain),
> +       SOC_ENUM_SINGLE(LINE2L_2_LADC_CTRL, 3, 9, aic3x_adc_gain),
> +       SOC_ENUM_SINGLE(LINE2R_2_RADC_CTRL, 3, 9, aic3x_adc_gain),
> +};

So the issue is that we have the DAPM switches controlling exactly the same
registers. I believe if you would set the gain in a way that bit0 is 1, then
DAPM will think that the path is disconnected.
Also if you would set the gain and then mute and unmute the path you would
have lost the gain you wanted to have..

The only way I can think of implementing these mixers is to have two sets of
custom callbacks. one set is to set/get the gain and the other is to set/get
the mute/disconnect on these.
When the path is disconnected you should not write the gain change to the
chip, but cache it and if the path is unmuted, you write the cached gain. When
you mute the path you should take the set gain first, cache it, then
disconnect the path.

For these gains you should have DECLARE_TLV_DB_SCALE() and use
SOC_SINGLE_TLV(). Make sure that the control name matches with the
corresponding DAPM widget's name so ALSA can match them correctly.

> +static const char *aic3x_headset_debounce[] = { "16ms", "32ms", "64ms",
> "128ms", "256ms", "512ms" };
> +static const struct soc_enum aic3x_headset_debounce_enum[] = {
> +    SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 2, 6, aic3x_headset_debounce),
> +};
> +static const char *aic3x_button_debounce[] = { "0ms", "8ms", "16ms", "32ms" };
> +static const struct soc_enum aic3x_button_debounce_enum[] = {
> +    SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 0, 4, aic3x_button_debounce),
> +};
> +
> +
> +
>  /*
>   * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps
>   */
> @@ -399,6 +432,19 @@ static const struct snd_kcontrol_new aic3x_snd_controls[]
> = {
>         SOC_DOUBLE_R("PGA Capture Switch", LADC_VOL, RADC_VOL, 7, 0x01, 1),
> 
>         SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]),
> +    /* Additional controls */
> +       SOC_ENUM("Mic3L LADC Gain", aic3x_adc_gain_enum[0]),
> +       SOC_ENUM("Mic3R RADC Gain", aic3x_adc_gain_enum[1]),
> +       SOC_ENUM("Line1L LADC Gain", aic3x_adc_gain_enum[2]),
> +       SOC_ENUM("Line1R RADC Gain", aic3x_adc_gain_enum[3]),
> +       SOC_ENUM("Line2L LADC Gain", aic3x_adc_gain_enum[4]),
> +       SOC_ENUM("Line2R RADC Gain", aic3x_adc_gain_enum[5]),

Please avoid using arrays as the driver no longer use these anymore.

> +    SOC_ENUM("Headset Debounce", aic3x_headset_debounce_enum),
> +    SOC_ENUM("Button Debounce", aic3x_button_debounce_enum),
> +    SOC_SINGLE("Headset Detected", AIC3X_HEADSET_DETECT_CTRL_A,
> AIC3X_HEADSET_DETECT_SHIFT, AIC3X_HEADSET_DETECT_MASK, 0),
> +    SOC_SINGLE("Headset Detect Enable", AIC3X_HEADSET_DETECT_CTRL_A, 7, 1, 0),
> +    SOC_SINGLE("High power output Ac-coupled", AIC3X_HEADSET_DETECT_CTRL_B,
> 7, 1, 0),
> +    SOC_SINGLE("Stereo pseudodifferential output",
> AIC3X_HEADSET_DETECT_CTRL_B, 3, 1, 0),
>  };
> 
>  static const struct snd_kcontrol_new aic3x_mono_controls[] = {
> @@ -614,7 +660,7 @@ static const struct snd_soc_dapm_widget
> aic3x_dapm_widgets[] = {
>         SND_SOC_DAPM_REG(snd_soc_dapm_micbias, "DMic Rate 32",
>                          AIC3X_ASD_INTF_CTRLA, 0, 3, 3, 0),
> 
> -       /* Mic Bias */
> +    /* Mic Bias */

nop change

>         SND_SOC_DAPM_SUPPLY("Mic Bias", MICBIAS_CTRL, 6, 0,
>                          mic_bias_event,
>                          SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
> @@ -692,7 +738,7 @@ static const struct snd_soc_dapm_route intercon[] = {
>         {"Left Line2L Mux", "single-ended", "LINE2L"},
>         {"Left Line2L Mux", "differential", "LINE2L"},
> 
> -       {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"},
> +    {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"},

nop change

>         {"Left PGA Mixer", "Line1R Switch", "Left Line1R Mux"},
>         {"Left PGA Mixer", "Line2L Switch", "Left Line2L Mux"},
>         {"Left PGA Mixer", "Mic3L Switch", "MIC3L"},
> @@ -814,6 +860,7 @@ static const struct snd_soc_dapm_route intercon[] = {
>         {"Right HPCOM Mux", "external feedback", "Right HPCOM Mixer"},
>         {"Right HP Com", NULL, "Right HPCOM Mux"},
>         {"HPRCOM", NULL, "Right HP Com"},
> +

why?

>  };
> 
>  static const struct snd_soc_dapm_route intercon_mono[] = {
> @@ -918,7 +965,9 @@ static int aic3x_hw_params(struct snd_pcm_substream
> *substream,
>         data = (LDAC2LCH | RDAC2RCH);
>         data |= (fsref == 44100) ? FSREF_44100 : FSREF_48000;
>         if (params_rate(params) >= 64000)
> +       {
>                 data |= DUAL_RATE_MODE;
> +       }

why?

>         snd_soc_write(codec, AIC3X_CODEC_DATAPATH_REG, data);
> 
>         /* codec sample rate select */
> -- 
> and tlv320aic3x.h
> ---
>  sound/soc/codecs/tlv320aic3x.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
> index e521ac3..8ff42d2 100644
> --- a/sound/soc/codecs/tlv320aic3x.h
> +++ b/sound/soc/codecs/tlv320aic3x.h
> @@ -271,6 +271,12 @@ enum {
>         AIC3X_BUTTON_DEBOUNCE_32MS      = 3
>  };
> 
> +typedef struct {
> +    struct platform_device* pdev;
> +    struct proc_dir_entry *proc_value;
> +    struct snd_soc_codec *codec;
> +} aic3106_detect_t;
> +
>  #define AIC3X_HEADSET_DETECT_ENABLED   0x80
>  #define AIC3X_HEADSET_DETECT_SHIFT     5
>  #define AIC3X_HEADSET_DETECT_MASK      3
> -- 


When submitting please separate the patches by functionality. It is easier to
review the changes.

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Peter Ujfalusi]

On 03/14/2016 04:57 PM, Timur Karaldin wrote:
> Hi, Peter!
> What's about placing capture controls in the Playback section of Alsamixer
> view?

Yeah, it is annoying. They can be fixed by:
"Line1L Switch" -> "Line1L Capture Switch" and so on. On the other hand we had
these unfortunate control names I think since the beginning and it is kind of
an ABI for existing products. I don't have issue changing them, but existing
setups relying on the control names might break...

> Does resetting of some registers during start/stop of recording
> connect with it?

Not sure what you mean.

-- 
Péter

> 
> 14.03.2016 17:47, Peter Ujfalusi пишет:
>> Hi Timur,
>>
>> On 03/14/16 13:06, Timur Karaldin wrote:
>>> Hi, colleagues!
>>>
>>> I'm newbie with ALSA devel. I need to get extra functionality from codec
>>> through ALSA mixer.
>>> Task: 1. making possibility to set-up gain for each input (XXX_2_LADCCTRL and
>>> XXX_2_RADCCTRL registers)
>> This part is a bit tricky as in the registers:
>> value 0..8 is gain from 0dB to -12dB in steps of -1.5dB and value 0xf means
>> basically mute/disconnect.
>>
>>> 2. making possibility to set-up debounce for button and headset detection (
>>> HEADSET_DETECT_CTRL_A register)
>>> 3. making possibility to enable high-power AC-coupled output
>>> (HEADSET_DETECT_CTRL_A register)
>>> 4. making possibility to enable stereo pseudo differential output
>>> (HEADSET_DETECT_CTRL_B register)
>>> 5. enable headset detection (HEADSET_DETECT_CTRL_A)
>>> 6. getting headset detect status (HEADSET_DETECT_CTRL_A)
>>>
>>> Well, my patch works in generally, but I don't like it very much, because:
>>> 1. All controls for input amplifiers' gain are shown in Alsamixer-Playback
>>> section. But not only mine, capture controls from base codec's version are
>>> shown in playback section too, for example (Right or Left) PGA Mixer Line1(L
>>> or R)/2(L or R)/MIC3(L or R), which is definitely capture settings. Should I
>>> worried about it? because, as I understand, it could not be setted correctly
>>> during changing playback/capture mode for power-saving reason.
>>> 2.  "Right PGA Mixer Line1R" control (and similar) using the same bits in
>>> LINE1R_2_RADCCTRL register as my "Line1R RADC Gain" control (and similar) and
>>> put there b"1111" when user switch disconnect it from ADC. So my control is
>>> not shown when user disconnect it from ADC.
>>> 3. When I start/stop arecord some of my options were reset (debounces, headset
>>> enable, high-power AC-coupled, pseudo differential output). When I set up it
>>> in shell and then start arecord, it's reset again. Is it the reason because of
>>> displaying controls in playback mixer section?
>>>
>>> I will be very pleased for any advise, thank you!
>>>
>>>
>>> here is my patch for tlv320aic3x.c
>>> ---
>>>   sound/soc/codecs/tlv320aic3x.c | 55
>>> +++++++++++++++++++++++++++++++++++++++---
>>>   1 file changed, 52 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
>>> index d7349bc..27d031e 100644
>>> --- a/sound/soc/codecs/tlv320aic3x.c
>>> +++ b/sound/soc/codecs/tlv320aic3x.c
>>> @@ -122,11 +122,24 @@ static const struct reg_default aic3x_reg[] = {
>>>          { 108, 0x00 }, { 109, 0x00 },
>>>   };
>>>
>>> +static bool aic3106_volatile(struct device *dev, unsigned int reg)
>>> +{
>>> +    switch(reg)
>>> +    {
>>> +        case AIC3X_HEADSET_DETECT_CTRL_A:
>>> +        case AIC3X_HEADSET_DETECT_CTRL_B:
>>> +            return true;
>>> +        default:
>>> +            return false;
>>> +    }
>>> +}
>>> +
>>>   static const struct regmap_config aic3x_regmap = {
>>>          .reg_bits = 8,
>>>          .val_bits = 8,
>>>
>>>          .max_register = DAC_ICC_ADJ,
>>> +    .volatile_reg = aic3106_volatile,
>>>          .reg_defaults = aic3x_reg,
>>>          .num_reg_defaults = ARRAY_SIZE(aic3x_reg),
>>>          .cache_type = REGCACHE_RBTREE,
>>> @@ -196,7 +209,7 @@ static int mic_bias_event(struct snd_soc_dapm_widget *w,
>>>          struct snd_kcontrol *kcontrol, int event)
>>>   {
>>>          struct snd_soc_codec *codec = w->codec;
>>> -       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
>>> +    struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
>> You have couple of these nop changes in the patch...
>>
>>>          switch (event) {
>>>          case SND_SOC_DAPM_POST_PMU:
>>> @@ -270,6 +283,26 @@ static const struct soc_enum aic3x_agc_decay_enum[] = {
>>>          SOC_ENUM_SINGLE(RAGC_CTRL_A, 0, 4, aic3x_agc_decay),
>>>   };
>>>
>>> +static const char *aic3x_adc_gain[] = { "0dB", "-1.5dB", "-3dB", "-4.5dB",
>>> "-6dB", "-7.5dB", "-9dB", "-10.5dB", "-12dB" };
>>> +static const struct soc_enum aic3x_adc_gain_enum[] = {
>>> +       SOC_ENUM_SINGLE(MIC3LR_2_LADC_CTRL, 4, 9, aic3x_adc_gain),
>>> +       SOC_ENUM_SINGLE(MIC3LR_2_RADC_CTRL, 0, 9, aic3x_adc_gain),
>>> +       SOC_ENUM_SINGLE(LINE1L_2_LADC_CTRL, 3, 9, aic3x_adc_gain),
>>> +       SOC_ENUM_SINGLE(LINE1R_2_RADC_CTRL, 3, 9, aic3x_adc_gain),
>>> +       SOC_ENUM_SINGLE(LINE2L_2_LADC_CTRL, 3, 9, aic3x_adc_gain),
>>> +       SOC_ENUM_SINGLE(LINE2R_2_RADC_CTRL, 3, 9, aic3x_adc_gain),
>>> +};
>> So the issue is that we have the DAPM switches controlling exactly the same
>> registers. I believe if you would set the gain in a way that bit0 is 1, then
>> DAPM will think that the path is disconnected.
>> Also if you would set the gain and then mute and unmute the path you would
>> have lost the gain you wanted to have..
>>
>> The only way I can think of implementing these mixers is to have two sets of
>> custom callbacks. one set is to set/get the gain and the other is to set/get
>> the mute/disconnect on these.
>> When the path is disconnected you should not write the gain change to the
>> chip, but cache it and if the path is unmuted, you write the cached gain. When
>> you mute the path you should take the set gain first, cache it, then
>> disconnect the path.
>>
>> For these gains you should have DECLARE_TLV_DB_SCALE() and use
>> SOC_SINGLE_TLV(). Make sure that the control name matches with the
>> corresponding DAPM widget's name so ALSA can match them correctly.
>>
>>> +static const char *aic3x_headset_debounce[] = { "16ms", "32ms", "64ms",
>>> "128ms", "256ms", "512ms" };
>>> +static const struct soc_enum aic3x_headset_debounce_enum[] = {
>>> +    SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 2, 6,
>>> aic3x_headset_debounce),
>>> +};
>>> +static const char *aic3x_button_debounce[] = { "0ms", "8ms", "16ms",
>>> "32ms" };
>>> +static const struct soc_enum aic3x_button_debounce_enum[] = {
>>> +    SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 0, 4,
>>> aic3x_button_debounce),
>>> +};
>>> +
>>> +
>>> +
>>>   /*
>>>    * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps
>>>    */
>>> @@ -399,6 +432,19 @@ static const struct snd_kcontrol_new aic3x_snd_controls[]
>>> = {
>>>          SOC_DOUBLE_R("PGA Capture Switch", LADC_VOL, RADC_VOL, 7, 0x01, 1),
>>>
>>>          SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]),
>>> +    /* Additional controls */
>>> +       SOC_ENUM("Mic3L LADC Gain", aic3x_adc_gain_enum[0]),
>>> +       SOC_ENUM("Mic3R RADC Gain", aic3x_adc_gain_enum[1]),
>>> +       SOC_ENUM("Line1L LADC Gain", aic3x_adc_gain_enum[2]),
>>> +       SOC_ENUM("Line1R RADC Gain", aic3x_adc_gain_enum[3]),
>>> +       SOC_ENUM("Line2L LADC Gain", aic3x_adc_gain_enum[4]),
>>> +       SOC_ENUM("Line2R RADC Gain", aic3x_adc_gain_enum[5]),
>> Please avoid using arrays as the driver no longer use these anymore.
>>
>>> +    SOC_ENUM("Headset Debounce", aic3x_headset_debounce_enum),
>>> +    SOC_ENUM("Button Debounce", aic3x_button_debounce_enum),
>>> +    SOC_SINGLE("Headset Detected", AIC3X_HEADSET_DETECT_CTRL_A,
>>> AIC3X_HEADSET_DETECT_SHIFT, AIC3X_HEADSET_DETECT_MASK, 0),
>>> +    SOC_SINGLE("Headset Detect Enable", AIC3X_HEADSET_DETECT_CTRL_A, 7, 1,
>>> 0),
>>> +    SOC_SINGLE("High power output Ac-coupled", AIC3X_HEADSET_DETECT_CTRL_B,
>>> 7, 1, 0),
>>> +    SOC_SINGLE("Stereo pseudodifferential output",
>>> AIC3X_HEADSET_DETECT_CTRL_B, 3, 1, 0),
>>>   };
>>>
>>>   static const struct snd_kcontrol_new aic3x_mono_controls[] = {
>>> @@ -614,7 +660,7 @@ static const struct snd_soc_dapm_widget
>>> aic3x_dapm_widgets[] = {
>>>          SND_SOC_DAPM_REG(snd_soc_dapm_micbias, "DMic Rate 32",
>>>                           AIC3X_ASD_INTF_CTRLA, 0, 3, 3, 0),
>>>
>>> -       /* Mic Bias */
>>> +    /* Mic Bias */
>> nop change
>>
>>>          SND_SOC_DAPM_SUPPLY("Mic Bias", MICBIAS_CTRL, 6, 0,
>>>                           mic_bias_event,
>>>                           SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
>>> @@ -692,7 +738,7 @@ static const struct snd_soc_dapm_route intercon[] = {
>>>          {"Left Line2L Mux", "single-ended", "LINE2L"},
>>>          {"Left Line2L Mux", "differential", "LINE2L"},
>>>
>>> -       {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"},
>>> +    {"Left PGA Mixer", "Line1L Switch", "Left Line1L Mux"},
>> nop change
>>
>>>          {"Left PGA Mixer", "Line1R Switch", "Left Line1R Mux"},
>>>          {"Left PGA Mixer", "Line2L Switch", "Left Line2L Mux"},
>>>          {"Left PGA Mixer", "Mic3L Switch", "MIC3L"},
>>> @@ -814,6 +860,7 @@ static const struct snd_soc_dapm_route intercon[] = {
>>>          {"Right HPCOM Mux", "external feedback", "Right HPCOM Mixer"},
>>>          {"Right HP Com", NULL, "Right HPCOM Mux"},
>>>          {"HPRCOM", NULL, "Right HP Com"},
>>> +
>> why?
>>
>>>   };
>>>
>>>   static const struct snd_soc_dapm_route intercon_mono[] = {
>>> @@ -918,7 +965,9 @@ static int aic3x_hw_params(struct snd_pcm_substream
>>> *substream,
>>>          data = (LDAC2LCH | RDAC2RCH);
>>>          data |= (fsref == 44100) ? FSREF_44100 : FSREF_48000;
>>>          if (params_rate(params) >= 64000)
>>> +       {
>>>                  data |= DUAL_RATE_MODE;
>>> +       }
>> why?
>>
>>>          snd_soc_write(codec, AIC3X_CODEC_DATAPATH_REG, data);
>>>
>>>          /* codec sample rate select */
>>> -- 
>>> and tlv320aic3x.h
>>> ---
>>>   sound/soc/codecs/tlv320aic3x.h | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
>>> index e521ac3..8ff42d2 100644
>>> --- a/sound/soc/codecs/tlv320aic3x.h
>>> +++ b/sound/soc/codecs/tlv320aic3x.h
>>> @@ -271,6 +271,12 @@ enum {
>>>          AIC3X_BUTTON_DEBOUNCE_32MS      = 3
>>>   };
>>>
>>> +typedef struct {
>>> +    struct platform_device* pdev;
>>> +    struct proc_dir_entry *proc_value;
>>> +    struct snd_soc_codec *codec;
>>> +} aic3106_detect_t;
>>> +
>>>   #define AIC3X_HEADSET_DETECT_ENABLED   0x80
>>>   #define AIC3X_HEADSET_DETECT_SHIFT     5
>>>   #define AIC3X_HEADSET_DETECT_MASK      3
>>> -- 
>>
>> When submitting please separate the patches by functionality. It is easier to
>> review the changes.
>>
> 
> 

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Timur Karaldin]

14.03.2016 19:38, Peter Ujfalusi пишет:
> On 03/14/2016 04:57 PM, Timur Karaldin wrote:
>> Hi, Peter!
>> What's about placing capture controls in the Playback section of Alsamixer
>> view?
> Yeah, it is annoying. They can be fixed by:
> "Line1L Switch" -> "Line1L Capture Switch" and so on. On the other hand we had
> these unfortunate control names I think since the beginning and it is kind of
> an ABI for existing products. I don't have issue changing them, but existing
> setups relying on the control names might break...
>
>> Does resetting of some registers during start/stop of recording
>> connect with it?
> Not sure what you mean.
>
I mean current situation:

#i2cdump -f -y -r 13-13 1 0x1b
Result: 0x00
#arecord test.wav &
#i2cdump -f -y -r 13-13 1 0x1b
Result: 0x00
# amixer sset 'Headset Detect Enable' *on*
#i2cdump -f -y -r 13-13 1 0x1b
Result:*0x80*
#killall arecord
#i2cdump -f -y -r 13-13 1 0x1b
Result: 0x00
#arecord test.wav &
#i2cdump -f -y -r 13-13 1 0x1b
Result:*0x00
*#killall arecord*
*amixer sset 'Headset Detect Enable'*on**
*#i2cdump -f -y -r 13-13 1 0x1b
Result:**0x00
**#amixer sset 'Headset Detect Enable' off
*amixer: Invalid command!*

So state of 'Headset Detect Enable' and others of my controls were not 
stored during arecord/aplay restart (open and close device).







_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Peter Ujfalusi]

On 03/15/16 12:17, Timur Karaldin wrote:
> 14.03.2016 19:38, Peter Ujfalusi пишет:
>> On 03/14/2016 04:57 PM, Timur Karaldin wrote:
>>> Hi, Peter!
>>> What's about placing capture controls in the Playback section of Alsamixer
>>> view?
>> Yeah, it is annoying. They can be fixed by:
>> "Line1L Switch" -> "Line1L Capture Switch" and so on. On the other hand we had
>> these unfortunate control names I think since the beginning and it is kind of
>> an ABI for existing products. I don't have issue changing them, but existing
>> setups relying on the control names might break...
>>
>>> Does resetting of some registers during start/stop of recording
>>> connect with it?
>> Not sure what you mean.
>>
> I mean current situation:
> 
> #i2cdump -f -y -r 13-13 1 0x1b
> Result: 0x00
> #arecord test.wav &
> #i2cdump -f -y -r 13-13 1 0x1b
> Result: 0x00
> # amixer sset 'Headset Detect Enable' *on*
> #i2cdump -f -y -r 13-13 1 0x1b
> Result:*0x80*
> #killall arecord
> #i2cdump -f -y -r 13-13 1 0x1b
> Result: 0x00
> #arecord test.wav &
> #i2cdump -f -y -r 13-13 1 0x1b
> Result:*0x00
> *#killall arecord*
> *amixer sset 'Headset Detect Enable'*on**
> *#i2cdump -f -y -r 13-13 1 0x1b
> Result:**0x00
> **#amixer sset 'Headset Detect Enable' off
> *amixer: Invalid command!*
> 
> So state of 'Headset Detect Enable' and others of my controls were not stored
> during arecord/aplay restart (open and close device).

aic3x_set_power() will soft reset the codec on power off which resets also the
headset detect registers.

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Timur Karaldin]

Hi Peter!

14.03.2016 17:47, Peter Ujfalusi пишет:
> So the issue is that we have the DAPM switches controlling exactly the 
> same registers. I believe if you would set the gain in a way that bit0 
> is 1, then DAPM will think that the path is disconnected. Also if you 
> would set the gain and then mute and unmute the path you would have 
> lost the gain you wanted to have.. The only way I can think of 
> implementing these mixers is to have two sets of custom callbacks. one 
> set is to set/get the gain and the other is to set/get the 
> mute/disconnect on these. When the path is disconnected you should not 
> write the gain change to the chip, but cache it and if the path is 
> unmuted, you write the cached gain. When you mute the path you should 
> take the set gain first, cache it, then disconnect the path. For these 
> gains you should have DECLARE_TLV_DB_SCALE() and use SOC_SINGLE_TLV(). 
> Make sure that the control name matches with the corresponding DAPM 
> widget's name so ALSA can match them correctly. 
That's not clear for me. As I understand, I need to create 
SOC_SINGLE_TLV() for each gain I would like to change, with the same 
name as SOC_DAPM_SINGLE_AIC3x (for example "Line1L Switch" for Line1) 
and make callbacks with cached gain values. Am I right?
May be the better sollution is to change SOC_DAPM_SINGLE_AIC3x on/off 
switch to other type with switch and gain control together? So it could 
have some connected states with different gains and disconnected state 
(Something like volume/mute control).

Cheers, Tim
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Peter Ujfalusi]

On 03/15/16 14:12, Timur Karaldin wrote:
> Hi Peter!
> 
> 14.03.2016 17:47, Peter Ujfalusi пишет:
>> So the issue is that we have the DAPM switches controlling exactly the same
>> registers. I believe if you would set the gain in a way that bit0 is 1, then
>> DAPM will think that the path is disconnected. Also if you would set the
>> gain and then mute and unmute the path you would have lost the gain you
>> wanted to have.. The only way I can think of implementing these mixers is to
>> have two sets of custom callbacks. one set is to set/get the gain and the
>> other is to set/get the mute/disconnect on these. When the path is
>> disconnected you should not write the gain change to the chip, but cache it
>> and if the path is unmuted, you write the cached gain. When you mute the
>> path you should take the set gain first, cache it, then disconnect the path.
>> For these gains you should have DECLARE_TLV_DB_SCALE() and use
>> SOC_SINGLE_TLV(). Make sure that the control name matches with the
>> corresponding DAPM widget's name so ALSA can match them correctly. 
> That's not clear for me. As I understand, I need to create SOC_SINGLE_TLV()
> for each gain I would like to change, with the same name as
> SOC_DAPM_SINGLE_AIC3x (for example "Line1L Switch" for Line1) and make
> callbacks with cached gain values. Am I right?

Mostly yes, but you need to name the controls:
existing switch + new gain control
"Line1L Switch" + "Line1L Volume"
"Line1R Switch" + "Line1R Volume"

In this way alsamixer can combine them.

> May be the better sollution is to change SOC_DAPM_SINGLE_AIC3x on/off switch
> to other type with switch and gain control together? So it could have some
> connected states with different gains and disconnected state (Something like
> volume/mute control).

Something based on SOC_DAPM_SINGLE_TLV should work. But that means again
change in Kcontrol names.

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Timur Karaldin]

Ok, I choose the way #1 without changing kcontrol names.
Another thing I don't understand is the way for caching gain volume if I 
have switches muted. Is there any proper way for storing cached values 
between gain and switch handlers includes soft reset or I need manually 
check kcontrol name in handler and cache values in extended private 
structure? The same question for switch handler, because I need to share 
cached values between these handlers.

15.03.2016 15:40, Peter Ujfalusi пишет:
> On 03/15/16 14:12, Timur Karaldin wrote:
>> Hi Peter!
>>
>> 14.03.2016 17:47, Peter Ujfalusi пишет:
>>> So the issue is that we have the DAPM switches controlling exactly the same
>>> registers. I believe if you would set the gain in a way that bit0 is 1, then
>>> DAPM will think that the path is disconnected. Also if you would set the
>>> gain and then mute and unmute the path you would have lost the gain you
>>> wanted to have.. The only way I can think of implementing these mixers is to
>>> have two sets of custom callbacks. one set is to set/get the gain and the
>>> other is to set/get the mute/disconnect on these. When the path is
>>> disconnected you should not write the gain change to the chip, but cache it
>>> and if the path is unmuted, you write the cached gain. When you mute the
>>> path you should take the set gain first, cache it, then disconnect the path.
>>> For these gains you should have DECLARE_TLV_DB_SCALE() and use
>>> SOC_SINGLE_TLV(). Make sure that the control name matches with the
>>> corresponding DAPM widget's name so ALSA can match them correctly.
>> That's not clear for me. As I understand, I need to create SOC_SINGLE_TLV()
>> for each gain I would like to change, with the same name as
>> SOC_DAPM_SINGLE_AIC3x (for example "Line1L Switch" for Line1) and make
>> callbacks with cached gain values. Am I right?
> Mostly yes, but you need to name the controls:
> existing switch + new gain control
> "Line1L Switch" + "Line1L Volume"
> "Line1R Switch" + "Line1R Volume"
>
> In this way alsamixer can combine them.
>
>> May be the better sollution is to change SOC_DAPM_SINGLE_AIC3x on/off switch
>> to other type with switch and gain control together? So it could have some
>> connected states with different gains and disconnected state (Something like
>> volume/mute control).
> Something based on SOC_DAPM_SINGLE_TLV should work. But that means again
> change in Kcontrol names.
>


-- 
С уважением,
Руководитель проектов
Морские комплексы и системы плюс
Каралдин Т.М.
----------------------------------------------------------------------------------
Best regards,
Projects director
Marine complexes and systems plus
Timur Karaldin

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Peter Ujfalusi]

Hi Timur,

please avoid top posting.

On 03/15/16 18:13, Timur Karaldin wrote:
> Ok, I choose the way #1 without changing kcontrol names.
> Another thing I don't understand is the way for caching gain volume if I have
> switches muted. Is there any proper way for storing cached values between gain
> and switch handlers includes soft reset or I need manually check kcontrol name
> in handler and cache values in extended private structure? The same question
> for switch handler, because I need to share cached values between these handlers.

Since the problematic registers are: 17, 18, .. 24. I would have an array
inside struct aic3x_priv to store the configured gain. You could store the
mute as well, but since regmap is caching the content I would simply read back
the gain value and if it is 0xf, it is muted, so update only the local shadow,
if it is not muted, write the gain.
In the mute/unmute controls I would then save the gain to shadow when muting
the path and write the cached gain to the register when unmuting it. You
should also return correct 0/1 for the unmute state and not the actual
register value.

> 
> 15.03.2016 15:40, Peter Ujfalusi пишет:
>> On 03/15/16 14:12, Timur Karaldin wrote:
>>> Hi Peter!
>>>
>>> 14.03.2016 17:47, Peter Ujfalusi пишет:
>>>> So the issue is that we have the DAPM switches controlling exactly the same
>>>> registers. I believe if you would set the gain in a way that bit0 is 1, then
>>>> DAPM will think that the path is disconnected. Also if you would set the
>>>> gain and then mute and unmute the path you would have lost the gain you
>>>> wanted to have.. The only way I can think of implementing these mixers is to
>>>> have two sets of custom callbacks. one set is to set/get the gain and the
>>>> other is to set/get the mute/disconnect on these. When the path is
>>>> disconnected you should not write the gain change to the chip, but cache it
>>>> and if the path is unmuted, you write the cached gain. When you mute the
>>>> path you should take the set gain first, cache it, then disconnect the path.
>>>> For these gains you should have DECLARE_TLV_DB_SCALE() and use
>>>> SOC_SINGLE_TLV(). Make sure that the control name matches with the
>>>> corresponding DAPM widget's name so ALSA can match them correctly.
>>> That's not clear for me. As I understand, I need to create SOC_SINGLE_TLV()
>>> for each gain I would like to change, with the same name as
>>> SOC_DAPM_SINGLE_AIC3x (for example "Line1L Switch" for Line1) and make
>>> callbacks with cached gain values. Am I right?
>> Mostly yes, but you need to name the controls:
>> existing switch + new gain control
>> "Line1L Switch" + "Line1L Volume"
>> "Line1R Switch" + "Line1R Volume"
>>
>> In this way alsamixer can combine them.
>>
>>> May be the better sollution is to change SOC_DAPM_SINGLE_AIC3x on/off switch
>>> to other type with switch and gain control together? So it could have some
>>> connected states with different gains and disconnected state (Something like
>>> volume/mute control).
>> Something based on SOC_DAPM_SINGLE_TLV should work. But that means again
>> change in Kcontrol names.
>>
> 
> 


-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Timur Karaldin]

Hi Peter!

16.03.2016 11:57, Peter Ujfalusi пишет:
> Hi Timur,
>
> please avoid top posting.
>
> On 03/15/16 18:13, Timur Karaldin wrote:
>> Ok, I choose the way #1 without changing kcontrol names.
>> Another thing I don't understand is the way for caching gain volume if I have
>> switches muted. Is there any proper way for storing cached values between gain
>> and switch handlers includes soft reset or I need manually check kcontrol name
>> in handler and cache values in extended private structure? The same question
>> for switch handler, because I need to share cached values between these handlers.
> Since the problematic registers are: 17, 18, .. 24. I would have an array
> inside struct aic3x_priv to store the configured gain. You could store the
> mute as well, but since regmap is caching the content I would simply read back
> the gain value and if it is 0xf, it is muted, so update only the local shadow,
> if it is not muted, write the gain.
> In the mute/unmute controls I would then save the gain to shadow when muting
> the path and write the cached gain to the register when unmuting it. You
> should also return correct 0/1 for the unmute state and not the actual
> register value.
Actually I try to avoid complex handler, with switch-case which depends 
on reg number or kcontrol name inside of it.
I have found similar discussion with you about one year ago here 
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-February/087430.html. 
There is a proposal to use TLV_DB_RANGE_HEAD() and TLV_DB_SCALE_ITEM() 
because of invert with SOC_DAPM_SINGLE_TLV. As I understand we could 
make tlv with using 0 as mute, skip values 1-6, use values 7-15 (7 = 
-12dB and 15=0dB) then declare SOC_DAPM_SINGLE_TLV as "Line1L Volume" 
for example. Is it still actual? Few things which I didn't understand:
- should I use "Line1L Switch" together with "Line1L Volume" in this 
case. I think no, because DAPM_SINGLE_TLV should replace switch.
- i don't understand how declare tlv with skipping values 1-6

Cheers, Tim
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Peter Ujfalusi]

On 03/16/2016 12:26 PM, Timur Karaldin wrote:
> Hi Peter!
> 
> 16.03.2016 11:57, Peter Ujfalusi пишет:
>> Hi Timur,
>>
>> please avoid top posting.
>>
>> On 03/15/16 18:13, Timur Karaldin wrote:
>>> Ok, I choose the way #1 without changing kcontrol names.
>>> Another thing I don't understand is the way for caching gain volume if I have
>>> switches muted. Is there any proper way for storing cached values between gain
>>> and switch handlers includes soft reset or I need manually check kcontrol name
>>> in handler and cache values in extended private structure? The same question
>>> for switch handler, because I need to share cached values between these
>>> handlers.
>> Since the problematic registers are: 17, 18, .. 24. I would have an array
>> inside struct aic3x_priv to store the configured gain. You could store the
>> mute as well, but since regmap is caching the content I would simply read back
>> the gain value and if it is 0xf, it is muted, so update only the local shadow,
>> if it is not muted, write the gain.
>> In the mute/unmute controls I would then save the gain to shadow when muting
>> the path and write the cached gain to the register when unmuting it. You
>> should also return correct 0/1 for the unmute state and not the actual
>> register value.

your mail client does not seem to wrap the lines correctly, can you check that.

> Actually I try to avoid complex handler, with switch-case which depends on reg
> number or kcontrol name inside of it.
> I have found similar discussion with you about one year ago here
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-February/087430.html. There
> is a proposal to use TLV_DB_RANGE_HEAD() and TLV_DB_SCALE_ITEM() because of
> invert with SOC_DAPM_SINGLE_TLV.

As I recall there were some issues with that back then, but can not recall. At
least I don't see followup patches regarding to that.

> As I understand we could make tlv with using
> 0 as mute, skip values 1-6, use values 7-15 (7 = -12dB and 15=0dB) then
> declare SOC_DAPM_SINGLE_TLV as "Line1L Volume" for example. Is it still
> actual?

Worth a try ;)
The naming needs to be different, take a look at twl4030 codec's
twl4030_dapm_dbypass_tlv and how it is used.

> Few things which I didn't understand:
> - should I use "Line1L Switch" together with "Line1L Volume" in this case. I
> think no, because DAPM_SINGLE_TLV should replace switch.

Since we will have the gain and mute with volume control we will have only one
control.

> - i don't understand how declare tlv with skipping values 1-6

Yeah, I'm not sure how leaving 'hole' between the TLV_DB_SCALE_ITEMs will be
handled if it works.

> 
> Cheers, Tim


-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Timur Karaldin]

16.03.2016 18:23, Peter Ujfalusi пишет:
> On 03/16/2016 12:26 PM, Timur Karaldin wrote:
>> Hi Peter!
>>
>> 16.03.2016 11:57, Peter Ujfalusi пишет:
>>> Hi Timur,
>>>
>>> please avoid top posting.
>>>
>>> On 03/15/16 18:13, Timur Karaldin wrote:
>>>> Ok, I choose the way #1 without changing kcontrol names.
>>>> Another thing I don't understand is the way for caching gain volume if I have
>>>> switches muted. Is there any proper way for storing cached values between gain
>>>> and switch handlers includes soft reset or I need manually check kcontrol name
>>>> in handler and cache values in extended private structure? The same question
>>>> for switch handler, because I need to share cached values between these
>>>> handlers.
>>> Since the problematic registers are: 17, 18, .. 24. I would have an array
>>> inside struct aic3x_priv to store the configured gain. You could store the
>>> mute as well, but since regmap is caching the content I would simply read back
>>> the gain value and if it is 0xf, it is muted, so update only the local shadow,
>>> if it is not muted, write the gain.
>>> In the mute/unmute controls I would then save the gain to shadow when muting
>>> the path and write the cached gain to the register when unmuting it. You
>>> should also return correct 0/1 for the unmute state and not the actual
>>> register value.
> your mail client does not seem to wrap the lines correctly, can you check that.
I have no idea how these lines should looks, so it's very hard for me to 
see what's wrong. Could you point me how it looks in original?
>
>> Actually I try to avoid complex handler, with switch-case which depends on reg
>> number or kcontrol name inside of it.
>> I have found similar discussion with you about one year ago here
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-February/087430.html. There
>> is a proposal to use TLV_DB_RANGE_HEAD() and TLV_DB_SCALE_ITEM() because of
>> invert with SOC_DAPM_SINGLE_TLV.
> As I recall there were some issues with that back then, but can not recall. At
> least I don't see followup patches regarding to that.
>
>> As I understand we could make tlv with using
>> 0 as mute, skip values 1-6, use values 7-15 (7 = -12dB and 15=0dB) then
>> declare SOC_DAPM_SINGLE_TLV as "Line1L Volume" for example. Is it still
>> actual?
> Worth a try ;)
> The naming needs to be different, take a look at twl4030 codec's
> twl4030_dapm_dbypass_tlv and how it is used.
>
>> Few things which I didn't understand:
>> - should I use "Line1L Switch" together with "Line1L Volume" in this case. I
>> think no, because DAPM_SINGLE_TLV should replace switch.
> Since we will have the gain and mute with volume control we will have only one
> control.
>
>> - i don't understand how declare tlv with skipping values 1-6
> Yeah, I'm not sure how leaving 'hole' between the TLV_DB_SCALE_ITEMs will be
> handled if it works.
>
>> Cheers, Tim
Ok, now it's much more clear for me.
Another question is register behaviour during soft reset. There is 
"aic3x_set_power" handle. In this handle kernel makes SOFT_RESET, markes 
cache as dirty, then power down the codec for handle power down request.
But as I could see main volumes like "HP DAC" and "PCM" stores values 
between close and open in mixer and I could not see any code to handle 
it. On the other hand my controls do not save states, as you mentioned 
because of SOFT_RESET, could you explain such different behaviour?

Cheers,
Tim
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Peter Ujfalusi]

On 03/16/16 18:53, Timur Karaldin wrote:
>> your mail client does not seem to wrap the lines correctly, can you check that.
> I have no idea how these lines should looks, so it's very hard for me to see
> what's wrong. Could you point me how it looks in original?

They are looong.

see: https://wiki.openstack.org/wiki/MailingListEtiquette

> Ok, now it's much more clear for me.
> Another question is register behaviour during soft reset. There is
> "aic3x_set_power" handle. In this handle kernel makes SOFT_RESET, markes cache
> as dirty, then power down the codec for handle power down request.
> But as I could see main volumes like "HP DAC" and "PCM" stores values between
> close and open in mixer and I could not see any code to handle it. On the
> other hand my controls do not save states, as you mentioned because of
> SOFT_RESET, could you explain such different behaviour?

All cached registers are going to be restored after power on with exception of
volatile registers. You need to restore the bits in a volatile registers in
the driver.

-- 
Péter

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Timur Karaldin]

Hi Peter!

17.03.2016 12:26, Peter Ujfalusi пишет:
>> Ok, now it's much more clear for me.
>> Another question is register behaviour during soft reset. There is
>> "aic3x_set_power" handle. In this handle kernel makes SOFT_RESET, markes cache
>> as dirty, then power down the codec for handle power down request.
>> But as I could see main volumes like "HP DAC" and "PCM" stores values between
>> close and open in mixer and I could not see any code to handle it. On the
>> other hand my controls do not save states, as you mentioned because of
>> SOFT_RESET, could you explain such different behaviour?

> All cached registers are going to be restored after power on with exception of
> volatile registers. You need to restore the bits in a volatile registers in
> the driver.
>
I'm very close to patch v2,  but I have some problems with 
misunderstanding the key moment with dapm handler 
snd_soc_dapm_put_volsw_aic3x I think.
I successfuly wrote put/get handlers for DOUBLE_R_EXT_TLV and 
SINGLE_EXT_TLV volume controls using snd_kcontrol_chip and 
snd_soc_codec_get_drvdata function for accessing codec's private data. 
It's work. But any attempts in snd_soc_dapm_put_volsw_aic3x to get priv 
data aix3x_priv* make kernel error:  Unable to handle kernel NULL 
pointer dereference at virtual address 0000009e. If I remove 
snd_soc_codec_get_drvdata call and accessing to cached regs it's become 
normal again.
Currently I use kernel 3.15 and there is some example in wm8903.c

static int wm8903_class_w_put(struct snd_kcontrol *kcontrol,
                               struct snd_ctl_elem_value *ucontrol)
{
          struct snd_soc_codec *codec = 
snd_soc_dapm_kcontrol_codec(kcontrol);
          struct wm8903_priv *wm8903 = snd_soc_codec_get_drvdata(codec);
         ...
and it's look ok, and I hope it's working, but not good for me in 
snd_soc_dapm_put_volsw_aic3x handler.
Do you have any idea or any tips I should keep in mind, when working in 
DAPM context?



Cheers, Tim
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Timur Karaldin]

Hi Peter!
Here is my patch v2. It works only for aic3106, but I don't have any 
others. Also patch contains some debug output, I would clean it. I like 
behaviour of this improvements and I didn't know any issue right now for 
these add-ons except debug output and that it's correct only for aic3106 
(didn't check for others even on datasheet level). Thank you for all 
your help!
-----------------------------------------------------------------------
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index d7349bc..2c06e21 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -89,6 +89,7 @@ struct aic3x_priv {

         /* Selects the micbias voltage */
         enum aic3x_micbias_voltage micbias_vg;
+       unsigned char cached_gain[LINE1L_2_RADC_CTRL-MIC3LR_2_LADC_CTRL+1];
  };

  static const struct reg_default aic3x_reg[] = {
@@ -134,16 +135,175 @@ static const struct regmap_config aic3x_regmap = {

  #define SOC_DAPM_SINGLE_AIC3X(xname, reg, shift, mask, invert) \
         SOC_SINGLE_EXT(xname, reg, shift, mask, invert, \
-               snd_soc_dapm_get_volsw, snd_soc_dapm_put_volsw_aic3x)
+               snd_soc_dapm_get_volsw_aic3x, snd_soc_dapm_put_volsw_aic3x)
+
+#define SOC_DOUBLE_R_AIC3X_TLV(xname, reg, rreg, shift, mask, invert) \
+       SOC_DOUBLE_R_EXT_TLV(xname, reg, rreg, shift, mask, invert, \
+       snd_soc_get_gain_aic3x, snd_soc_put_gain_aic3x, gain_stage_tlv)
+
+#define SOC_SINGLE_AIC3X_TLV(xname, reg, shift, mask, invert) \
+       SOC_SINGLE_EXT_TLV(xname, reg, shift, mask, invert, \
+       snd_soc_get_gain_aic3x, snd_soc_put_gain_aic3x, gain_stage_tlv)
+
+#define        SOC_SINGLE_EXT_VOL(xname, reg, shift, mask, invert) \
+       SOC_SINGLE_EXT(xname, reg, shift, mask, invert, \
+               snd_soc_get_volsw_uncached, snd_soc_put_volsw)
+
+/*
+ * Headset detect flag, button press detect flag and headset type flag 
is read only register,
+ * but we could not declare these regs as volatile because other bits 
are RW, so we use
+ * unchached get hanler for these regs.
+*/
+static int snd_soc_get_volsw_uncached( struct snd_kcontrol *kcontrol,
+               struct snd_ctl_elem_value * ucontrol)
+{
+       int ret;
+       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+
+       regcache_cache_bypass(aic3x->regmap, true);
+       ret = snd_soc_get_volsw(kcontrol, ucontrol);
+       regcache_cache_bypass(aic3x->regmap, false);
+       return ret;
+}
+
+/*
+ * All input lines have additional gain controls which can be controled 
by volume control.
+ * 0x0 - means 0dB and 0x8 - means -12 dB. It's a litlle bit tricky 
because value 0xF means mute.
+ *  value 0x9-0xE reserverd, so in case of switch muted we should store 
value in cache and should
+ * not set up it to register
+*/
+static int snd_soc_put_gain_aic3x(struct snd_kcontrol* kcontrol,
+                                       struct snd_ctl_elem_value* ucontrol)
+{
+       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+       struct soc_mixer_control *mc = (struct soc_mixer_control 
*)kcontrol->private_value;
+       unsigned int reg;
+       unsigned int shift = mc->shift;
+       int max = mc->max;
+       unsigned int mask = (1<<fls(max))-1;
+       unsigned int invert = mc->invert;
+
+       u8 regdata, newval, regval;
+        u8 loop = 0;
+
+       //First of all we need to cache value. Then we need to test each 
value on mute in register
+       //If not muted, we should setup new value
+       reg = mc->reg;
+        newval = ( ucontrol->value.integer.value[0] & mask );
+       printk(KERN_INFO "set gain control reg:%d val:%0X mask:%0X 
shift:%0X invert %d\n", reg,newval, mask, shift,invert);
+        do
+        {
+               if(invert)
+               {
+                       newval = ( 8 - newval ) & mask;
+                       printk(KERN_INFO "inverted val %0X\n", newval);
+               }
+               //read, test and update if first reg if needs
+               regdata = snd_soc_read(codec, reg);
+               regval = ( regdata >> shift) & mask;
+
+               if( reg >= MIC3LR_2_LADC_CTRL && reg <= LINE1L_2_RADC_CTRL )
+               {
+                       printk(KERN_INFO "cached_gain before 
%0X\n",aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]);
+ aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]&=~(mask<<shift);
+ aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]|=(newval<<shift);
+                       printk(KERN_INFO "cached_gain after 
%0X\n",aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]);
+               }
+
+               if ( regval!=0xf && regval!=newval )
+                       snd_soc_update_bits( codec, reg, mask<<shift, 
newval<<shift );
+
+                if(snd_soc_volsw_is_stereo(mc))
+                {
+                        loop=!loop;     //switch loop, so for first 
time loop will be true after and for second time it will be false, so we 
will finish loop
+                        if(loop) //if first time
+                        {
+                                reg=mc->rreg;
+                                newval = 
(ucontrol->value.integer.value[1] & mask);
+                               printk(KERN_INFO "control is stereo 
second reg:%d val:%0X\n", reg,newval);
+                                continue;
+                        }
+                }
+
+        }while(loop);
+       return 0;
+}
+
+/*
+ * All input lines have additional gain controls which can be controled 
by volume control.
+ * 0x0 - means 0dB and 0x8 - means -12 dB. It's a litlle bit tricky 
because value 0xF means mute.
+ * value 0x9-0xE reserverd, so in case of switch muted we should read 
value from cache in aic3x_priv
+ * and should not read it from regcache or register
+*/
+static int snd_soc_get_gain_aic3x( struct snd_kcontrol *kcontrol,
+                                  struct snd_ctl_elem_value *ucontrol)
+{
+       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+       struct soc_mixer_control *mc = (struct soc_mixer_control 
*)kcontrol->private_value;
+       unsigned int reg;
+       unsigned int shift = mc->shift;
+       int max = mc->max;
+       unsigned int mask = (1<<fls(max))-1;
+       unsigned int invert = mc->invert;
+
+       u8 regdata, regval;
+        u8 loop = 0;
+
+       //First of all we need to cache value. Then we need to test each 
value on mute in register
+       //If not muted, we should set up new value
+       reg = mc->reg;
+
+       printk(KERN_INFO "get gain control reg:%d mask:%0X shift:%0X 
invert %d\n", reg, mask, shift,invert);
+        do
+        {
+               regdata = snd_soc_read(codec, reg);
+               regval = ( regdata >> shift) & mask;
+               printk(KERN_INFO "regval:%0X\n", regval);
+
+               //check if register is muted then return cached value
+               if( regval == 0xf )
+               {
+                       if( reg >=MIC3LR_2_LADC_CTRL && reg <= 
LINE1L_2_RADC_CTRL )
+                               regval = 
(aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]>>shift) & mask;
+                       printk(KERN_INFO "regval from cache:%0X\n", regval);
+
+               }
+               if( invert )
+               {
+                       regval = (8 - regval) & mask;
+                       printk(KERN_INFO "regval after invert:%0X\n", 
regval);
+               }
+               ucontrol->value.integer.value[loop] = regval;
+
+                if(snd_soc_volsw_is_stereo(mc))
+                {
+                        loop=loop?0:1; //switch loop, so for first time 
loop will be 1 after and for second time it will be 0, so we will exit 
from while-loop
+                        if(loop) //if first time
+                        {
+                                reg=mc->rreg;
+                               printk(KERN_INFO "control is stereo 
second reg:%d\n", reg);
+                                continue;
+                        }
+                }
+
+        }while(loop);
+       return 0;
+}

  /*
   * All input lines are connected when !0xf and disconnected with 0xf 
bit field,
- * so we have to use specific dapm_put call for input mixer
+ * so we have to use specific dapm_put call for input mixer. In case of 
unmute
+ * we should set up register value from cached values from aic3x_priv 
struct.
   */
  static int snd_soc_dapm_put_volsw_aic3x(struct snd_kcontrol *kcontrol,
                                         struct snd_ctl_elem_value 
*ucontrol)
  {
         struct snd_soc_codec *codec = 
snd_soc_dapm_kcontrol_codec(kcontrol);
+       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+
         struct soc_mixer_control *mc =
                 (struct soc_mixer_control *)kcontrol->private_value;
         unsigned int reg = mc->reg;
@@ -151,20 +311,32 @@ static int snd_soc_dapm_put_volsw_aic3x(struct 
snd_kcontrol *kcontrol,
         int max = mc->max;
         unsigned int mask = (1 << fls(max)) - 1;
         unsigned int invert = mc->invert;
-       unsigned short val;
+       unsigned int val;
         struct snd_soc_dapm_update update;
         int connect, change;

         val = (ucontrol->value.integer.value[0] & mask);
+       printk(KERN_INFO "dapm put volsw val:%X reg:%X mask:%X shift:%X 
invert:%X\n", val, reg, mask, shift, invert);

         mask = 0xf;
         if (val)
+       {
                 val = mask;
-
+               printk(KERN_INFO "new val:%X\n", val);
+       }
         connect = !!val;

         if (invert)
+       {
                 val = mask - val;
+               printk(KERN_INFO "val after invert:%X\n", val);
+       }
+       if (!val)
+       {
+               if( reg>=MIC3LR_2_LADC_CTRL && reg<=LINE1L_2_RADC_CTRL )
+                       val = 
(aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]>>shift) & mask;
+               printk(KERN_INFO "cached val:%X\n", val);
+       }

         mask <<= shift;
         val <<= shift;
@@ -183,6 +355,36 @@ static int snd_soc_dapm_put_volsw_aic3x(struct 
snd_kcontrol *kcontrol,
         return change;
  }

+/*
+ * Based on standart handler snd_soc_dapm_get_volsw, but changing mask 
to 0xF
+ *
+ */
+static int snd_soc_dapm_get_volsw_aic3x(struct snd_kcontrol *kcontrol,
+                                       struct snd_ctl_elem_value *ucontrol)
+{
+       struct soc_mixer_control *mc =
+               (struct soc_mixer_control *)kcontrol->private_value;
+       unsigned int reg = mc->reg;
+       unsigned int shift = mc->shift;
+       int max = mc->max;
+       unsigned int mask = (1 << fls(max)) - 1;
+       int ret;
+
+       //register contains 4 bits, so we change max temoprally to read 
register by original handler, then return it back
+       mc->max = 15;
+       ret = snd_soc_dapm_get_volsw( kcontrol, ucontrol);
+       printk(KERN_INFO "soc_dapm_get_volsw_aic3x:%ld reg:%X shift:%X 
max:%X newmax:%X\n", ucontrol->value.integer.value[0], reg, shift, max, 
mc->max );
+       mc->max = max;
+
+       //all gains except mute (0xf) after invertion is not equl to 0, 
so we need to set 1(on)
+       //for all values except 0. 0 is mute, so we do not need to change it
+       if(ucontrol->value.integer.value[0]!=0)
+               ucontrol->value.integer.value[0]=1;
+       printk(KERN_INFO "new value:%ld\n", 
ucontrol->value.integer.value[0]);
+
+       return ret;
+}
+
  /*
   * mic bias power on/off share the same register bits with
   * output voltage of mic bias. when power on mic bias, we
@@ -270,6 +472,14 @@ static const struct soc_enum aic3x_agc_decay_enum[] = {
         SOC_ENUM_SINGLE(RAGC_CTRL_A, 0, 4, aic3x_agc_decay),
  };

+static const char *aic3x_headset_debounce[] = { "16ms", "32ms", "64ms", 
"128ms", "256ms", "512ms" };
+static const struct soc_enum aic3x_headset_debounce_enum =
+SOC_ENUM_SINGLE( AIC3X_HEADSET_DETECT_CTRL_A, 2, 6, 
aic3x_headset_debounce);
+
+static const char *aic3x_button_debounce[] = { "0ms", "8ms", "16ms", 
"32ms" };
+static const struct soc_enum aic3x_button_debounce_enum =
+SOC_ENUM_SINGLE( AIC3X_HEADSET_DETECT_CTRL_A, 0, 4, aic3x_button_debounce);
+
  /*
   * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps
   */
@@ -287,6 +497,9 @@ static DECLARE_TLV_DB_SCALE(adc_tlv, 0, 50, 0);
   */
  static DECLARE_TLV_DB_SCALE(output_stage_tlv, -5900, 50, 1);

+static DECLARE_TLV_DB_SCALE(gain_stage_tlv, -1200, 150, 0);
+
+
  static const struct snd_kcontrol_new aic3x_snd_controls[] = {
         /* Output */
         SOC_DOUBLE_R_TLV("PCM Playback Volume",
@@ -399,6 +612,24 @@ static const struct snd_kcontrol_new 
aic3x_snd_controls[] = {
         SOC_DOUBLE_R("PGA Capture Switch", LADC_VOL, RADC_VOL, 7, 0x01, 1),

         SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]),
+       /* Additional controls */
+       SOC_DOUBLE_R_AIC3X_TLV("Mic3L Volume", MIC3LR_2_LADC_CTRL, 
MIC3LR_2_RADC_CTRL, 4, 8, 1),
+       SOC_DOUBLE_R_AIC3X_TLV("Mic3R Volume", MIC3LR_2_LADC_CTRL, 
MIC3LR_2_RADC_CTRL, 0, 8, 1),
+       SOC_DOUBLE_R_AIC3X_TLV("Line1L Volume", LINE1L_2_LADC_CTRL, 
LINE1L_2_RADC_CTRL, 3, 8, 1),
+       SOC_DOUBLE_R_AIC3X_TLV("Line1R Volume", LINE1R_2_LADC_CTRL, 
LINE1R_2_RADC_CTRL, 3, 8, 1),
+       SOC_SINGLE_AIC3X_TLV("Line2L Volume", LINE2L_2_LADC_CTRL, 3, 8, 1),
+       SOC_SINGLE_AIC3X_TLV("Line2R Volume", LINE2R_2_RADC_CTRL, 3, 8, 1),
+       SOC_ENUM("Headset Jack Debounce", aic3x_headset_debounce_enum ),
+       SOC_ENUM("Button Press Debounce", aic3x_button_debounce_enum ),
+       SOC_SINGLE("Headset Detect Enable", AIC3X_HEADSET_DETECT_CTRL_A, 
7, 1, 0),
+       SOC_SINGLE_EXT_VOL("Headset Detect Type", 
AIC3X_HEADSET_DETECT_CTRL_A,
+               AIC3X_HEADSET_DETECT_A_SHIFT, 
AIC3X_HEADSET_DETECT_A_MASK, 0),
+       SOC_SINGLE_EXT_VOL("Button Detect Flag", 
AIC3X_HEADSET_DETECT_CTRL_B,
+               AIC3X_BUTTON_DETECT_SHIFT, AIC3X_BUTTON_DETECT_MASK, 0),
+       SOC_SINGLE_EXT_VOL("Headset Detect Flag", 
AIC3X_HEADSET_DETECT_CTRL_B,
+               AIC3X_HEADSET_DETECT_B_SHIFT, 
AIC3X_HEADSET_DETECT_B_MASK, 0),
+       SOC_SINGLE("High power output Ac-coupled", 
AIC3X_HEADSET_DETECT_CTRL_B, 7, 1, 0),
+       SOC_SINGLE("Stereo pseudodifferential output", 
AIC3X_HEADSET_DETECT_CTRL_B, 3, 1, 0),
  };

  static const struct snd_kcontrol_new aic3x_mono_controls[] = {
--------------------------------------
diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
index e521ac3..bcac87f 100644
--- a/sound/soc/codecs/tlv320aic3x.h
+++ b/sound/soc/codecs/tlv320aic3x.h
@@ -271,9 +271,20 @@ enum {
         AIC3X_BUTTON_DEBOUNCE_32MS      = 3
  };

+typedef struct {
+    struct platform_device* pdev;
+    struct proc_dir_entry *proc_value;
+    struct snd_soc_codec *codec;
+} aic3106_detect_t;
+
  #define AIC3X_HEADSET_DETECT_ENABLED   0x80
-#define AIC3X_HEADSET_DETECT_SHIFT     5
-#define AIC3X_HEADSET_DETECT_MASK      3
+#define AIC3X_HEADSET_DETECT_A_SHIFT   5
+#define AIC3X_HEADSET_DETECT_A_MASK    3
+
+#define AIC3X_BUTTON_DETECT_SHIFT      5
+#define AIC3X_BUTTON_DETECT_MASK       1
+#define AIC3X_HEADSET_DETECT_B_SHIFT   4
+#define AIC3X_HEADSET_DETECT_B_MASK    1
  #define AIC3X_HEADSET_DEBOUNCE_SHIFT   2
  #define AIC3X_HEADSET_DEBOUNCE_MASK    7
  #define AIC3X_BUTTON_DEBOUNCE_SHIFT    0
--------------------------------------

17.03.2016 12:26, Peter Ujfalusi пишет:
> On 03/16/16 18:53, Timur Karaldin wrote:
>>> your mail client does not seem to wrap the lines correctly, can you check that.
>> I have no idea how these lines should looks, so it's very hard for me to see
>> what's wrong. Could you point me how it looks in original?
> They are looong.
>
> see: https://wiki.openstack.org/wiki/MailingListEtiquette
>
>> Ok, now it's much more clear for me.
>> Another question is register behaviour during soft reset. There is
>> "aic3x_set_power" handle. In this handle kernel makes SOFT_RESET, markes cache
>> as dirty, then power down the codec for handle power down request.
>> But as I could see main volumes like "HP DAC" and "PCM" stores values between
>> close and open in mixer and I could not see any code to handle it. On the
>> other hand my controls do not save states, as you mentioned because of
>> SOFT_RESET, could you explain such different behaviour?
> All cached registers are going to be restored after power on with exception of
> volatile registers. You need to restore the bits in a volatile registers in
> the driver.
>


-- 
С уважением,
Руководитель проектов
Морские комплексы и системы плюс
Каралдин Т.М.
----------------------------------------------------------------------------------
Best regards,
Projects director
Marine complexes and systems plus
Timur Karaldin

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: ASoC: TLV320AIC3x: Adding additional functionality for 3106 with [Patch] for discuss

[Peter Ujfalusi]

Timur,

On 03/23/16 17:21, Timur Karaldin wrote:
> Hi Peter!
> Here is my patch v2. It works only for aic3106, but I don't have any others.
> Also patch contains some debug output, I would clean it. I like behaviour of
> this improvements and I didn't know any issue right now for these add-ons
> except debug output and that it's correct only for aic3106 (didn't check for
> others even on datasheet level). Thank you for all your help!

sorry for the delay, I was dragged away for a bit.

Can you send these as separated patches per new functionality? Please use
git format-patch to create the patches and try to clean them up so it is going
to be easier to understand them.

Can you check your mail client to wrap lines correctly?
You have very long lines, for example:
http://www.spinics.net/lists/alsa-devel/msg47793.html
http://www.spinics.net/lists/alsa-devel/msg48199.html

While they should be something like:
http://www.spinics.net/lists/alsa-devel/msg47797.html

Couple of comments, but it would be more beneficial to see a series...

> -----------------------------------------------------------------------
> diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
> index d7349bc..2c06e21 100644
> --- a/sound/soc/codecs/tlv320aic3x.c
> +++ b/sound/soc/codecs/tlv320aic3x.c
> @@ -89,6 +89,7 @@ struct aic3x_priv {
> 
>         /* Selects the micbias voltage */
>         enum aic3x_micbias_voltage micbias_vg;
> +       unsigned char cached_gain[LINE1L_2_RADC_CTRL-MIC3LR_2_LADC_CTRL+1];

spaces around '+' '-' '='

See: Documentation/CodingStyle

Also, make sure that tabs are not replaced by spaces and check the patch for
long lines also.

Run scripts/checkpatch.pl --strict on the patches.

>  };
> 
>  static const struct reg_default aic3x_reg[] = {
> @@ -134,16 +135,175 @@ static const struct regmap_config aic3x_regmap = {
> 
>  #define SOC_DAPM_SINGLE_AIC3X(xname, reg, shift, mask, invert) \
>         SOC_SINGLE_EXT(xname, reg, shift, mask, invert, \
> -               snd_soc_dapm_get_volsw, snd_soc_dapm_put_volsw_aic3x)
> +               snd_soc_dapm_get_volsw_aic3x, snd_soc_dapm_put_volsw_aic3x)
> +
> +#define SOC_DOUBLE_R_AIC3X_TLV(xname, reg, rreg, shift, mask, invert) \
> +       SOC_DOUBLE_R_EXT_TLV(xname, reg, rreg, shift, mask, invert, \
> +       snd_soc_get_gain_aic3x, snd_soc_put_gain_aic3x, gain_stage_tlv)
> +
> +#define SOC_SINGLE_AIC3X_TLV(xname, reg, shift, mask, invert) \
> +       SOC_SINGLE_EXT_TLV(xname, reg, shift, mask, invert, \
> +       snd_soc_get_gain_aic3x, snd_soc_put_gain_aic3x, gain_stage_tlv)
> +
> +#define        SOC_SINGLE_EXT_VOL(xname, reg, shift, mask, invert) \
> +       SOC_SINGLE_EXT(xname, reg, shift, mask, invert, \
> +               snd_soc_get_volsw_uncached, snd_soc_put_volsw)
> +
> +/*
> + * Headset detect flag, button press detect flag and headset type flag is
> read only register,
> + * but we could not declare these regs as volatile because other bits are RW,
> so we use
> + * unchached get hanler for these regs.
> +*/
> +static int snd_soc_get_volsw_uncached( struct snd_kcontrol *kcontrol,
> +               struct snd_ctl_elem_value * ucontrol)
> +{
> +       int ret;
> +       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
> +       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
> +
> +       regcache_cache_bypass(aic3x->regmap, true);
> +       ret = snd_soc_get_volsw(kcontrol, ucontrol);
> +       regcache_cache_bypass(aic3x->regmap, false);
> +       return ret;
> +}
> +
> +/*
> + * All input lines have additional gain controls which can be controled by
> volume control.
> + * 0x0 - means 0dB and 0x8 - means -12 dB. It's a litlle bit tricky because
> value 0xF means mute.
> + *  value 0x9-0xE reserverd, so in case of switch muted we should store value
> in cache and should
> + * not set up it to register
> +*/
> +static int snd_soc_put_gain_aic3x(struct snd_kcontrol* kcontrol,
> +                                       struct snd_ctl_elem_value* ucontrol)

Please keep the parameter indentation uniform.

> +{
> +       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
> +       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
> +       struct soc_mixer_control *mc = (struct soc_mixer_control
> *)kcontrol->private_value;
> +       unsigned int reg;
> +       unsigned int shift = mc->shift;
> +       int max = mc->max;
> +       unsigned int mask = (1<<fls(max))-1;
> +       unsigned int invert = mc->invert;
> +
> +       u8 regdata, newval, regval;
> +        u8 loop = 0;
> +
> +       //First of all we need to cache value. Then we need to test each value
> on mute in register
> +       //If not muted, we should setup new value
> +       reg = mc->reg;
> +        newval = ( ucontrol->value.integer.value[0] & mask );
> +       printk(KERN_INFO "set gain control reg:%d val:%0X mask:%0X shift:%0X
> invert %d\n", reg,newval, mask, shift,invert);
> +        do
> +        {

It would be better to do this w/o do {}while loop, see snd_soc_put_volsw() for
example.

> +               if(invert)
> +               {
> +                       newval = ( 8 - newval ) & mask;
> +                       printk(KERN_INFO "inverted val %0X\n", newval);
> +               }
> +               //read, test and update if first reg if needs
> +               regdata = snd_soc_read(codec, reg);
> +               regval = ( regdata >> shift) & mask;
> +
> +               if( reg >= MIC3LR_2_LADC_CTRL && reg <= LINE1L_2_RADC_CTRL )
> +               {
> +                       printk(KERN_INFO "cached_gain before
> %0X\n",aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]);
> + aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]&=~(mask<<shift);
> + aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]|=(newval<<shift);
> +                       printk(KERN_INFO "cached_gain after
> %0X\n",aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]);
> +               }
> +
> +               if ( regval!=0xf && regval!=newval )
> +                       snd_soc_update_bits( codec, reg, mask<<shift,
> newval<<shift );
> +
> +                if(snd_soc_volsw_is_stereo(mc))
> +                {
> +                        loop=!loop;     //switch loop, so for first time loop
> will be true after and for second time it will be false, so we will finish loop
> +                        if(loop) //if first time
> +                        {
> +                                reg=mc->rreg;
> +                                newval = (ucontrol->value.integer.value[1] &
> mask);
> +                               printk(KERN_INFO "control is stereo second
> reg:%d val:%0X\n", reg,newval);
> +                                continue;
> +                        }
> +                }
> +
> +        }while(loop);
> +       return 0;
> +}
> +
> +/*
> + * All input lines have additional gain controls which can be controled by
> volume control.
> + * 0x0 - means 0dB and 0x8 - means -12 dB. It's a litlle bit tricky because
> value 0xF means mute.
> + * value 0x9-0xE reserverd, so in case of switch muted we should read value
> from cache in aic3x_priv
> + * and should not read it from regcache or register
> +*/
> +static int snd_soc_get_gain_aic3x( struct snd_kcontrol *kcontrol,
> +                                  struct snd_ctl_elem_value *ucontrol)
> +{
> +       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
> +       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
> +       struct soc_mixer_control *mc = (struct soc_mixer_control
> *)kcontrol->private_value;
> +       unsigned int reg;
> +       unsigned int shift = mc->shift;
> +       int max = mc->max;
> +       unsigned int mask = (1<<fls(max))-1;
> +       unsigned int invert = mc->invert;
> +
> +       u8 regdata, regval;
> +        u8 loop = 0;
> +
> +       //First of all we need to cache value. Then we need to test each value
> on mute in register
> +       //If not muted, we should set up new value
> +       reg = mc->reg;
> +
> +       printk(KERN_INFO "get gain control reg:%d mask:%0X shift:%0X invert
> %d\n", reg, mask, shift,invert);
> +        do
> +        {

same here, please remove the loop.

> +               regdata = snd_soc_read(codec, reg);
> +               regval = ( regdata >> shift) & mask;
> +               printk(KERN_INFO "regval:%0X\n", regval);
> +
> +               //check if register is muted then return cached value
> +               if( regval == 0xf )
> +               {
> +                       if( reg >=MIC3LR_2_LADC_CTRL && reg <=
> LINE1L_2_RADC_CTRL )
> +                               regval =
> (aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]>>shift) & mask;
> +                       printk(KERN_INFO "regval from cache:%0X\n", regval);
> +
> +               }
> +               if( invert )
> +               {
> +                       regval = (8 - regval) & mask;
> +                       printk(KERN_INFO "regval after invert:%0X\n", regval);
> +               }
> +               ucontrol->value.integer.value[loop] = regval;
> +
> +                if(snd_soc_volsw_is_stereo(mc))
> +                {
> +                        loop=loop?0:1; //switch loop, so for first time loop
> will be 1 after and for second time it will be 0, so we will exit from while-loop
> +                        if(loop) //if first time
> +                        {
> +                                reg=mc->rreg;
> +                               printk(KERN_INFO "control is stereo second
> reg:%d\n", reg);
> +                                continue;
> +                        }
> +                }
> +
> +        }while(loop);
> +       return 0;
> +}
> 
>  /*
>   * All input lines are connected when !0xf and disconnected with 0xf bit field,
> - * so we have to use specific dapm_put call for input mixer
> + * so we have to use specific dapm_put call for input mixer. In case of unmute
> + * we should set up register value from cached values from aic3x_priv struct.
>   */
>  static int snd_soc_dapm_put_volsw_aic3x(struct snd_kcontrol *kcontrol,
>                                         struct snd_ctl_elem_value *ucontrol)
>  {
>         struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
> +       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
> +
>         struct soc_mixer_control *mc =
>                 (struct soc_mixer_control *)kcontrol->private_value;
>         unsigned int reg = mc->reg;
> @@ -151,20 +311,32 @@ static int snd_soc_dapm_put_volsw_aic3x(struct
> snd_kcontrol *kcontrol,
>         int max = mc->max;
>         unsigned int mask = (1 << fls(max)) - 1;
>         unsigned int invert = mc->invert;
> -       unsigned short val;
> +       unsigned int val;
>         struct snd_soc_dapm_update update;
>         int connect, change;
> 
>         val = (ucontrol->value.integer.value[0] & mask);
> +       printk(KERN_INFO "dapm put volsw val:%X reg:%X mask:%X shift:%X
> invert:%X\n", val, reg, mask, shift, invert);
> 
>         mask = 0xf;
>         if (val)
> +       {
>                 val = mask;
> -
> +               printk(KERN_INFO "new val:%X\n", val);
> +       }
>         connect = !!val;
> 
>         if (invert)
> +       {
>                 val = mask - val;
> +               printk(KERN_INFO "val after invert:%X\n", val);
> +       }
> +       if (!val)
> +       {
> +               if( reg>=MIC3LR_2_LADC_CTRL && reg<=LINE1L_2_RADC_CTRL )
> +                       val =
> (aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]>>shift) & mask;
> +               printk(KERN_INFO "cached val:%X\n", val);
> +       }

this generally looks good, but please clean up the patch.

> 
>         mask <<= shift;
>         val <<= shift;
> @@ -183,6 +355,36 @@ static int snd_soc_dapm_put_volsw_aic3x(struct
> snd_kcontrol *kcontrol,
>         return change;
>  }
> 
> +/*
> + * Based on standart handler snd_soc_dapm_get_volsw, but changing mask to 0xF
> + *
> + */
> +static int snd_soc_dapm_get_volsw_aic3x(struct snd_kcontrol *kcontrol,
> +                                       struct snd_ctl_elem_value *ucontrol)
> +{
> +       struct soc_mixer_control *mc =
> +               (struct soc_mixer_control *)kcontrol->private_value;
> +       unsigned int reg = mc->reg;
> +       unsigned int shift = mc->shift;
> +       int max = mc->max;
> +       unsigned int mask = (1 << fls(max)) - 1;
> +       int ret;
> +
> +       //register contains 4 bits, so we change max temoprally to read
> register by original handler, then return it back
> +       mc->max = 15;
> +       ret = snd_soc_dapm_get_volsw( kcontrol, ucontrol);
> +       printk(KERN_INFO "soc_dapm_get_volsw_aic3x:%ld reg:%X shift:%X max:%X
> newmax:%X\n", ucontrol->value.integer.value[0], reg, shift, max, mc->max );
> +       mc->max = max;
> +
> +       //all gains except mute (0xf) after invertion is not equl to 0, so we
> need to set 1(on)
> +       //for all values except 0. 0 is mute, so we do not need to change it
> +       if(ucontrol->value.integer.value[0]!=0)
> +               ucontrol->value.integer.value[0]=1;
> +       printk(KERN_INFO "new value:%ld\n", ucontrol->value.integer.value[0]);
> +
> +       return ret;
> +}
> +
>  /*
>   * mic bias power on/off share the same register bits with
>   * output voltage of mic bias. when power on mic bias, we
> @@ -270,6 +472,14 @@ static const struct soc_enum aic3x_agc_decay_enum[] = {
>         SOC_ENUM_SINGLE(RAGC_CTRL_A, 0, 4, aic3x_agc_decay),
>  };
> 
> +static const char *aic3x_headset_debounce[] = { "16ms", "32ms", "64ms",
> "128ms", "256ms", "512ms" };
> +static const struct soc_enum aic3x_headset_debounce_enum =
> +SOC_ENUM_SINGLE( AIC3X_HEADSET_DETECT_CTRL_A, 2, 6, aic3x_headset_debounce);
> +
> +static const char *aic3x_button_debounce[] = { "0ms", "8ms", "16ms", "32ms" };
> +static const struct soc_enum aic3x_button_debounce_enum =
> +SOC_ENUM_SINGLE( AIC3X_HEADSET_DETECT_CTRL_A, 0, 4, aic3x_button_debounce);
> +
>  /*
>   * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps
>   */
> @@ -287,6 +497,9 @@ static DECLARE_TLV_DB_SCALE(adc_tlv, 0, 50, 0);
>   */
>  static DECLARE_TLV_DB_SCALE(output_stage_tlv, -5900, 50, 1);
> 
> +static DECLARE_TLV_DB_SCALE(gain_stage_tlv, -1200, 150, 0);
> +
> +
>  static const struct snd_kcontrol_new aic3x_snd_controls[] = {
>         /* Output */
>         SOC_DOUBLE_R_TLV("PCM Playback Volume",
> @@ -399,6 +612,24 @@ static const struct snd_kcontrol_new aic3x_snd_controls[]
> = {
>         SOC_DOUBLE_R("PGA Capture Switch", LADC_VOL, RADC_VOL, 7, 0x01, 1),
> 
>         SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]),
> +       /* Additional controls */
> +       SOC_DOUBLE_R_AIC3X_TLV("Mic3L Volume", MIC3LR_2_LADC_CTRL,
> MIC3LR_2_RADC_CTRL, 4, 8, 1),
> +       SOC_DOUBLE_R_AIC3X_TLV("Mic3R Volume", MIC3LR_2_LADC_CTRL,
> MIC3LR_2_RADC_CTRL, 0, 8, 1),
> +       SOC_DOUBLE_R_AIC3X_TLV("Line1L Volume", LINE1L_2_LADC_CTRL,
> LINE1L_2_RADC_CTRL, 3, 8, 1),
> +       SOC_DOUBLE_R_AIC3X_TLV("Line1R Volume", LINE1R_2_LADC_CTRL,
> LINE1R_2_RADC_CTRL, 3, 8, 1),
> +       SOC_SINGLE_AIC3X_TLV("Line2L Volume", LINE2L_2_LADC_CTRL, 3, 8, 1),
> +       SOC_SINGLE_AIC3X_TLV("Line2R Volume", LINE2R_2_RADC_CTRL, 3, 8, 1),
> +       SOC_ENUM("Headset Jack Debounce", aic3x_headset_debounce_enum ),
> +       SOC_ENUM("Button Press Debounce", aic3x_button_debounce_enum ),
> +       SOC_SINGLE("Headset Detect Enable", AIC3X_HEADSET_DETECT_CTRL_A, 7, 1,
> 0),
> +       SOC_SINGLE_EXT_VOL("Headset Detect Type", AIC3X_HEADSET_DETECT_CTRL_A,
> +               AIC3X_HEADSET_DETECT_A_SHIFT, AIC3X_HEADSET_DETECT_A_MASK, 0),
> +       SOC_SINGLE_EXT_VOL("Button Detect Flag", AIC3X_HEADSET_DETECT_CTRL_B,
> +               AIC3X_BUTTON_DETECT_SHIFT, AIC3X_BUTTON_DETECT_MASK, 0),
> +       SOC_SINGLE_EXT_VOL("Headset Detect Flag", AIC3X_HEADSET_DETECT_CTRL_B,
> +               AIC3X_HEADSET_DETECT_B_SHIFT, AIC3X_HEADSET_DETECT_B_MASK, 0),
> +       SOC_SINGLE("High power output Ac-coupled",
> AIC3X_HEADSET_DETECT_CTRL_B, 7, 1, 0),
> +       SOC_SINGLE("Stereo pseudodifferential output",
> AIC3X_HEADSET_DETECT_CTRL_B, 3, 1, 0),
>  };
> 
>  static const struct snd_kcontrol_new aic3x_mono_controls[] = {
> --------------------------------------
> diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
> index e521ac3..bcac87f 100644
> --- a/sound/soc/codecs/tlv320aic3x.h
> +++ b/sound/soc/codecs/tlv320aic3x.h
> @@ -271,9 +271,20 @@ enum {
>         AIC3X_BUTTON_DEBOUNCE_32MS      = 3
>  };
> 
> +typedef struct {
> +    struct platform_device* pdev;
> +    struct proc_dir_entry *proc_value;
> +    struct snd_soc_codec *codec;
> +} aic3106_detect_t;
> +
>  #define AIC3X_HEADSET_DETECT_ENABLED   0x80
> -#define AIC3X_HEADSET_DETECT_SHIFT     5
> -#define AIC3X_HEADSET_DETECT_MASK      3
> +#define AIC3X_HEADSET_DETECT_A_SHIFT   5
> +#define AIC3X_HEADSET_DETECT_A_MASK    3
> +
> +#define AIC3X_BUTTON_DETECT_SHIFT      5
> +#define AIC3X_BUTTON_DETECT_MASK       1
> +#define AIC3X_HEADSET_DETECT_B_SHIFT   4
> +#define AIC3X_HEADSET_DETECT_B_MASK    1
>  #define AIC3X_HEADSET_DEBOUNCE_SHIFT   2
>  #define AIC3X_HEADSET_DEBOUNCE_MASK    7
>  #define AIC3X_BUTTON_DEBOUNCE_SHIFT    0
> --------------------------------------

It would be great if you could clean this up, have separate patches in order
to be able to review the changes and also to be able to test it.
It looks promising ;)

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH V2 1/2] ASoC: TLV320AIC3x: Adding additional functionality for 3106

[Timur Karaldin]

Here is the result.
----------------------------------------------------------------------------
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index d7349bc..3b0fae1 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -89,6 +89,9 @@ struct aic3x_priv {

         /* Selects the micbias voltage */
         enum aic3x_micbias_voltage micbias_vg;
+       unsigned char cached_gain[LINE1L_2_RADC_CTRL -
+                       MIC3LR_2_LADC_CTRL + 1];
+       int bypass;
  };

  static const struct reg_default aic3x_reg[] = {
@@ -134,16 +137,166 @@ static const struct regmap_config aic3x_regmap = {

  #define SOC_DAPM_SINGLE_AIC3X(xname, reg, shift, mask, invert) \
         SOC_SINGLE_EXT(xname, reg, shift, mask, invert, \
-               snd_soc_dapm_get_volsw, snd_soc_dapm_put_volsw_aic3x)
+               snd_soc_dapm_get_volsw_aic3x, snd_soc_dapm_put_volsw_aic3x)
+
+#define SOC_DOUBLE_R_AIC3X_TLV(xname, reg, rreg, shift, mask, invert) \
+       SOC_DOUBLE_R_EXT_TLV(xname, reg, rreg, shift, mask, invert, \
+       snd_soc_get_gain_aic3x, snd_soc_put_gain_aic3x, gain_stage_tlv)
+
+#define SOC_SINGLE_AIC3X_TLV(xname, reg, shift, mask, invert) \
+       SOC_SINGLE_EXT_TLV(xname, reg, shift, mask, invert, \
+       snd_soc_get_gain_aic3x, snd_soc_put_gain_aic3x, gain_stage_tlv)
+
+#define        SOC_SINGLE_EXT_VOL(xname, reg, shift, mask, invert) \
+       SOC_SINGLE_EXT(xname, reg, shift, mask, invert, \
+               snd_soc_get_volsw_uncached, snd_soc_put_volsw)
+
+/*
+ * The headset detect flag, the button press detect flag and the headset
+ * type flag are stored in read only registers, but we could not declare
+ * these regs as volatile because other bits are RW, so we use unchached
+ * get hanler for these regs.
+ */
+static int snd_soc_get_volsw_uncached(struct snd_kcontrol *kcontrol,
+                                     struct snd_ctl_elem_value *ucontrol)
+{
+       int ret;
+       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+       if (aic3x->power)
+               regcache_cache_bypass(aic3x->regmap, true);
+       ret = snd_soc_get_volsw(kcontrol, ucontrol);
+       if (aic3x->power)
+               regcache_cache_bypass(aic3x->regmap, false);
+       return ret;
+}
+
+/*
+ * All input lines have additional volume gain controls. Value 0x0 is 0dB gain
+ * and value 0x8 is -12 dB gain. It's a little bit tricky because value 0xF
+ * means mute. values 0x9-0xE are reserved. If switch is muted we should store
+ * value in cache and should not set up register.
+ */
+static int snd_soc_put_gain_aic3x(struct snd_kcontrol *kcontrol,
+                                 struct snd_ctl_elem_value *ucontrol)
+{
+       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+       struct soc_mixer_control *mc =
+               (struct soc_mixer_control *)kcontrol->private_value;
+       unsigned int reg;
+       unsigned int shift = mc->shift;
+       int max = mc->max;
+       unsigned int mask = (1<<fls(max))-1;
+       unsigned int invert = mc->invert;
+
+       u8 regdata, newval, regval;
+       u8 loop = 0;
+
+       /* We need to cache value. Then we need to test each value on
+        * mute in hw register. If not muted, we should set up new value */
+       reg = mc->reg;
+       newval = (ucontrol->value.integer.value[0] & mask);
+       do {
+               if (invert)
+                       newval = (8 - newval) & mask;
+               /* read, test and update if first reg if needs */
+               regdata = snd_soc_read(codec, reg);
+               regval = (regdata >> shift) & mask;
+
+               if (reg >= MIC3LR_2_LADC_CTRL && reg <= LINE1L_2_RADC_CTRL) {
+                       aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]
+                               &= ~(mask<<shift);
+                       aic3x->cached_gain[reg-MIC3LR_2_LADC_CTRL]
+                               |= (newval<<shift);
+               }
+
+               if (regval != 0xf && regval != newval)
+                       snd_soc_update_bits(codec, reg, mask<<shift,
+                                           newval<<shift);
+
+               if (snd_soc_volsw_is_stereo(mc)) {
+                       /* first time loop will be switched to true and
+                       * second time loop will be switched to false, so
+                       *  we could finish loop */
+                       loop = !loop;
+                       if (loop) { /* reinit vars for second loop */
+                               reg = mc->rreg;
+                               newval = (ucontrol->value.integer.value[1]
+                                        & mask);
+                               continue;
+                       }
+               }
+
+       } while (loop);
+       return 0;
+}
+
+/*
+ * All input lines have additional volume gain controls. Value 0x0 is 0dB gain
+ * and value 0x8 is -12 dB gain. It's a little bit tricky because value 0xF
+ * means mute. Values 0x9-0xE are reserved. If switch is muted we should read
+ * value from cache but not from hw register.
+ */
+static int snd_soc_get_gain_aic3x(struct snd_kcontrol *kcontrol,
+                                 struct snd_ctl_elem_value *ucontrol)
+{
+       struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+       struct soc_mixer_control *mc =
+               (struct soc_mixer_control *)kcontrol->private_value;
+       unsigned int reg;
+       unsigned int shift = mc->shift;
+       int max = mc->max;
+       unsigned int mask = (1<<fls(max))-1;
+       unsigned int invert = mc->invert;
+
+       u8 regdata, regval;
+       u8 loop = 0;
+
+       reg = mc->reg;
+
+       do {
+               regdata = snd_soc_read(codec, reg);
+               regval = (regdata >> shift) & mask;
+
+               /* check if register is muted then return cached value */
+               if (regval == 0xf) {
+                       if (reg >= MIC3LR_2_LADC_CTRL &&
+                           reg <= LINE1L_2_RADC_CTRL)
+                               regval = (aic3x->cached_gain[reg-
+                                       MIC3LR_2_LADC_CTRL]>>shift) & mask;
+               }
+               if (invert)
+                       regval = (8 - regval) & mask;
+
+               ucontrol->value.integer.value[loop] = regval;
+
+               if (snd_soc_volsw_is_stereo(mc)) {
+                       /* first time loop will be switched to 1 and second time
+                       * loop will be switched to 0, so we could finish loop */
+                       loop = loop ? 0 : 1;
+                       if (loop) { /* reinit vars for second loop */
+                               reg = mc->rreg;
+                               continue;
+                       }
+               }
+
+       } while (loop);
+       return 0;
+}

  /*
- * All input lines are connected when !0xf and disconnected with 0xf bit field,
- * so we have to use specific dapm_put call for input mixer
+ * All input lines are connected when gain is not equal to 0xf and disconnected
+ * in other cases. We have to use the specific dapm_put call for input mixer.
+ * In case of unmute we should set up register's value by cached value.
   */
  static int snd_soc_dapm_put_volsw_aic3x(struct snd_kcontrol *kcontrol,
                                         struct snd_ctl_elem_value *ucontrol)
  {
         struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
+       struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+
         struct soc_mixer_control *mc =
                 (struct soc_mixer_control *)kcontrol->private_value;
         unsigned int reg = mc->reg;
@@ -151,7 +304,7 @@ static int snd_soc_dapm_put_volsw_aic3x(struct snd_kcontrol 
*kcontrol,
         int max = mc->max;
         unsigned int mask = (1 << fls(max)) - 1;
         unsigned int invert = mc->invert;
-       unsigned short val;
+       unsigned int val;
         struct snd_soc_dapm_update update;
         int connect, change;

@@ -166,6 +319,12 @@ static int snd_soc_dapm_put_volsw_aic3x(struct snd_kcontrol 
*kcontrol,
         if (invert)
                 val = mask - val;

+       if (!val) {
+               if (reg >= MIC3LR_2_LADC_CTRL && reg <= LINE1L_2_RADC_CTRL)
+                       val = (aic3x->cached_gain[reg
+                               -MIC3LR_2_LADC_CTRL]>>shift) & mask;
+       }
+
         mask <<= shift;
         val <<= shift;

@@ -184,6 +343,32 @@ static int snd_soc_dapm_put_volsw_aic3x(struct snd_kcontrol 
*kcontrol,
  }

  /*
+ * Based on standart handler snd_soc_dapm_get_volsw, but changing mask to 0xF
+ */
+static int snd_soc_dapm_get_volsw_aic3x(struct snd_kcontrol *kcontrol,
+                                       struct snd_ctl_elem_value *ucontrol)
+{
+       struct soc_mixer_control *mc =
+               (struct soc_mixer_control *)kcontrol->private_value;
+       int max = mc->max;
+       int ret;
+
+       /* the register contains 4 bits, so we would change max temporally
+       * to read register by original handler, then return max back */
+       mc->max = 15;
+       ret = snd_soc_dapm_get_volsw(kcontrol, ucontrol);
+       mc->max = max;
+
+       /* all gain values (except mute value) after invertion is not equl to 0,
+       * so we need to set 1(on) for all values except 0.
+       * 0 means mute, so we do not need to change it. */
+       if (ucontrol->value.integer.value[0] != 0)
+               ucontrol->value.integer.value[0] = 1;
+
+       return ret;
+}
+
+/*
   * mic bias power on/off share the same register bits with
   * output voltage of mic bias. when power on mic bias, we
   * need reclaim it to voltage value.
@@ -270,6 +455,16 @@ static const struct soc_enum aic3x_agc_decay_enum[] = {
         SOC_ENUM_SINGLE(RAGC_CTRL_A, 0, 4, aic3x_agc_decay),
  };

+static const char * const aic3x_headset_debounce[] = {
+       "16ms", "32ms", "64ms", "128ms", "256ms", "512ms" };
+static const struct soc_enum aic3x_headset_debounce_enum =
+SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 2, 6, aic3x_headset_debounce);
+
+static const char * const aic3x_button_debounce[] = {
+       "0ms", "8ms", "16ms", "32ms" };
+static const struct soc_enum aic3x_button_debounce_enum =
+SOC_ENUM_SINGLE(AIC3X_HEADSET_DETECT_CTRL_A, 0, 4, aic3x_button_debounce);
+
  /*
   * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps
   */
@@ -287,6 +482,9 @@ static DECLARE_TLV_DB_SCALE(adc_tlv, 0, 50, 0);
   */
  static DECLARE_TLV_DB_SCALE(output_stage_tlv, -5900, 50, 1);

+static DECLARE_TLV_DB_SCALE(gain_stage_tlv, -1200, 150, 0);
+
+
  static const struct snd_kcontrol_new aic3x_snd_controls[] = {
         /* Output */
         SOC_DOUBLE_R_TLV("PCM Playback Volume",
@@ -399,6 +597,36 @@ static const struct snd_kcontrol_new aic3x_snd_controls[] = {
         SOC_DOUBLE_R("PGA Capture Switch", LADC_VOL, RADC_VOL, 7, 0x01, 1),

         SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]),
+       /* Additional controls */
+       SOC_DOUBLE_R_AIC3X_TLV("Mic3L Volume",
+                              MIC3LR_2_LADC_CTRL, MIC3LR_2_RADC_CTRL, 4, 8, 1),
+       SOC_DOUBLE_R_AIC3X_TLV("Mic3R Volume",
+                              MIC3LR_2_LADC_CTRL, MIC3LR_2_RADC_CTRL, 0, 8, 1),
+       SOC_DOUBLE_R_AIC3X_TLV("Line1L Volume",
+                              LINE1L_2_LADC_CTRL, LINE1L_2_RADC_CTRL, 3, 8, 1),
+       SOC_DOUBLE_R_AIC3X_TLV("Line1R Volume",
+                              LINE1R_2_LADC_CTRL, LINE1R_2_RADC_CTRL, 3, 8, 1),
+       SOC_SINGLE_AIC3X_TLV("Line2L Volume",
+                            LINE2L_2_LADC_CTRL, 3, 8, 1),
+       SOC_SINGLE_AIC3X_TLV("Line2R Volume",
+                            LINE2R_2_RADC_CTRL, 3, 8, 1),
+       SOC_ENUM("Headset Jack Debounce", aic3x_headset_debounce_enum),
+       SOC_ENUM("Button Press Debounce", aic3x_button_debounce_enum),
+       SOC_SINGLE("Headset Detect Enable",
+                  AIC3X_HEADSET_DETECT_CTRL_A, 7, 1, 0),
+       SOC_SINGLE_EXT_VOL("Headset Detect Type", AIC3X_HEADSET_DETECT_CTRL_A,
+                          AIC3X_HEADSET_DETECT_A_SHIFT,
+                          AIC3X_HEADSET_DETECT_A_MASK, 0),
+       SOC_SINGLE_EXT_VOL("Button Detect Flag", AIC3X_HEADSET_DETECT_CTRL_B,
+                          AIC3X_BUTTON_DETECT_SHIFT,
+                          AIC3X_BUTTON_DETECT_MASK, 0),
+       SOC_SINGLE_EXT_VOL("Headset Detect Flag", AIC3X_HEADSET_DETECT_CTRL_B,
+                          AIC3X_HEADSET_DETECT_B_SHIFT,
+                          AIC3X_HEADSET_DETECT_B_MASK, 0),
+       SOC_SINGLE("High power output Ac-coupled",
+                  AIC3X_HEADSET_DETECT_CTRL_B, 7, 1, 0),
+       SOC_SINGLE("Stereo pseudodifferential output",
+                  AIC3X_HEADSET_DETECT_CTRL_B, 3, 1, 0),
  };

  static const struct snd_kcontrol_new aic3x_mono_controls[] = {
----------------------------------------------------------------------------
P.S. I didn't remove loops because functions will be to big to fit in one screen, 
actually I don't like it.

Cheers,
Tim Karaldin

[PATCH V2 2/2] ASoC: TLV320AIC3x: Adding additional functionality for 3106

[Timur Karaldin]

Here is the result:
----------------------------------------------------------------------------
diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
index e521ac3..22bd0be 100644
--- a/sound/soc/codecs/tlv320aic3x.h
+++ b/sound/soc/codecs/tlv320aic3x.h
@@ -272,8 +272,13 @@ enum {
  };

  #define AIC3X_HEADSET_DETECT_ENABLED   0x80
-#define AIC3X_HEADSET_DETECT_SHIFT     5
-#define AIC3X_HEADSET_DETECT_MASK      3
+#define AIC3X_HEADSET_DETECT_A_SHIFT   5
+#define AIC3X_HEADSET_DETECT_A_MASK    3
+
+#define AIC3X_BUTTON_DETECT_SHIFT      5
+#define AIC3X_BUTTON_DETECT_MASK       1
+#define AIC3X_HEADSET_DETECT_B_SHIFT   4
+#define AIC3X_HEADSET_DETECT_B_MASK    1
  #define AIC3X_HEADSET_DEBOUNCE_SHIFT   2
  #define AIC3X_HEADSET_DEBOUNCE_MASK    7
  #define AIC3X_BUTTON_DEBOUNCE_SHIFT    0
----------------------------------------------------------------------------
Cheers,
Timur Karaldin