Re: [PATCH v2] ASoC: AK4671: add ak4671 codec driver

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

On Mon, Sep 07, 2009 at 12:15:48PM +0900, Joonyoung Shim wrote:

> +static int ak4671_pll_event(struct snd_soc_dapm_widget *w,
> +		struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_codec *codec = w->codec;
> +	u8 reg;
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		reg = snd_soc_read(codec, AK4671_PLL_MODE_SELECT1);
> +		reg |= AK4671_PMPLL;
> +		snd_soc_write(codec, AK4671_PLL_MODE_SELECT1, reg);
> +		/* pll lock time: max 40ms */
> +		mdelay(40);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		reg = snd_soc_read(codec, AK4671_PLL_MODE_SELECT1);
> +		reg &= ~AK4671_PMPLL;
> +		snd_soc_write(codec, AK4671_PLL_MODE_SELECT1, reg);
> +		break;
> +	}

I think you need some reference counting or a supply widget here since
this event is there for all the DACs and ADCs:

> +	/* DAC */
> +	SND_SOC_DAPM_DAC_E("DAC Left", "Left HiFi Playback",
> +			AK4671_AD_DA_POWER_MANAGEMENT, 6, 0, ak4671_pll_event,
> +			SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
> +	SND_SOC_DAPM_DAC_E("DAC Right", "Right HiFi Playback",
> +			AK4671_AD_DA_POWER_MANAGEMENT, 7, 0, ak4671_pll_event,
> +			SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
> +
> +	/* ADC */
> +	SND_SOC_DAPM_ADC_E("ADC Left", "Left HiFi Capture",
> +			AK4671_AD_DA_POWER_MANAGEMENT, 4, 0, ak4671_pll_event,
> +			SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
> +	SND_SOC_DAPM_ADC_E("ADC Right", "Right HiFi Capture",
> +			AK4671_AD_DA_POWER_MANAGEMENT, 5, 0, ak4671_pll_event,
> +			SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),

This shouldn't be a problem if they all go on or off at once (except for
the extra latency from waiting for the PLL to go on) but unless I'm
missing something if capture and playback are stopped independantly I'd
expect that the PLL will get shut down when the first one happens,
causing problems for the still active stream.