[RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Jarkko Nikula]

This makes possible to register auxiliary dailess codecs in a machine
driver. Term dailess is used here for amplifiers and codecs without DAI or
DAI being unused.

Dailess auxiliary codecs are kept in struct snd_soc_aux_dev and those codecs
are probed after initializing the DAI links. There are no major differences
between DAI link codecs and dailess codecs in ASoC core point of view. DAPM
handles them equally and sysfs and debugfs directories for dailess codecs
are similar except the pmdown_time node is not created.

Only suspend and resume functions are modified to traverse all probed codecs
instead of DAI link codecs.

Example below shows a dailess codec registration.

struct snd_soc_aux_dev foo_aux_dev[] = {
	{
		.name = "Amp",
		.codec_name = "codec.2",
		.init = foo_init2,
	},
};

static struct snd_soc_card card = {
	...
	.aux_dev = foo_aux_dev,
	.num_aux_devs = ARRAY_SIZE(foo_aux_dev),
};

Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
---
Idea sharing version, not to be applied. Needs at least cross-device DAPM
to be usuful and e.g. soc_probe_dai_link and soc_probe_aux_dev share a lot
of same code.

I'm not entirely sure of reusing struct snd_soc_pcm_runtime but having some
common struct on top of it for registering the sysfs nodes and passing to
snd_soc_dapm_sys_add didn't sound clear either.
Anyway suspend/resume is working with this version and doesn't need any other
modifications to soc_suspend/soc_resume than traversing through the registered
codecs instead of doing bunch of rtd->dailess etc hacks there.
---
 include/sound/soc.h  |   17 ++++++
 sound/soc/soc-core.c |  155 ++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 166 insertions(+), 6 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 346c59e..c62225e 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -583,6 +583,14 @@ struct snd_soc_prefix_map {
 	const char *name_prefix;
 };
 
+struct snd_soc_aux_dev {
+	const char *name;		/* Codec name */
+	const char *codec_name;		/* for multi-codec */
+
+	/* codec/machine specific init - e.g. add machine controls */
+	int (*init)(struct snd_soc_dapm_context *dapm);
+};
+
 /* SoC card */
 struct snd_soc_card {
 	const char *name;
@@ -624,6 +632,15 @@ struct snd_soc_card {
 	struct snd_soc_prefix_map *prefix_map;
 	int num_prefixes;
 
+	/*
+	 * optional auxiliary devices such as amplifiers or codecs with DAI
+	 * link unused
+	 */
+	struct snd_soc_aux_dev *aux_dev;
+	int num_aux_devs;
+	struct snd_soc_pcm_runtime *rtd_aux;
+	int num_aux_rtd;
+
 	struct work_struct deferred_resume_work;
 
 	/* lists of probed devices belonging to this card */
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b0e1aea..d7fc3a6 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -986,6 +986,7 @@ static int soc_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct snd_soc_card *card = platform_get_drvdata(pdev);
+	struct snd_soc_codec *codec;
 	int i;
 
 	/* If the initialization of this soc device failed, there is no codec
@@ -1064,8 +1065,7 @@ static int soc_suspend(struct device *dev)
 	}
 
 	/* suspend all CODECs */
-	for (i = 0; i < card->num_rtd; i++) {
-		struct snd_soc_codec *codec = card->rtd[i].codec;
+	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
 		/* If there are paths active then the CODEC will be held with
 		 * bias _ON and should not be suspended. */
 		if (!codec->suspended && codec->driver->suspend) {
@@ -1106,6 +1106,7 @@ static void soc_resume_deferred(struct work_struct *work)
 	struct snd_soc_card *card =
 			container_of(work, struct snd_soc_card, deferred_resume_work);
 	struct platform_device *pdev = to_platform_device(card->dev);
+	struct snd_soc_codec *codec;
 	int i;
 
 	/* our power state is still SNDRV_CTL_POWER_D3hot from suspend time,
@@ -1131,8 +1132,7 @@ static void soc_resume_deferred(struct work_struct *work)
 			cpu_dai->driver->resume(cpu_dai);
 	}
 
-	for (i = 0; i < card->num_rtd; i++) {
-		struct snd_soc_codec *codec = card->rtd[i].codec;
+	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
 		/* If the CODEC was idle over suspend then it will have been
 		 * left with bias OFF or STANDBY and suspended so we must now
 		 * resume.  Otherwise the suspend was suppressed.
@@ -1604,6 +1604,130 @@ static void soc_unregister_ac97_dai_link(struct snd_soc_codec *codec)
 }
 #endif
 
+static int soc_probe_aux_dev(struct snd_soc_card *card, int num)
+{
+	struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
+	struct snd_soc_pcm_runtime *rtd = &card->rtd_aux[num];
+	struct snd_soc_codec *codec;
+	const char *temp;
+	int ret = 0;
+
+	/* find CODEC from registered CODECs*/
+	list_for_each_entry(codec, &codec_list, list) {
+		if (!strcmp(codec->name, aux_dev->codec_name)) {
+			if (codec->probed) {
+				dev_err(codec->dev,
+					"asoc: codec already probed");
+				ret = -EBUSY;
+				goto out;
+			}
+			break;
+		}
+	}
+
+	if (!try_module_get(codec->dev->driver->owner))
+		return -ENODEV;
+
+	codec->card = card;
+	codec->dapm.card = card;
+
+	soc_set_name_prefix(card, codec);
+	if (codec->driver->probe) {
+		ret = codec->driver->probe(codec);
+		if (ret < 0) {
+			dev_err(codec->dev, "asoc: failed to probe CODEC");
+			return ret;
+		}
+	}
+
+	soc_init_codec_debugfs(codec);
+
+	/* mark codec as probed and add to card codec list */
+	codec->probed = 1;
+	list_add(&codec->card_list, &card->codec_dev_list);
+	list_add(&codec->dapm.list, &card->dapm_list);
+
+	/* now that all clients have probed, initialise the DAI link */
+	if (aux_dev->init) {
+		/* machine controls, routes and widgets are not prefixed */
+		temp = codec->name_prefix;
+		codec->name_prefix = NULL;
+		ret = aux_dev->init(&codec->dapm);
+		if (ret < 0) {
+			dev_err(codec->dev,
+				"asoc: failed to init %s\n", aux_dev->name);
+			return ret;
+		}
+		codec->name_prefix = temp;
+	}
+
+	/* Make sure all DAPM widgets are instantiated */
+	snd_soc_dapm_new_widgets(&codec->dapm);
+	snd_soc_dapm_sync(&codec->dapm);
+
+	/* register the rtd device */
+	rtd->codec = codec;
+	rtd->card = card;
+	rtd->dev.parent = card->dev;
+	rtd->dev.release = rtd_release;
+	rtd->dev.init_name = aux_dev->name;
+	ret = device_register(&rtd->dev);
+	if (ret < 0) {
+		dev_err(codec->dev,
+			"asoc: failed to register aux runtime device %d\n",
+			ret);
+		return ret;
+	}
+	rtd->dev_registered = 1;
+
+	/* add DAPM sysfs entries for this codec */
+	ret = snd_soc_dapm_sys_add(&rtd->dev);
+	if (ret < 0)
+		dev_err(codec->dev,
+			"asoc: failed to add codec dapm sysfs entries\n");
+
+	/* add codec sysfs entries */
+	ret = device_create_file(&rtd->dev, &dev_attr_codec_reg);
+	if (ret < 0)
+		dev_err(codec->dev, "asoc: failed to add codec sysfs files\n");
+
+out:
+	return ret;
+}
+
+static void soc_remove_aux_dev(struct snd_soc_card *card, int num)
+{
+	struct snd_soc_pcm_runtime *rtd = &card->rtd_aux[num];
+	struct snd_soc_codec *codec = rtd->codec;
+	int err;
+
+	/* unregister the rtd device */
+	if (rtd->dev_registered) {
+		device_unregister(&rtd->dev);
+		rtd->dev_registered = 0;
+	}
+
+	/* remove the CODEC */
+	if (codec && codec->probed) {
+		if (codec->driver->remove) {
+			err = codec->driver->remove(codec);
+			if (err < 0)
+				dev_err(codec->dev,
+					"asoc: failed to remove %s\n",
+					codec->name);
+		}
+
+		/* Make sure all DAPM widgets are freed */
+		snd_soc_dapm_free(&codec->dapm);
+
+		soc_cleanup_codec_debugfs(codec);
+		device_remove_file(&rtd->dev, &dev_attr_codec_reg);
+		codec->probed = 0;
+		list_del(&codec->card_list);
+		module_put(codec->dev->driver->owner);
+	}
+}
+
 static void snd_soc_instantiate_card(struct snd_soc_card *card)
 {
 	struct platform_device *pdev = to_platform_device(card->dev);
@@ -1658,6 +1782,15 @@ static void snd_soc_instantiate_card(struct snd_soc_card *card)
 		}
 	}
 
+	for (i = 0; i < card->num_aux_devs; i++) {
+		ret = soc_probe_aux_dev(card, i);
+		if (ret < 0) {
+			pr_err("asoc: failed to add auxiliary devices %s: %d\n",
+			       card->name, ret);
+			goto probe_aux_dev_err;
+		}
+	}
+
 	snprintf(card->snd_card->shortname, sizeof(card->snd_card->shortname),
 		 "%s",  card->name);
 	snprintf(card->snd_card->longname, sizeof(card->snd_card->longname),
@@ -1684,6 +1817,10 @@ static void snd_soc_instantiate_card(struct snd_soc_card *card)
 	mutex_unlock(&card->mutex);
 	return;
 
+probe_aux_dev_err:
+	for (i = 0; i < card->num_aux_devs; i++)
+		soc_remove_aux_dev(card, i);
+
 probe_dai_err:
 	for (i = 0; i < card->num_links; i++)
 		soc_remove_dai_link(card, i);
@@ -1748,6 +1885,10 @@ static int soc_remove(struct platform_device *pdev)
 			run_delayed_work(&rtd->delayed_work);
 		}
 
+		/* 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);
@@ -2950,10 +3091,12 @@ static int snd_soc_register_card(struct snd_soc_card *card)
 	if (!card->name || !card->dev)
 		return -EINVAL;
 
-	card->rtd = kzalloc(sizeof(struct snd_soc_pcm_runtime) * card->num_links,
-			GFP_KERNEL);
+	card->rtd = kzalloc(sizeof(struct snd_soc_pcm_runtime) *
+			    (card->num_links + card->num_aux_devs),
+			    GFP_KERNEL);
 	if (card->rtd == NULL)
 		return -ENOMEM;
+	card->rtd_aux = &card->rtd[card->num_links];
 
 	for (i = 0; i < card->num_links; i++)
 		card->rtd[i].dai_link = &card->dai_link[i];
-- 
1.7.2.3

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Liam Girdwood]

Hi Jarkko,

Sorry, didn't get a chance to discuss on IRC but I have some comments
below :-

On Thu, 2010-11-25 at 17:47 +0200, Jarkko Nikula wrote:
> This makes possible to register auxiliary dailess codecs in a machine
> driver. Term dailess is used here for amplifiers and codecs without DAI or
> DAI being unused.
> 
> Dailess auxiliary codecs are kept in struct snd_soc_aux_dev and those codecs
> are probed after initializing the DAI links. There are no major differences
> between DAI link codecs and dailess codecs in ASoC core point of view. DAPM
> handles them equally and sysfs and debugfs directories for dailess codecs
> are similar except the pmdown_time node is not created.
> 
> Only suspend and resume functions are modified to traverse all probed codecs
> instead of DAI link codecs.
> 
> Example below shows a dailess codec registration.
> 
> struct snd_soc_aux_dev foo_aux_dev[] = {
> 	{
> 		.name = "Amp",
> 		.codec_name = "codec.2",
> 		.init = foo_init2,
> 	},
> };
> 
> static struct snd_soc_card card = {
> 	...
> 	.aux_dev = foo_aux_dev,
> 	.num_aux_devs = ARRAY_SIZE(foo_aux_dev),
> };
> 
> Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
> ---
> Idea sharing version, not to be applied. Needs at least cross-device DAPM
> to be usuful and e.g. soc_probe_dai_link and soc_probe_aux_dev share a lot
> of same code.
> 
> I'm not entirely sure of reusing struct snd_soc_pcm_runtime but having some
> common struct on top of it for registering the sysfs nodes and passing to
> snd_soc_dapm_sys_add didn't sound clear either.
> Anyway suspend/resume is working with this version and doesn't need any other
> modifications to soc_suspend/soc_resume than traversing through the registered
> codecs instead of doing bunch of rtd->dailess etc hacks there.
> ---
>  include/sound/soc.h  |   17 ++++++
>  sound/soc/soc-core.c |  155 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 166 insertions(+), 6 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 346c59e..c62225e 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -583,6 +583,14 @@ struct snd_soc_prefix_map {
>  	const char *name_prefix;
>  };
>  
> +struct snd_soc_aux_dev {
> +	const char *name;		/* Codec name */
> +	const char *codec_name;		/* for multi-codec */
> +
> +	/* codec/machine specific init - e.g. add machine controls */
> +	int (*init)(struct snd_soc_dapm_context *dapm);
> +};
> +
>  /* SoC card */
>  struct snd_soc_card {
>  	const char *name;
> @@ -624,6 +632,15 @@ struct snd_soc_card {
>  	struct snd_soc_prefix_map *prefix_map;
>  	int num_prefixes;
>  
> +	/*
> +	 * optional auxiliary devices such as amplifiers or codecs with DAI
> +	 * link unused
> +	 */
> +	struct snd_soc_aux_dev *aux_dev;
> +	int num_aux_devs;
> +	struct snd_soc_pcm_runtime *rtd_aux;
> +	int num_aux_rtd;
> +

Could we not just make this a new component type and have a list of aux
devices ?

>  
>  	/* lists of probed devices belonging to this card */
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index b0e1aea..d7fc3a6 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -986,6 +986,7 @@ static int soc_suspend(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct snd_soc_card *card = platform_get_drvdata(pdev);
> +	struct snd_soc_codec *codec;
>  	int i;
>  
>  	/* If the initialization of this soc device failed, there is no codec
> @@ -1064,8 +1065,7 @@ static int soc_suspend(struct device *dev)
>  	}
>  
>  	/* suspend all CODECs */
> -	for (i = 0; i < card->num_rtd; i++) {
> -		struct snd_soc_codec *codec = card->rtd[i].codec;
> +	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
>  		/* If there are paths active then the CODEC will be held with
>  		 * bias _ON and should not be suspended. */
>  		if (!codec->suspended && codec->driver->suspend) {
> @@ -1106,6 +1106,7 @@ static void soc_resume_deferred(struct work_struct *work)
>  	struct snd_soc_card *card =
>  			container_of(work, struct snd_soc_card, deferred_resume_work);
>  	struct platform_device *pdev = to_platform_device(card->dev);
> +	struct snd_soc_codec *codec;
>  	int i;
>  
>  	/* our power state is still SNDRV_CTL_POWER_D3hot from suspend time,
> @@ -1131,8 +1132,7 @@ static void soc_resume_deferred(struct work_struct *work)
>  			cpu_dai->driver->resume(cpu_dai);
>  	}
>  
> -	for (i = 0; i < card->num_rtd; i++) {
> -		struct snd_soc_codec *codec = card->rtd[i].codec;
> +	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
>  		/* If the CODEC was idle over suspend then it will have been
>  		 * left with bias OFF or STANDBY and suspended so we must now
>  		 * resume.  Otherwise the suspend was suppressed.
> @@ -1604,6 +1604,130 @@ static void soc_unregister_ac97_dai_link(struct snd_soc_codec *codec)
>  }
>  #endif
>  
> +static int soc_probe_aux_dev(struct snd_soc_card *card, int num)
> +{
> +	struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
> +	struct snd_soc_pcm_runtime *rtd = &card->rtd_aux[num];
> +	struct snd_soc_codec *codec;
> +	const char *temp;
> +	int ret = 0;
> +
> +	/* find CODEC from registered CODECs*/
> +	list_for_each_entry(codec, &codec_list, list) {
> +		if (!strcmp(codec->name, aux_dev->codec_name)) {
> +			if (codec->probed) {
> +				dev_err(codec->dev,
> +					"asoc: codec already probed");
> +				ret = -EBUSY;
> +				goto out;
> +			}
> +			break;
> +		}
> +	}
> +

Why do aux devices need to be coupled with CODECs here ? 

Thanks

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

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Mark Brown]

On Thu, Nov 25, 2010 at 07:18:23PM +0000, Liam Girdwood wrote:
> On Thu, 2010-11-25 at 17:47 +0200, Jarkko Nikula wrote:

> > +	struct snd_soc_aux_dev *aux_dev;
> > +	int num_aux_devs;
> > +	struct snd_soc_pcm_runtime *rtd_aux;
> > +	int num_aux_rtd;

> Could we not just make this a new component type and have a list of aux
> devices ?

I'd certainly rather we didn't have to deal with the PCM runtime data
since obviously the main feature of these things is that there's no PCM
involved.  Need to review this properly to remind myself why we're doing
this...

> > +	/* find CODEC from registered CODECs*/
> > +	list_for_each_entry(codec, &codec_list, list) {
> > +		if (!strcmp(codec->name, aux_dev->codec_name)) {
> > +			if (codec->probed) {
> > +				dev_err(codec->dev,
> > +					"asoc: codec already probed");
> > +				ret = -EBUSY;
> > +				goto out;
> > +			}
> > +			break;
> > +		}
> > +	}

> Why do aux devices need to be coupled with CODECs here ? 

The way we've got things set up currently the CODEC isn't really just a
CODEC, it's got a bunch of other random services which are more generic
chip services associated with it like the register cache and the bias
management.  There's so much overlap between the devices that I'm not
sure that it's really worth splitting the types up too much at the root
level (CODEC is pretty much a superclass that has everything on it but
DMA).

Given how widely used snd_soc_codec is I'm not sure it's worth fixing
the naming thing here, especially since 99% of the time the device is
actually a CODEC.

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Jarkko Nikula]

On Thu, 25 Nov 2010 21:32:54 +0000
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Thu, Nov 25, 2010 at 07:18:23PM +0000, Liam Girdwood wrote:
> > On Thu, 2010-11-25 at 17:47 +0200, Jarkko Nikula wrote:
> 
> > > +	struct snd_soc_aux_dev *aux_dev;
> > > +	int num_aux_devs;
> > > +	struct snd_soc_pcm_runtime *rtd_aux;
> > > +	int num_aux_rtd;
> 
> > Could we not just make this a new component type and have a list of aux
> > devices ?
> 
> I'd certainly rather we didn't have to deal with the PCM runtime data
> since obviously the main feature of these things is that there's no PCM
> involved.  Need to review this properly to remind myself why we're doing
> this...
> 
Exactly, that was the part I didn't like myself either. Only use for
rtd here is to hold struct device for virtual device creation
under /sys/devices/platform/soc-audio and share the use of
codec_reg_show and dapm_widget_show functions.

Probably something like below can be used as a common struct device
holder and carry either rtd or dapm to sysfs functions. E.g. in
dapm_widget_show we would be accessing the dapm and rtd in
pmdown_time_show.

struct foo {
	struct device dev;
	union {
		struct snd_soc_pcm_runtime *rtd;
		struct snd_soc_dapm_context *dapm;
	} context;
};

> > Why do aux devices need to be coupled with CODECs here ? 
> 
> The way we've got things set up currently the CODEC isn't really just a
> CODEC, it's got a bunch of other random services which are more generic
> chip services associated with it like the register cache and the bias
> management.  There's so much overlap between the devices that I'm not
> sure that it's really worth splitting the types up too much at the root
> level (CODEC is pretty much a superclass that has everything on it but
> DMA).
> 
> Given how widely used snd_soc_codec is I'm not sure it's worth fixing
> the naming thing here, especially since 99% of the time the device is
> actually a CODEC.

And codecs can be used as an amplifer only by not connecting the DAI.
Like if codec chip is cheaper or better available than amplifier or if
multicodec package saves board space compared to separate codec and
amplifier chips.

If I counted correctly we have currently only three amplifier drivers:
tpa6130a2.c, wm2000.c and wm9090.c so separation doesn't sound worth of
trouble at this point as the core serves well those cases also.


-- 
Jarkko

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Peter Ujfalusi]

Hi,

On Friday 26 November 2010 09:25:24 ext Jarkko Nikula wrote:
> And codecs can be used as an amplifer only by not connecting the DAI.
> Like if codec chip is cheaper or better available than amplifier or if
> multicodec package saves board space compared to separate codec and
> amplifier chips.

We could as well see these things as components on the audio path.
A standard CODEC would have DAI + DAPM (analog/digital routings, amps) + gain 
controls.
In case of an external amplifier we only have the DAPM part + gain control.

If a component has DAI, than the normal PCM operations would apply to them, if 
the component does not have DAI, than those are not applicable.
I'm not sure, but I think we do have some level of separation of DAPM and PCM 
operations, right? So why not to utilize, and extend that route?
All component drivers would use the same registration, core would knows which 
component has DAI, and which does not. Machine driver could specify a list of 
components, provides the DAPM connection between the components.

If DAPM core knows which widget belongs to which component, than I see no real 
problem with this method. The DAPM would work just fine. The PCM operatins would 
only apply to component with DAI.

> If I counted correctly we have currently only three amplifier drivers:
> tpa6130a2.c, wm2000.c and wm9090.c so separation doesn't sound worth of
> trouble at this point as the core serves well those cases also.

One more: max9877, if I recall correctly that was the first amp driver?

-- 
Péter

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Jarkko Nikula]

On Fri, 26 Nov 2010 14:19:42 +0200
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> We could as well see these things as components on the audio path.
> A standard CODEC would have DAI + DAPM (analog/digital routings, amps) + gain 
> controls.
> In case of an external amplifier we only have the DAPM part + gain control.
> 
> If a component has DAI, than the normal PCM operations would apply to them, if 
> the component does not have DAI, than those are not applicable.
> I'm not sure, but I think we do have some level of separation of DAPM and PCM 
> operations, right? So why not to utilize, and extend that route?
> All component drivers would use the same registration, core would knows which 
> component has DAI, and which does not. Machine driver could specify a list of 
> components, provides the DAPM connection between the components.
> 
> If DAPM core knows which widget belongs to which component, than I see no real 
> problem with this method. The DAPM would work just fine. The PCM operatins would 
> only apply to component with DAI.
> 
Actually these are already well separated in current ASoC. Like my RFC
patch didn't not need touch soc-dapm.c at all and efectively other
changes were around codec probing/removal and by not registering the
PCM.

> > If I counted correctly we have currently only three amplifier drivers:
> > tpa6130a2.c, wm2000.c and wm9090.c so separation doesn't sound worth of
> > trouble at this point as the core serves well those cases also.
> 
> One more: max9877, if I recall correctly that was the first amp driver?
> 
Yeah, true. Looks like wm9090.c doesn't need any conversion after we
are able to register dailess codecs but those three can be then
converted to use standard probing mechanism and let them register itself
own widgets and controls. I.e. machine drivers don't need to call those
tailored _add_controls functions.


-- 
Jarkko

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Mark Brown]

On Fri, Nov 26, 2010 at 03:35:48PM +0200, Jarkko Nikula wrote:
> Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> > If DAPM core knows which widget belongs to which component, than I see no real 
> > problem with this method. The DAPM would work just fine. The PCM operatins would 
> > only apply to component with DAI.

> Actually these are already well separated in current ASoC. Like my RFC
> patch didn't not need touch soc-dapm.c at all and efectively other
> changes were around codec probing/removal and by not registering the
> PCM.

That's pretty much where I'm coming from - we already have most of the
infrastructure, we just need to get the devices into the system but once
we do that we should be able to cope with everything already.

> > > If I counted correctly we have currently only three amplifier drivers:
> > > tpa6130a2.c, wm2000.c and wm9090.c so separation doesn't sound worth of
> > > trouble at this point as the core serves well those cases also.

> > One more: max9877, if I recall correctly that was the first amp driver?

> Yeah, true. Looks like wm9090.c doesn't need any conversion after we
> are able to register dailess codecs but those three can be then
> converted to use standard probing mechanism and let them register itself
> own widgets and controls. I.e. machine drivers don't need to call those
> tailored _add_controls functions.

Yup, WM9090 is a DAIless CODEC already as it's using the register cache.
The systems where it's typically deployed have a stub CODEC normally so
this happens to work as-is, it wouldn't work as-is otherwise.

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Peter Ujfalusi]

On Friday 26 November 2010 15:41:52 ext Mark Brown wrote:
> On Fri, Nov 26, 2010 at 03:35:48PM +0200, Jarkko Nikula wrote:
> > Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:
> > > If DAPM core knows which widget belongs to which component, than I see
> > > no real problem with this method. The DAPM would work just fine. The
> > > PCM operatins would only apply to component with DAI.
> > 
> > Actually these are already well separated in current ASoC. Like my RFC
> > patch didn't not need touch soc-dapm.c at all and efectively other
> > changes were around codec probing/removal and by not registering the
> > PCM.
> 
> That's pretty much where I'm coming from - we already have most of the
> infrastructure, we just need to get the devices into the system but once
> we do that we should be able to cope with everything already.

So, if let's say I rewrite the tpa6130a2 driver as DAIless CODEC driver, and 
connect to a system, where we already have a proper codec. What should I expect?
Can this work with 'reasonable' ;) amount of work (or non) on the core side?

-- 
Péter

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Mark Brown]

On Fri, Nov 26, 2010 at 03:55:34PM +0200, Peter Ujfalusi wrote:

> So, if let's say I rewrite the tpa6130a2 driver as DAIless CODEC driver, and 
> connect to a system, where we already have a proper codec. What should I expect?
> Can this work with 'reasonable' ;) amount of work (or non) on the core side?

Jarkko's patch should make it work, we just need to figure out if the
pcm_runtime thing (I'm living in the hope that he's tested it!).

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Jarkko Nikula]

On Fri, 26 Nov 2010 15:55:34 +0200
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> > That's pretty much where I'm coming from - we already have most of the
> > infrastructure, we just need to get the devices into the system but once
> > we do that we should be able to cope with everything already.
> 
> So, if let's say I rewrite the tpa6130a2 driver as DAIless CODEC driver, and 
> connect to a system, where we already have a proper codec. What should I expect?
> Can this work with 'reasonable' ;) amount of work (or non) on the core side?
> 
I'm happy to hear does it work or not :-)

Basically you would need the cross-device set [1] and this RFC. The
corss-device set has some trivial merge issues I think now but I can
send you an updated version if you like. We don't want to merge
cross-device set before a problem with DAPMless codecs is solved.

Then in your machine driver you need to have this struct
snd_soc_aux_dev for tpa6130a2 and machine init for it would just add a
map that connects output of DAI codec into input of tpa6130a2 and call
snd_soc_dapm_sync as usual.


-- 
Jarkko
1.
http://mailman.alsa-project.org/pipermail/alsa-devel/2010-November/033583.html

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Mark Brown]

On Thu, Nov 25, 2010 at 05:47:38PM +0200, Jarkko Nikula wrote:

> I'm not entirely sure of reusing struct snd_soc_pcm_runtime but having some
> common struct on top of it for registering the sysfs nodes and passing to
> snd_soc_dapm_sys_add didn't sound clear either.
> Anyway suspend/resume is working with this version and doesn't need any other
> modifications to soc_suspend/soc_resume than traversing through the registered
> codecs instead of doing bunch of rtd->dailess etc hacks there.

So, the reason we're doing this is that the sysfs nodes for DAPM and
regular CODEC stuff are using the device node in the PCM runtime data to
look up the data structures they need to do things.  The CODEC probably
isn't an ideal place for the PCM runtime data anyway, it feels like a
card thing (as it spans multiple devices within the card except in the
most baroque designs).

Since we appear to be abusing sysfs here anyway (we've got an empty
release function) I think we should just look into fixing this properly,
probably by having a device directly on the CODEC and using that for the
CODEC-specific sysfs files.

That said, this shouldn't affect the external interfaces here - it
should only have any material impact on the core - so probably shouldn't
be a blocker for adding this feature.  Liam, does this analysis all
sound sane to you?

Re: [RFC don't apply] ASoC: Add support for optional auxiliary dailess codecs

[Mark Brown]

On Sun, Nov 28, 2010 at 11:50:04AM +0000, Mark Brown wrote:

> That said, this shouldn't affect the external interfaces here - it
> should only have any material impact on the core - so probably shouldn't
> be a blocker for adding this feature.  Liam, does this analysis all
> sound sane to you?

Just discussed offline with Liam and he's happy with this approach so
I've now applied the patch - thanks!  I'll look into faffing about with
sysfs myself.