Re: [PATCH 2/2] sound: usb: allow clock source validity interrupts

[Takashi Iwai <tiwai@suse.de>]

On Sat, 09 Apr 2016 11:16:59 +0200,
Daniel Mack wrote:
> 
> On 04/09/2016 10:52 AM, Takashi Iwai wrote:
> > On Fri, 08 Apr 2016 19:52:02 +0200,
> > Daniel Mack wrote:
> >>
> >> miniDSP USBStreamer UAC2 devices send clock validity changes with the
> >> control field set to zero. The current interrupt handler ignores all
> >> packets if the control field does not match the mixer element's, but
> >> it really should only do that in case that field is needed to
> >> distinguish multiple elements with the same ID.
> >>
> >> This patch implements a logic that lets notifications packets pass
> >> if the element ID is unique for a given device.
> >>
> >> Signed-off-by: Daniel Mack <daniel@zonque.org>
> >> ---
> >>  sound/usb/mixer.c | 9 ++++++++-
> >>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> >> index 973274b..aa6f16e 100644
> >> --- a/sound/usb/mixer.c
> >> +++ b/sound/usb/mixer.c
> >> @@ -2371,6 +2371,7 @@ static void snd_usb_mixer_interrupt_v2(struct usb_mixer_interface *mixer,
> >>  	__u8 unitid = (index >> 8) & 0xff;
> >>  	__u8 control = (value >> 8) & 0xff;
> >>  	__u8 channel = value & 0xff;
> >> +	unsigned int count = 0;
> >>  
> >>  	if (channel >= MAX_CHANNELS) {
> >>  		usb_audio_dbg(mixer->chip,
> >> @@ -2379,6 +2380,12 @@ static void snd_usb_mixer_interrupt_v2(struct usb_mixer_interface *mixer,
> >>  		return;
> >>  	}
> >>  
> >> +	for (list = mixer->id_elems[unitid]; list; list = list->next_id_elem)
> >> +		count++;
> >> +
> >> +	if (count == 0)
> >> +		return;
> >> +
> >>  	for (list = mixer->id_elems[unitid]; list; list = list->next_id_elem) {
> >>  		struct usb_mixer_elem_info *info;
> >>  
> >> @@ -2386,7 +2393,7 @@ static void snd_usb_mixer_interrupt_v2(struct usb_mixer_interface *mixer,
> >>  			continue;
> >>  
> >>  		info = (struct usb_mixer_elem_info *)list;
> >> -		if (info->control != control)
> >> +		if (count > 1 && info->control != control)
> >>  			continue;
> > 
> > Just for checking count=0 and count=1, we need no loop to count
> > beforehand.
> > 		if (info->control != control &&
> > 		    (list != mixer->id_elems[unit] ||
> > 		     list->list_next_id_elem))
> > 			continue;
> > 
> > But, this doesn't look better and is more harder to understand, so I'm
> > not willing to sell it :)
> 
> I had something like that before but opted for the more readable
> version. But you're right. I'll add a comment and do it your way.

Oh no, sorry, I wasn't clear: I meant that my version is worse in the
end, and I prefer your first version, just for simplicity.


Takashi