[alsa-devel] [PATCH] ASoC: add ak4554 driver

[Kuninori Morimoto]

Hi Lars

Thank you for checking patch

> > +static int ak4554_dai_hw_params(struct snd_pcm_substream *substream,
> > +				struct snd_pcm_hw_params *params,
> > +				struct snd_soc_dai *dai)
> > +{
> > +	struct ak4554_priv *priv = dev_get_drvdata(dai->dev);
> > +
> > +	if ((priv->usrcnt > 0) &&
> > +	    (priv->rate != params_rate(params))) {
> > +		dev_err(dai->dev, "asymmetric rate\n");
> > +		return -EIO;
> > +	}
> > +
> > +	priv->usrcnt++;
> > +	priv->rate = params_rate(params);
> 
> Set the symmetric_rates field of the dai driver to 1 to enforce symmetric
> rates. No need to implement this by hand.

Hmm...
I guess this "symmetric_rates" works if dai has both .playback / .capture

 ak4554_dai
   - playback
   - capture

But, this ak4554 can't use above style, because its format is not symmetric.
 (playback is SND_SOC_DAIFMT_RIGHT_J, capture is SND_SOC_DAIFMT_LEFT_J)
So, it is using

 ak4554_dai-playback
   - playback
 ak4554_dai-capture
   - capture

I guess it can't use "symmetric_rates" on this.
We talked this on
http://comments.gmane.org/gmane.linux.alsa.devel/108346

And, I can update my log comment if it was un-understandable.

> > +struct snd_soc_dai_driver ak4554_dai[] = {
> > +	{
> > +		.name = "ak4554-hifi-playback",
> > +		.playback = {
> > +			.stream_name = "Playback",
> > +			.channels_min = 2,
> > +			.channels_max = 2,
> > +			.rates = SNDRV_PCM_RATE_8000_48000,
> > +			.formats = SNDRV_PCM_FMTBIT_S16_LE,
> > +		},
> > +		.ops = &ak4554_dai_ops,
> > +	}, {
> > +		.name = "ak4554-hifi-capture",
> > +		.capture = {
> > +			.stream_name = "Capture",
> > +			.channels_min = 2,
> > +			.channels_max = 2,
> > +			.rates = SNDRV_PCM_RATE_8000_48000,
> > +			.formats = SNDRV_PCM_FMTBIT_S16_LE,
> > +		},
> > +		.ops = &ak4554_dai_ops,
> > +	}
> > +};
> > +EXPORT_SYMBOL_GPL(ak4554_dai);
> 
> I don't think this needs to be exported and the struct should be static

Indeed.
Thank you, I will remove it in v2

> > +static int ak4554_probe(struct snd_soc_codec *codec)
> > +{
> > +	dev_info(codec->dev, "probed\n");
> > +	return 0;
> > +}
> > +
> > +static int ak4554_remove(struct snd_soc_codec *codec)
> > +{
> > +	dev_info(codec->dev, "removed\n");
> > +	return 0;
> > +}
> 
> Remove the two functions above, there is not much point in printing these messages.

OK, it will be removed in v2

Best regards
---
Kuninori Morimoto