On 04/01/2009 02:10 PM, Mark Brown wrote:
On Wed, Apr 01, 2009 at 12:56:55PM +0200, Daniel Glöckner wrote:
+static int aic3x_trigger(struct snd_pcm_substream *substream, int cmd,
struct snd_soc_dai *codec_dai)+{
struct snd_soc_codec *codec = codec_dai->codec;struct aic3x_priv *aic3x = codec->private_data;switch (cmd) {case SNDRV_PCM_TRIGGER_START:case SNDRV_PCM_TRIGGER_RESUME:case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:aic3x->running[substream->stream] = 1;break;Hrm, does the chip support asymmetric configurations for playback and capture?
No, why do you ask?
+/*
- Enables or disables the left/right ADC/DAC according to new.
- Bits in new:
- 0 left ADC
- 1 right ADC
- 2 left DAC
- 4 right DAC
- A set bit enables the component.
- Returns the previous state of those components encoded in the same way.
- */
Is this really required? I can't see it being a good idea to bounce the power of the DAC or ADC while they are live since that will impact the audio stream noticably. It looks like it'd be better to do a check which refuses to do the reconfiguration when it's not possible.
Powering off is required in case the stream has been closed previously but the close_delayed_work has not yet been run. That was the case where I observed the problems this patch tries to fix.
It's especially suspicious since the power of the DAC and ADC is managed via DAPM; the only current user should be safe since it saves then restores the state but it does raise alarm bells.
How about making it a function that always disables the four components and referring to snd_soc_dapm_sync to restore the state?
In this version of the patch I removed the line that restores the state when setting the sampling rate failed in-between. Is it safe to assume that DAPM will not enable the ADC/DAC on other occasions before a valid sampling rate has been configured? One could reorder aic3x_hw_params to detect the error before the first registers are written..
case SND_SOC_BIAS_OFF:
/* force all power off */reg = aic3x_read_reg_cache(codec, LINE1L_2_LADC_CTRL);aic3x_write(codec, LINE1L_2_LADC_CTRL, reg & ~LADC_PWR_ON);This should be split into a separate patch; it's a separate change.
Ok
Daniel