[PATCH 0/3] ASoC: davinci: devm_snd_soc_register_platform and mcasp cleanup

[PATCH 0/3] ASoC: davinci: devm_snd_soc_register_platform and mcasp cleanup

[Peter Ujfalusi]

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(-)

-- 
1.9.2

[PATCH 1/3] ASoC: davinci-pcm: Convert to use devm_snd_soc_register_platform()

[Peter Ujfalusi]

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;
 }
-- 
1.9.2

Re: [PATCH 1/3] ASoC: davinci-pcm: Convert to use devm_snd_soc_register_platform()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 191 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 2/3] ASoC: davinci-mcasp: Convert to use devm_snd_soc_register_component()

[Peter Ujfalusi]

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);
 
-- 
1.9.2

Re: [PATCH 2/3] ASoC: davinci-mcasp: Convert to use devm_snd_soc_register_component()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 133 bytes --]

On Tue, Apr 22, 2014 at 02:03:13PM +0300, Peter Ujfalusi wrote:
> It allows to remove code from the cleanup paths.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 3/3] ASoC: davinci-mcasp: Optimize pm_runtime usage and clean up the init code

[Peter Ujfalusi]

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,
-- 
1.9.2

Re: [PATCH 3/3] ASoC: davinci-mcasp: Optimize pm_runtime usage and clean up the init code

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 539 bytes --]

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?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 3/3] ASoC: davinci-mcasp: Optimize pm_runtime usage and clean up the init code

[Peter Ujfalusi]

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.

-- 
Péter

Re: [PATCH 3/3] ASoC: davinci-mcasp: Optimize pm_runtime usage and clean up the init code

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 963 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]