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

[Joonyoung Shim <jy0922.shim@samsung.com>]

On 9/7/2009 7:23 PM, Mark Brown wrote:
> 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.
> 

OK. I was not far-sighted enough to think of that. I will do additional 
try to handle it.

Thanks.