Hello, Looks good, but I have one comment, you can consider if you like it... ... > +static int omap_mcbsp2_st_set_mode(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + u8 value = ucontrol->value.integer.value[0]; > + > + if (value == omap_st_is_enabled(1)) > + return 0; > + > + if (value) > + omap_st_enable(1); > + else > + omap_st_disable(1); > + > + return 1; > +} > + > +static int omap_mcbsp2_st_get_mode(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + ucontrol->value.integer.value[0] = omap_st_is_enabled(1); > + return 0; > +} > + > +static int omap_mcbsp3_st_set_mode(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + u8 value = ucontrol->value.integer.value[0]; > + > + if (value == omap_st_is_enabled(2)) > + return 0; > + > + if (value) > + omap_st_enable(2); > + else > + omap_st_disable(2); > + > + return 1; > +} > + > +static int omap_mcbsp3_st_get_mode(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + > + ucontrol->value.integer.value[0] = omap_st_is_enabled(2); > + return 0; > +} Instead of having these two set of function, I would have only one: static int omap_mcbsp_st_put_mode(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; u8 value = ucontrol->value.integer.value[0]; if (value == omap_st_is_enabled(mc->reg)) return 0; if (value) omap_st_enable(mc->reg); else omap_st_disable(mc->reg); return 1; } static int omap_mcbsp_st_get_mode(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; ucontrol->value.integer.value[0] = omap_st_is_enabled(mc->reg); return 0; } Than > +static const struct snd_kcontrol_new omap_mcbsp2_st_controls[] = { > + SOC_SINGLE_EXT("McBSP2 Sidetone Switch", 0, 0, 1, 0, > + omap_mcbsp2_st_get_mode, omap_mcbsp2_st_set_mode), SOC_SINGLE_EXT("McBSP2 Sidetone Switch", 1, 0, 1, 0, omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode), > + OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 0 Volume", > + -32768, 32767, > + omap_mcbsp2_get_st_ch0_volume, > + omap_mcbsp2_set_st_ch0_volume), > + OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 1 Volume", > + -32768, 32767, > + omap_mcbsp2_get_st_ch1_volume, > + omap_mcbsp2_set_st_ch1_volume), > +}; > + > +static const struct snd_kcontrol_new omap_mcbsp3_st_controls[] = { > + SOC_SINGLE_EXT("McBSP3 Sidetone Switch", 0, 0, 1, 0, > + omap_mcbsp3_st_get_mode, omap_mcbsp3_st_set_mode), SOC_SINGLE_EXT("McBSP3 Sidetone Switch", 2, 0, 1, 0, omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode), > + OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 0 Volume", > + -32768, 32767, > + omap_mcbsp3_get_st_ch0_volume, > + omap_mcbsp3_set_st_ch0_volume), > + OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 1 Volume", > + -32768, 32767, > + omap_mcbsp3_get_st_ch1_volume, > + omap_mcbsp3_set_st_ch1_volume), > +}; > + > +int omap_mcbsp_st_add_controls(struct snd_soc_codec *codec, int mcbsp_id) > +{ > + if (!cpu_is_omap34xx()) > + return -ENODEV; > + > + switch (mcbsp_id) { > + case 2: /* McBSP 2 */ > + return snd_soc_add_controls(codec, omap_mcbsp2_st_controls, > + ARRAY_SIZE(omap_mcbsp2_st_controls)); > + case 3: /* McBSP 3 */ > + return snd_soc_add_controls(codec, omap_mcbsp3_st_controls, > + ARRAY_SIZE(omap_mcbsp3_st_controls)); > + default: > + break; > + } > + > + return -EINVAL; > +} > +EXPORT_SYMBOL_GPL(omap_mcbsp_st_add_controls); > + > static int __init snd_omap_mcbsp_init(void) > { > return snd_soc_register_dais(omap_mcbsp_dai, > diff --git a/sound/soc/omap/omap-mcbsp.h b/sound/soc/omap/omap-mcbsp.h > index 1968d03..6c363e5 100644 > --- a/sound/soc/omap/omap-mcbsp.h > +++ b/sound/soc/omap/omap-mcbsp.h > @@ -57,4 +57,6 @@ enum omap_mcbsp_div { > > extern struct snd_soc_dai omap_mcbsp_dai[NUM_LINKS]; > > +int omap_mcbsp_st_add_controls(struct snd_soc_codec *codec, int mcbsp_id); > + > #endif -- Péter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html