Re: [PATCH 11/11] ASoC: codecs: rt298: Fix jack detection

From: Mark Brown <broonie@kernel.org>
To: "Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>
Cc: Oder Chiou <oder_chiou@realtek.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Cezary Rojewski <cezary.rojewski@intel.com>,
	alsa-devel@alsa-project.org, Takashi Iwai <tiwai@suse.com>,
	Liam Girdwood <lgirdwood@gmail.com>
Subject: Re: [PATCH 11/11] ASoC: codecs: rt298: Fix jack detection
Date: Thu, 9 Jun 2022 15:55:00 +0100	[thread overview]
Message-ID: <YqIJxPFwwsbAPJ14@sirena.org.uk> (raw)
In-Reply-To: <20220609133541.3984886-12-amadeuszx.slawinski@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]

On Thu, Jun 09, 2022 at 03:35:41PM +0200, Amadeusz Sławiński wrote:
> On our RVP platforms using rt298 with combojack we've seen issues with
> controls being in incorrect state after suspend/resume cycle. This is
> caused by codec driver not setting pins to correct state and causing
> codec suspend method to not be called. Which on resume caused codec
> registers to be in undefined state. Fix this by setting pins correctly
> in jack detect function.

Again fixes should go before cleanups.  Could you be more specific about
what was wrong with the existing code and how this fixes it?  

>  static int rt298_mic_detect(struct snd_soc_component *component,
>  			    struct snd_soc_jack *jack, void *data)
>  {
> +	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
>  	struct rt298_priv *rt298 = snd_soc_component_get_drvdata(component);
> -	struct snd_soc_dapm_context *dapm;
> -	bool hp = false;
> -	bool mic = false;
> -	int status = 0;
>  
>  	rt298->jack = jack;
>  
> -	/* If jack in NULL, disable HS jack */
> -	if (!jack) {
> +	if (jack) {
> +		/* enable IRQ */
> +		if (rt298->jack->status & SND_JACK_HEADPHONE)
> +			snd_soc_dapm_force_enable_pin(dapm, "LDO1");
> +		if (rt298->jack->status & SND_JACK_MICROPHONE) {
> +			snd_soc_dapm_force_enable_pin(dapm, "HV");
> +			snd_soc_dapm_force_enable_pin(dapm, "VREF");
> +		}
> +		regmap_update_bits(rt298->regmap, RT298_IRQ_CTRL, 0x2, 0x2);
> +		enable_irq(rt298->i2c->irq);
> +		snd_soc_jack_report(rt298->jack, rt298->jack->status,
> +				    SND_JACK_MICROPHONE | SND_JACK_HEADPHONE);

It looks rt298_jack_detect() already forces the pins on?  It's not clear
to me what the relationship between this code and the existing code is.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]