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

From: Mark Brown <broonie@kernel.org>
To: Nicolas Boichat <drinkcat@chromium.org>
Cc: Oder Chiou <oder_chiou@realtek.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Takashi Iwai <tiwai@suse.de>, Liam Girdwood <lgirdwood@gmail.com>,
	"koro.chen@mediatek.com" <koro.chen@mediatek.com>,
	Bard Liao <bardliao@realtek.com>
Subject: Re: [PATCH v2 1/3] ASoC: rt5645: Simplify rt5645_enable_push_button_irq
Date: Wed, 15 Jul 2015 12:56:44 +0100	[thread overview]
Message-ID: <20150715115644.GQ11162@sirena.org.uk> (raw)
In-Reply-To: <CANMq1KB_qkH41AiBeMDX6oJVyeOMEi1Tgwoxt9jar0Upqb=L4A@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1512 bytes --]

On Wed, Jul 15, 2015 at 07:50:50PM +0800, Nicolas Boichat wrote:

> > 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);
> > +               }

> Just to make sure I understand... With the code above, the dapm state
> is consistent. However, DAPM will only set the regmap bits when the
> card is instantiated. So why do we still need to update the regmap? To
> make sure we do not miss an early jack/button event? Or would we still
> get jack irq if the pins are enabled a little later? (I guess we can
> live with missing a button event at that stage, but we need the jack
> state to be correct)

I'm assuming it's something to do with early detection, I don't really
know though.

> Also, I'm going to update rt5645_irq_detection to do nothing if the
> codec is not initialized yet (just like rt286.c does). That should be
> ok as we call rt5645_irq from rt5645_set_jack_detect, after the codec
> is ready, which will update the initial jack status, and setup the
> DAPM pins. Does that sound ok?

Yes.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]