Re: [PATCH 3/4] ASoC: enable ssp on aspenite

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

On Wed, Mar 10, 2010 at 08:15:28AM -0500, Haojian Zhuang wrote:

> Append ssp driver for pxa168 since PLL could be generated by a new way.

This really doesn't seem like an optimal solution - if all that's
changed is that there's a new PLL then it should be possible to handle
that through either librification like the Samsung IISv2 block or with
runtime conditional code like the existing support for the clock
dithering on the PXA3xx.  Forking the entire driver makes maintinance
much harder.

> Append aspenite also that could support pxa168-ssp.

This should be a separate patch.

> +	ret = seek_mclk_conf(rate, width, channel);
> +	if (ret < 0)
> +		return ret;
> +	index = ret;
> +	mclk = get_mclk(ret);

seek_mclk_conf() doesn't appear to have been declared anywhere.

> +	/* set codec DAI configuration */
> +	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S
> +				  | SND_SOC_DAIFMT_NB_IF
> +				  | SND_SOC_DAIFMT_CBS_CFS);
> +	if (ret < 0)
> +		return ret;

Why are you inverting the frame?  It shouldn't hurt but it's an odd
choice.

> +static int aspenite_hifi_hw_free(struct snd_pcm_substream *substream)
> +{
> +	return 0;
> +}

Remove this if it's empty.

> +	/* Static setup for now */
> +	snd_soc_dapm_enable_pin(codec, "Headset Speaker");
> +	snd_soc_dapm_enable_pin(codec, "Headset Mic");
> +	snd_soc_dapm_enable_pin(codec, "Headphone");

No need to do this, pins are enaled by default.

> +	snd_soc_dapm_sync(codec);

This shouldn't be needed any more, it should get done by the core.