Re: [PATCH v2 1/3] ASoC: rt5645: Simplify rt5645_enable_push_button_irq

[Bard Liao <bardliao@realtek.com>]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Tuesday, July 14, 2015 5:53 PM
> To: Nicolas Boichat
> Cc: Bard Liao; Oder Chiou; Liam Girdwood; Jaroslav Kysela; Takashi Iwai;
> alsa-devel@alsa-project.org; koro.chen@mediatek.com
> Subject: Re: [PATCH v2 1/3] ASoC: rt5645: Simplify
> rt5645_enable_push_button_irq
> 
> On Tue, Jul 14, 2015 at 02:51:25PM +0800, Nicolas Boichat wrote:
> 
> > +		if (codec->component.card->instantiated) {
> > +			snd_soc_dapm_force_enable_pin(dapm, "ADC L power");
> > +			snd_soc_dapm_force_enable_pin(dapm, "ADC R power");
> > +			snd_soc_dapm_sync(dapm);
> > +		} else {
> > +			regmap_update_bits(rt5645->regmap, RT5645_PWR_DIG1,
> > +				RT5645_PWR_ADC_L_BIT | RT5645_PWR_ADC_R_BIT,
> > +				RT5645_PWR_ADC_L_BIT |
> RT5645_PWR_ADC_R_BIT);
> > +		}
> 
> I don't understand why this isn't updating the DAPM state if the device is
> not yet instantiated - this means that when the card instantiates the pins
> will be turned off which presumably isn't what you want given the manual
> register map futzing in the else case.  What's going on here?

Thanks for the review. I think what we need is something like
+		snd_soc_dapm_force_enable_pin(dapm, "ADC L power");
+		snd_soc_dapm_force_enable_pin(dapm, "ADC R power");
+		snd_soc_dapm_sync(dapm);
+		if (!codec->component.card->instantiated) {
+			regmap_update_bits(rt5645->regmap, RT5645_PWR_DIG1,
+				RT5645_PWR_ADC_L_BIT | RT5645_PWR_ADC_R_BIT,
+				RT5645_PWR_ADC_L_BIT | RT5645_PWR_ADC_R_BIT);
+		}

> 
> ------Please consider the environment before printing this e-mail.