[PATCH] ASoC: Make register_card API available to drivers

[PATCH] ASoC: Make register_card API available to drivers

[Koul, Vinod]

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
-- 
1.7.2.3

Re: [PATCH] ASoC: Make register_card API available to drivers

[Mark Brown]

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

Re: [PATCH] ASoC: Make register_card API available to drivers

[Koul, Vinod]

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

Re: [PATCH] ASoC: Make register_card API available to drivers

[Mark Brown]

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.

Re: [PATCH] ASoC: Make register_card API available to drivers

[Koul, Vinod]

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

Re: [PATCH] ASoC: Make register_card API available to drivers

[Mark Brown]

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.

Re: [PATCH] ASoC: Make register_card API available to drivers

[Koul, Vinod]

> 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

Re: [PATCH] ASoC: Make register_card API available to drivers

[Mark Brown]

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.

Re: [PATCH] ASoC: Make register_card API available to drivers

[Koul, Vinod]

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