From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 2/2] sound: usb: allow clock source validity interrupts Date: Sat, 09 Apr 2016 11:51:40 +0200 Message-ID: References: <1460137922-8993-1-git-send-email-daniel@zonque.org> <1460137922-8993-2-git-send-email-daniel@zonque.org> <5708C88B.8080809@zonque.org> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 5DE7F2615F7 for ; Sat, 9 Apr 2016 11:51:41 +0200 (CEST) In-Reply-To: <5708C88B.8080809@zonque.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Daniel Mack Cc: orm.finnendahl@selma.hfmdk-frankfurt.de, alsa-devel@alsa-project.org, clemens@ladisch.de, jan.baumgart@selma.hfmdk-frankfurt.de List-Id: alsa-devel@alsa-project.org 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 > >> --- > >> 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