[PATCH v2] ASoC: rt711-sdca: change capture switch controls

[Shuming [范書銘]]

> >
> > +static void rt711_sdca_set_fu0f_capture_ctl(struct rt711_sdca_priv
> > +*rt711) {
> > +	if (rt711->fu0f_dapm_mute || rt711->fu0f_mixer_mute) {
> > +		regmap_write(rt711->regmap,
> > +			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC,
> RT711_SDCA_ENT_USER_FU0F,
> > +				RT711_SDCA_CTL_FU_MUTE, CH_L), 0x01);
> > +		regmap_write(rt711->regmap,
> > +			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC,
> RT711_SDCA_ENT_USER_FU0F,
> > +				RT711_SDCA_CTL_FU_MUTE, CH_R), 0x01);
> > +	} else {
> > +		regmap_write(rt711->regmap,
> > +			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC,
> RT711_SDCA_ENT_USER_FU0F,
> > +				RT711_SDCA_CTL_FU_MUTE, CH_L), 0x00);
> > +		regmap_write(rt711->regmap,
> > +			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC,
> RT711_SDCA_ENT_USER_FU0F,
> > +				RT711_SDCA_CTL_FU_MUTE, CH_R), 0x00);
> > +	}
> > +}
> > +
> 
> Please, keep the idependent mute functionality for left and right channel. Use
> bitmap instead bool for kcontrol put/get . I appologize, if my example code
> confused you. I just wanted to describe the logic.
> 
> Also, perhaps, you may change the register with one regmap_write() ?

I see. I think the kcontrol could use 'SOC_DOUBLE_EXT' macro to create it.
The put callback function checks the bitmap to mute/unmute the left/right channel.

> > +static void rt711_sdca_set_fu1e_capture_ctl(struct rt711_sdca_priv
> > +*rt711) {
> > +	if (rt711->fu1e_dapm_mute || rt711->fu1e_mixer_mute) {
> > +		regmap_write(rt711->regmap,
> > +			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC,
> RT711_SDCA_ENT_USER_FU1E,
> > +				RT711_SDCA_CTL_FU_MUTE, CH_L), 0x01);
> > +		regmap_write(rt711->regmap,
> > +			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC,
> RT711_SDCA_ENT_USER_FU1E,
> > +				RT711_SDCA_CTL_FU_MUTE, CH_R), 0x01);
> > +	} else {
> > +		regmap_write(rt711->regmap,
> > +			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC,
> RT711_SDCA_ENT_USER_FU1E,
> > +				RT711_SDCA_CTL_FU_MUTE, CH_L), 0x00);
> > +		regmap_write(rt711->regmap,
> > +			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC,
> RT711_SDCA_ENT_USER_FU1E,
> > +				RT711_SDCA_CTL_FU_MUTE, CH_R), 0x00);
> > +	}
> > +}
> > +
> > +static int rt711_sdca_capture_switch_get(struct snd_kcontrol *kcontrol,
> > +			struct snd_ctl_elem_value *ucontrol) {
> > +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> > +	struct rt711_sdca_priv *rt711 =
> > +snd_soc_component_get_drvdata(component);
> > +
> > +	if (strstr(ucontrol->id.name, "FU1E Capture Switch"))
> > +		ucontrol->value.integer.value[0] = !rt711->fu1e_mixer_mute;
> > +	else if (strstr(ucontrol->id.name, "FU0F Capture Switch"))
> > +		ucontrol->value.integer.value[0] = !rt711->fu0f_mixer_mute;
> > +	return 0;
> > +}
> 
> It's not so nice (strstr). Please, use diferent functions to set/get FU1E and FU0F
> controls.

OK, I will create the different functions for FU1E and FU0F.

> > +
> > +static int rt711_sdca_capture_switch_put(struct snd_kcontrol *kcontrol,
> > +			struct snd_ctl_elem_value *ucontrol) {
> > +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> > +	struct rt711_sdca_priv *rt711 =
> > +snd_soc_component_get_drvdata(component);
> > +
> > +	if (strstr(ucontrol->id.name, "FU1E Capture Switch")) {
> > +		rt711->fu1e_mixer_mute = !ucontrol->value.integer.value[0];
> > +		rt711_sdca_set_fu1e_capture_ctl(rt711);
> > +	} else if (strstr(ucontrol->id.name, "FU0F Capture Switch")) {
> > +		rt711->fu0f_mixer_mute = !ucontrol->value.integer.value[0];
> > +		rt711_sdca_set_fu0f_capture_ctl(rt711);
> > +	}
> > +	return 0;
> > +}
> 
> The return value for the kcontrol put callback should be:
> 
> a) a negative error code
> b) 0 - no change
> c) 1 - the value was changed
> 
> If you don't return 1 on change, the other user space applications which are
> monitoring the given kcontrol won't be notified about changes.
> 
> Perhaps, other put callbacks (functions) in this driver require this cleanup, too.

Ok, I will add the return value and test that. Thanks.

> 				Jaroslav
> 
> --
> Jaroslav Kysela <perex at perex.cz>
> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
> ------Please consider the environment before printing this e-mail.