I don't see the problem because my codec waits until trigger to activate its interface.
Thanks for the feedback. I'm using the tlv320aic3x codec driver. Which codec are you using?
A modified tlv320aic23. I wrote it before it was available in the kernel, and never finished a merge.
So the question is, who should be responsible for the final turn on?
My thought was that the device (master) which starts the external wires to wiggling should be last.
Fair enough. I suppose that is the reasoning for the addition of the prepare function. The assumption being that a cpu_dai callback to the prepare function will always proceed a call to the codec_dai trigger function. In this way the serial port can be configured and enabled. Then the codec can turn on the bit/frame clocks which will start the flow of data. Although it would seem the codec is unmuted a bit prematurely in this scenario since it happens before the clocks are enabled in the core prepare function -- at least I think this is the case.
However there would still seem to be spurious, or at least superfluous, calls to mcbsp_stop/start when just requesting the device to enter the pause state.
I guess the call tree is then different in the case where the cpu, or machine, is the clock master. This has pros/cons obviously.
If the codec is master, and starts the clocks before the mcbsp is ready that could also cause a pop or noise.
I still a newbie when it comes to the ALSA architecture. Is the proper signal indicating everything is ready, including valid data, a trigger call with cmd=START?
That is my understanding.
I think the main problem with using trigger to start the codec wiggling wires is the need to use schedule_work to do i2c writes. That's probably why the mainline aic23 driver doesn't use trigger. Sample below.
static void codec_trigger_deferred(struct work_struct *work) { struct aic23 *aic23 = container_of(work, struct aic23, deferred_trigger_work); struct snd_soc_codec *codec = &aic23->codec; int playback = aic23->active_mask & ACTIVE_PLAYBACK; u16 dia = (aic23->active_mask) ? 1 : 0; if (playback) { tlv320aic23_set(codec, TLV320AIC23_ACTIVE, 1); tlv320aic23_modify(codec, TLV320AIC23_ANLG, 0, TLV320AIC23_DAC_SELECTED); tlv320aic23_mute_volume(codec, 0); } else { tlv320aic23_mute_volume(codec, 1); if (dia) tlv320aic23_set(codec, TLV320AIC23_ACTIVE, dia); tlv320aic23_modify(codec, TLV320AIC23_ANLG, TLV320AIC23_DAC_SELECTED, 0); if (!dia) tlv320aic23_set(codec, TLV320AIC23_ACTIVE, dia); } }
static int tlv320aic23_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_device *socdev = rtd->socdev; struct snd_soc_codec *codec = socdev->card->codec; struct aic23 *aic23 = container_of(codec, struct aic23, codec); int ret = 0; int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK); unsigned char mask = (playback)? ACTIVE_PLAYBACK : ACTIVE_CAPTURE;
switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: aic23->active_mask |= mask; schedule_work(&aic23->deferred_trigger_work); break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: aic23->active_mask &= ~mask; schedule_work(&aic23->deferred_trigger_work); /* don't stop driving data lines * until digital_mute done */ break; default: return ret; }