On 04/23/2012 09:05 PM, Mark Brown wrote:
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.
We will add more codecs to be matched up the same machine-driver and I found it useful to have this split. It just separates the callbacks related to each codec added in the dai-link-struct. I would like to keep this division if that is OK.
- /* 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.
The bit delay is the number of bit-clocks from the framesync to the first data-bit. For the AB8500-chip it is set by the bit AB8500_DIGIFCONF2_IF0DEL. I would have put this in the set_dai_fmt but I have not found a bit that is controlling this.
- } 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.
OK.
- 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.
Ah, then I have to put back all the disables instead, which I removed before sending the patch =)