[PATCH v2 0/5] ASoC: codecs: Series of fixes for realtek codecs used on RVPs
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(-)
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);
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; }
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
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
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
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
participants (2)
-
Amadeusz Sławiński -
Mark Brown