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

[Liam Girdwood <lrg@ti.com>]

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