[PATCH 0/4] ASoC: codecs: Realtek/SoundWire: fix remove/suspend issues
These issues and fixes were identified during our module load/unload and suspend/resume stress tests.
Bard Liao (1): ASoC: rt5682: do nothing in rt5682_suspend/resume in sdw mode
Pierre-Louis Bossart (3): ASoC: rt700-sdw: use cancel_work_sync() in .remove as well as .suspend ASoC: rt711-sdw: use cancel_work_sync() for .remove ASoC: rt5682-sdw: cancel_work_sync() in .remove and .suspend
sound/soc/codecs/rt5682-sdw.c | 4 +++- sound/soc/codecs/rt5682.c | 6 ++++++ sound/soc/codecs/rt700-sdw.c | 4 ++-- sound/soc/codecs/rt711-sdw.c | 4 ++-- 4 files changed, 13 insertions(+), 5 deletions(-)
Make sure the workqueues are not running after the .remove() callback, which can lead to timeout errors.
A previous fix to use cancel_work_sync was applied for the suspend case but the remove case is missing
Fixes: 5f2df2a4583b ('ASoC: rt700: wait for the delayed work to finish when the system suspends') Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Rander Wang rander.wang@linux.intel.com --- sound/soc/codecs/rt700-sdw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c index ce9255b881d4..4001612dfd73 100644 --- a/sound/soc/codecs/rt700-sdw.c +++ b/sound/soc/codecs/rt700-sdw.c @@ -462,8 +462,8 @@ static int rt700_sdw_remove(struct sdw_slave *slave) struct rt700_priv *rt700 = dev_get_drvdata(&slave->dev);
if (rt700 && rt700->hw_init) { - cancel_delayed_work(&rt700->jack_detect_work); - cancel_delayed_work(&rt700->jack_btn_check_work); + cancel_delayed_work_sync(&rt700->jack_detect_work); + cancel_delayed_work_sync(&rt700->jack_btn_check_work); }
return 0;
Make sure the workqueues are not running after the .remove() callback, which can lead to timeout errors.
A previous fix to use cancel_work_sync was applied for the suspend case but the remove case is missing
Fixes: 501ef013390b ('ASoC: rt711: wait for the delayed work to finish when the system suspends') Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Rander Wang rander.wang@linux.intel.com --- sound/soc/codecs/rt711-sdw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c index 756c0ada3b31..2beb4286d997 100644 --- a/sound/soc/codecs/rt711-sdw.c +++ b/sound/soc/codecs/rt711-sdw.c @@ -463,8 +463,8 @@ static int rt711_sdw_remove(struct sdw_slave *slave) struct rt711_priv *rt711 = dev_get_drvdata(&slave->dev);
if (rt711 && rt711->hw_init) { - cancel_delayed_work(&rt711->jack_detect_work); - cancel_delayed_work(&rt711->jack_btn_check_work); + cancel_delayed_work_sync(&rt711->jack_detect_work); + cancel_delayed_work_sync(&rt711->jack_btn_check_work); cancel_work_sync(&rt711->calibration_work); }
Follow pattern from other drivers and use cancel_work_sync() for both .remove() and .suspend().
Fixes: 03f6fc6de919 ('ASoC: rt5682: Add the soundwire support') Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Rander Wang rander.wang@linux.intel.com --- sound/soc/codecs/rt5682-sdw.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 69f046bba67a..b49f1e16125d 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -710,7 +710,7 @@ static int rt5682_sdw_remove(struct sdw_slave *slave) struct rt5682_priv *rt5682 = dev_get_drvdata(&slave->dev);
if (rt5682 && rt5682->hw_init) - cancel_delayed_work(&rt5682->jack_detect_work); + cancel_delayed_work_sync(&rt5682->jack_detect_work);
return 0; } @@ -728,6 +728,8 @@ static int __maybe_unused rt5682_dev_suspend(struct device *dev) if (!rt5682->hw_init) return 0;
+ cancel_delayed_work_sync(&rt5682->jack_detect_work); + regcache_cache_only(rt5682->regmap, true); regcache_mark_dirty(rt5682->regmap);
From: Bard Liao yung-chuan.liao@linux.intel.com
regcache sync will be done in sdw device suspend/resume functions. And we have different jack detection mechanism for SoundWire.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/codecs/rt5682.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/sound/soc/codecs/rt5682.c b/sound/soc/codecs/rt5682.c index 185906d6d4e0..b306ac4b9b2e 100644 --- a/sound/soc/codecs/rt5682.c +++ b/sound/soc/codecs/rt5682.c @@ -2910,6 +2910,9 @@ static int rt5682_suspend(struct snd_soc_component *component) { struct rt5682_priv *rt5682 = snd_soc_component_get_drvdata(component);
+ if (rt5682->is_sdw) + return 0; + regcache_cache_only(rt5682->regmap, true); regcache_mark_dirty(rt5682->regmap); return 0; @@ -2919,6 +2922,9 @@ static int rt5682_resume(struct snd_soc_component *component) { struct rt5682_priv *rt5682 = snd_soc_component_get_drvdata(component);
+ if (rt5682->is_sdw) + return 0; + regcache_cache_only(rt5682->regmap, false); regcache_sync(rt5682->regmap);
On Thu, 4 Feb 2021 14:17:35 -0600, Pierre-Louis Bossart wrote:
These issues and fixes were identified during our module load/unload and suspend/resume stress tests.
Bard Liao (1): ASoC: rt5682: do nothing in rt5682_suspend/resume in sdw mode
Pierre-Louis Bossart (3): ASoC: rt700-sdw: use cancel_work_sync() in .remove as well as .suspend ASoC: rt711-sdw: use cancel_work_sync() for .remove ASoC: rt5682-sdw: cancel_work_sync() in .remove and .suspend
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: rt700-sdw: use cancel_work_sync() in .remove as well as .suspend commit: 737ee8bdf682cedb3c42b713d20ffa5c899591fb [2/4] ASoC: rt711-sdw: use cancel_work_sync() for .remove commit: 121871a75ae4032cf5e506ba5159761805709def [3/4] ASoC: rt5682-sdw: cancel_work_sync() in .remove and .suspend commit: c792c3690b82c8d26c01494a51ebf66d9cae7e72 [4/4] ASoC: rt5682: do nothing in rt5682_suspend/resume in sdw mode commit: 30fd8f65af78d0ac0859cf436beed14834b39802
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)
-
Mark Brown -
Pierre-Louis Bossart