Messages in this thread | | | Subject | Re: [PATCH 08/12] ASoC: wcd9335: add basic controls | From | Srinivas Kandagatla <> | Date | Fri, 27 Jul 2018 10:55:11 +0100 |
| |
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 >
| |