[alsa-devel] [PATCH] ALSA: usb-audio: Recurse before saving terminal properties

[Julian Scheel]

Hi Daniel,

On 16.08.15 13:45, Daniel Mack wrote:
> On 08/14/2015 03:25 PM, Julian Scheel wrote:
>> Am 14.08.2015 um 15:06 schrieb Takashi Iwai:
>
>>>> @@ -715,15 +715,16 @@ static int check_input_term(struct mixer_build *state, int id,
>>>>    				term->name = d->iTerminal;
>>>>    			} else { /* UAC_VERSION_2 */
>>>>    				struct uac2_input_terminal_descriptor *d = p1;
>>>> -				term->type = le16_to_cpu(d->wTerminalType);
>>>> -				term->channels = d->bNrChannels;
>>>> -				term->chconfig = le32_to_cpu(d->bmChannelConfig);
>>>> -				term->name = d->iTerminal;
>>>>
>>>>    				/* call recursively to get the clock selectors */
>>>>    				err = check_input_term(state, d->bCSourceID, term);
>>>>    				if (err < 0)
>>>>    					return err;
>>>> +
>>>> +				term->type = le16_to_cpu(d->wTerminalType);
>>>> +				term->channels = d->bNrChannels;
>>>> +				term->chconfig = le32_to_cpu(d->bmChannelConfig);
>>>> +				term->name = d->iTerminal;
>>>
>>> ... by this override, essentially all fields except for id are
>>> restored.  Then what's the point to call check_input_term() for the
>>> clock source?
>>
>> This is a good point indeed, but due do my understanding the recursion
>> is not about filling missing information, but about verifying a valid
>> descriptor. So in case there are broken references in the descriptor it
>> gets abandoned. I'm happy to be corrected if this understanding is wrong.
>
> Hmm, good point. It' been a while since I worked on this, but looking at
> the code right now, you're right, the mixer unit for input terminal is
> indeed just overridden by the information of the clock selector on UAC2.
> That doesn't seem right.
>
> I guess what we really want is to add a 2nd mixer control in such cases,
> and that would mean we need to call into parse_audio_selector_unit()
> instead.

Ah, you're right. The Clock Selector shall actually be exposed as a 
mixer unit. This never happened with the current code, but it just 
renamed all other selectors to the name of the clock selector if it existed.

> I can cook up or test a patch next week, when I have access to test
> hardware.

I can update my patch to reflect this as well. If you haven't done 
before I'll try to do it later today.

-Julian