[alsa-devel] [PATCH 1/5] ASoC: Allow platform drivers to have no ops structure
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com --- sound/soc/soc-core.c | 49 ++++++++++++++++++++++++++++++------------------- 1 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index a6f37d4..cb93b79 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -545,6 +545,12 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
mutex_lock(&pcm_mutex);
+ /* CODEC<->CODEC DAI link, no CPU interface */ + if (rtd->dai_link->no_cpu) { + ret = -EINVAL; + goto out; + } + /* startup the audio subsystem */ if (cpu_dai->driver->ops->startup) { ret = cpu_dai->driver->ops->startup(substream, cpu_dai); @@ -555,7 +561,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) } }
- if (platform->driver->ops->open) { + if (platform->driver->ops && platform->driver->ops->open) { ret = platform->driver->ops->open(substream); if (ret < 0) { printk(KERN_ERR "asoc: can't open platform %s\n", platform->name); @@ -685,7 +691,7 @@ machine_err: codec_dai->driver->ops->shutdown(substream, codec_dai);
codec_dai_err: - if (platform->driver->ops->close) + if (platform->driver->ops && platform->driver->ops->close) platform->driver->ops->close(substream);
platform_err: @@ -767,7 +773,7 @@ static int soc_codec_close(struct snd_pcm_substream *substream) if (rtd->dai_link->ops && rtd->dai_link->ops->shutdown) rtd->dai_link->ops->shutdown(substream);
- if (platform->driver->ops->close) + if (platform->driver->ops && platform->driver->ops->close) platform->driver->ops->close(substream); cpu_dai->runtime = NULL;
@@ -810,7 +816,7 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream) } }
- if (platform->driver->ops->prepare) { + if (platform->driver->ops && platform->driver->ops->prepare) { ret = platform->driver->ops->prepare(substream); if (ret < 0) { printk(KERN_ERR "asoc: platform prepare error\n"); @@ -899,7 +905,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, } }
- if (platform->driver->ops->hw_params) { + if (platform->driver->ops && platform->driver->ops->hw_params) { ret = platform->driver->ops->hw_params(substream, params); if (ret < 0) { printk(KERN_ERR "asoc: platform %s hw params failed\n", @@ -952,7 +958,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) rtd->dai_link->ops->hw_free(substream);
/* free any DMA resources */ - if (platform->driver->ops->hw_free) + if (platform->driver->ops && platform->driver->ops->hw_free) platform->driver->ops->hw_free(substream);
/* now free hw params for the DAIs */ @@ -980,7 +986,7 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) return ret; }
- if (platform->driver->ops->trigger) { + if (platform->driver->ops && platform->driver->ops->trigger) { ret = platform->driver->ops->trigger(substream, cmd); if (ret < 0) return ret; @@ -1009,7 +1015,7 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) snd_pcm_uframes_t offset = 0; snd_pcm_sframes_t delay = 0;
- if (platform->driver->ops->pointer) + if (platform->driver->ops && platform->driver->ops->pointer) offset = platform->driver->ops->pointer(substream);
if (cpu_dai->driver->ops->delay) @@ -2125,13 +2131,15 @@ static int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
rtd->pcm = pcm; pcm->private_data = rtd; - soc_pcm_ops.mmap = platform->driver->ops->mmap; - soc_pcm_ops.pointer = platform->driver->ops->pointer; - soc_pcm_ops.ioctl = platform->driver->ops->ioctl; - soc_pcm_ops.copy = platform->driver->ops->copy; - soc_pcm_ops.silence = platform->driver->ops->silence; - soc_pcm_ops.ack = platform->driver->ops->ack; - soc_pcm_ops.page = platform->driver->ops->page; + if (platform->driver->ops) { + soc_pcm_ops.mmap = platform->driver->ops->mmap; + soc_pcm_ops.pointer = platform->driver->ops->pointer; + soc_pcm_ops.ioctl = platform->driver->ops->ioctl; + soc_pcm_ops.copy = platform->driver->ops->copy; + soc_pcm_ops.silence = platform->driver->ops->silence; + soc_pcm_ops.ack = platform->driver->ops->ack; + soc_pcm_ops.page = platform->driver->ops->page; + }
if (playback) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &soc_pcm_ops); @@ -2139,10 +2147,13 @@ static int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (capture) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &soc_pcm_ops);
- ret = platform->driver->pcm_new(rtd->card->snd_card, codec_dai, pcm); - if (ret < 0) { - printk(KERN_ERR "asoc: platform pcm constructor failed\n"); - return ret; + if (platform->driver->pcm_new) { + ret = platform->driver->pcm_new(rtd->card->snd_card, + codec_dai, pcm); + if (ret < 0) { + pr_err("asoc: platform pcm constructor failed\n"); + return ret; + } }
pcm->private_free = platform->driver->pcm_free;
Since we can now support multiple platforms allow machines to not specify a platform in a DAI link. Since the rest of the code requires that we have a struct device for all objects we do this by substituting in a dummy device that we register automatically.
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com --- sound/soc/soc-core.c | 15 +++++++++---- sound/soc/soc-utils.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index cb93b79..e851aa7 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1357,13 +1357,18 @@ find_codec: dai_link->codec_name);
find_platform: - /* do we already have the CODEC DAI for this link ? */ - if (rtd->platform) { + /* do we need a platform? */ + if (rtd->platform) goto out; - } - /* no, then find CPU DAI from registered DAIs*/ + + /* if there's no platform we match on the empty platform */ + platform_name = dai_link->platform_name; + if (!platform_name) + platform_name = "snd-soc-dummy"; + + /* no, then find one from the set of registered platforms */ list_for_each_entry(platform, &platform_list, list) { - if (!strcmp(platform->name, dai_link->platform_name)) { + if (!strcmp(platform->name, platform_name)) { rtd->platform = platform; goto out; } diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 3f45e6a..2865791 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -13,6 +13,7 @@ * option) any later version. */
+#include <linux/platform_device.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -55,3 +56,57 @@ int snd_soc_params_to_bclk(struct snd_pcm_hw_params *params) return ret; } EXPORT_SYMBOL_GPL(snd_soc_params_to_bclk); + +static struct snd_soc_platform_driver dummy_platform; + +static __devinit int snd_soc_dummy_probe(struct platform_device *pdev) +{ + return snd_soc_register_platform(&pdev->dev, &dummy_platform); +} + +static __devexit int snd_soc_dummy_remove(struct platform_device *pdev) +{ + snd_soc_unregister_platform(&pdev->dev); + + return 0; +} + +static struct platform_driver soc_dummy_driver = { + .driver = { + .name = "snd-soc-dummy", + .owner = THIS_MODULE, + }, + .probe = snd_soc_dummy_probe, + .remove = __devexit_p(snd_soc_dummy_remove), +}; + +static struct platform_device *soc_dummy_dev; + +static int __init snd_soc_util_init(void) +{ + int ret; + + soc_dummy_dev = platform_device_alloc("snd-soc-dummy", -1); + if (!soc_dummy_dev) + return -ENOMEM; + + ret = platform_device_add(soc_dummy_dev); + if (ret != 0) { + platform_device_put(soc_dummy_dev); + return ret; + } + + ret = platform_driver_register(&soc_dummy_driver); + if (ret != 0) + platform_device_unregister(soc_dummy_dev); + + return ret; +} +module_init(snd_soc_util_init); + +static void __exit snd_soc_util_exit(void) +{ + platform_device_unregister(soc_dummy_dev); + platform_driver_unregister(&soc_dummy_driver); +} +module_exit(snd_soc_util_exit);
On Wed, Apr 27, 2011 at 10:57 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
Since we can now support multiple platforms allow machines to not specify a platform in a DAI link. Since the rest of the code requires that we have a struct device for all objects we do this by substituting in a dummy device that we register automatically.
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com
2-5 as well...
Acked-by: Jassi Brar jassisinghbrar@gmail.com
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com --- sound/soc/samsung/speyside.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/sound/soc/samsung/speyside.c b/sound/soc/samsung/speyside.c index 337855a..360a333 100644 --- a/sound/soc/samsung/speyside.c +++ b/sound/soc/samsung/speyside.c @@ -183,7 +183,6 @@ static struct snd_soc_dai_link speyside_dai[] = { .cpu_dai_name = "wm8915-aif2", .codec_dai_name = "wm1250-ev1", .codec_name = "wm1250-ev1.1-0027", - .platform_name = "samsung-audio", .ops = &speyside_ops, .ignore_suspend = 1, },
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com --- sound/soc/samsung/neo1973_wm8753.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/sound/soc/samsung/neo1973_wm8753.c b/sound/soc/samsung/neo1973_wm8753.c index 4522309..16152ed 100644 --- a/sound/soc/samsung/neo1973_wm8753.c +++ b/sound/soc/samsung/neo1973_wm8753.c @@ -432,7 +432,6 @@ static struct snd_soc_dai_link neo1973_dai[] = { { /* Voice via BT */ .name = "Bluetooth", .stream_name = "Voice", - .platform_name = "samsung-audio", .cpu_dai_name = "dfbmcs320-pcm", .codec_dai_name = "wm8753-voice", .codec_name = "wm8753-codec.0-001a",
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com --- sound/soc/samsung/goni_wm8994.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/sound/soc/samsung/goni_wm8994.c b/sound/soc/samsung/goni_wm8994.c index 0e80dae..eb6d72e 100644 --- a/sound/soc/samsung/goni_wm8994.c +++ b/sound/soc/samsung/goni_wm8994.c @@ -246,7 +246,6 @@ static struct snd_soc_dai_link goni_dai[] = { .stream_name = "Voice", .cpu_dai_name = "goni-voice-dai", .codec_dai_name = "wm8994-aif2", - .platform_name = "samsung-audio", .codec_name = "wm8994-codec.0-001a", .ops = &goni_voice_ops, },
On 04/27/2011 07:27 PM, Mark Brown wrote:
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com
sound/soc/soc-core.c | 49 ++++++++++++++++++++++++++++++------------------- 1 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index a6f37d4..cb93b79 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -545,6 +545,12 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
mutex_lock(&pcm_mutex);
- /* CODEC<->CODEC DAI link, no CPU interface */
- if (rtd->dai_link->no_cpu) {
ret = -EINVAL;goto out;- }
- /* startup the audio subsystem */ if (cpu_dai->driver->ops->startup) { ret = cpu_dai->driver->ops->startup(substream, cpu_dai);
This hunk looks as if it belongs to a different series. There is currently no 'no_cpu' field in the snd_soc_dai_link struct.
- Lars
On Wed, Apr 27, 2011 at 07:49:55PM +0200, Lars-Peter Clausen wrote:
This hunk looks as if it belongs to a different series. There is currently no 'no_cpu' field in the snd_soc_dai_link struct.
Yes, it does. Please review assuming that's dropped.
On Wed, 2011-04-27 at 18:27 +0100, Mark Brown wrote:
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com
sound/soc/soc-core.c | 49 ++++++++++++++++++++++++++++++------------------- 1 files changed, 30 insertions(+), 19 deletions(-)
All
Acked-by: Liam Girdwood lrg@ti.com
On Wed, Apr 27, 2011 at 10:57 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com
sound/soc/soc-core.c | 49 ++++++++++++++++++++++++++++++------------------- 1 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index a6f37d4..cb93b79 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -545,6 +545,12 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
mutex_lock(&pcm_mutex);
- /* CODEC<->CODEC DAI link, no CPU interface */
- if (rtd->dai_link->no_cpu) {
- ret = -EINVAL;
- goto out;
- }
/* startup the audio subsystem */ if (cpu_dai->driver->ops->startup) { ret = cpu_dai->driver->ops->startup(substream, cpu_dai); @@ -555,7 +561,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) } }
- if (platform->driver->ops->open) {
- if (platform->driver->ops && platform->driver->ops->open) {
ret = platform->driver->ops->open(substream); if (ret < 0) { printk(KERN_ERR "asoc: can't open platform %s\n", platform->name); @@ -685,7 +691,7 @@ machine_err: codec_dai->driver->ops->shutdown(substream, codec_dai);
codec_dai_err:
- if (platform->driver->ops->close)
- if (platform->driver->ops && platform->driver->ops->close)
platform->driver->ops->close(substream);
platform_err: @@ -767,7 +773,7 @@ static int soc_codec_close(struct snd_pcm_substream *substream) if (rtd->dai_link->ops && rtd->dai_link->ops->shutdown) rtd->dai_link->ops->shutdown(substream);
- if (platform->driver->ops->close)
- if (platform->driver->ops && platform->driver->ops->close)
platform->driver->ops->close(substream); cpu_dai->runtime = NULL;
@@ -810,7 +816,7 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream) } }
- if (platform->driver->ops->prepare) {
- if (platform->driver->ops && platform->driver->ops->prepare) {
ret = platform->driver->ops->prepare(substream); if (ret < 0) { printk(KERN_ERR "asoc: platform prepare error\n"); @@ -899,7 +905,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, } }
- if (platform->driver->ops->hw_params) {
- if (platform->driver->ops && platform->driver->ops->hw_params) {
ret = platform->driver->ops->hw_params(substream, params); if (ret < 0) { printk(KERN_ERR "asoc: platform %s hw params failed\n", @@ -952,7 +958,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) rtd->dai_link->ops->hw_free(substream);
/* free any DMA resources */
- if (platform->driver->ops->hw_free)
- if (platform->driver->ops && platform->driver->ops->hw_free)
platform->driver->ops->hw_free(substream);
/* now free hw params for the DAIs */ @@ -980,7 +986,7 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) return ret; }
- if (platform->driver->ops->trigger) {
- if (platform->driver->ops && platform->driver->ops->trigger) {
ret = platform->driver->ops->trigger(substream, cmd); if (ret < 0) return ret; @@ -1009,7 +1015,7 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) snd_pcm_uframes_t offset = 0; snd_pcm_sframes_t delay = 0;
- if (platform->driver->ops->pointer)
- if (platform->driver->ops && platform->driver->ops->pointer)
offset = platform->driver->ops->pointer(substream);
if (cpu_dai->driver->ops->delay) @@ -2125,13 +2131,15 @@ static int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
rtd->pcm = pcm; pcm->private_data = rtd;
- soc_pcm_ops.mmap = platform->driver->ops->mmap;
- soc_pcm_ops.pointer = platform->driver->ops->pointer;
- soc_pcm_ops.ioctl = platform->driver->ops->ioctl;
- soc_pcm_ops.copy = platform->driver->ops->copy;
- soc_pcm_ops.silence = platform->driver->ops->silence;
- soc_pcm_ops.ack = platform->driver->ops->ack;
- soc_pcm_ops.page = platform->driver->ops->page;
- if (platform->driver->ops) {
- soc_pcm_ops.mmap = platform->driver->ops->mmap;
- soc_pcm_ops.pointer = platform->driver->ops->pointer;
- soc_pcm_ops.ioctl = platform->driver->ops->ioctl;
- soc_pcm_ops.copy = platform->driver->ops->copy;
- soc_pcm_ops.silence = platform->driver->ops->silence;
- soc_pcm_ops.ack = platform->driver->ops->ack;
- soc_pcm_ops.page = platform->driver->ops->page;
- }
if (playback) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &soc_pcm_ops); @@ -2139,10 +2147,13 @@ static int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (capture) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &soc_pcm_ops);
- ret = platform->driver->pcm_new(rtd->card->snd_card, codec_dai, pcm);
- if (ret < 0) {
- printk(KERN_ERR "asoc: platform pcm constructor failed\n");
- return ret;
- if (platform->driver->pcm_new) {
- ret = platform->driver->pcm_new(rtd->card->snd_card,
- codec_dai, pcm);
- if (ret < 0) {
- pr_err("asoc: platform pcm constructor failed\n");
- return ret;
- }
}
pcm->private_free = platform->driver->pcm_free;
I have nothing against this approach, but just a thought... Is there any option like having 'dummy' versions of cpu, codec and dma dais, maybe ops too, and have machine drivers explicitly use them. That will preserve what has always been expected off of ASoC drivers and keep the core 'simple' (not to mean the checks you add above are serious overhead).
Thanks.
On Thu, Apr 28, 2011 at 12:17:46AM +0530, Jassi Brar wrote:
Please delete unneeded context, makes it much easier to find your text...
I have nothing against this approach, but just a thought... Is there any option like having 'dummy' versions of cpu, codec and dma dais, maybe ops too, and have machine drivers explicitly use them. That will preserve what has always been expected off of ASoC drivers and keep the core 'simple' (not to mean the checks you add above are serious overhead).
I considered that, my thought was mostly that it's better for the core to do a bit of work than for each user to have to do typing. I'd probably have gone the other way if we weren't already having to check to see if the op itself was there - this is why there's a dummy driver created and substituted in where there's no platform at all. Had I done the second patch first I'd probably never have written this one but it seemed like something someone might come up with another sane use case for some time.
In the DAIs we're actually substituting a dummy ops in but we shouldn't really be doing that as it ought to be possible for the drivers to be const.
On Thu, Apr 28, 2011 at 12:35 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Thu, Apr 28, 2011 at 12:17:46AM +0530, Jassi Brar wrote:
Please delete unneeded context, makes it much easier to find your text...
There wasn't anything to quote from changelog and my comments encompass your every change, so I commented at the end.
I have nothing against this approach, but just a thought... Is there any option like having 'dummy' versions of cpu, codec and dma dais, maybe ops too, and have machine drivers explicitly use them. That will preserve what has always been expected off of ASoC drivers and keep the core 'simple' (not to mean the checks you add above are serious overhead).
I considered that, my thought was mostly that it's better for the core to do a bit of work than for each user to have to do typing. I'd probably have gone the other way if we weren't already having to check to see if the op itself was there - this is why there's a dummy driver created and substituted in where there's no platform at all. Had I done the second patch first I'd probably never have written this one but it seemed like something someone might come up with another sane use case for some time.
In the DAIs we're actually substituting a dummy ops in but we shouldn't really be doing that as it ought to be possible for the drivers to be const.
OK.
Acked-by: Jassi Brar jassisinghbrar@gmail.com
participants (4)
-
Jassi Brar -
Lars-Peter Clausen -
Liam Girdwood -
Mark Brown