linux-kernel - Re: [PATCH 3/4] ASoC: amd: acp: Add machine driver that enables sound for systems with a ES8336 codec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 22 Mar 2023 00:17:24 +0200
From:   Marian Postevca <posteuca@...ex.one>
To:     Mark Brown <broonie@...nel.org>
Cc:     Takashi Iwai <tiwai@...e.com>, Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>, linux-kernel@...r.kernel.org,
        alsa-devel@...a-project.org
Subject: Re: [PATCH 3/4] ASoC: amd: acp: Add machine driver that enables
 sound for systems with a ES8336 codec

Mark Brown <broonie@...nel.org> writes:

>> +static int acp3x_es83xx_speaker_power_event(struct snd_soc_dapm_widget *w,
>> +					    struct snd_kcontrol *kcontrol, int event)
>> +{
>> +	struct acp3x_es83xx_private *priv = get_mach_priv(w->dapm->card);
>> +
>> +	dev_dbg(priv->codec_dev, "speaker power event: %d\n", event);
>> +	if (SND_SOC_DAPM_EVENT_ON(event))
>> +		acp3x_es83xx_set_gpios_values(priv, 1, 0);
>> +	else
>> +		acp3x_es83xx_set_gpios_values(priv, 0, 1);
>
> Why are these two GPIOs tied together like this?
>

These GPIOs represent the speaker and the headphone switches. When
activating the speaker GPIO you have to deactivate the headphone GPIO
and vice versa. The logic is taken from the discussion on the sofproject
pull request:
https://github.com/thesofproject/linux/pull/4112/commits/810d03e0aecdf0caf580a5179ee6873fb33485ab
and
https://github.com/thesofproject/linux/pull/4066

>> +static int acp3x_es83xx_suspend_pre(struct snd_soc_card *card)
>> +{
>> +	struct acp3x_es83xx_private *priv = get_mach_priv(card);
>> +
>> +	dev_dbg(priv->codec_dev, "card suspend\n");
>> +	snd_soc_component_set_jack(priv->codec, NULL, NULL);
>> +	return 0;
>> +}
>
> That's weird, why do that?

This is needed because if suspending the laptop with the headphones
inserted, when resuming, the sound is not working anymore. Sound stops
working on speakers and headphones. Reinsertion and removals of the
headphone doesn't solve the problem.

This seems to be caused by the fact
that the GPIO IRQ stops working in es8316_irq() after resume.
Now the call to snd_soc_component_set_jack() in suspend disables the
GPIO IRQ and in resume the GPIO IRQ is reactivated.
By the way this sequence is also used in bytcht_es8316.c in suspend and
resume:

static int byt_cht_es8316_suspend(struct snd_soc_card *card)
{
	struct snd_soc_component *component;

	for_each_card_components(card, component) {
		if (!strcmp(component->name, codec_name)) {
			dev_dbg(component->dev, "disabling jack detect before suspend\n");
			snd_soc_component_set_jack(component, NULL, NULL);
			break;
		}
	}

	return 0;
}

static int byt_cht_es8316_resume(struct snd_soc_card *card)
{
	struct byt_cht_es8316_private *priv = snd_soc_card_get_drvdata(card);
	struct snd_soc_component *component;

	for_each_card_components(card, component) {
		if (!strcmp(component->name, codec_name)) {
			dev_dbg(component->dev, "re-enabling jack detect after resume\n");
			snd_soc_component_set_jack(component, &priv->jack, NULL);
			break;
		}
	}