Skip to content

Commit

Permalink
ASoC: codecs: rt700/rt711/rt711-sdca: resume bus/codec in .set_jack_d…
Browse files Browse the repository at this point in the history
…etect

The .set_jack_detect() codec component callback is invoked during card
registration, which happens when the machine driver is probed.

The issue is that this callback can race with the bus suspend/resume,
and IO timeouts can happen. This can be reproduced very easily if the
machine driver is 'blacklisted' and manually probed after the bus
suspends. The bus and codec need to be re-initialized using pm_runtime
helpers.

Previous contributions tried to make sure accesses to the bus during
the .set_jack_detect() component callback only happen when the bus is
active. This was done by changing the regcache status on a component
remove. This is however a layering violation, the regcache status
should only be modified on device probe, suspend and resume. The
component probe/remove should not modify how the device regcache is
handled. This solution also didn't handle all the possible race
conditions, and the RT700 headset codec was not handled.

BugLink: #3643
Fixes: 7ad4d23 ('ASoC: rt711-sdca: Add RT711 SDCA vendor-specific driver')
Fixes: 899b125 ('ASoC: rt711: add snd_soc_component remove callback')
Signed-off-by: Pierre-Louis Bossart <[email protected]>
  • Loading branch information
plbossart committed May 18, 2022
1 parent 5ba1b2e commit f4e8191
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 35 deletions.
16 changes: 10 additions & 6 deletions sound/soc/codecs/rt700.c
Original file line number Diff line number Diff line change
Expand Up @@ -315,17 +315,21 @@ static int rt700_set_jack_detect(struct snd_soc_component *component,
struct snd_soc_jack *hs_jack, void *data)
{
struct rt700_priv *rt700 = snd_soc_component_get_drvdata(component);
int ret;

rt700->hs_jack = hs_jack;

if (!rt700->hw_init) {
dev_dbg(&rt700->slave->dev,
"%s hw_init not ready yet\n", __func__);
return 0;
ret = pm_runtime_resume_and_get(component->dev);
if (ret < 0 && ret != -EACCES) {
dev_err(component->dev, "%s: failed to resume %d\n", __func__, ret);
return ret;
}

rt700->hs_jack = hs_jack;

rt700_jack_init(rt700);

pm_runtime_mark_last_busy(component->dev);
pm_runtime_put_autosuspend(component->dev);

return 0;
}

Expand Down
26 changes: 11 additions & 15 deletions sound/soc/codecs/rt711-sdca.c
Original file line number Diff line number Diff line change
Expand Up @@ -487,16 +487,21 @@ static int rt711_sdca_set_jack_detect(struct snd_soc_component *component,
struct snd_soc_jack *hs_jack, void *data)
{
struct rt711_sdca_priv *rt711 = snd_soc_component_get_drvdata(component);
int ret;

rt711->hs_jack = hs_jack;

if (!rt711->hw_init) {
dev_dbg(&rt711->slave->dev,
"%s hw_init not ready yet\n", __func__);
return 0;
ret = pm_runtime_resume_and_get(component->dev);
if (ret < 0 && ret != -EACCES) {
dev_err(component->dev, "%s: failed to resume %d\n", __func__, ret);
return ret;
}

rt711->hs_jack = hs_jack;

rt711_sdca_jack_init(rt711);

pm_runtime_mark_last_busy(component->dev);
pm_runtime_put_autosuspend(component->dev);

return 0;
}

Expand Down Expand Up @@ -1190,14 +1195,6 @@ static int rt711_sdca_probe(struct snd_soc_component *component)
return 0;
}

static void rt711_sdca_remove(struct snd_soc_component *component)
{
struct rt711_sdca_priv *rt711 = snd_soc_component_get_drvdata(component);

regcache_cache_only(rt711->regmap, true);
regcache_cache_only(rt711->mbq_regmap, true);
}

static const struct snd_soc_component_driver soc_sdca_dev_rt711 = {
.probe = rt711_sdca_probe,
.controls = rt711_sdca_snd_controls,
Expand All @@ -1207,7 +1204,6 @@ static const struct snd_soc_component_driver soc_sdca_dev_rt711 = {
.dapm_routes = rt711_sdca_audio_map,
.num_dapm_routes = ARRAY_SIZE(rt711_sdca_audio_map),
.set_jack = rt711_sdca_set_jack_detect,
.remove = rt711_sdca_remove,
.endianness = 1,
};

Expand Down
24 changes: 10 additions & 14 deletions sound/soc/codecs/rt711.c
Original file line number Diff line number Diff line change
Expand Up @@ -457,17 +457,21 @@ static int rt711_set_jack_detect(struct snd_soc_component *component,
struct snd_soc_jack *hs_jack, void *data)
{
struct rt711_priv *rt711 = snd_soc_component_get_drvdata(component);
int ret;

rt711->hs_jack = hs_jack;

if (!rt711->hw_init) {
dev_dbg(&rt711->slave->dev,
"%s hw_init not ready yet\n", __func__);
return 0;
ret = pm_runtime_resume_and_get(component->dev);
if (ret < 0 && ret != -EACCES) {
dev_err(component->dev, "%s: failed to resume %d\n", __func__, ret);
return ret;
}

rt711->hs_jack = hs_jack;

rt711_jack_init(rt711);

pm_runtime_mark_last_busy(component->dev);
pm_runtime_put_autosuspend(component->dev);

return 0;
}

Expand Down Expand Up @@ -932,13 +936,6 @@ static int rt711_probe(struct snd_soc_component *component)
return 0;
}

static void rt711_remove(struct snd_soc_component *component)
{
struct rt711_priv *rt711 = snd_soc_component_get_drvdata(component);

regcache_cache_only(rt711->regmap, true);
}

static const struct snd_soc_component_driver soc_codec_dev_rt711 = {
.probe = rt711_probe,
.set_bias_level = rt711_set_bias_level,
Expand All @@ -949,7 +946,6 @@ static const struct snd_soc_component_driver soc_codec_dev_rt711 = {
.dapm_routes = rt711_audio_map,
.num_dapm_routes = ARRAY_SIZE(rt711_audio_map),
.set_jack = rt711_set_jack_detect,
.remove = rt711_remove,
.endianness = 1,
};

Expand Down

0 comments on commit f4e8191

Please sign in to comment.