[PATCH] ASoC: ams-delta: Let the core to register DAPM widgets/routes and controls

[PATCH] ASoC: ams-delta: Let the core to register DAPM widgets/routes and controls

[Peter Ujfalusi]

Pass the DAPM widgets/routes and static controls via the
snd_soc_card struct to core. In this way the machine driver
does not need to handle the DAPM widgets/routes.

Move the setup for initial pin constellation to late_probe
time.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/omap/ams-delta.c |   56 +++++++++++++++----------------------------
 1 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
index 10d3d41..2153a1c 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c
@@ -536,48 +536,23 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
 
 	/* Register optional line discipline for over the modem control */
 	ret = tty_register_ldisc(N_V253, &cx81801_ops);
-	if (ret) {
+	if (ret)
 		dev_warn(card->dev,
 				"Failed to register line discipline, "
 				"will continue without any controls.\n");
-		return 0;
-	}
-
-	/* Add board specific DAPM widgets and routes */
-	ret = snd_soc_dapm_new_controls(dapm, ams_delta_dapm_widgets,
-					ARRAY_SIZE(ams_delta_dapm_widgets));
-	if (ret) {
-		dev_warn(card->dev,
-				"Failed to register DAPM controls, "
-				"will continue without any.\n");
-		return 0;
-	}
 
-	ret = snd_soc_dapm_add_routes(dapm, ams_delta_audio_map,
-					ARRAY_SIZE(ams_delta_audio_map));
-	if (ret) {
-		dev_warn(card->dev,
-				"Failed to set up DAPM routes, "
-				"will continue with codec default map.\n");
-		return 0;
-	}
+	return 0;
+}
 
+static int ams_delta_late_probe(struct snd_soc_card *card)
+{
 	/* Set up initial pin constellation */
-	snd_soc_dapm_disable_pin(dapm, "Mouthpiece");
-	snd_soc_dapm_enable_pin(dapm, "Earpiece");
-	snd_soc_dapm_enable_pin(dapm, "Microphone");
-	snd_soc_dapm_disable_pin(dapm, "Speaker");
-	snd_soc_dapm_disable_pin(dapm, "AGCIN");
-	snd_soc_dapm_disable_pin(dapm, "AGCOUT");
-	snd_soc_dapm_new_widgets(dapm);
-
-	/* Add virtual switch */
-	ret = snd_soc_add_controls(codec, ams_delta_audio_controls,
-					ARRAY_SIZE(ams_delta_audio_controls));
-	if (ret)
-		dev_warn(card->dev,
-				"Failed to register audio mode control, "
-				"will continue without it.\n");
+	snd_soc_dapm_disable_pin(&card->dapm, "Mouthpiece");
+	snd_soc_dapm_enable_pin(&card->dapm, "Earpiece");
+	snd_soc_dapm_enable_pin(&card->dapm, "Microphone");
+	snd_soc_dapm_disable_pin(&card->dapm, "Speaker");
+	snd_soc_dapm_disable_pin(&card->dapm, "AGCIN");
+	snd_soc_dapm_disable_pin(&card->dapm, "AGCOUT");
 
 	return 0;
 }
@@ -600,6 +575,15 @@ static struct snd_soc_card ams_delta_audio_card = {
 	.dai_link = &ams_delta_dai_link,
 	.num_links = 1,
 	.set_bias_level = ams_delta_set_bias_level,
+
+	.controls = ams_delta_audio_controls,
+	.num_controls = ARRAY_SIZE(ams_delta_audio_controls),
+	.dapm_widgets = ams_delta_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(ams_delta_dapm_widgets),
+	.dapm_routes = ams_delta_audio_map,
+	.num_dapm_routes = ARRAY_SIZE(ams_delta_audio_map),
+
+	.late_probe = ams_delta_late_probe,
 };
 
 /* Module init/exit */
-- 
1.7.7

Re: [PATCH] ASoC: ams-delta: Let the core to register DAPM widgets/routes and controls

[Janusz Krzysztofik]

On Fri, 7 Oct 2011 at 11:03:19 Peter Ujfalusi wrote:
> Pass the DAPM widgets/routes and static controls via the
> snd_soc_card struct to core. In this way the machine driver
> does not need to handle the DAPM widgets/routes.

Hi Peter,
This change really simplifies things by making better use of platform 
provided functionality instead of reimplementing parts of it, but it 
breaks the intentional logic of conditionally enabling card widgets, 
controls and routes. OTOH, the negative condition here is 
tty_register_ldisc() failure, which seems hardly possible in a stable, 
correctly configured kernel. So, if you really think such conditional 
logic is no longer required in a driver which has proven its stability, 
please go on, I've tested your changes, they work for me.

Otherwise, I can try to re-factor the driver code, keeping that 
conditional logic, but using it for conditional runtime assignment of 
card->{widgets,routes,controls} instead of conditionally calling 
snd_soc_dapm_{new_controls,add_controls,add_routes}(). What do you 
think?

Thanks,
Janusz

> Move the setup for initial pin constellation to late_probe
> time.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  sound/soc/omap/ams-delta.c |   56 +++++++++++++++----------------------------
>  1 files changed, 20 insertions(+), 36 deletions(-)
> 
> diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
> index 10d3d41..2153a1c 100644
> --- a/sound/soc/omap/ams-delta.c
> +++ b/sound/soc/omap/ams-delta.c
> @@ -536,48 +536,23 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
>  
>  	/* Register optional line discipline for over the modem control */
>  	ret = tty_register_ldisc(N_V253, &cx81801_ops);
> -	if (ret) {
> +	if (ret)
>  		dev_warn(card->dev,
>  				"Failed to register line discipline, "
>  				"will continue without any controls.\n");
> -		return 0;
> -	}
> -
> -	/* Add board specific DAPM widgets and routes */
> -	ret = snd_soc_dapm_new_controls(dapm, ams_delta_dapm_widgets,
> -					ARRAY_SIZE(ams_delta_dapm_widgets));
> -	if (ret) {
> -		dev_warn(card->dev,
> -				"Failed to register DAPM controls, "
> -				"will continue without any.\n");
> -		return 0;
> -	}
>  
> -	ret = snd_soc_dapm_add_routes(dapm, ams_delta_audio_map,
> -					ARRAY_SIZE(ams_delta_audio_map));
> -	if (ret) {
> -		dev_warn(card->dev,
> -				"Failed to set up DAPM routes, "
> -				"will continue with codec default map.\n");
> -		return 0;
> -	}
> +	return 0;
> +}
>  
> +static int ams_delta_late_probe(struct snd_soc_card *card)
> +{
>  	/* Set up initial pin constellation */
> -	snd_soc_dapm_disable_pin(dapm, "Mouthpiece");
> -	snd_soc_dapm_enable_pin(dapm, "Earpiece");
> -	snd_soc_dapm_enable_pin(dapm, "Microphone");
> -	snd_soc_dapm_disable_pin(dapm, "Speaker");
> -	snd_soc_dapm_disable_pin(dapm, "AGCIN");
> -	snd_soc_dapm_disable_pin(dapm, "AGCOUT");
> -	snd_soc_dapm_new_widgets(dapm);
> -
> -	/* Add virtual switch */
> -	ret = snd_soc_add_controls(codec, ams_delta_audio_controls,
> -					ARRAY_SIZE(ams_delta_audio_controls));
> -	if (ret)
> -		dev_warn(card->dev,
> -				"Failed to register audio mode control, "
> -				"will continue without it.\n");
> +	snd_soc_dapm_disable_pin(&card->dapm, "Mouthpiece");
> +	snd_soc_dapm_enable_pin(&card->dapm, "Earpiece");
> +	snd_soc_dapm_enable_pin(&card->dapm, "Microphone");
> +	snd_soc_dapm_disable_pin(&card->dapm, "Speaker");
> +	snd_soc_dapm_disable_pin(&card->dapm, "AGCIN");
> +	snd_soc_dapm_disable_pin(&card->dapm, "AGCOUT");
>  
>  	return 0;
>  }
> @@ -600,6 +575,15 @@ static struct snd_soc_card ams_delta_audio_card = {
>  	.dai_link = &ams_delta_dai_link,
>  	.num_links = 1,
>  	.set_bias_level = ams_delta_set_bias_level,
> +
> +	.controls = ams_delta_audio_controls,
> +	.num_controls = ARRAY_SIZE(ams_delta_audio_controls),
> +	.dapm_widgets = ams_delta_dapm_widgets,
> +	.num_dapm_widgets = ARRAY_SIZE(ams_delta_dapm_widgets),
> +	.dapm_routes = ams_delta_audio_map,
> +	.num_dapm_routes = ARRAY_SIZE(ams_delta_audio_map),
> +
> +	.late_probe = ams_delta_late_probe,
>  };
>  
>  /* Module init/exit */
> 

Re: [PATCH] ASoC: ams-delta: Let the core to register DAPM widgets/routes and controls

[Péter Ujfalusi]

Hi Janusz,

On Saturday 08 October 2011 13:47:57 Janusz Krzysztofik wrote:
> On Fri, 7 Oct 2011 at 11:03:19 Peter Ujfalusi wrote:
> > Pass the DAPM widgets/routes and static controls via the
> > snd_soc_card struct to core. In this way the machine driver
> > does not need to handle the DAPM widgets/routes.
> 
> Hi Peter,
> This change really simplifies things by making better use of platform
> provided functionality instead of reimplementing parts of it, but it
> breaks the intentional logic of conditionally enabling card widgets,
> controls and routes. OTOH, the negative condition here is
> tty_register_ldisc() failure, which seems hardly possible in a stable,
> correctly configured kernel. So, if you really think such conditional
> logic is no longer required in a driver which has proven its stability,
> please go on, I've tested your changes, they work for me.

I was a bit hesitant to do this change in the first place.
We can keep the current flow in the ams-delta to be on the safe side.
I'm going to send out another series for the omap platforms soon which will 
keep the error handling logic of ams-delta intact.
 
> Otherwise, I can try to re-factor the driver code, keeping that
> conditional logic, but using it for conditional runtime assignment of
> card->{widgets,routes,controls} instead of conditionally calling
> snd_soc_dapm_{new_controls,add_controls,add_routes}(). What do you
> think?

I don't think it worth the trouble to do this.

--
Péter