Re: [PATCH 08/12] ASoC: wcd9335: add basic controls

[Srinivas Kandagatla]

Thanks Vinod for review comments,

On 25/07/18 13:04, Vinod wrote:

On 23-07-18, 16:54, Srinivas Kandagatla wrote:

+static int wcd9335_set_compander(struct snd_kcontrol *kc,
+                                struct snd_ctl_elem_value *ucontrol)
+{
+       struct snd_soc_component *component = snd_soc_kcontrol_component(kc);
+       struct wcd9335_codec *wcd = dev_get_drvdata(component->dev);
+       int comp = ((struct soc_mixer_control *) kc->private_value)->shift;
+       int value = ucontrol->value.integer.value[0];
+       int sel;
+
+       wcd->comp_enabled[comp] = value;
+       sel = value ? WCD9335_HPH_GAIN_SRC_SEL_COMPANDER :
+               WCD9335_HPH_GAIN_SRC_SEL_REGISTER;
+
+       /* Any specific register configuration for compander */
+       switch (comp) {
+       case COMPANDER_1:
+               /* Set Gain Source Select based on compander enable/disable */
+               snd_soc_component_update_bits(component, WCD9335_HPH_L_EN,
+                                     WCD9335_HPH_GAIN_SRC_SEL_MASK, sel);
+               break;
+       case COMPANDER_2:
+               snd_soc_component_update_bits(component, WCD9335_HPH_R_EN,
+                                     WCD9335_HPH_GAIN_SRC_SEL_MASK, sel);
+               break;
+       case COMPANDER_3:
+               break;
+       case COMPANDER_4:
+               break;
+       case COMPANDER_5:
+               snd_soc_component_update_bits(component, WCD9335_SE_LO_LO3_GAIN,
+                                     WCD9335_HPH_GAIN_SRC_SEL_MASK, sel);
+               break;
+       case COMPANDER_6:
+               snd_soc_component_update_bits(component, WCD9335_SE_LO_LO4_GAIN,
+                                     WCD9335_HPH_GAIN_SRC_SEL_MASK, sel);
+               break;
+       case COMPANDER_7:
+               break;
+       case COMPANDER_8:
+               break;

why not do:

         case COMPANDER_3:
         case COMPANDER_4:
         case COMPANDER_7:
         case COMPANDER_8:
                 break;

thereby clubbing all the ones not handled together..

I removed the un-handled ones in v2.

+static int wcd9335_rx_hph_mode_get(struct snd_kcontrol *kc,
+                                struct snd_ctl_elem_value *ucontrol)
+{
+       struct snd_soc_component *component = snd_soc_kcontrol_component(kc);
+       struct wcd9335_codec *wcd = dev_get_drvdata(component->dev);
+
+       ucontrol->value.integer.value[0] = wcd->hph_mode;

i think this is an enum so shouldn't value.enumerated.item[0] be used?

Yep, there are one more instances like this, i fixed this in v2.

+
+       return 0;
+}
+
+static int wcd9335_rx_hph_mode_put(struct snd_kcontrol *kc,
+                                struct snd_ctl_elem_value *ucontrol)
+{
+       struct snd_soc_component *component = snd_soc_kcontrol_component(kc);
+       struct wcd9335_codec *wcd = dev_get_drvdata(component->dev);
+       u32 mode_val;
+
+       mode_val = ucontrol->value.enumerated.item[0];

yaaay

--srini