[alsa-devel] [PATCH] ASoC: SAMSUNG: Add sound card driver for Snow board

[Mark Brown]

On Tue, Apr 22, 2014 at 01:33:54PM +0530, Tushar Behera wrote:

> Added machine driver to instantiate I2S based sound card on Snow
> board. It has MAX98095 audio codec on board.

In general this isn't up to modern standards, please do try to check
that new code is following best practices.  Did the support for setting
the clocking up in the device tree get merged already?

Please do also pay attention to the CC lists when posting patches, this
seems to have been sent to a fairly random selection of people and
lists.

> +	switch (params_format(params)) {

params_width().

> +	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
> +					     SND_SOC_DAIFMT_NB_NF |
> +					     SND_SOC_DAIFMT_CBS_CFS);

Set this in the dai_link.

> +	ret = snd_soc_dai_set_sysclk(codec_dai, 0,
> +					FIN_PLL_RATE, SND_SOC_CLOCK_IN);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_CDCLK,
> +					0, SND_SOC_CLOCK_OUT);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = snd_soc_dai_set_clkdiv(cpu_dai, SAMSUNG_I2S_DIV_BCLK, bfs);
> +	if (ret < 0)
> +		return ret;

Set this stuff up on probe.  I'm surprised that you need to set BCLK at
all...

> +static int snow_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct snd_soc_codec *codec = rtd->codec;
> +	struct snd_soc_dapm_context *dapm = &codec->dapm;
> +
> +	snd_soc_dapm_sync(dapm);
> +
> +	return 0;
> +}

This does nothing, remove it.

> +	if (!pdev->dev.platform_data && !pdev->dev.of_node) {
> +		dev_err(&pdev->dev, "No platform data supplied\n");
> +		return -EINVAL;
> +	}
> +
> +	name = of_get_property(pdev->dev.of_node, "card-name", NULL);
> +	if (name)
> +		card->name = name;
> +
> +	i2s_node = of_parse_phandle(pdev->dev.of_node,
> +				    "samsung,i2s-controller", 0);
> +	if (!i2s_node) {
> +		dev_err(&pdev->dev,
> +			"Property 'i2s-controller' missing or invalid\n");
> +		return -EINVAL;
> +	}

You're allowing platform data above but I see DT is mandatory here.

> +	ret = snd_soc_register_card(card);
> +	if (ret) {
> +		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
> +		return ret;
> +	}

devm_snd_soc_register_card().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20140422/fc76a035/attachment.sig>