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