[alsa-devel] [PATCH 0/3] ASoC: davinci: devm_snd_soc_register_platform and mcasp cleanup
Hi,
Now that we have resource managed version of snd_soc_register_platform() convert the davinci ASoC code to use it.
Clean the mcasp driver's init code while we are here so we can remove the platform driver's remove function since we now have managed resources only.
Regards, Peter --- Peter Ujfalusi (3): ASoC: davinci-pcm: Convert to use devm_snd_soc_register_platform() ASoC: davinci-mcasp: Convert to use devm_snd_soc_register_component() ASoC: davinci-mcasp: Optimize pm_runtime usage and clean up the init code
sound/soc/davinci/davinci-i2s.c | 1 - sound/soc/davinci/davinci-mcasp.c | 69 ++++++++++----------------------------- sound/soc/davinci/davinci-pcm.c | 8 +---- sound/soc/davinci/davinci-pcm.h | 4 --- sound/soc/davinci/davinci-vcif.c | 1 - 5 files changed, 19 insertions(+), 64 deletions(-)
Remove the cleanup code related to the platform from the DAI drivers at the same time to avoid breakage.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-i2s.c | 1 - sound/soc/davinci/davinci-mcasp.c | 15 --------------- sound/soc/davinci/davinci-pcm.c | 8 +------- sound/soc/davinci/davinci-pcm.h | 4 ---- sound/soc/davinci/davinci-vcif.c | 1 - 5 files changed, 1 insertion(+), 28 deletions(-)
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c index ebe82947bab3..7682af31d6e6 100644 --- a/sound/soc/davinci/davinci-i2s.c +++ b/sound/soc/davinci/davinci-i2s.c @@ -757,7 +757,6 @@ static int davinci_i2s_remove(struct platform_device *pdev) struct davinci_mcbsp_dev *dev = dev_get_drvdata(&pdev->dev);
snd_soc_unregister_component(&pdev->dev); - davinci_soc_platform_unregister(&pdev->dev);
clk_disable(dev->clk); clk_put(dev->clk); diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index d505fe7292a4..44a03840f76c 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -1254,23 +1254,8 @@ err_release_clk:
static int davinci_mcasp_remove(struct platform_device *pdev) { - struct davinci_mcasp *mcasp = dev_get_drvdata(&pdev->dev); - snd_soc_unregister_component(&pdev->dev);
- switch (mcasp->version) { - case MCASP_VERSION_1: - case MCASP_VERSION_2: - case MCASP_VERSION_3: - davinci_soc_platform_unregister(&pdev->dev); - break; - case MCASP_VERSION_4: - /* Using the resource managed omap-pcm as platform driver */ - break; - default: - break; - } - pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev);
diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c index 14145cdf8a11..7809e9d935fc 100644 --- a/sound/soc/davinci/davinci-pcm.c +++ b/sound/soc/davinci/davinci-pcm.c @@ -852,16 +852,10 @@ static struct snd_soc_platform_driver davinci_soc_platform = {
int davinci_soc_platform_register(struct device *dev) { - return snd_soc_register_platform(dev, &davinci_soc_platform); + return devm_snd_soc_register_platform(dev, &davinci_soc_platform); } EXPORT_SYMBOL_GPL(davinci_soc_platform_register);
-void davinci_soc_platform_unregister(struct device *dev) -{ - snd_soc_unregister_platform(dev); -} -EXPORT_SYMBOL_GPL(davinci_soc_platform_unregister); - MODULE_AUTHOR("Vladimir Barinov"); MODULE_DESCRIPTION("TI DAVINCI PCM DMA module"); MODULE_LICENSE("GPL"); diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h index 5fd4737ab398..0fe2346a9aa2 100644 --- a/sound/soc/davinci/davinci-pcm.h +++ b/sound/soc/davinci/davinci-pcm.h @@ -31,15 +31,11 @@ struct davinci_pcm_dma_params {
#if IS_ENABLED(CONFIG_SND_DAVINCI_SOC) int davinci_soc_platform_register(struct device *dev); -void davinci_soc_platform_unregister(struct device *dev); #else static inline int davinci_soc_platform_register(struct device *dev) { return 0; } -static inline void davinci_soc_platform_unregister(struct device *dev) -{ -} #endif /* CONFIG_SND_DAVINCI_SOC */
#endif diff --git a/sound/soc/davinci/davinci-vcif.c b/sound/soc/davinci/davinci-vcif.c index 30587c0cdbd2..77aef05588c3 100644 --- a/sound/soc/davinci/davinci-vcif.c +++ b/sound/soc/davinci/davinci-vcif.c @@ -258,7 +258,6 @@ static int davinci_vcif_probe(struct platform_device *pdev) static int davinci_vcif_remove(struct platform_device *pdev) { snd_soc_unregister_component(&pdev->dev); - davinci_soc_platform_unregister(&pdev->dev);
return 0; }
On Tue, Apr 22, 2014 at 02:03:12PM +0300, Peter Ujfalusi wrote:
Remove the cleanup code related to the platform from the DAI drivers at the same time to avoid breakage.
Applied, thanks.
It allows to remove code from the cleanup paths.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-mcasp.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 44a03840f76c..14058dc6eaf8 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -1134,7 +1134,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev) if (!mcasp->base) { dev_err(&pdev->dev, "ioremap failed\n"); ret = -ENOMEM; - goto err_release_clk; + goto err; }
mcasp->op_mode = pdata->op_mode; @@ -1215,11 +1215,12 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
mcasp_reparent_fck(pdev);
- ret = snd_soc_register_component(&pdev->dev, &davinci_mcasp_component, - &davinci_mcasp_dai[pdata->op_mode], 1); + ret = devm_snd_soc_register_component(&pdev->dev, + &davinci_mcasp_component, + &davinci_mcasp_dai[pdata->op_mode], 1);
if (ret != 0) - goto err_release_clk; + goto err;
switch (mcasp->version) { case MCASP_VERSION_1: @@ -1239,14 +1240,12 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
if (ret) { dev_err(&pdev->dev, "register PCM failed: %d\n", ret); - goto err_unregister_component; + goto err; }
return 0;
-err_unregister_component: - snd_soc_unregister_component(&pdev->dev); -err_release_clk: +err: pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); return ret; @@ -1254,8 +1253,6 @@ err_release_clk:
static int davinci_mcasp_remove(struct platform_device *pdev) { - snd_soc_unregister_component(&pdev->dev); - pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev);
Move the pm_runtime_enable/disable to dai driver probe/remove callbacks from module probe/remove callbacks. With this change we can remove the platform driver's remove function since it became NULL.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-mcasp.c | 45 +++++++++++++-------------------------- 1 file changed, 15 insertions(+), 30 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 14058dc6eaf8..c858c9ccf774 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -778,6 +778,8 @@ static int davinci_mcasp_dai_probe(struct snd_soc_dai *dai) { struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai);
+ pm_runtime_enable(mcasp->dev); + if (mcasp->version == MCASP_VERSION_4) { /* Using dmaengine PCM */ dai->playback_dma_data = @@ -793,6 +795,15 @@ static int davinci_mcasp_dai_probe(struct snd_soc_dai *dai) return 0; }
+static int davinci_mcasp_dai_remove(struct snd_soc_dai *dai) +{ + struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai); + + pm_runtime_disable(mcasp->dev); + + return 0; +} + #ifdef CONFIG_PM_SLEEP static int davinci_mcasp_suspend(struct snd_soc_dai *dai) { @@ -847,6 +858,7 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = { { .name = "davinci-mcasp.0", .probe = davinci_mcasp_dai_probe, + .remove = davinci_mcasp_dai_remove, .suspend = davinci_mcasp_suspend, .resume = davinci_mcasp_resume, .playback = { @@ -867,6 +879,7 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = { { .name = "davinci-mcasp.1", .probe = davinci_mcasp_dai_probe, + .remove = davinci_mcasp_dai_remove, .playback = { .channels_min = 1, .channels_max = 384, @@ -1122,19 +1135,10 @@ static int davinci_mcasp_probe(struct platform_device *pdev) return -EBUSY; }
- pm_runtime_enable(&pdev->dev); - - ret = pm_runtime_get_sync(&pdev->dev); - if (IS_ERR_VALUE(ret)) { - dev_err(&pdev->dev, "pm_runtime_get_sync() failed\n"); - return ret; - } - mcasp->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem)); if (!mcasp->base) { dev_err(&pdev->dev, "ioremap failed\n"); - ret = -ENOMEM; - goto err; + return -ENOMEM; }
mcasp->op_mode = pdata->op_mode; @@ -1220,7 +1224,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev) &davinci_mcasp_dai[pdata->op_mode], 1);
if (ret != 0) - goto err; + return ret;
switch (mcasp->version) { case MCASP_VERSION_1: @@ -1238,30 +1242,11 @@ static int davinci_mcasp_probe(struct platform_device *pdev) break; }
- if (ret) { - dev_err(&pdev->dev, "register PCM failed: %d\n", ret); - goto err; - } - - return 0; - -err: - pm_runtime_put_sync(&pdev->dev); - pm_runtime_disable(&pdev->dev); return ret; }
-static int davinci_mcasp_remove(struct platform_device *pdev) -{ - pm_runtime_put_sync(&pdev->dev); - pm_runtime_disable(&pdev->dev); - - return 0; -} - static struct platform_driver davinci_mcasp_driver = { .probe = davinci_mcasp_probe, - .remove = davinci_mcasp_remove, .driver = { .name = "davinci-mcasp", .owner = THIS_MODULE,
On Tue, Apr 22, 2014 at 02:03:14PM +0300, Peter Ujfalusi wrote:
Move the pm_runtime_enable/disable to dai driver probe/remove callbacks from module probe/remove callbacks. With this change we can remove the platform driver's remove function since it became NULL.
This does mean that if the device gets enumerated but isn't used in the system it won't go to runtime idle since the DAI level probe is only called when we're building a card. That doesn't seem like it's a win. How about creating a devm_pm_runtime_enable() instead?
On 04/22/2014 02:47 PM, Mark Brown wrote:
On Tue, Apr 22, 2014 at 02:03:14PM +0300, Peter Ujfalusi wrote:
Move the pm_runtime_enable/disable to dai driver probe/remove callbacks from module probe/remove callbacks. With this change we can remove the platform driver's remove function since it became NULL.
This does mean that if the device gets enumerated but isn't used in the system it won't go to runtime idle since the DAI level probe is only called when we're building a card.
If the given mcasp is not used as part of a card, it should not have been probed in the first place (at least with DT boot we can control this). If the driver is probed for a mcasp instance and it is not part of any card than it can be left disabled IMHO no need for runtime pm to take care of it. I might missed something related to runtime pm, but this is my understanding.
That doesn't seem like it's a win. How about creating a devm_pm_runtime_enable() instead?
I was also considered to do this but ended up moving the pm_runtime_enable/disable instead.
On Tue, Apr 22, 2014 at 03:20:33PM +0300, Peter Ujfalusi wrote:
On 04/22/2014 02:47 PM, Mark Brown wrote:
This does mean that if the device gets enumerated but isn't used in the system it won't go to runtime idle since the DAI level probe is only called when we're building a card.
If the given mcasp is not used as part of a card, it should not have been probed in the first place (at least with DT boot we can control this). If the driver is probed for a mcasp instance and it is not part of any card than it can be left disabled IMHO no need for runtime pm to take care of it. I might missed something related to runtime pm, but this is my understanding.
Sure, but you then also have the cases where for whatever reason the card doesn't probe (some other driver missing for example). Probably almost all the time it's not going to make a practical difference but it just feels like a step in the wrong direction for a minimal gain.
participants (2)
-
Mark Brown -
Peter Ujfalusi