From 6b45b3a08b60a3e2812d01c3db2069604e225c90 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 6 Mar 2024 08:31:14 -0600 Subject: [PATCH 1/4] 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 25dba9eb383cfa..b8bb7e630ee432 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -245,12 +245,12 @@ static int hda_sdw_exit(struct snd_sof_dev *sdev) hdev = sdev->pdata->hw_pdata; - 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; } From b974100fc9ec5ae6dc75f8463fc47fc38a385a94 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Thu, 29 Feb 2024 16:40:54 +0800 Subject: [PATCH 2/4] soundwire: intel_auxdevice: use pm_runtime_resume() instead of pm_request_resume() We need to wait for each child to fully resume. pm_request_resume() is asynchronous, what we need is to wait synchronously to avoid race conditions. Signed-off-by: Bard Liao --- drivers/soundwire/intel_auxdevice.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c index 93698532deac40..2b9dbd10de560a 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; } @@ -499,9 +499,9 @@ static int __maybe_unused intel_pm_prepare(struct device *dev) * first resume the device for this link. This will also by construction * resume the PCI parent device. */ - 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 0; } From f56be06551c7a67605592b75d8b2573a043d86b1 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Thu, 29 Feb 2024 16:56:43 +0800 Subject: [PATCH 3/4] 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 2b9dbd10de560a..d6566387ff3cd3 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 32f632b22d212ab3b70b1cdadd507735f7569f6c Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Fri, 23 Feb 2024 21:39:30 +0800 Subject: [PATCH 4/4] soundwire: intel_init: resume all devices on exit. When the manager becomes pm_runtime active in the remove procedure, peripherals will become attached, and do the initialization process. We have to wait until all the devices are fully resumed before the cleanup, otherwise there is a possible race condition where asynchronous workqueues initiate transfers on the bus that cannot complete. 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 | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 534c8795e7e85d..a09134b97cd666 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" @@ -356,6 +357,19 @@ EXPORT_SYMBOL_NS(sdw_intel_startup, SOUNDWIRE_INTEL_INIT); */ void sdw_intel_exit(struct sdw_intel_ctx *ctx) { + struct sdw_intel_link_res *link; + + /* we first resume links and devices and wait synchronously before the cleanup */ + 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); + } + sdw_intel_cleanup(ctx); kfree(ctx->ids); kfree(ctx->ldev);