Re: [PATCH 2/2] ASoC: Blackfin: new machine driver for ADAV801/ADAV803 codecs

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

On 7 Aug 2010, at 16:27, Mike Frysinger <vapier@gentoo.org> wrote:

> +config SND_BF5XX_SOC_ADAV80X
> +	tristate "SoC ADAV801/3 Audio support"
> +	depends on SND_BF5XX_I2S && (SPI_MASTER || I2C)
> +	select SND_BF5XX_SOC_I2S
> +	select SND_SOC_ADAV80X
> +	help
> +	  Say Y if you want to add support for ADAV80X SoC audio.

This help text is really not terribly descriptive - this is support for that CODEC on a particular board, not support for the CODEC itself. 

As with the other Blackfin machine drivers you really should look at reporting the particular reference board this is on rather than providing a single driver which assumes a particular hookup for the system. Things like this...

> +	/* For the ADAV80X evaluation board, use the on board crystal
> +	   as XIN, and use XIN as source for PLL1 */
> +	ret = snd_soc_dai_set_pll(codec_dai, ADAV80X_CLK_PLL1,
> +			ADAV80X_CLK_XIN, 27000000, clk);

...aren't going to be true for every system.

> +	/* If you want to use XIN as clock source */
> +	/* ret = snd_soc_dai_set_pll(codec_dai, 0, ADAV80X_CLK_XIN,
> +		27000000, 0); */

This should probably be highlighted by an ifdef or even Kconfig option. Some sort comment explaining why people might want to choose one option or the other would also be useful.