On Mon, 18 Dec 2017 16:30:13 +0100, Mauro Santos wrote: > > On 18-12-2017 13:53, Takashi Iwai wrote: > > On Mon, 18 Dec 2017 14:44:44 +0100, > > Greg KH wrote: > >> > >> On Sun, Dec 17, 2017 at 06:56:05PM +0000, Mauro Santos wrote: > >>> I believe this is the right place to report this problem, but if it > >>> isn't please point me in the right direction. > >> > >> Adding the developer of that patch, and the sound maintainer and > >> developers to the thread. > >> > >>> I have noticed that after the update from kernel 4.14.5 to 4.14.6 > >>> alsamixer does not show the usual volume controls for my usb soundcard. > >>> > >>> Reverting 3884d12e17ab770aa0f5d4bc65bfbfd006f417fa ALSA: usb-audio: Add > >>> check return value for usb_string() (from linux-stable) makes the > >>> controls come back again with kernel 4.14.6. > > (snip) > >> > >> This is odd, Takashi, I thought we fixed up the problem that if the > >> string was invalid, the code would continue to go on, it's not a "real" > >> error. Did that not get marked for the stable trees? > > > > Yes, it was marked as stable, and it's odd that the revert of the > > given commit changes the behavior in that way. > > > > Mauro, could you double-check whether reverting the commit really does > > fix it as expected? If yes, could you put some debug print at the > > part the patch touches, and check which unit id gives len=0 from > > snd_usb_copy_string_desc()? > > I'm sure reverting that patch makes things work as expected. I noticed > the problem after an update and that is the only thing I revert on top > of the kernel provided by the distro (Arch Linux). Did you revert only one patch, not both patches? Just to be sure. > Alsamixer works fine for the built in soundcard in my laptop, but the > mixer for the usb soundcard was not working so it's specific to usb and > only 2 patches touch the mixer.c file between 4.14.5 and 4.14.6. I've > tried reversing both, one at a time, and only reverting this one > restored the expected behavior. > > Regarding adding the debug print I'm going to need guidance. Without > reverting anything, would adding at line 2178 of sound/usb/mixer.c the > following be enough? > > printk("usbmixdbg: nameid=%d len=%d id.name=%s\n",nameid,len,kctl->id.name); > > It would then look like this (minus the line wrapping): > len = check_mapped_name(map, kctl->id.name, sizeof(kctl->id.name)); > printk("usbmixdbg: nameid=%d len=%d id.name=%s\n",nameid,len,kctl->id.name); > if (len) Well, at that point, there should be no difference. The difference is after that, so what I'd like to see is something like: --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -2175,14 +2175,18 @@ static int parse_audio_selector_unit(struct mixer_build *state, int unitid, nameid = uac_selector_unit_iSelector(desc); len = check_mapped_name(map, kctl->id.name, sizeof(kctl->id.name)); + pr_info("XXX id=%d, check_mapped_name=%d\n", id, len); if (len) ; - else if (nameid) + else if (nameid) { len = snd_usb_copy_string_desc(state, nameid, kctl->id.name, sizeof(kctl->id.name)); - else + pr_info("XXX id=%d, copy_string=%d\n", len); + } else { len = get_term_name(state, &state->oterm, kctl->id.name, sizeof(kctl->id.name), 0); + pr_info("XXX id=%d, get_term_name=%d\n", len); + } if (!len) { strlcpy(kctl->id.name, "USB", sizeof(kctl->id.name)); If you see copy_string=0, it means that your hardware reports a bogus entry, and the driver does it correctly. If ignoring that error really restores the old behavior back, it essentially means that it worked casually in the past... thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html