Re: [patch 2/2] alsa: s6105 IP camera machine specific ASoC code

[Mark Brown <broonie@sirena.org.uk>]

On Thu, Mar 26, 2009 at 02:36:14PM +0100, Daniel Gl??ckner wrote:

> This patch adds machine specific code for the audio part of the Stretch
> s6105 IP camera reference design.

This looks mostly good, though some of the comments on the platform
support affect this driver too.  You should also check for current ASoC
APIs.

> +	/* set the codec system clock */
> +	ret = snd_soc_dai_set_sysclk(codec_dai, 0, S6105_CODEC_CLOCK,
> +					    SND_SOC_CLOCK_OUT);

Are you sure you're applying the correct configuration to the correct
DAI here - S6105_CODEC_CLOCK is a CPU define, not a CODEC define?

> +static int snd_soc_dapm_set_endpoint(struct snd_soc_codec *codec, char *name,
> +				     int status)
> +{
> +	if (status)
> +		return snd_soc_dapm_enable_pin(codec, name);
> +	else
> +		return snd_soc_dapm_disable_pin(codec, name);
> +}

This shouldn't be in a driver!

> +	snd_soc_dapm_set_endpoint(codec, "Audio Out Differential", !val);
> +	snd_soc_dapm_set_endpoint(codec, "Audio Out Stereo", val);
> +	snd_soc_dapm_sync(codec);

Hrm.  Given the fact that you can't have the stereo and differential
outputs enabled simultaneously is it not safer to first disable the
active output, then sync, then enable the new output?