[alsa-devel] [PATCH] ASoC: Make register_card API available to drivers
From: Vinod Koul vinod.koul@intel.com
The register_card is an internal API which soc_probe uses to register the card. In order to create multiple cards we need to create multiple instances of soc-audio device. If we exposes the register card API for drivers to use, then machine driver can register their cards directly
This patch makes register_card and unregister_card API available to machine drivers. This also maintains compatibility with older method of registration which can be marked depreciated and removed eventually.
Signed-off-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Harsha Priya priya.harsha@intel.com --- include/sound/soc.h | 2 + sound/soc/soc-core.c | 90 +++++++++++++++++++++++++++---------------------- 2 files changed, 52 insertions(+), 40 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index d609232..b515eb6 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -258,6 +258,8 @@ enum snd_soc_compress_type { SND_SOC_RBTREE_COMPRESSION };
+int snd_soc_register_card(struct snd_soc_card *card); +int snd_soc_unregister_card(struct snd_soc_card *card); int snd_soc_register_platform(struct device *dev, struct snd_soc_platform_driver *platform_drv); void snd_soc_unregister_platform(struct device *dev); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index cbac50b..0bd07f6 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -58,8 +58,6 @@ static LIST_HEAD(dai_list); static LIST_HEAD(platform_list); static LIST_HEAD(codec_list);
-static int snd_soc_register_card(struct snd_soc_card *card); -static int snd_soc_unregister_card(struct snd_soc_card *card); static int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num);
/* @@ -1870,16 +1868,12 @@ static int soc_probe(struct platform_device *pdev) struct snd_soc_card *card = platform_get_drvdata(pdev); int ret = 0;
+ /* no card, so machine driver will register explictly */ + if (!card) + return 0; + /* Bodge while we unpick instantiation */ card->dev = &pdev->dev; - INIT_LIST_HEAD(&card->dai_dev_list); - INIT_LIST_HEAD(&card->codec_dev_list); - INIT_LIST_HEAD(&card->platform_dev_list); - INIT_LIST_HEAD(&card->widgets); - INIT_LIST_HEAD(&card->paths); - INIT_LIST_HEAD(&card->dapm_list); - - soc_init_card_debugfs(card);
ret = snd_soc_register_card(card); if (ret != 0) { @@ -1890,37 +1884,46 @@ static int soc_probe(struct platform_device *pdev) return 0; }
-/* removes a socdev */ -static int soc_remove(struct platform_device *pdev) +static int soc_cleanup_card_resources(struct snd_soc_card *card) { - struct snd_soc_card *card = platform_get_drvdata(pdev); + struct platform_device *pdev = to_platform_device(card->dev); int i;
- if (card->instantiated) { + /* make sure any delayed work runs */ + for (i = 0; i < card->num_rtd; i++) { + struct snd_soc_pcm_runtime *rtd = &card->rtd[i]; + flush_delayed_work_sync(&rtd->delayed_work); + }
- /* make sure any delayed work runs */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_pcm_runtime *rtd = &card->rtd[i]; - flush_delayed_work_sync(&rtd->delayed_work); - } + /* remove auxiliary devices */ + for (i = 0; i < card->num_aux_devs; i++) + soc_remove_aux_dev(card, i);
- /* remove auxiliary devices */ - for (i = 0; i < card->num_aux_devs; i++) - soc_remove_aux_dev(card, i); + /* remove and free each DAI */ + for (i = 0; i < card->num_rtd; i++) + soc_remove_dai_link(card, i);
- /* remove and free each DAI */ - for (i = 0; i < card->num_rtd; i++) - soc_remove_dai_link(card, i); + soc_cleanup_card_debugfs(card);
- soc_cleanup_card_debugfs(card); + /* remove the card */ + if (card->remove) + card->remove(pdev); + + kfree(card->rtd); + snd_card_free(card->snd_card); + return 0;
- /* remove the card */ - if (card->remove) - card->remove(pdev); +} + +/* removes a socdev */ +int soc_remove(struct platform_device *pdev) +{ + struct snd_soc_card *card = platform_get_drvdata(pdev); + + + if (!card) + return 0;
- kfree(card->rtd); - snd_card_free(card->snd_card); - } snd_soc_unregister_card(card); return 0; } @@ -3107,17 +3110,23 @@ EXPORT_SYMBOL_GPL(snd_soc_dai_digital_mute); * * @card: Card to register * - * Note that currently this is an internal only function: it will be - * exposed to machine drivers after further backporting of ASoC v2 - * registration APIs. */ -static int snd_soc_register_card(struct snd_soc_card *card) +int snd_soc_register_card(struct snd_soc_card *card) { int i;
if (!card->name || !card->dev) return -EINVAL;
+ INIT_LIST_HEAD(&card->dai_dev_list); + INIT_LIST_HEAD(&card->codec_dev_list); + INIT_LIST_HEAD(&card->platform_dev_list); + INIT_LIST_HEAD(&card->widgets); + INIT_LIST_HEAD(&card->paths); + INIT_LIST_HEAD(&card->dapm_list); + + soc_init_card_debugfs(card); + card->rtd = kzalloc(sizeof(struct snd_soc_pcm_runtime) * (card->num_links + card->num_aux_devs), GFP_KERNEL); @@ -3141,25 +3150,26 @@ static int snd_soc_register_card(struct snd_soc_card *card)
return 0; } +EXPORT_SYMBOL_GPL(snd_soc_register_card);
/** * snd_soc_unregister_card - Unregister a card with the ASoC core * * @card: Card to unregister * - * Note that currently this is an internal only function: it will be - * exposed to machine drivers after further backporting of ASoC v2 - * registration APIs. */ -static int snd_soc_unregister_card(struct snd_soc_card *card) +int snd_soc_unregister_card(struct snd_soc_card *card) { mutex_lock(&client_mutex); list_del(&card->list); mutex_unlock(&client_mutex); + if (card->instantiated) + soc_cleanup_card_resources(card); dev_dbg(card->dev, "Unregistered card '%s'\n", card->name);
return 0; } +EXPORT_SYMBOL_GPL(snd_soc_unregister_card);
/* * Simplify DAI link configuration by removing ".-1" from device names
On Thu, Jan 13, 2011 at 12:19:18PM +0530, Koul, Vinod wrote:
The register_card is an internal API which soc_probe uses to register the card. In order to create multiple cards we need to create multiple instances of soc-audio device. If we exposes the register card API for drivers to use, then machine driver can register their cards directly
This patch makes register_card and unregister_card API available to machine drivers. This also maintains compatibility with older method of registration which can be marked depreciated and removed eventually.
This is probably mostly OK but it'd be good if you could break it down into a small patch series. At present it does a bunch of refactoring and also adds the API - it'd be better if there were a sequence of patches, each doing a single refactor, followed by a final patch which exports the API.
Splitting it up like this will allow bisection if there's a problem introduced by the change and makes review very much easier as there's only one change to think about in a given patch.
@@ -1870,16 +1868,12 @@ static int soc_probe(struct platform_device *pdev) struct snd_soc_card *card = platform_get_drvdata(pdev); int ret = 0;
- /* no card, so machine driver will register explictly */
- if (!card)
return 0;
Hrm, this looks a bit surprising. When we're registering directly from the machine drivers I'd not expect us to end up going through soc-probe at all and...
/* Bodge while we unpick instantiation */ card->dev = &pdev->dev;
...that the card device would be something passed in from the machine driver (probably something that the architecture code registered in order to allow the machine driver to probe, but possibly something else).
This patch makes register_card and unregister_card API available to machine drivers. This also maintains compatibility with older method of registration which can be marked depreciated and removed eventually.
This is probably mostly OK but it'd be good if you could break it down into a small patch series. At present it does a bunch of refactoring and also adds the API - it'd be better if there were a sequence of patches, each doing a single refactor, followed by a final patch which exports the API.
Sure will do that...
@@ -1870,16 +1868,12 @@ static int soc_probe(struct platform_device *pdev) struct snd_soc_card *card = platform_get_drvdata(pdev); int ret = 0;
- /* no card, so machine driver will register explictly */
- if (!card)
return 0;
Hrm, this looks a bit surprising. When we're registering directly from the machine drivers I'd not expect us to end up going through soc-probe at all and...
/* Bodge while we unpick instantiation */ card->dev = &pdev->dev;
...that the card device would be something passed in from the machine driver (probably something that the architecture code registered in order to allow the machine driver to probe, but possibly something else).
Yes on both of these, but I didn't want to break current method. So I kept these to ensure both methods work fine.
One we have converted all driver to use register then we can remove these. Would have made sense to put a comment for that
~Vinod
On Thu, Jan 13, 2011 at 05:06:19PM +0530, Koul, Vinod wrote:
Yes on both of these, but I didn't want to break current method. So I kept these to ensure both methods work fine.
One we have converted all driver to use register then we can remove these. Would have made sense to put a comment for that
My plan for this when I originally wrote the comment had been to make the contents of the soc-audio device probe and removal look exactly like what you'd write directly in a machine driver so no code except for the code doing the soc-audio device had any idea it was there. This would keep backwards compatiblity without special casing, making things more maintainable.
Yes on both of these, but I didn't want to break current method. So I kept these to ensure both methods work fine.
One we have converted all driver to use register then we can remove these. Would have made sense to put a comment for that
My plan for this when I originally wrote the comment had been to make the contents of the soc-audio device probe and removal look exactly like what you'd write directly in a machine driver so no code except for the code doing the soc-audio device had any idea it was there. This would keep backwards compatiblity without special casing, making things more maintainable.
Hmmmm, I didn't want to change the current machine drivers yet, hence tried to move the code of soc_probe into card_register. So that works neatly (doesn't matter who is doing, machine or core) Only thing which I kept is to check for card pointer and if valid then call register (assuming someone is still using old method) which we should mark deprecated
card->dev = &pdev->dev;
This would be required only for current method hence kept it there. For anyone who uses register, the machine should set this up properly
And I agree that these things should be done by machine. But wanted to split the change to avoid any issues
So what I propose is keep it as is, I will resend the split patch series, and then after machine drivers are changed we remove these.
Btw this make me wonder what will soc_probe be used (it is not doing anything) Do we still need this and creating soc-audio device?
~Vinod
On Thu, Jan 13, 2011 at 06:54:52PM +0530, Koul, Vinod wrote:
My plan for this when I originally wrote the comment had been to make the contents of the soc-audio device probe and removal look exactly like what you'd write directly in a machine driver so no code except for the code doing the soc-audio device had any idea it was there. This would keep backwards compatiblity without special casing, making things more maintainable.
Hmmmm, I didn't want to change the current machine drivers yet, hence tried to move the code of soc_probe into card_register. So that works neatly (doesn't matter who is doing, machine or core) Only thing which I kept is to check for card pointer and if valid then call register (assuming someone is still using old method) which we should mark deprecated
The above doesn't require any change in existing machine drivers - they still pass the card data structure into the soc-audio device which then registers with the core. This the whole point, move the soc-audio device over to using an API that can also be used externally then convert the machine drivers to stop using the soc-audio device.
Btw this make me wonder what will soc_probe be used (it is not doing anything) Do we still need this and creating soc-audio device?
That's the point I'm making above. It shouldn't be involved at all except as part of the soc-audio device code, anything registering a card directly should never call it.
The above doesn't require any change in existing machine drivers - they still pass the card data structure into the soc-audio device which then registers with the core. This the whole point, move the soc-audio device over to using an API that can also be used externally then convert the machine drivers to stop using the soc-audio device.
Btw this make me wonder what will soc_probe be used (it is not doing anything) Do we still need this and creating soc-audio device?
That's the point I'm making above. It shouldn't be involved at all except as part of the soc-audio device code, anything registering a card directly should never call it.
Agreed :) Knowing the context helps...
Above works well for probe, but not sure if that is intention for remove(). Do you expect the machine driver to call codec and platform remove before it unregisters the card. The code inside: if (card->instantiated)
~Vinod
On Thu, Jan 13, 2011 at 07:31:48PM +0530, Koul, Vinod wrote:
Above works well for probe, but not sure if that is intention for remove(). Do you expect the machine driver to call codec and platform remove before it unregisters the card. The code inside: if (card->instantiated)
No, the core code needs to do that as part of removing the card.
Above works well for probe, but not sure if that is intention for remove(). Do you expect the machine driver to call codec and platform remove before it unregisters the card. The code inside: if (card->instantiated)
No, the core code needs to do that as part of removing the card.
Agreed, sending you're the split patchset, code is almost same
~Vinod
participants (2)
-
Koul, Vinod
-
Mark Brown