Re: [PATCH 07/12] add phycore sound support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 19, 2009 at 04:48:21PM +0100, Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>

> +	if (machine_is_pcm038() || machine_is_pcm037()) {

Just check when probing the driver.

> +		snd_soc_dai_set_fmt(cpu_dai,
> +				SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> +				SND_SOC_DAIFMT_CBM_CFM);
> +		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +			snd_soc_dai_set_fmt(codec_dai,
> +				SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
> +				SND_SOC_DAIFMT_CBM_CFM);
> +		} else {
> +			snd_soc_dai_set_fmt(codec_dai,
> +				SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF |
> +				SND_SOC_DAIFMT_CBM_CFM);
> +			snd_soc_dai_set_tdm_slot(codec_dai, 0, 0, 4, 0);
> +		}

This looks very wrong.  The CPU is unconditionally configured to use I2S
mode but the CODEC may be configured to use either I2S or DSP mode
depending on the direction of audio.  The CPU and CODEC should at least
be in the same mode...

> +		/* Put DC field of STCCR to 1 (not zero) */
> +		ret = cpu_dai->ops->set_tdm_slot(cpu_dai, 0x0, 0xfffffffc, 4, 0);

That comment doesn't really clarify anything for me...

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

> +static int imx_phycore_probe(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static int imx_phycore_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}

Remove these if they have no content.

> +#ifdef CONFIG_SND_SOC_WM9712
> +static struct snd_soc_dai_link imx_phycore_dai_ac97[] = {
> +	{
> +		.name		= "HiFi",
> +		.stream_name	= "HiFi",
> +		.codec_dai	= &wm9712_dai[WM9712_DAI_AC97_HIFI],
> +		.ops		= &imx_phycore_hifi_ops,
> +	},
> +};
> +#endif

I'd make this a second machine driver, it doesn't appear to have
anything in common with the Atlas based system and the conditionals make
everything more complicated.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux