[PATCH] ASoC: dapm - fix prefix for DAPM muxes

[PATCH] ASoC: dapm - fix prefix for DAPM muxes

[Liam Girdwood]

Make sure DAPM muxes have a valid kcontrol name instead of NULL.

Signed-off-by: Liam Girdwood <lrg@ti.com>
---
 sound/soc/soc-dapm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 456617e..c5d98e3 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -489,10 +489,10 @@ static int dapm_new_mux(struct snd_soc_dapm_context *dapm,
 	wlist->widgets[wlistentries - 1] = w;
 
 	if (!kcontrol) {
-		if (dapm->codec)
+		if (dapm->codec && dapm->codec->name_prefix)
 			prefix = dapm->codec->name_prefix;
 		else
-			prefix = NULL;
+			prefix = w->name;
 
 		if (shared) {
 			name = w->kcontrol_news[0].name;
-- 
1.7.4.1

Re: [PATCH] ASoC: dapm - fix prefix for DAPM muxes

[Jarkko Nikula]

On Tue, 24 May 2011 17:39:09 +0100
Liam Girdwood <lrg@ti.com> wrote:

> Make sure DAPM muxes have a valid kcontrol name instead of NULL.
> 
> Signed-off-by: Liam Girdwood <lrg@ti.com>
> ---
>  sound/soc/soc-dapm.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 456617e..c5d98e3 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -489,10 +489,10 @@ static int dapm_new_mux(struct snd_soc_dapm_context *dapm,
>  	wlist->widgets[wlistentries - 1] = w;
>  
>  	if (!kcontrol) {
> -		if (dapm->codec)
> +		if (dapm->codec && dapm->codec->name_prefix)
>  			prefix = dapm->codec->name_prefix;
>  		else
> -			prefix = NULL;
> +			prefix = w->name;
>  
This didn't look correct to play with prefix. A quick look revealed
that there is indeed breakage from commit af46800 ("ASoC: Implement mux
control sharing") and this fix doesn't fix it on rx51.

I think the breakage must come from dapm_is_shared_kcontrol() that
returns always 1 and dapm_new_mux() ends up using w->kcontrol_news
[0].name instead of previous w->name.

dapm_is_shared_kcontrol() returns always 1 since it gets
&w->kcontrol_news[0] as an argument and &w->kcontrol_news[i] will
match when iterating the same widget with i==0.

	list_for_each_entry(w, &dapm->card->widgets, list) {
		for (i = 0; i < w->num_kcontrols; i++) {
			if (&w->kcontrol_news[i] == kcontrol_new) {
				if (w->kcontrols)
					*kcontrol = w->kcontrols[i];
				return 1;
			}
		}
	}

It works rx51 if I start the iteration from i==1 but it looks the
actual fix is to the check that we are not iterating the same widget.

-- 
Jarkko

Re: [PATCH] ASoC: dapm - fix prefix for DAPM muxes

[Stephen Warren]

Jarkko Nikula wrote at Tuesday, May 24, 2011 12:45 PM:
> On Tue, 24 May 2011 17:39:09 +0100 Liam Girdwood <lrg@ti.com> wrote:
> > Make sure DAPM muxes have a valid kcontrol name instead of NULL.
> >
> > Signed-off-by: Liam Girdwood <lrg@ti.com>
> > ---
> >  sound/soc/soc-dapm.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> > index 456617e..c5d98e3 100644
> > --- a/sound/soc/soc-dapm.c
> > +++ b/sound/soc/soc-dapm.c
> > @@ -489,10 +489,10 @@ static int dapm_new_mux(struct snd_soc_dapm_context *dapm,
> >  	wlist->widgets[wlistentries - 1] = w;
> >
> >  	if (!kcontrol) {
> > -		if (dapm->codec)
> > +		if (dapm->codec && dapm->codec->name_prefix)
> >  			prefix = dapm->codec->name_prefix;
> >  		else
> > -			prefix = NULL;
> > +			prefix = w->name;
> >
> This didn't look correct to play with prefix. A quick look revealed
> that there is indeed breakage from commit af46800 ("ASoC: Implement mux
> control sharing") and this fix doesn't fix it on rx51.
> 
> I think the breakage must come from dapm_is_shared_kcontrol() that
> returns always 1 and dapm_new_mux() ends up using w->kcontrol_news
> [0].name instead of previous w->name.
> 
> dapm_is_shared_kcontrol() returns always 1 since it gets
> &w->kcontrol_news[0] as an argument and &w->kcontrol_news[i] will
> match when iterating the same widget with i==0.
> 
> 	list_for_each_entry(w, &dapm->card->widgets, list) {
> 		for (i = 0; i < w->num_kcontrols; i++) {
> 			if (&w->kcontrol_news[i] == kcontrol_new) {
> 				if (w->kcontrols)
> 					*kcontrol = w->kcontrols[i];
> 				return 1;
> 			}
> 		}
> 	}
> 
> It works rx51 if I start the iteration from i==1 but it looks the
> actual fix is to the check that we are not iterating the same widget.

Yes, I think just add a "struct snd_soc_dapm_widget *kcontrolw" parameter
to dapm_is_shared_kcontrol(), and make the list_for_each continue if
w == kcontrolw.

Sorry, I didn't test this on a system with control name prefixes.

I can whip up the patch if you want, but won't be able to test that
it fixes your problems, since none of my systems have prefixes.
Let me know either way.

-- 
nvpublic

Re: [PATCH] ASoC: dapm - fix prefix for DAPM muxes

[Liam Girdwood]

On 24/05/11 20:37, Stephen Warren wrote:
> Jarkko Nikula wrote at Tuesday, May 24, 2011 12:45 PM:
>> On Tue, 24 May 2011 17:39:09 +0100 Liam Girdwood <lrg@ti.com> wrote:
>>> Make sure DAPM muxes have a valid kcontrol name instead of NULL.
>>>
>>> Signed-off-by: Liam Girdwood <lrg@ti.com>
>>> ---
>>>  sound/soc/soc-dapm.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
>>> index 456617e..c5d98e3 100644
>>> --- a/sound/soc/soc-dapm.c
>>> +++ b/sound/soc/soc-dapm.c
>>> @@ -489,10 +489,10 @@ static int dapm_new_mux(struct snd_soc_dapm_context *dapm,
>>>  	wlist->widgets[wlistentries - 1] = w;
>>>
>>>  	if (!kcontrol) {
>>> -		if (dapm->codec)
>>> +		if (dapm->codec && dapm->codec->name_prefix)
>>>  			prefix = dapm->codec->name_prefix;
>>>  		else
>>> -			prefix = NULL;
>>> +			prefix = w->name;
>>>
>> This didn't look correct to play with prefix. A quick look revealed
>> that there is indeed breakage from commit af46800 ("ASoC: Implement mux
>> control sharing") and this fix doesn't fix it on rx51.
>>
>> I think the breakage must come from dapm_is_shared_kcontrol() that
>> returns always 1 and dapm_new_mux() ends up using w->kcontrol_news
>> [0].name instead of previous w->name.
>>
>> dapm_is_shared_kcontrol() returns always 1 since it gets
>> &w->kcontrol_news[0] as an argument and &w->kcontrol_news[i] will
>> match when iterating the same widget with i==0.
>>
>> 	list_for_each_entry(w, &dapm->card->widgets, list) {
>> 		for (i = 0; i < w->num_kcontrols; i++) {
>> 			if (&w->kcontrol_news[i] == kcontrol_new) {
>> 				if (w->kcontrols)
>> 					*kcontrol = w->kcontrols[i];
>> 				return 1;
>> 			}
>> 		}
>> 	}
>>
>> It works rx51 if I start the iteration from i==1 but it looks the
>> actual fix is to the check that we are not iterating the same widget.
> 
> Yes, I think just add a "struct snd_soc_dapm_widget *kcontrolw" parameter
> to dapm_is_shared_kcontrol(), and make the list_for_each continue if
> w == kcontrolw.
> 
> Sorry, I didn't test this on a system with control name prefixes.
> 
> I can whip up the patch if you want, but won't be able to test that
> it fixes your problems, since none of my systems have prefixes.
> Let me know either way.
> 

Please do. I'll give it a test tomorrow.

Thanks !

Liam