From 9c3e1698a218b88d27fd8c8a6644b8a4bebf521d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 25 Jan 2019 16:07:25 -0600 Subject: [PATCH 1/5] ASoC: SOF: add probe_complete callback definition With the device model, there is no guarantee that the driver probe of the child platform device will complete synchronously. The errors are also squelched so that e.g. the pci driver has no way of knowing that the firmware boot failed. Also the probe might be implemented in a workqueue (e.g. if there are any dependencies on modules or long hardware initialization times), so we really need an asynchronous way of notifying the parent device that all probe-related functionality completed successfully in the "sof-audio" device. The simplest possible implementation is a callback added to the sof_pdata structure. Signed-off-by: Pierre-Louis Bossart --- include/sound/sof.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/sound/sof.h b/include/sound/sof.h index 7c11d16207e478..251103f46fe51a 100644 --- a/include/sound/sof.h +++ b/include/sound/sof.h @@ -30,6 +30,16 @@ struct snd_sof_pdata { /* parent device */ struct device *dev; + /* + * notification callback used if the hardware initialization + * can take time or is handled in a workqueue. This callback + * can be used by the parent device to e.g. enable runtime_pm + * or limit functionality until all low-level inits are + * complete. The device argument refers to the parent, not the + * "sof-audio" device. + */ + void (*sof_probe_complete)(struct device *dev); + /* descriptor */ const struct sof_dev_desc *desc; From 0138be371152f1fd82984c13d5d972345642bab4 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 28 Jan 2019 12:07:09 -0600 Subject: [PATCH 2/5] ASoC: SOF: core: only set drvdata on successful probe There's no point in storing information if the probe fails later on. Move the set_drvdata before returning success. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/core.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 1eff06acb9f057..fb7b4199fa6802 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -282,7 +282,6 @@ static int sof_probe(struct platform_device *pdev) INIT_LIST_HEAD(&sdev->widget_list); INIT_LIST_HEAD(&sdev->dai_list); INIT_LIST_HEAD(&sdev->route_list); - dev_set_drvdata(&pdev->dev, sdev); spin_lock_init(&sdev->ipc_lock); spin_lock_init(&sdev->hw_lock); @@ -388,6 +387,12 @@ static int sof_probe(struct platform_device *pdev) dev_dbg(sdev->dev, "created machine %s\n", dev_name(&plat_data->pdev_mach->dev)); + /* + * so far so good, set the drvdata which can be used to test probe + * success. + */ + dev_set_drvdata(&pdev->dev, sdev); + return 0; comp_err: From e7845e1a924da19f1c07361ba8b491e16c635361 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 28 Jan 2019 12:14:04 -0600 Subject: [PATCH 3/5] ASoC: SOF: PM: check that sdev is valid If the probe isn't successful, the drvdata will not be set. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/pm.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 93102993b508dc..47f30ba84ecd35 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -234,6 +234,10 @@ static int sof_resume(struct device *dev, bool runtime_resume) struct snd_sof_dev *sdev = dev_get_drvdata(&priv->pdev_pcm->dev); int ret = 0; + /* do nothing if the probe failed somehow */ + if (!sdev) + return 0; + /* do nothing if dsp resume callbacks are not set */ if (!sof_ops(sdev)->resume || !sof_ops(sdev)->runtime_resume) return 0; @@ -304,6 +308,10 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) struct snd_sof_dev *sdev = dev_get_drvdata(&priv->pdev_pcm->dev); int ret = 0; + /* do nothing if the probe failed somehow */ + if (!sdev) + return 0; + /* do nothing if dsp suspend callback is not set */ if (!sof_ops(sdev)->suspend) return 0; From 4476a56b2060c60077535bf52da5c942ba565615 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 25 Jan 2019 16:11:57 -0600 Subject: [PATCH 4/5] ASoC: SOF: core: invoke notification callback If the "sof-audio" probe completes successfully, invoke the sof_probe_complete notification callback, if desired by the parent. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index fb7b4199fa6802..78af9987053040 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -389,9 +389,11 @@ static int sof_probe(struct platform_device *pdev) /* * so far so good, set the drvdata which can be used to test probe - * success. + * success. Also let the parent device know the probe completed */ dev_set_drvdata(&pdev->dev, sdev); + if (plat_data->sof_probe_complete) + plat_data->sof_probe_complete(sdev->parent); return 0; From 0770b3699cb16a104ab4b31e55c5ba10394883dc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 25 Jan 2019 16:13:36 -0600 Subject: [PATCH 5/5] ASoC: SOF: pci: enable sof_probe_complete callback We have no way of knowing if low-level layers probed successfully. Move all the runtime_pm enablement to the sof_probe_complete callback to avoid any possible race conditions. FIXME: can we have race conditions between the notification and the remove? Is a lock necessary? Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/sof-pci-dev.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c index 8893ce224b0ecb..f4833e4d0ce007 100644 --- a/sound/soc/sof/sof-pci-dev.c +++ b/sound/soc/sof/sof-pci-dev.c @@ -150,6 +150,24 @@ static const struct dev_pm_ops sof_pci_pm = { NULL) }; +static void sof_pci_probe_complete(struct device *dev) +{ + dev_dbg(dev, "Completing SOF PCI probe"); + + /* allow runtime_pm */ + pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS); + pm_runtime_use_autosuspend(dev); + + /* + * runtime pm for pci device is "forbidden" by default. + * so call pm_runtime_allow() to enable it. + */ + pm_runtime_allow(dev); + + /* follow recommendation in pci-driver.c to decrement usage counter */ + pm_runtime_put_noidle(dev); +} + static int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id) { @@ -217,6 +235,9 @@ static int sof_pci_probe(struct pci_dev *pci, sof_pdata->dev = &pci->dev; sof_pdata->platform = "sof-audio"; + /* set callback to enable runtime_pm */ + sof_pdata->sof_probe_complete = sof_pci_probe_complete; + /* register sof-audio platform driver */ ret = sof_create_platform_device(priv); if (ret) { @@ -224,19 +245,6 @@ static int sof_pci_probe(struct pci_dev *pci, goto release_regions; } - /* allow runtime_pm */ - pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS); - pm_runtime_use_autosuspend(dev); - - /* - * runtime pm for pci device is "forbidden" by default. - * so call pm_runtime_allow() to enable it. - */ - pm_runtime_allow(dev); - - /* follow recommendation in pci-driver.c to decrement usage counter */ - pm_runtime_put_noidle(dev); - return ret; release_regions: