diff mbox series

ASoC: SOF: fix runtime pm usage mismatch after probe errors

Message ID 20210210105237.2179273-1-kai.vehmanen@linux.intel.com (mailing list archive)
State Accepted
Commit 271d9373db1c76f239fe3124e552b6b58b2af984
Headers show
Series ASoC: SOF: fix runtime pm usage mismatch after probe errors | expand

Commit Message

Kai Vehmanen Feb. 10, 2021, 10:52 a.m. UTC
With current delayed probe implementation, sof_probe_complete is not
called in case of errors. And as this function is responsible for
decrementing runtime pm usage counter, this will result in following
problem:

 - probe driver in conditions where probe will fail (to force
   the condition on Intel SOF systems, set
   "snd_sof_intel_hda_common.codec_mask=0")
 - unload driver (runtime-pm usage_count is leaked)
 - fix the issue by installing missing fw, modifying module parameters,
   etc actions
 - try to load driver again -> success, probe ok
 -> device never enters runtime suspend

Fix the issue by storing result of delayed probe to a state variable and
providing new snd_sof_device_probe_completed() to be queried from SOF
PCI/ACPI/OF drivers.

If probe never completed successfully, runtime PM was not set up and
thus at remove(), we should not increment usage count anymore.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@intel.com>
---
 sound/soc/sof/core.c        | 10 ++++++++++
 sound/soc/sof/sof-pci-dev.c |  3 ++-
 sound/soc/sof/sof-priv.h    |  2 ++
 3 files changed, 14 insertions(+), 1 deletion(-)


base-commit: d40dac7ae8c0f213ac1da7896c35ddc2c58419ab

Comments

Mark Brown Feb. 10, 2021, 8:11 p.m. UTC | #1
On Wed, 10 Feb 2021 12:52:37 +0200, Kai Vehmanen wrote:
> With current delayed probe implementation, sof_probe_complete is not
> called in case of errors. And as this function is responsible for
> decrementing runtime pm usage counter, this will result in following
> problem:
> 
>  - probe driver in conditions where probe will fail (to force
>    the condition on Intel SOF systems, set
>    "snd_sof_intel_hda_common.codec_mask=0")
>  - unload driver (runtime-pm usage_count is leaked)
>  - fix the issue by installing missing fw, modifying module parameters,
>    etc actions
>  - try to load driver again -> success, probe ok
>  -> device never enters runtime suspend
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: SOF: fix runtime pm usage mismatch after probe errors
      commit: 271d9373db1c76f239fe3124e552b6b58b2af984

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
diff mbox series

Patch

diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
index 8d13eb13fe08..6d8f7d9fd192 100644
--- a/sound/soc/sof/core.c
+++ b/sound/soc/sof/core.c
@@ -246,6 +246,8 @@  static int sof_probe_continue(struct snd_sof_dev *sdev)
 	if (plat_data->sof_probe_complete)
 		plat_data->sof_probe_complete(sdev->dev);
 
+	sdev->probe_completed = true;
+
 	return 0;
 
 fw_trace_err:
@@ -340,6 +342,14 @@  int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data)
 }
 EXPORT_SYMBOL(snd_sof_device_probe);
 
+bool snd_sof_device_probe_completed(struct device *dev)
+{
+	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
+
+	return sdev->probe_completed;
+}
+EXPORT_SYMBOL(snd_sof_device_probe_completed);
+
 int snd_sof_device_remove(struct device *dev)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
index 388772f9c4d2..84990cc8aa7d 100644
--- a/sound/soc/sof/sof-pci-dev.c
+++ b/sound/soc/sof/sof-pci-dev.c
@@ -447,7 +447,8 @@  static void sof_pci_remove(struct pci_dev *pci)
 	snd_sof_device_remove(&pci->dev);
 
 	/* follow recommendation in pci-driver.c to increment usage counter */
-	if (!(sof_pci_debug & SOF_PCI_DISABLE_PM_RUNTIME))
+	if (snd_sof_device_probe_completed(&pci->dev) &&
+	    !(sof_pci_debug & SOF_PCI_DISABLE_PM_RUNTIME))
 		pm_runtime_get_noresume(&pci->dev);
 
 	/* release pci regions and disable device */
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 682c4b6d01ef..ad0d7ba2708c 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -389,6 +389,7 @@  struct snd_sof_dev {
 
 	/* work queue in case the probe is implemented in two steps */
 	struct work_struct probe_work;
+	bool probe_completed;
 
 	/* DSP HW differentiation */
 	struct snd_sof_pdata *pdata;
@@ -464,6 +465,7 @@  struct snd_sof_dev {
 int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data);
 int snd_sof_device_remove(struct device *dev);
 int snd_sof_device_shutdown(struct device *dev);
+bool snd_sof_device_probe_completed(struct device *dev);
 
 int snd_sof_runtime_suspend(struct device *dev);
 int snd_sof_runtime_resume(struct device *dev);