>From: Ujfalusi Peter (Nokia-D/Tampere)
>Sent: 18 February, 2010 10:15
>
>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
Makes sense - I'll change it.
Cheers, Ilkka
>> +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
