[PATCH 4/4] ASoC: soc core allow machine driver to register the card

[PATCH 4/4] ASoC: soc core allow machine driver to register the card

[Koul, Vinod]

From: Vinod Koul <vinod.koul@intel.com>

The machine driver can't register the card directly and need to do this thru
soc-audio device creation

This patch allows the register and unregister card to be directly called by
machine drivers

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 |   21 +++++++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 541ddfa..9952254 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 8305712..69117b6 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,6 +1868,13 @@ 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 should be registering card
+	 * we should not be here in that case so ret error
+	 */
+	if (!card)
+		return -EINVAL;
+
 	/* Bodge while we unpick instantiation */
 	card->dev = &pdev->dev;
 	snd_soc_initialize_card_lists(card);
@@ -3105,11 +3110,8 @@ 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;
 
@@ -3141,17 +3143,15 @@ 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)
 {
 	if (card->instantiated)
 		soc_cleanup_card_resources(card);
@@ -3162,6 +3162,7 @@ static int snd_soc_unregister_card(struct snd_soc_card *card)
 
 	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 4/4] ASoC: soc core allow machine driver to register the card

[Liam Girdwood]

On Fri, 2011-01-14 at 19:22 +0530, Koul, Vinod wrote:
> From: Vinod Koul <vinod.koul@intel.com>
> 
> The machine driver can't register the card directly and need to do this thru
> soc-audio device creation
> 
> This patch allows the register and unregister card to be directly called by
> machine drivers
> 
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> Signed-off-by: Harsha Priya <priya.harsha@intel.com>
> ---

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>

Fwiw, adding this functionality to core has been on my TODO list for a
while. I can delete this task now :)

Thanks !

Liam  
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 4/4] ASoC: soc core allow machine driver to register the card

[Mark Brown]

On Fri, Jan 14, 2011 at 07:22:48PM +0530, Koul, Vinod wrote:
> From: Vinod Koul <vinod.koul@intel.com>
> 
> The machine driver can't register the card directly and need to do this thru
> soc-audio device creation
> 
> This patch allows the register and unregister card to be directly called by
> machine drivers

Applied, thanks.

[PATCH 4/4] ASoC: soc core allow machine driver to register the card

[Koul, Vinod]

From: Vinod Koul <vinod.koul@intel.com>

The machine driver can't register the card directly and need to do this thru
soc-audio device creation

This patch allows the register and unregister card to be called by
machine drivers

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Harsha Priya <harsha.priya@intel.com>
---
 include/sound/soc.h  |    2 ++
 sound/soc/soc-core.c |   21 +++++++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 541ddfa..9952254 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 8305712..87348ab 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,6 +1868,10 @@ 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 is registering card */
+	if (!card)
+		return 0;
+
 	/* Bodge while we unpick instantiation */
 	card->dev = &pdev->dev;
 	snd_soc_initialize_card_lists(card);
@@ -1919,6 +1921,9 @@ static int soc_remove(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = platform_get_drvdata(pdev);
 
+	if (!card)
+		return 0;
+
 	snd_soc_unregister_card(card);
 	return 0;
 }
@@ -3105,11 +3110,8 @@ 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;
 
@@ -3141,17 +3143,15 @@ 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)
 {
 	if (card->instantiated)
 		soc_cleanup_card_resources(card);
@@ -3162,6 +3162,7 @@ static int snd_soc_unregister_card(struct snd_soc_card *card)
 
 	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 4/4] ASoC: soc core allow machine driver to register the card

[Mark Brown]

On Thu, Jan 13, 2011 at 10:49:10PM +0530, Koul, Vinod wrote:

> @@ -1870,6 +1868,10 @@ 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 is registering card */
> +	if (!card)
> +		return 0;
> +

I'll review this one more closely tomorrow - as I said with the combined
patch I'd not expect the soc-audio device to be involved when the
machine driver is registering a card so I'd like to understand in more
detail why this is still required.

Re: [PATCH 4/4] ASoC: soc core allow machine driver to register the card

[Koul, Vinod]

> > @@ -1870,6 +1868,10 @@ 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 is registering card */
> > +	if (!card)
> > +		return 0;
> > +
> 
> I'll review this one more closely tomorrow - as I said with the combined
> patch I'd not expect the soc-audio device to be involved when the
> machine driver is registering a card so I'd like to understand in more
> detail why this is still required.

If someone is still creating soc-audio device and not adding the drvdata the
code following this will use a null pointer :(. So tried to protect, but again
if people are doing right, then this shouldn't be invoked at all

~Vinod

Re: [PATCH 4/4] ASoC: soc core allow machine driver to register the card

[Mark Brown]

On Fri, Jan 14, 2011 at 09:54:59AM +0530, Koul, Vinod wrote:

> > > +	/* no card, so machine driver is registering card */
> > > +	if (!card)
> > > +		return 0;
> > > +

> > I'll review this one more closely tomorrow - as I said with the combined
> > patch I'd not expect the soc-audio device to be involved when the
> > machine driver is registering a card so I'd like to understand in more
> > detail why this is still required.

> If someone is still creating soc-audio device and not adding the drvdata the
> code following this will use a null pointer :(. So tried to protect, but again
> if people are doing right, then this shouldn't be invoked at all

If that's all it is then the check should be changed to return -EINVAL
or similar as this is an error which prevents the device being
instantiated.  It shouldn't pretend it succeeded, and not doing so will
save us having to check in any of the other functions.

Re: [PATCH 4/4] ASoC: soc core allow machine driver to register the card

[Koul, Vinod]

> > > I'll review this one more closely tomorrow - as I said with the combined
> > > patch I'd not expect the soc-audio device to be involved when the
> > > machine driver is registering a card so I'd like to understand in more
> > > detail why this is still required.
> 
> > If someone is still creating soc-audio device and not adding the drvdata the
> > code following this will use a null pointer :(. So tried to protect, but again
> > if people are doing right, then this shouldn't be invoked at all
> 
> If that's all it is then the check should be changed to return -EINVAL
> or similar as this is an error which prevents the device being
> instantiated.  It shouldn't pretend it succeeded, and not doing so will
> save us having to check in any of the other functions.

Agreed, but on the other hand if someone is using register api they should
not be creating the device, so probe shouldn't be called at all.
We can leave this check. What do you think?

~Vinod

Re: [PATCH 4/4] ASoC: soc core allow machine driver to register the card

[Mark Brown]

On Fri, Jan 14, 2011 at 04:55:47PM +0530, Koul, Vinod wrote:

> > If that's all it is then the check should be changed to return -EINVAL
> > or similar as this is an error which prevents the device being
> > instantiated.  It shouldn't pretend it succeeded, and not doing so will
> > save us having to check in any of the other functions.

> Agreed, but on the other hand if someone is using register api they should
> not be creating the device, so probe shouldn't be called at all.
> We can leave this check. What do you think?

I think the check should be changed to report an error since they should
never be calling this function in the first place; this is the standard
thing for devices with an invalid setup.

Re: [PATCH 4/4] ASoC: soc core allow machine driver to register the card

[Koul, Vinod]

> > > If that's all it is then the check should be changed to return -EINVAL
> > > or similar as this is an error which prevents the device being
> > > instantiated.  It shouldn't pretend it succeeded, and not doing so will
> > > save us having to check in any of the other functions.
> 
> > Agreed, but on the other hand if someone is using register api they should
> > not be creating the device, so probe shouldn't be called at all.
> > We can leave this check. What do you think?
> 
> I think the check should be changed to report an error since they should
> never be calling this function in the first place; this is the standard
> thing for devices with an invalid setup.
Yes, that's a better idea.
Sent you this patch updated.

~Vinod