Re: [PATCH] ASoC: Tegra: Add support of tegra boards based on ALC5632 codec

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

On Mon, Nov 21, 2011 at 10:08:08PM +0200, Leon Romanovsky wrote:

Mostly OK, a few things below but they're very minor.

> +	err = snd_soc_dai_set_fmt(cpu_dai,
> +					SND_SOC_DAIFMT_I2S |
> +					SND_SOC_DAIFMT_NB_NF |
> +					SND_SOC_DAIFMT_CBS_CFS);
> +	if (err < 0) {
> +		dev_err(card->dev, "cpu_dai fmt not set\n");
> +		return err;
> +	}
> +
> +	err = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
> +					SND_SOC_CLOCK_IN);
> +	if (err < 0) {
> +		dev_err(card->dev, "codec_dai clock not set\n");
> +		return err;
> +	}

These should use the .dai_fmt member in the dai_link struct.

> +	ret = snd_soc_add_controls(codec, tegra_alc5632_controls,
> +				   ARRAY_SIZE(tegra_alc5632_controls));
> +	if (ret < 0)
> +		return ret;
> +
> +	snd_soc_dapm_new_controls(dapm, tegra_alc5632_dapm_widgets,
> +		ARRAY_SIZE(tegra_alc5632_dapm_widgets));
> +
> +	snd_soc_dapm_add_routes(dapm, tegra_alc5632_audio_map,
> +		ARRAY_SIZE(tegra_alc5632_audio_map));

These should use the controls, dapm_widgets and dapm_routes members of
the card structure.

> +
> +	snd_soc_dapm_sync(dapm);

No need for this, the core will sync for you.

> +	if (!machine_is_paz00()) {
> +		dev_err(&pdev->dev, "Not running on Toshiba AC100!\n");
> +		return -ENODEV;
> +	}

Should be no need to check for htis as you're using an explicit platform
device - if the device is registered you should be OK.

> +static int __init tegra_alc5632_init(void)
> +{
> +	return platform_driver_register(&tegra_alc5632_driver);
> +}
> +module_init(tegra_alc5632_init);
> +
> +static void __exit tegra_alc5632_exit(void)
> +{
> +	platform_driver_unregister(&tegra_alc5632_driver);
> +}
> +module_exit(tegra_alc5632_exit);

Use platform_module_driver for this.