From c2a734e9c1c74b7bd143f7b51c6e422b9303fc69 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Thu, 29 Feb 2024 16:40:54 +0800 Subject: [PATCH 1/6] soundwire: intel_auxdevice: use pm_resume_resume to resume child We need to wait for each child to fully resume. Use pm_resume_resume to ensure it. Signed-off-by: Bard Liao --- drivers/soundwire/intel_auxdevice.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c index 93698532deac40..48f6677b054ad1 100644 --- a/drivers/soundwire/intel_auxdevice.c +++ b/drivers/soundwire/intel_auxdevice.c @@ -454,9 +454,9 @@ static int intel_resume_child_device(struct device *dev, void *data) return 0; } - ret = pm_request_resume(dev); + ret = pm_runtime_resume(dev); if (ret < 0) { - dev_err(dev, "%s: pm_request_resume failed: %d\n", __func__, ret); + dev_err(dev, "%s: pm_runtime_resume failed: %d\n", __func__, ret); return ret; } From 6b1b861752c8cc35a33e23bb5c2072b4a1da1150 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Thu, 29 Feb 2024 16:56:43 +0800 Subject: [PATCH 2/6] soundwire: intel: export intel_resume_child_device We will resume each child in the next patch, and intel_resume_child_device() will be used. Signed-off-by: Bard Liao --- drivers/soundwire/intel_auxdevice.c | 2 +- drivers/soundwire/intel_auxdevice.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c index 48f6677b054ad1..84c291ac51db66 100644 --- a/drivers/soundwire/intel_auxdevice.c +++ b/drivers/soundwire/intel_auxdevice.c @@ -440,7 +440,7 @@ int intel_link_process_wakeen_event(struct auxiliary_device *auxdev) * PM calls */ -static int intel_resume_child_device(struct device *dev, void *data) +int intel_resume_child_device(struct device *dev, void *data) { int ret; struct sdw_slave *slave = dev_to_sdw_dev(dev); diff --git a/drivers/soundwire/intel_auxdevice.h b/drivers/soundwire/intel_auxdevice.h index a00ecde9556333..acaae0bd6592b8 100644 --- a/drivers/soundwire/intel_auxdevice.h +++ b/drivers/soundwire/intel_auxdevice.h @@ -6,6 +6,7 @@ int intel_link_startup(struct auxiliary_device *auxdev); int intel_link_process_wakeen_event(struct auxiliary_device *auxdev); +int intel_resume_child_device(struct device *dev, void *data); struct sdw_intel_link_dev { struct auxiliary_device auxdev; From 94afd7f2eea933b999c853b604b5a0c1bf0ffc11 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Fri, 23 Feb 2024 21:39:30 +0800 Subject: [PATCH 3/6] ASoC/soundwire: Intel: add sdw_intel_pre_exit helper We will disable SDW interrupt in hda_sdw_exit() before sdw_intel_exit() and peripherals will on longer access SDW registers. However, when the manager became pm_runtime active in the remove procedure, peripherals will become attached, and do the initialization process. The commit suggests a sdw_intel_pre_exit() function and use pm_runtime_resume() to wait for the peripherals being fully resumed. This will ensure there are no SoundWire registers accessed after the bus is powered-down. Signed-off-by: Bard Liao --- drivers/soundwire/intel_init.c | 17 +++++++++++++++++ include/linux/soundwire/sdw_intel.h | 1 + sound/soc/sof/intel/hda.c | 3 +++ 3 files changed, 21 insertions(+) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 534c8795e7e85d..f128670b2a6f4f 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -16,6 +16,7 @@ #include #include #include "cadence_master.h" +#include "bus.h" #include "intel.h" #include "intel_auxdevice.h" @@ -363,6 +364,22 @@ void sdw_intel_exit(struct sdw_intel_ctx *ctx) } EXPORT_SYMBOL_NS(sdw_intel_exit, SOUNDWIRE_INTEL_INIT); +void sdw_intel_pre_exit(struct sdw_intel_ctx *ctx) +{ + struct sdw_intel_link_res *link; + + list_for_each_entry(link, &ctx->link_list, list) { + struct sdw_bus *bus = &link->cdns->bus; + int ret; + + ret = device_for_each_child(bus->dev, NULL, intel_resume_child_device); + if (ret < 0) + dev_err(bus->dev, "%s: intel_resume_child_device failed: %d\n", + __func__, ret); + } +} +EXPORT_SYMBOL_NS(sdw_intel_pre_exit, SOUNDWIRE_INTEL_INIT); + void sdw_intel_process_wakeen_event(struct sdw_intel_ctx *ctx) { struct sdw_intel_link_dev *ldev; diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 8e78417156e3c6..1b3562815b1425 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -373,6 +373,7 @@ sdw_intel_probe(struct sdw_intel_res *res); int sdw_intel_startup(struct sdw_intel_ctx *ctx); void sdw_intel_exit(struct sdw_intel_ctx *ctx); +void sdw_intel_pre_exit(struct sdw_intel_ctx *ctx); irqreturn_t sdw_intel_thread(int irq, void *dev_id); diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 25dba9eb383cfa..d38ccaf9980ad7 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -245,6 +245,9 @@ static int hda_sdw_exit(struct snd_sof_dev *sdev) hdev = sdev->pdata->hw_pdata; + if (hdev->sdw) + sdw_intel_pre_exit(hdev->sdw); + hda_sdw_int_enable(sdev, false); if (hdev->sdw) From 18c1ac2c8cdd449fc1a9b80f6c9f2673c3461c24 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 5 Mar 2024 11:53:07 -0600 Subject: [PATCH 4/6] [HACK] add traces to debug race conditions Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel_auxdevice.c | 4 ++++ sound/soc/codecs/rt1308-sdw.c | 8 ++++++++ sound/soc/codecs/rt700-sdw.c | 8 ++++++++ sound/soc/codecs/rt711-sdw.c | 8 ++++++++ 4 files changed, 28 insertions(+) diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c index 84c291ac51db66..0967447ba465c6 100644 --- a/drivers/soundwire/intel_auxdevice.c +++ b/drivers/soundwire/intel_auxdevice.c @@ -454,12 +454,16 @@ int intel_resume_child_device(struct device *dev, void *data) return 0; } + dev_dbg(dev, "runtime_resume started\n"); + ret = pm_runtime_resume(dev); if (ret < 0) { dev_err(dev, "%s: pm_runtime_resume failed: %d\n", __func__, ret); return ret; } + dev_dbg(dev, "runtime_resume done\n"); + return 0; } diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index 63d4abf964d45c..cbe5d4010dc754 100644 --- a/sound/soc/codecs/rt1308-sdw.c +++ b/sound/soc/codecs/rt1308-sdw.c @@ -742,8 +742,12 @@ static int rt1308_sdw_probe(struct sdw_slave *slave, static int rt1308_sdw_remove(struct sdw_slave *slave) { + dev_dbg(&slave->dev, "remove started\n"); + pm_runtime_disable(&slave->dev); + dev_dbg(&slave->dev, "remove done\n"); + return 0; } @@ -773,6 +777,8 @@ static int __maybe_unused rt1308_dev_resume(struct device *dev) struct rt1308_sdw_priv *rt1308 = dev_get_drvdata(dev); unsigned long time; + dev_dbg(&slave->dev, "Resume started\n"); + if (!rt1308->first_hw_init) return 0; @@ -793,6 +799,8 @@ static int __maybe_unused rt1308_dev_resume(struct device *dev) regcache_cache_only(rt1308->regmap, false); regcache_sync_region(rt1308->regmap, 0xc000, 0xcfff); + dev_dbg(&slave->dev, "Resume done\n"); + return 0; } diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c index 52c33d56b143a9..f432654beec835 100644 --- a/sound/soc/codecs/rt700-sdw.c +++ b/sound/soc/codecs/rt700-sdw.c @@ -459,6 +459,8 @@ static int rt700_sdw_remove(struct sdw_slave *slave) { struct rt700_priv *rt700 = dev_get_drvdata(&slave->dev); + dev_dbg(&slave->dev, "remove started\n"); + if (rt700->hw_init) { cancel_delayed_work_sync(&rt700->jack_detect_work); cancel_delayed_work_sync(&rt700->jack_btn_check_work); @@ -466,6 +468,8 @@ static int rt700_sdw_remove(struct sdw_slave *slave) pm_runtime_disable(&slave->dev); + dev_dbg(&slave->dev, "remove done\n"); + return 0; } @@ -526,6 +530,8 @@ static int __maybe_unused rt700_dev_resume(struct device *dev) struct rt700_priv *rt700 = dev_get_drvdata(dev); unsigned long time; + dev_dbg(&slave->dev, "Resume started\n"); + if (!rt700->first_hw_init) return 0; @@ -547,6 +553,8 @@ static int __maybe_unused rt700_dev_resume(struct device *dev) regcache_sync_region(rt700->regmap, 0x3000, 0x8fff); regcache_sync_region(rt700->regmap, 0x752010, 0x75206b); + dev_dbg(&slave->dev, "Resume done\n"); + return 0; } diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c index 988451f24a7562..cd91da04911f08 100644 --- a/sound/soc/codecs/rt711-sdw.c +++ b/sound/soc/codecs/rt711-sdw.c @@ -460,6 +460,8 @@ static int rt711_sdw_remove(struct sdw_slave *slave) { struct rt711_priv *rt711 = dev_get_drvdata(&slave->dev); + dev_dbg(&slave->dev, "remove started\n"); + if (rt711->hw_init) { cancel_delayed_work_sync(&rt711->jack_detect_work); cancel_delayed_work_sync(&rt711->jack_btn_check_work); @@ -471,6 +473,8 @@ static int rt711_sdw_remove(struct sdw_slave *slave) mutex_destroy(&rt711->calibrate_mutex); mutex_destroy(&rt711->disable_irq_lock); + dev_dbg(&slave->dev, "remove done\n"); + return 0; } @@ -532,6 +536,8 @@ static int __maybe_unused rt711_dev_resume(struct device *dev) struct rt711_priv *rt711 = dev_get_drvdata(dev); unsigned long time; + dev_dbg(&slave->dev, "Resume started\n"); + if (!rt711->first_hw_init) return 0; @@ -558,6 +564,8 @@ static int __maybe_unused rt711_dev_resume(struct device *dev) regcache_sync_region(rt711->regmap, 0x3000, 0x8fff); regcache_sync_region(rt711->regmap, 0x752009, 0x752091); + dev_dbg(&slave->dev, "Resume done\n"); + return 0; } From ab3714f42b9056c6e1e169c45c747d57b29ba28d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 5 Mar 2024 17:15:11 -0600 Subject: [PATCH 5/6] more traces Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 191e6cc6f96276..9cd133e653c936 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1761,9 +1761,14 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) goto io_err; } + dev_dbg(&slave->dev, "SDW_SCP_INT1 write successful iteration %d\n", + count); + /* at this point all initial interrupt sources were handled */ slave->first_interrupt_done = true; + dev_dbg(&slave->dev, "SDW_SCP_INT1 read started iteration %d\n", + count); /* * Read status again to ensure no new interrupts arrived * while servicing interrupts. @@ -1774,6 +1779,10 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) "SDW_SCP_INT1 recheck read failed:%d\n", ret); goto io_err; } + + dev_dbg(&slave->dev, "SDW_SCP_INT1 read done iteration %d\n", + count); + buf = ret; ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, buf2); From 2e6c5c54f45c0cc23d2d11722718c2f23fa7396d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 6 Mar 2024 08:31:14 -0600 Subject: [PATCH 6/6] ASoC: SOF: Intel: hda: disable SoundWire interrupt later The SoundWire interrupts can be masked at two levels a) in the Cadence IP b) at the HDaudio controller level We have an existing mechanism with cancel_work_sync() and status flags to make sure all existing interrupts are handled in the Cadence IP, and likewise no new interrupts can be generated before turning off the links. However on remove we first use the higher-level mask at the controller level, which is a sledgehammer preventing interrupts from all links. This is very racy and not necessary. We can disable the SoundWire interrupts after all the cleanups are done without any loss of functionality. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index d38ccaf9980ad7..25451a0d7322ea 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -248,12 +248,12 @@ static int hda_sdw_exit(struct snd_sof_dev *sdev) if (hdev->sdw) sdw_intel_pre_exit(hdev->sdw); - hda_sdw_int_enable(sdev, false); - if (hdev->sdw) sdw_intel_exit(hdev->sdw); hdev->sdw = NULL; + hda_sdw_int_enable(sdev, false); + return 0; }