Re: [PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Thu, Feb 03, 2011 at 01:56:18PM -0700, Stephen Warren wrote:

> At present, I'm not sure how best to resolve this. Downstream drivers
> directly tweaked the wm8903's registers so that both ADCs processed the
> same input channel.

The CODEC driver needs to have support for routing the left and right
channels like things like the WM8993 do.

>  	{"IN1L", NULL, "Mic Jack"},
> +	{"IN1R", NULL, "Mic Jack"},

This looks odd - I'd expect to see separate widgets for the internal
microphone rather than using the same widget.  This would greatly
simplify the driver code without really impacting the level of
configuration applications need to do (and allowing them to use both
simultaneously if they want to).

> +static int harmony_set_mic_selection(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
> +	struct snd_soc_card *card = codec->card;
> +	struct tegra_harmony *harmony = snd_soc_card_get_drvdata(card);
> +
> +	if (harmony->mic_selection == ucontrol->value.integer.value[0])
> +		return 0;
> +
> +	harmony->mic_selection = ucontrol->value.integer.value[0];
> +	harmony_mic_control(codec);

This should be locking the CODEC mutex while doing the update.

> +static const struct soc_enum harmony_enum[] = {
> +	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(mic_selection_names),
> +			    mic_selection_names),
>  };

Don't create arrays of enums, declare individual varaibles and reference
them...

> +	SOC_ENUM_EXT("Mic Selection", harmony_enum[0],
> +		     harmony_get_mic_selection, harmony_set_mic_selection),

...as it makes the references to them much easier to follow and less
error prone.