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

[Ola Lilja]

On 04/27/2012 01:15 PM, Mark Brown wrote:

> On Fri, Apr 27, 2012 at 12:59:06PM +0200, Ola Lilja wrote:
> 
>> 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.
> 
> No, I really don't see any value at all in it.  The machine drivers
> aren't actually sharing anything visible and the effect of what you're
> doing is to make the selection of machine a compile time one instead of
> a runtime one.


No, that is a misunderstanding. We are just dividing the machine-driver file
into one main-file and then calling functions from other ones. It is not
affecting the framework in any way. We just want to divide the code in a way we
find useful. One file calling functions from another one. I don't see how that
can be a problem.

> 
>> >> +	/* 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.
> 
> But what are you actually tying to do with this?  It sounds rather like
> you're selecting between DSP A and B modes...


Yes, I've moved this into the DSP A/B selection for the platform-DAI. I will do
the same for the codec-DAI.