diff mbox

[v2,2/5] ASoC: Revert "ASoC: dapm: Fix double prefix addition"

Message ID 1399472428-11034-2-git-send-email-lars@metafoo.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lars-Peter Clausen May 7, 2014, 2:20 p.m. UTC
This reverts commit bd23c5b661858446267f4d6b2fb4edd8eb710dda.

The patch claims that the patch is necessary to avoid double prefix addition
when calling snd_soc_dapm_add_route() from snd_soc_dapm_connect_dai_link_widgets().
But snd_soc_dapm_add_route() is called with the card's DAPM context, which does
not have a prefix, which means there is no prefix that could be added a second
time.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
I might be missing something. My best guess is that this was needed in some
vendor tree, but not in upstream.
---
 sound/soc/soc-dapm.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Lars-Peter Clausen May 12, 2014, 3 p.m. UTC | #1
On 05/07/2014 04:20 PM, Lars-Peter Clausen wrote:
> This reverts commit bd23c5b661858446267f4d6b2fb4edd8eb710dda.
>
> The patch claims that the patch is necessary to avoid double prefix addition
> when calling snd_soc_dapm_add_route() from snd_soc_dapm_connect_dai_link_widgets().
> But snd_soc_dapm_add_route() is called with the card's DAPM context, which does
> not have a prefix, which means there is no prefix that could be added a second
> time.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> I might be missing something. My best guess is that this was needed in some
> vendor tree, but not in upstream.

Hi Songhee, Arun,

Can you take a look at this? I think the reason why you had this patch is 
because this was needed to prevent double prefix addition in 
snd_soc_dapm_link_dai_widgets(). All other places where dapm_add_route() 
is/was used do not suffer the problem of double prefix addition. But 
snd_soc_dapm_link_dai_widgets() was changed in commit commit 2553628e 
("ASoC: dapm: Add snd_soc_dapm_add_path() helper function") to use 
dapm_add_path which does not have the problem since it doesn't try to add a 
prefix. v3.12 was the first kernel that had this commit and I think that you 
forward ported this change from a vendor tree that was based on an earlier 
version. So in summery I think the issue that you tried to fix this patch 
was already fixed by the time you submitted the patch.

Thanks,
- Lars


> ---
>   sound/soc/soc-dapm.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 142a738..08d869c 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -2381,8 +2381,7 @@ err:
>   }
>
>   static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm,
> -				  const struct snd_soc_dapm_route *route,
> -				  unsigned int is_prefixed)
> +				  const struct snd_soc_dapm_route *route)
>   {
>   	struct snd_soc_dapm_widget *wsource = NULL, *wsink = NULL, *w;
>   	struct snd_soc_dapm_widget *wtsource = NULL, *wtsink = NULL;
> @@ -2392,7 +2391,7 @@ static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm,
>   	char prefixed_source[80];
>   	int ret;
>
> -	if (dapm->codec && dapm->codec->name_prefix && !is_prefixed) {
> +	if (dapm->codec && dapm->codec->name_prefix) {
>   		snprintf(prefixed_sink, sizeof(prefixed_sink), "%s %s",
>   			 dapm->codec->name_prefix, route->sink);
>   		sink = prefixed_sink;
> @@ -2520,7 +2519,7 @@ int snd_soc_dapm_add_routes(struct snd_soc_dapm_context *dapm,
>
>   	mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_INIT);
>   	for (i = 0; i < num; i++) {
> -		r = snd_soc_dapm_add_route(dapm, route, false);
> +		r = snd_soc_dapm_add_route(dapm, route);
>   		if (r < 0) {
>   			dev_err(dapm->dev, "ASoC: Failed to add route %s -> %s -> %s\n",
>   				route->source,
> @@ -3430,7 +3429,7 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card)
>   				cpu_dai->codec->name, r.source,
>   				codec_dai->platform->name, r.sink);
>
> -			snd_soc_dapm_add_route(&card->dapm, &r, true);
> +			snd_soc_dapm_add_route(&card->dapm, &r);
>   		}
>
>   		/* connect BE DAI capture if widgets are valid */
> @@ -3441,7 +3440,7 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card)
>   				codec_dai->codec->name, r.source,
>   				cpu_dai->platform->name, r.sink);
>
> -			snd_soc_dapm_add_route(&card->dapm, &r, true);
> +			snd_soc_dapm_add_route(&card->dapm, &r);
>   		}
>
>   	}
>
Arun Shamanna Lakshmi May 12, 2014, 8:22 p.m. UTC | #2
> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Monday, May 12, 2014 8:00 AM
> To: Lars-Peter Clausen
> Cc: Mark Brown; Liam Girdwood; Songhee Baek; Arun Shamanna Lakshmi;
> alsa-devel@alsa-project.org
> Subject: Re: [alsa-devel] [PATCH v2 2/5] ASoC: Revert "ASoC: dapm: Fix
> double prefix addition"
> 
> On 05/07/2014 04:20 PM, Lars-Peter Clausen wrote:
> > This reverts commit bd23c5b661858446267f4d6b2fb4edd8eb710dda.
> >
> > The patch claims that the patch is necessary to avoid double prefix
> > addition when calling snd_soc_dapm_add_route() from
> snd_soc_dapm_connect_dai_link_widgets().
> > But snd_soc_dapm_add_route() is called with the card's DAPM context,
> > which does not have a prefix, which means there is no prefix that
> > could be added a second time.
> >
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > ---
> > I might be missing something. My best guess is that this was needed in
> > some vendor tree, but not in upstream.
> 
> Hi Songhee, Arun,
> 
> Can you take a look at this? I think the reason why you had this patch is
> because this was needed to prevent double prefix addition in
> snd_soc_dapm_link_dai_widgets(). All other places where
> dapm_add_route() is/was used do not suffer the problem of double prefix
> addition. But
> snd_soc_dapm_link_dai_widgets() was changed in commit commit
> 2553628e
> ("ASoC: dapm: Add snd_soc_dapm_add_path() helper function") to use
> dapm_add_path which does not have the problem since it doesn't try to
> add a prefix. v3.12 was the first kernel that had this commit and I think
> that you forward ported this change from a vendor tree that was based
> on an earlier version. So in summery I think the issue that you tried to fix
> this patch was already fixed by the time you submitted the patch.
> 
> Thanks,
> - Lars

We tested the latest upstream kernel and it works without our patch. So, it's
okay to revert this and sorry to push this patch in the first place.

> 
> > ---
> >   sound/soc/soc-dapm.c | 11 +++++------
> >   1 file changed, 5 insertions(+), 6 deletions(-)
> >

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
Mark Brown May 12, 2014, 8:35 p.m. UTC | #3
On Wed, May 07, 2014 at 04:20:25PM +0200, Lars-Peter Clausen wrote:
> This reverts commit bd23c5b661858446267f4d6b2fb4edd8eb710dda.
> 
> The patch claims that the patch is necessary to avoid double prefix addition
> when calling snd_soc_dapm_add_route() from snd_soc_dapm_connect_dai_link_widgets().

Applied, thanks.  Please take care over the word wrapping in your commit
logs - they're bumping right at the 80 column limit usually.
diff mbox

Patch

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 142a738..08d869c 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -2381,8 +2381,7 @@  err:
 }
 
 static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm,
-				  const struct snd_soc_dapm_route *route,
-				  unsigned int is_prefixed)
+				  const struct snd_soc_dapm_route *route)
 {
 	struct snd_soc_dapm_widget *wsource = NULL, *wsink = NULL, *w;
 	struct snd_soc_dapm_widget *wtsource = NULL, *wtsink = NULL;
@@ -2392,7 +2391,7 @@  static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm,
 	char prefixed_source[80];
 	int ret;
 
-	if (dapm->codec && dapm->codec->name_prefix && !is_prefixed) {
+	if (dapm->codec && dapm->codec->name_prefix) {
 		snprintf(prefixed_sink, sizeof(prefixed_sink), "%s %s",
 			 dapm->codec->name_prefix, route->sink);
 		sink = prefixed_sink;
@@ -2520,7 +2519,7 @@  int snd_soc_dapm_add_routes(struct snd_soc_dapm_context *dapm,
 
 	mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_INIT);
 	for (i = 0; i < num; i++) {
-		r = snd_soc_dapm_add_route(dapm, route, false);
+		r = snd_soc_dapm_add_route(dapm, route);
 		if (r < 0) {
 			dev_err(dapm->dev, "ASoC: Failed to add route %s -> %s -> %s\n",
 				route->source,
@@ -3430,7 +3429,7 @@  void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card)
 				cpu_dai->codec->name, r.source,
 				codec_dai->platform->name, r.sink);
 
-			snd_soc_dapm_add_route(&card->dapm, &r, true);
+			snd_soc_dapm_add_route(&card->dapm, &r);
 		}
 
 		/* connect BE DAI capture if widgets are valid */
@@ -3441,7 +3440,7 @@  void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card)
 				codec_dai->codec->name, r.source,
 				cpu_dai->platform->name, r.sink);
 
-			snd_soc_dapm_add_route(&card->dapm, &r, true);
+			snd_soc_dapm_add_route(&card->dapm, &r);
 		}
 
 	}