All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
@ 2014-03-08 18:16 Lars-Peter Clausen
  2014-03-08 18:16 ` [PATCH 2/3] ASoC: rx51: Convert to table based control and DAPM setup Lars-Peter Clausen
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Lars-Peter Clausen @ 2014-03-08 18:16 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Peter Ujfalusi, alsa-devel, Lars-Peter Clausen,
	Grazvydas Ignotas, Jarkko Nikula

When using table based DAPM setup there is no need to register DAPM elements for
different sub-components separately. The widgets will be registered before the
first sub-component is initialized, the routes are only added after the last
sub-component has been initialized, meaning everything will be available when it
is needed.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/omap/omap-abe-twl6040.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/sound/soc/omap/omap-abe-twl6040.c b/sound/soc/omap/omap-abe-twl6040.c
index ebb1390..5011bfa 100644
--- a/sound/soc/omap/omap-abe-twl6040.c
+++ b/sound/soc/omap/omap-abe-twl6040.c
@@ -163,6 +163,10 @@ static const struct snd_soc_dapm_route audio_map[] = {
 
 	{"AFML", NULL, "Line In"},
 	{"AFMR", NULL, "Line In"},
+
+	/* DMIC routing */
+	{"DMic", NULL, "Digital Mic"},
+	{"Digital Mic", NULL, "Digital Mic1 Bias"},
 };
 
 static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd)
@@ -196,20 +200,6 @@ static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd)
 	return ret;
 }
 
-static const struct snd_soc_dapm_route dmic_audio_map[] = {
-	{"DMic", NULL, "Digital Mic"},
-	{"Digital Mic", NULL, "Digital Mic1 Bias"},
-};
-
-static int omap_abe_dmic_init(struct snd_soc_pcm_runtime *rtd)
-{
-	struct snd_soc_codec *codec = rtd->codec;
-	struct snd_soc_dapm_context *dapm = &codec->dapm;
-
-	return snd_soc_dapm_add_routes(dapm, dmic_audio_map,
-				ARRAY_SIZE(dmic_audio_map));
-}
-
 /* Digital audio interface glue - connects codec <--> CPU */
 static struct snd_soc_dai_link abe_twl6040_dai_links[] = {
 	{
@@ -229,7 +219,6 @@ static struct snd_soc_dai_link abe_twl6040_dai_links[] = {
 		.codec_dai_name = "dmic-hifi",
 		.platform_name = "omap-pcm-audio",
 		.codec_name = "dmic-codec",
-		.init = omap_abe_dmic_init,
 		.ops = &omap_abe_dmic_ops,
 	},
 };
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] ASoC: rx51: Convert to table based control and DAPM setup
  2014-03-08 18:16 [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly Lars-Peter Clausen
@ 2014-03-08 18:16 ` Lars-Peter Clausen
  2014-03-08 18:16 ` [PATCH 3/3] ASoC: omap3pandora: Convert to table based " Lars-Peter Clausen
  2014-03-10  8:13 ` [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly Peter Ujfalusi
  2 siblings, 0 replies; 12+ messages in thread
From: Lars-Peter Clausen @ 2014-03-08 18:16 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Peter Ujfalusi, alsa-devel, Lars-Peter Clausen,
	Grazvydas Ignotas, Jarkko Nikula

Use table based setup to register the controls and DAPM widgets and routes. This
on one hand makes the code a bit shorter and cleaner and on the other hand the
board level DAPM elements get registered in the card's DAPM context rather than
in the CODEC's DAPM context.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/omap/rx51.c | 47 +++++++----------------------------------------
 1 file changed, 7 insertions(+), 40 deletions(-)

diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c
index 611179c..a15ffda 100644
--- a/sound/soc/omap/rx51.c
+++ b/sound/soc/omap/rx51.c
@@ -233,9 +233,6 @@ static const struct snd_soc_dapm_widget aic34_dapm_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone Jack", rx51_hp_event),
 	SND_SOC_DAPM_MIC("HS Mic", NULL),
 	SND_SOC_DAPM_LINE("FM Transmitter", NULL),
-};
-
-static const struct snd_soc_dapm_widget aic34_dapm_widgetsb[] = {
 	SND_SOC_DAPM_SPK("Earphone", NULL),
 };
 
@@ -249,9 +246,7 @@ static const struct snd_soc_dapm_route audio_map[] = {
 
 	{"DMic Rate 64", NULL, "Mic Bias"},
 	{"Mic Bias", NULL, "DMic"},
-};
 
-static const struct snd_soc_dapm_route audio_mapb[] = {
 	{"b LINE2R", NULL, "MONO_LOUT"},
 	{"Earphone", NULL, "b HPLOUT"},
 
@@ -277,9 +272,6 @@ static const struct snd_kcontrol_new aic34_rx51_controls[] = {
 	SOC_ENUM_EXT("Jack Function", rx51_enum[2],
 		     rx51_get_jack, rx51_set_jack),
 	SOC_DAPM_PIN_SWITCH("FM Transmitter"),
-};
-
-static const struct snd_kcontrol_new aic34_rx51_controlsb[] = {
 	SOC_DAPM_PIN_SWITCH("Earphone"),
 };
 
@@ -294,19 +286,6 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_nc_pin(dapm, "MIC3R");
 	snd_soc_dapm_nc_pin(dapm, "LINE1R");
 
-	/* Add RX-51 specific controls */
-	err = snd_soc_add_card_controls(rtd->card, aic34_rx51_controls,
-				   ARRAY_SIZE(aic34_rx51_controls));
-	if (err < 0)
-		return err;
-
-	/* Add RX-51 specific widgets */
-	snd_soc_dapm_new_controls(dapm, aic34_dapm_widgets,
-				  ARRAY_SIZE(aic34_dapm_widgets));
-
-	/* Set up RX-51 specific audio path audio_map */
-	snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map));
-
 	err = tpa6130a2_add_controls(codec);
 	if (err < 0)
 		return err;
@@ -329,24 +308,6 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
 	return err;
 }
 
-static int rx51_aic34b_init(struct snd_soc_dapm_context *dapm)
-{
-	int err;
-
-	err = snd_soc_add_card_controls(dapm->card, aic34_rx51_controlsb,
-				   ARRAY_SIZE(aic34_rx51_controlsb));
-	if (err < 0)
-		return err;
-
-	err = snd_soc_dapm_new_controls(dapm, aic34_dapm_widgetsb,
-					ARRAY_SIZE(aic34_dapm_widgetsb));
-	if (err < 0)
-		return 0;
-
-	return snd_soc_dapm_add_routes(dapm, audio_mapb,
-				       ARRAY_SIZE(audio_mapb));
-}
-
 /* Digital audio interface glue - connects codec <--> CPU */
 static struct snd_soc_dai_link rx51_dai[] = {
 	{
@@ -367,7 +328,6 @@ static struct snd_soc_aux_dev rx51_aux_dev[] = {
 	{
 		.name = "TLV320AIC34b",
 		.codec_name = "tlv320aic3x-codec.2-0019",
-		.init = rx51_aic34b_init,
 	},
 };
 
@@ -388,6 +348,13 @@ static struct snd_soc_card rx51_sound_card = {
 	.num_aux_devs = ARRAY_SIZE(rx51_aux_dev),
 	.codec_conf = rx51_codec_conf,
 	.num_configs = ARRAY_SIZE(rx51_codec_conf),
+
+	.controls = aic34_rx51_controls,
+	.num_controls = ARRAY_SIZE(aic34_rx51_controls),
+	.dapm_widgets = aic34_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(aic34_dapm_widgets),
+	.dapm_routes = audio_map,
+	.num_dapm_routes = ARRAY_SIZE(audio_map),
 };
 
 static struct platform_device *rx51_snd_device;
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] ASoC: omap3pandora: Convert to table based DAPM setup
  2014-03-08 18:16 [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly Lars-Peter Clausen
  2014-03-08 18:16 ` [PATCH 2/3] ASoC: rx51: Convert to table based control and DAPM setup Lars-Peter Clausen
@ 2014-03-08 18:16 ` Lars-Peter Clausen
  2014-03-10  8:13 ` [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly Peter Ujfalusi
  2 siblings, 0 replies; 12+ messages in thread
From: Lars-Peter Clausen @ 2014-03-08 18:16 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Peter Ujfalusi, alsa-devel, Lars-Peter Clausen,
	Grazvydas Ignotas, Jarkko Nikula

Use table based setup to register the controls and DAPM widgets and routes. This
on one hand makes the code a bit shorter and cleaner and on the other hand the
board level DAPM elements get registered in the card's DAPM context rather than
in the CODEC's DAPM context.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/omap/omap3pandora.c | 31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

diff --git a/sound/soc/omap/omap3pandora.c b/sound/soc/omap/omap3pandora.c
index cf604a2..02181bb 100644
--- a/sound/soc/omap/omap3pandora.c
+++ b/sound/soc/omap/omap3pandora.c
@@ -121,7 +121,7 @@ static int omap3pandora_hp_event(struct snd_soc_dapm_widget *w,
  *  |A| <~~clk~~+
  *  |P| <--- TWL4030 <--------- Line In and MICs
  */
-static const struct snd_soc_dapm_widget omap3pandora_out_dapm_widgets[] = {
+static const struct snd_soc_dapm_widget omap3pandora_dapm_widgets[] = {
 	SND_SOC_DAPM_DAC_E("PCM DAC", "HiFi Playback", SND_SOC_NOPM,
 			   0, 0, omap3pandora_dac_event,
 			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
@@ -130,22 +130,18 @@ static const struct snd_soc_dapm_widget omap3pandora_out_dapm_widgets[] = {
 			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
 	SND_SOC_DAPM_HP("Headphone Jack", NULL),
 	SND_SOC_DAPM_LINE("Line Out", NULL),
-};
 
-static const struct snd_soc_dapm_widget omap3pandora_in_dapm_widgets[] = {
 	SND_SOC_DAPM_MIC("Mic (internal)", NULL),
 	SND_SOC_DAPM_MIC("Mic (external)", NULL),
 	SND_SOC_DAPM_LINE("Line In", NULL),
 };
 
-static const struct snd_soc_dapm_route omap3pandora_out_map[] = {
+static const struct snd_soc_dapm_route omap3pandora_map[] = {
 	{"PCM DAC", NULL, "APLL Enable"},
 	{"Headphone Amplifier", NULL, "PCM DAC"},
 	{"Line Out", NULL, "PCM DAC"},
 	{"Headphone Jack", NULL, "Headphone Amplifier"},
-};
 
-static const struct snd_soc_dapm_route omap3pandora_in_map[] = {
 	{"AUXL", NULL, "Line In"},
 	{"AUXR", NULL, "Line In"},
 
@@ -160,7 +156,6 @@ static int omap3pandora_out_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_codec *codec = rtd->codec;
 	struct snd_soc_dapm_context *dapm = &codec->dapm;
-	int ret;
 
 	/* All TWL4030 output pins are floating */
 	snd_soc_dapm_nc_pin(dapm, "EARPIECE");
@@ -174,20 +169,13 @@ static int omap3pandora_out_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_nc_pin(dapm, "HFR");
 	snd_soc_dapm_nc_pin(dapm, "VIBRA");
 
-	ret = snd_soc_dapm_new_controls(dapm, omap3pandora_out_dapm_widgets,
-				ARRAY_SIZE(omap3pandora_out_dapm_widgets));
-	if (ret < 0)
-		return ret;
-
-	return snd_soc_dapm_add_routes(dapm, omap3pandora_out_map,
-		ARRAY_SIZE(omap3pandora_out_map));
+	return 0;
 }
 
 static int omap3pandora_in_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_codec *codec = rtd->codec;
 	struct snd_soc_dapm_context *dapm = &codec->dapm;
-	int ret;
 
 	/* Not comnnected */
 	snd_soc_dapm_nc_pin(dapm, "HSMIC");
@@ -195,13 +183,7 @@ static int omap3pandora_in_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_nc_pin(dapm, "DIGIMIC0");
 	snd_soc_dapm_nc_pin(dapm, "DIGIMIC1");
 
-	ret = snd_soc_dapm_new_controls(dapm, omap3pandora_in_dapm_widgets,
-				ARRAY_SIZE(omap3pandora_in_dapm_widgets));
-	if (ret < 0)
-		return ret;
-
-	return snd_soc_dapm_add_routes(dapm, omap3pandora_in_map,
-		ARRAY_SIZE(omap3pandora_in_map));
+	return 0;
 }
 
 static struct snd_soc_ops omap3pandora_ops = {
@@ -241,6 +223,11 @@ static struct snd_soc_card snd_soc_card_omap3pandora = {
 	.owner = THIS_MODULE,
 	.dai_link = omap3pandora_dai,
 	.num_links = ARRAY_SIZE(omap3pandora_dai),
+
+	.dapm_widgets = omap3pandora_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(omap3pandora_dapm_widgets),
+	.dapm_routes = omap3pandora_map,
+	.num_dapm_routes = ARRAY_SIZE(omap3pandora_map),
 };
 
 static struct platform_device *omap3pandora_snd_device;
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-08 18:16 [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly Lars-Peter Clausen
  2014-03-08 18:16 ` [PATCH 2/3] ASoC: rx51: Convert to table based control and DAPM setup Lars-Peter Clausen
  2014-03-08 18:16 ` [PATCH 3/3] ASoC: omap3pandora: Convert to table based " Lars-Peter Clausen
@ 2014-03-10  8:13 ` Peter Ujfalusi
  2014-03-10  8:18   ` Lars-Peter Clausen
  2 siblings, 1 reply; 12+ messages in thread
From: Peter Ujfalusi @ 2014-03-10  8:13 UTC (permalink / raw)
  To: Lars-Peter Clausen, Mark Brown, Liam Girdwood
  Cc: alsa-devel, Grazvydas Ignotas, Jarkko Nikula

Hi,

On 03/08/2014 08:16 PM, Lars-Peter Clausen wrote:
> When using table based DAPM setup there is no need to register DAPM elements for
> different sub-components separately. The widgets will be registered before the
> first sub-component is initialized, the routes are only added after the last
> sub-component has been initialized, meaning everything will be available when it
> is needed.

The reason why we add the DMIC routes in the way we do is that not all boards
have DMIC in use. PandaBoards does not have DMIC while SDP/Blaze have. On
PandaBoard we do not register the dmic dai link so the widgets are not going
to be added and also the dmic DAI and codec will be not loaded on PandaBoards.
I think this will cause some warning because of missing "DMic" widget?

-- 
Péter

> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  sound/soc/omap/omap-abe-twl6040.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/sound/soc/omap/omap-abe-twl6040.c b/sound/soc/omap/omap-abe-twl6040.c
> index ebb1390..5011bfa 100644
> --- a/sound/soc/omap/omap-abe-twl6040.c
> +++ b/sound/soc/omap/omap-abe-twl6040.c
> @@ -163,6 +163,10 @@ static const struct snd_soc_dapm_route audio_map[] = {
>  
>  	{"AFML", NULL, "Line In"},
>  	{"AFMR", NULL, "Line In"},
> +
> +	/* DMIC routing */
> +	{"DMic", NULL, "Digital Mic"},
> +	{"Digital Mic", NULL, "Digital Mic1 Bias"},
>  };
>  
>  static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd)
> @@ -196,20 +200,6 @@ static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd)
>  	return ret;
>  }
>  
> -static const struct snd_soc_dapm_route dmic_audio_map[] = {
> -	{"DMic", NULL, "Digital Mic"},
> -	{"Digital Mic", NULL, "Digital Mic1 Bias"},
> -};
> -
> -static int omap_abe_dmic_init(struct snd_soc_pcm_runtime *rtd)
> -{
> -	struct snd_soc_codec *codec = rtd->codec;
> -	struct snd_soc_dapm_context *dapm = &codec->dapm;
> -
> -	return snd_soc_dapm_add_routes(dapm, dmic_audio_map,
> -				ARRAY_SIZE(dmic_audio_map));
> -}
> -
>  /* Digital audio interface glue - connects codec <--> CPU */
>  static struct snd_soc_dai_link abe_twl6040_dai_links[] = {
>  	{
> @@ -229,7 +219,6 @@ static struct snd_soc_dai_link abe_twl6040_dai_links[] = {
>  		.codec_dai_name = "dmic-hifi",
>  		.platform_name = "omap-pcm-audio",
>  		.codec_name = "dmic-codec",
> -		.init = omap_abe_dmic_init,
>  		.ops = &omap_abe_dmic_ops,
>  	},
>  };
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-10  8:13 ` [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly Peter Ujfalusi
@ 2014-03-10  8:18   ` Lars-Peter Clausen
  2014-03-10  9:12     ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Lars-Peter Clausen @ 2014-03-10  8:18 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: alsa-devel, Mark Brown, Liam Girdwood, Jarkko Nikula, Grazvydas Ignotas

On 03/10/2014 09:13 AM, Peter Ujfalusi wrote:
> Hi,
>
> On 03/08/2014 08:16 PM, Lars-Peter Clausen wrote:
>> When using table based DAPM setup there is no need to register DAPM elements for
>> different sub-components separately. The widgets will be registered before the
>> first sub-component is initialized, the routes are only added after the last
>> sub-component has been initialized, meaning everything will be available when it
>> is needed.
>
> The reason why we add the DMIC routes in the way we do is that not all boards
> have DMIC in use. PandaBoards does not have DMIC while SDP/Blaze have. On
> PandaBoard we do not register the dmic dai link so the widgets are not going
> to be added and also the dmic DAI and codec will be not loaded on PandaBoards.
> I think this will cause some warning because of missing "DMic" widget?
>

Hm, ok, missed that part. Makes sense. I'll respin the patch to just use the 
card's DAPM context when registering the DMIC DAPM routes.

- Lars

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-10  8:18   ` Lars-Peter Clausen
@ 2014-03-10  9:12     ` Mark Brown
  2014-03-10  9:24       ` Lars-Peter Clausen
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2014-03-10  9:12 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Peter Ujfalusi, alsa-devel, Liam Girdwood, Jarkko Nikula,
	Grazvydas Ignotas


[-- Attachment #1.1: Type: text/plain, Size: 419 bytes --]

On Mon, Mar 10, 2014 at 09:18:50AM +0100, Lars-Peter Clausen wrote:

> Hm, ok, missed that part. Makes sense. I'll respin the patch to just
> use the card's DAPM context when registering the DMIC DAPM routes.

In general anything keying this stuff off DT is going to have that sort
of thing going on - most if not all of the things that are registering
in several chunks are doing so because some of it is conditional.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-10  9:12     ` Mark Brown
@ 2014-03-10  9:24       ` Lars-Peter Clausen
  2014-03-10 10:10         ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Lars-Peter Clausen @ 2014-03-10  9:24 UTC (permalink / raw)
  To: Mark Brown
  Cc: Peter Ujfalusi, alsa-devel, Liam Girdwood, Jarkko Nikula,
	Grazvydas Ignotas

On 03/10/2014 10:12 AM, Mark Brown wrote:
> On Mon, Mar 10, 2014 at 09:18:50AM +0100, Lars-Peter Clausen wrote:
>
>> Hm, ok, missed that part. Makes sense. I'll respin the patch to just
>> use the card's DAPM context when registering the DMIC DAPM routes.
>
> In general anything keying this stuff off DT is going to have that sort
> of thing going on - most if not all of the things that are registering
> in several chunks are doing so because some of it is conditional.
>

Before we had the support for card table based setup you'd had to register 
them in chunks, because the components became available one after another 
and you couldn't register DAPM elements for one component in the callback of 
another one. When using the table based setup the widgets are registered 
before all components are registered and the routes are registered after all 
the components have been registered.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-10  9:24       ` Lars-Peter Clausen
@ 2014-03-10 10:10         ` Mark Brown
  2014-03-10 10:27           ` Lars-Peter Clausen
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2014-03-10 10:10 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Peter Ujfalusi, alsa-devel, Liam Girdwood, Jarkko Nikula,
	Grazvydas Ignotas


[-- Attachment #1.1: Type: text/plain, Size: 1144 bytes --]

On Mon, Mar 10, 2014 at 10:24:49AM +0100, Lars-Peter Clausen wrote:
> On 03/10/2014 10:12 AM, Mark Brown wrote:

> >In general anything keying this stuff off DT is going to have that sort
> >of thing going on - most if not all of the things that are registering
> >in several chunks are doing so because some of it is conditional.

> Before we had the support for card table based setup you'd had to
> register them in chunks, because the components became available one
> after another and you couldn't register DAPM elements for one
> component in the callback of another one. When using the table based
> setup the widgets are registered before all components are
> registered and the routes are registered after all the components
> have been registered.

No, that's not the case at all - what makes you think that's the case?
All table based setup did was move things from code to data, I'm not
sure what you mean by registering DAPM elements from one component in
the callback for another.  The only thing that should be registering
things for other components is the card and the general idea was to
register everything in late_probe().

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-10 10:10         ` Mark Brown
@ 2014-03-10 10:27           ` Lars-Peter Clausen
  2014-03-10 11:05             ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Lars-Peter Clausen @ 2014-03-10 10:27 UTC (permalink / raw)
  To: Mark Brown
  Cc: Peter Ujfalusi, alsa-devel, Liam Girdwood, Jarkko Nikula,
	Grazvydas Ignotas

On 03/10/2014 11:10 AM, Mark Brown wrote:
> On Mon, Mar 10, 2014 at 10:24:49AM +0100, Lars-Peter Clausen wrote:
>> On 03/10/2014 10:12 AM, Mark Brown wrote:
>
>>> In general anything keying this stuff off DT is going to have that sort
>>> of thing going on - most if not all of the things that are registering
>>> in several chunks are doing so because some of it is conditional.
>
>> Before we had the support for card table based setup you'd had to
>> register them in chunks, because the components became available one
>> after another and you couldn't register DAPM elements for one
>> component in the callback of another one. When using the table based
>> setup the widgets are registered before all components are
>> registered and the routes are registered after all the components
>> have been registered.
>
> No, that's not the case at all - what makes you think that's the case?

The code ;)

> All table based setup did was move things from code to data, I'm not
> sure what you mean by registering DAPM elements from one component in
> the callback for another.

I meant the card level DAPM elements that are related to a certain 
component. Not the DAPM elements of the component itself.

> The only thing that should be registering
> things for other components is the card and the general idea was to
> register everything in late_probe().
>

What most machine drivers did before card table based setup is to have a 
per-component callback in which they did register the card DAPM elements 
associated with that component. E.g. routes from the CODEC output to a 
speaker, etc. You couldn't register the card level DAPM elements for one 
component in the init callback component of another one since the routes 
depend on the widgets being registered first. So those board drivers kept 
separate routes and widget tables for separate components. With table based 
setup for the card the routes are registered after all components have been 
registered, which means you can register all the routes via one table since 
all the dependencies are ready. Same is true if you use the card's 
late_probe callback to register the DAPM routes and widgets.

- Lars

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-10 10:27           ` Lars-Peter Clausen
@ 2014-03-10 11:05             ` Mark Brown
  2014-03-10 11:29               ` Lars-Peter Clausen
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2014-03-10 11:05 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Peter Ujfalusi, alsa-devel, Liam Girdwood, Jarkko Nikula,
	Grazvydas Ignotas


[-- Attachment #1.1: Type: text/plain, Size: 1819 bytes --]

On Mon, Mar 10, 2014 at 11:27:39AM +0100, Lars-Peter Clausen wrote:
> On 03/10/2014 11:10 AM, Mark Brown wrote:

> >The only thing that should be registering
> >things for other components is the card and the general idea was to
> >register everything in late_probe().

> What most machine drivers did before card table based setup is to
> have a per-component callback in which they did register the card
> DAPM elements associated with that component. E.g. routes from the
> CODEC output to a speaker, etc. You couldn't register the card level
> DAPM elements for one component in the init callback component of
> another one since the routes depend on the widgets being registered
> first. So those board drivers kept separate routes and widget tables
> for separate components. With table based setup for the card the

I'm sorry but this just isn't what was happening at all.  Remember that
most of the code that you're looking at only ever had one component so
it really never made any difference which callback people happened to
pick so long as it was one of the ones that ran after the CODEC was up.

The only driver that I can think of which did anything like what you
describe was smdk-wm8580 and that was still only using one component,
it's just that Jassi preferred to split the input and output paths since
the DAIs were separate on the CODEC and he felt that was clearer.  It
didn't make any practical difference, it certainly wasn't due to startup
ordering.

> routes are registered after all components have been registered,
> which means you can register all the routes via one table since all
> the dependencies are ready. Same is true if you use the card's
> late_probe callback to register the DAPM routes and widgets.

Right, anything that actually cared would be using the late_probe()
callback.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-10 11:05             ` Mark Brown
@ 2014-03-10 11:29               ` Lars-Peter Clausen
  2014-03-10 12:09                 ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Lars-Peter Clausen @ 2014-03-10 11:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: Peter Ujfalusi, alsa-devel, Liam Girdwood, Jarkko Nikula,
	Grazvydas Ignotas

On 03/10/2014 12:05 PM, Mark Brown wrote:
> On Mon, Mar 10, 2014 at 11:27:39AM +0100, Lars-Peter Clausen wrote:
>> On 03/10/2014 11:10 AM, Mark Brown wrote:
>
>>> The only thing that should be registering
>>> things for other components is the card and the general idea was to
>>> register everything in late_probe().
>
>> What most machine drivers did before card table based setup is to
>> have a per-component callback in which they did register the card
>> DAPM elements associated with that component. E.g. routes from the
>> CODEC output to a speaker, etc. You couldn't register the card level
>> DAPM elements for one component in the init callback component of
>> another one since the routes depend on the widgets being registered
>> first. So those board drivers kept separate routes and widget tables
>> for separate components. With table based setup for the card the
>
> I'm sorry but this just isn't what was happening at all.  Remember that
> most of the code that you're looking at only ever had one component so
> it really never made any difference which callback people happened to
> pick so long as it was one of the ones that ran after the CODEC was up.
>
> The only driver that I can think of which did anything like what you
> describe was smdk-wm8580 and that was still only using one component,
> it's just that Jassi preferred to split the input and output paths since
> the DAIs were separate on the CODEC and he felt that was clearer.  It
> didn't make any practical difference, it certainly wasn't due to startup
> ordering.

Take a look at e.g. omap/rx51.c it's doing what I'm describing and I presume 
it does this for the reasons I described.

But it doesn't really matter. The important thing about this series is that 
card level DAPM elements and controls should be registered with the card not 
the CODEC and I think we can agree on that one.

- Lars

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly
  2014-03-10 11:29               ` Lars-Peter Clausen
@ 2014-03-10 12:09                 ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-03-10 12:09 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Peter Ujfalusi, alsa-devel, Liam Girdwood, Jarkko Nikula,
	Grazvydas Ignotas


[-- Attachment #1.1: Type: text/plain, Size: 1217 bytes --]

On Mon, Mar 10, 2014 at 12:29:33PM +0100, Lars-Peter Clausen wrote:
> On 03/10/2014 12:05 PM, Mark Brown wrote:

> >The only driver that I can think of which did anything like what you
> >describe was smdk-wm8580 and that was still only using one component,
> >it's just that Jassi preferred to split the input and output paths since
> >the DAIs were separate on the CODEC and he felt that was clearer.  It
> >didn't make any practical difference, it certainly wasn't due to startup
> >ordering.

> Take a look at e.g. omap/rx51.c it's doing what I'm describing and I
> presume it does this for the reasons I described.

Oh, rx51 is just generally fun - I'd be a bit surprised if it still
works.  It's actually the out of ASoC probe stuff that's causing
problems there, it was trying to do multi-component prior that being
supported.

> But it doesn't really matter. The important thing about this series
> is that card level DAPM elements and controls should be registered
> with the card not the CODEC and I think we can agree on that one.

Right, it's mostly just an alarm bell for review (I'd not looked at the
series yet, I tend to defer things until the people working on the
driver have had a chance to look).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-03-10 12:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-08 18:16 [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly Lars-Peter Clausen
2014-03-08 18:16 ` [PATCH 2/3] ASoC: rx51: Convert to table based control and DAPM setup Lars-Peter Clausen
2014-03-08 18:16 ` [PATCH 3/3] ASoC: omap3pandora: Convert to table based " Lars-Peter Clausen
2014-03-10  8:13 ` [PATCH 1/3] ASoC: omap-abe-twl6040: No need to register DMIC routes seperatly Peter Ujfalusi
2014-03-10  8:18   ` Lars-Peter Clausen
2014-03-10  9:12     ` Mark Brown
2014-03-10  9:24       ` Lars-Peter Clausen
2014-03-10 10:10         ` Mark Brown
2014-03-10 10:27           ` Lars-Peter Clausen
2014-03-10 11:05             ` Mark Brown
2014-03-10 11:29               ` Lars-Peter Clausen
2014-03-10 12:09                 ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.