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

["Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>]

On 6/9/2022 4:55 PM, Mark Brown wrote:
> 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.

This aligns the code to be similar to other two rt2xx drivers and fixes 
a problem on our side.

Original code doesn't reach rt298_jack_detect() when jack == NULL, so it 
never disables those pins in this case.

I could probably fix this by moving rt298_jack_detect() call, but as 
drivers for rt2xx codecs are quite similar to each other I opted to fix 
the issue by minimizing the differences between them.