[PATCH v2 0/5] ASoC: codecs: Series of fixes for realtek codecs used on RVPs

[PATCH v2 0/5] ASoC: codecs: Series of fixes for realtek codecs used on RVPs

[Amadeusz Sławiński]

Our tests platforms do use realtek codecs, while implementing avs driver
and machine boards for it, we identified some problems with those codec
drivers.

This series aims to fix those issues.

Changes in v2:
 - Drop patches which were merged from v1
 - Drop patches changing IRQ behaviour
 - Add patches to set component to NULL on remove - disabling IRQs
   worked around the problem, so those patches weren't needed in earlier
   series

Amadeusz Sławiński (5):
  ASoC: codecs: rt298: Fix NULL jack in interrupt
  ASoC: codecs: rt298: Fix jack detection
  ASoC: codecs: rt286: Set component to NULL on remove
  ASoC: codecs: rt298: Set component to NULL on remove
  ASoC: codecs: rt274: Set component to NULL on remove

 sound/soc/codecs/rt274.c |  1 +
 sound/soc/codecs/rt286.c |  1 +
 sound/soc/codecs/rt298.c | 43 ++++++++++++++++++++--------------------
 3 files changed, 23 insertions(+), 22 deletions(-)

-- 
2.25.1

[PATCH v2 1/5] ASoC: codecs: rt298: Fix NULL jack in interrupt

[Amadeusz Sławiński]

Set rt298->jack to passed value in mic_detect, otherwise when jack is
set to NULL on next interrupt call, we may use freed pointer.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/codecs/rt298.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt298.c b/sound/soc/codecs/rt298.c
index 266a2cc55b8d..6a615943f983 100644
--- a/sound/soc/codecs/rt298.c
+++ b/sound/soc/codecs/rt298.c
@@ -335,6 +335,8 @@ static int rt298_mic_detect(struct snd_soc_component *component,
 	bool mic = false;
 	int status = 0;
 
+	rt298->jack = jack;
+
 	/* If jack in NULL, disable HS jack */
 	if (!jack) {
 		regmap_update_bits(rt298->regmap, RT298_IRQ_CTRL, 0x2, 0x0);
@@ -344,7 +346,6 @@ static int rt298_mic_detect(struct snd_soc_component *component,
 		return 0;
 	}
 
-	rt298->jack = jack;
 	regmap_update_bits(rt298->regmap, RT298_IRQ_CTRL, 0x2, 0x2);
 
 	rt298_jack_detect(rt298, &hp, &mic);
-- 
2.25.1

[PATCH v2 2/5] ASoC: codecs: rt298: Fix jack detection

[Amadeusz Sławiński]

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.

Above problem is caused by the fact that when jack == NULL code doesn't
reach rt298_jack_detect() function which sets pins. Alternatively
problem could be fixed by just moving rt298_jack_detect, but as rt298
codec is similar to rt286, align the code by setting pins explicitly.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/codecs/rt298.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/sound/soc/codecs/rt298.c b/sound/soc/codecs/rt298.c
index 6a615943f983..e1d94f128fd9 100644
--- a/sound/soc/codecs/rt298.c
+++ b/sound/soc/codecs/rt298.c
@@ -329,34 +329,31 @@ static void rt298_jack_detect_work(struct work_struct *work)
 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);
+		/* Send an initial empty report */
+		snd_soc_jack_report(rt298->jack, rt298->jack->status,
+				    SND_JACK_MICROPHONE | SND_JACK_HEADPHONE);
+	} else {
+		/* Disable IRQ */
 		regmap_update_bits(rt298->regmap, RT298_IRQ_CTRL, 0x2, 0x0);
-		dapm = snd_soc_component_get_dapm(component);
+		snd_soc_dapm_disable_pin(dapm, "HV");
+		snd_soc_dapm_disable_pin(dapm, "VREF");
 		snd_soc_dapm_disable_pin(dapm, "LDO1");
-		snd_soc_dapm_sync(dapm);
-		return 0;
 	}
-
-	regmap_update_bits(rt298->regmap, RT298_IRQ_CTRL, 0x2, 0x2);
-
-	rt298_jack_detect(rt298, &hp, &mic);
-	if (hp)
-		status |= SND_JACK_HEADPHONE;
-
-	if (mic)
-		status |= SND_JACK_MICROPHONE;
-
-	snd_soc_jack_report(rt298->jack, status,
-		SND_JACK_MICROPHONE | SND_JACK_HEADPHONE);
+	snd_soc_dapm_sync(dapm);
 
 	return 0;
 }
-- 
2.25.1

[PATCH v2 3/5] ASoC: codecs: rt286: Set component to NULL on remove

[Amadeusz Sławiński]

Make sure that component is set to proper value, otherwise we may
dereference freed component in interrupt.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/codecs/rt286.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/codecs/rt286.c b/sound/soc/codecs/rt286.c
index b2b0b2b1e4d0..c4f7c4c2d793 100644
--- a/sound/soc/codecs/rt286.c
+++ b/sound/soc/codecs/rt286.c
@@ -960,6 +960,7 @@ static void rt286_remove(struct snd_soc_component *component)
 	struct rt286_priv *rt286 = snd_soc_component_get_drvdata(component);
 
 	cancel_delayed_work_sync(&rt286->jack_detect_work);
+	rt286->component = NULL;
 }
 
 #ifdef CONFIG_PM
-- 
2.25.1

[PATCH v2 4/5] ASoC: codecs: rt298: Set component to NULL on remove

[Amadeusz Sławiński]

Make sure that component is set to proper value, otherwise we may
dereference freed component in interrupt.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/codecs/rt298.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/codecs/rt298.c b/sound/soc/codecs/rt298.c
index e1d94f128fd9..b0b53d4f07df 100644
--- a/sound/soc/codecs/rt298.c
+++ b/sound/soc/codecs/rt298.c
@@ -1022,6 +1022,7 @@ static void rt298_remove(struct snd_soc_component *component)
 	struct rt298_priv *rt298 = snd_soc_component_get_drvdata(component);
 
 	cancel_delayed_work_sync(&rt298->jack_detect_work);
+	rt298->component = NULL;
 }
 
 #ifdef CONFIG_PM
-- 
2.25.1

[PATCH v2 5/5] ASoC: codecs: rt274: Set component to NULL on remove

[Amadeusz Sławiński]

Make sure that component is set to proper value, otherwise we may
dereference freed component in interrupt.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/codecs/rt274.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/codecs/rt274.c b/sound/soc/codecs/rt274.c
index 6b208f9eb503..f2c50b11e4d0 100644
--- a/sound/soc/codecs/rt274.c
+++ b/sound/soc/codecs/rt274.c
@@ -993,6 +993,7 @@ static void rt274_remove(struct snd_soc_component *component)
 	struct rt274_priv *rt274 = snd_soc_component_get_drvdata(component);
 
 	cancel_delayed_work_sync(&rt274->jack_detect_work);
+	rt274->component = NULL;
 }
 
 #ifdef CONFIG_PM
-- 
2.25.1

Re: [PATCH v2 0/5] ASoC: codecs: Series of fixes for realtek codecs used on RVPs

[Mark Brown]

On Thu, 7 Jul 2022 14:56:56 +0200, Amadeusz Sławiński wrote:
> Our tests platforms do use realtek codecs, while implementing avs driver
> and machine boards for it, we identified some problems with those codec
> drivers.
> 
> This series aims to fix those issues.
> 
> Changes in v2:
>  - Drop patches which were merged from v1
>  - Drop patches changing IRQ behaviour
>  - Add patches to set component to NULL on remove - disabling IRQs
>    worked around the problem, so those patches weren't needed in earlier
>    series
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] ASoC: codecs: rt298: Fix NULL jack in interrupt
      commit: 9b6803ec1fe0f10942b9297d2d60ec46f2999323
[2/5] ASoC: codecs: rt298: Fix jack detection
      commit: c0c5a242bba878b4d34f7c9612fd6cd6c404d414
[3/5] ASoC: codecs: rt286: Set component to NULL on remove
      commit: c1d7ebda11aae4659b665af61d7277dd351659b9
[4/5] ASoC: codecs: rt298: Set component to NULL on remove
      commit: af3b33b9707d453a12e0cf5ac35d7b97b3524ace
[5/5] ASoC: codecs: rt274: Set component to NULL on remove
      commit: b9f098aa7ae2a022dee06a8ca363e3e0e077f05a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark