Re: [PATCH 8/8] ASoC: Ux500: Add machine-driver

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

[-- Attachment #1.1: Type: text/plain, Size: 1200 bytes --]

On Fri, Apr 20, 2012 at 11:33:29AM +0200, Ola Lilja wrote:

> +snd-soc-ux500-mach-objs := u8500.o ux500_ab8500.o
> +obj-$(CONFIG_SND_SOC_UX500_AB8500) += snd-soc-ux500-mach.o

This split into multiple files *really* doesn't seem like it adds
anything but complexity, the small amount of reuse just doesn't seem
worth it.

> +	/* Setup codec depending on driver-mode */
> +	driver_mode = (channels == 8) ?
> +		DRIVERMODE_CODEC_ONLY : DRIVERMODE_NORMAL;
> +	dev_dbg(dev, "%s: Driver-mode: %s.\n", __func__,
> +		(driver_mode == DRIVERMODE_NORMAL) ? "NORMAL" : "CODEC_ONLY");
> +
> +	ab8500_audio_set_bit_delay(codec_dai, 1);

What's this configuring?  I didn't notice it on the CODEC driver as the
function wasn't exported IIRC.

> +	} else {
> +		ab8500_audio_set_word_length(codec_dai, 20);

This should be done by using the TDM slot API - the slot length is one
of the parameters.

> +	status = snd_soc_add_codec_controls(codec, ux500_ab8500_ctrls,
> +			ARRAY_SIZE(ux500_ab8500_ctrls));

Do this from the driver.

> +	status = snd_soc_dapm_enable_pin(&codec->dapm, "Headset Left");
> +	status |= snd_soc_dapm_enable_pin(&codec->dapm, "Headset Right");

No need to do this, everything defaults on.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]