| Message ID | 1460137922-8993-2-git-send-email-daniel@zonque.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 :) thanks, Takashi
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. Thanks! Daniel
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
On 04/09/2016 11:51 AM, Takashi Iwai wrote: > On Sat, 09 Apr 2016 11:16:59 +0200, Daniel Mack wrote: >> On 04/09/2016 10:52 AM, Takashi Iwai wrote: >>> 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. Ah ok :) Well, ultimately up to you; you have both versions, and the two patches should apply independently anyway. Or I can resend both if you prefer that. Thanks, Daniel
On Sat, 09 Apr 2016 16:37:28 +0200, Daniel Mack wrote: > > On 04/09/2016 11:51 AM, Takashi Iwai wrote: > > On Sat, 09 Apr 2016 11:16:59 +0200, Daniel Mack wrote: > >> On 04/09/2016 10:52 AM, Takashi Iwai wrote: > > >>> 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. > > Ah ok :) Well, ultimately up to you; you have both versions, and the two > patches should apply independently anyway. > > Or I can resend both if you prefer that. Don't worry, I picked the preferred patches now (v2 for patch 1 and v1 for patch 2). thanks, Takashi
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; switch (attribute) {
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(-)