[alsa-devel] JIVE: Add ASoC audio support

[Mark Brown]

On Mon, Jun 23, 2008 at 12:54:23PM +0100, Ben Dooks wrote:

This looks mostly good - a few nits:

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.26-rc5-quilt2/sound/soc/s3c24xx/jive_wm8750.c	2008-06-12 11:07:04.000000000 +0100
> @@ -0,0 +1,266 @@
> +/* sound/soc/s3c24xx/jive_wm8705.c

Typo :)

> +static inline int codec_set_dai_fmt(struct snd_soc_codec_dai *dai,
> +				    unsigned int format)
> +{
> +	return dai->dai_ops.set_fmt(dai, format);
> +}

These functions are a good idea - the equivalents are already in ASoC v2
but haven't been backported into mainline yet - but they should be in
the ASoC core rather than in a machine driver.  Please either remove
them or integrate them into the core (obviously the latter would be nice
but the former is less work).

> +static const struct snd_kcontrol_new wm8750_jive_controls[] = {
> +
> +};

This (and the code to process it) could go since it's empty.

> +
> +	/* Add jive specific widgets */
> +	for (i = 0; i < ARRAY_SIZE(wm8750_dapm_widgets); i++) {
> +		snd_soc_dapm_new_control(codec, &wm8750_dapm_widgets[i]);
> +	}

These should all be updated to use the new bulk registration APIs (these
were added to the ASoC core this year, after the driver appears to have
been written).  Here the equivalent is snd_soc_dapm_new_controls().

> +	/* Set up jive specific audio path audio_map */
> +	for (i = 0; audio_map[i][0] != NULL; i++) {
> +		printk(KERN_INFO "mapping %s => %s\n", audio_map[i][0], audio_map[i][1]);
> +		snd_soc_dapm_connect_input(codec, audio_map[i][0], NULL,
> +					   audio_map[i][1]);
> +	}

Here it's snd_soc_dapm_add_routes().

> +static int __init jive_init(void)
> +{
> +	int ret;
> +
> +	printk("JIVE WM8750 Audio support\n");

This should have a KERN_INFO?