From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: Mixer regression with usb soundcard Date: Mon, 18 Dec 2017 18:13:08 +0100 Message-ID: References: <0f95a1cc-c438-ca4e-cc5f-d311e33a496e@gmail.com> <20171218134444.GA18133@kroah.com> <9dff0299-5621-2670-50b6-ee1851bb40f4@gmail.com> 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 3CAA8267544 for ; Mon, 18 Dec 2017 18:13:09 +0100 (CET) In-Reply-To: <9dff0299-5621-2670-50b6-ee1851bb40f4@gmail.com> 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: Mauro Santos Cc: Greg KH , Jaejoong Kim , linux-usb@vger.kernel.org, alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org On Mon, 18 Dec 2017 18:05:18 +0100, Mauro Santos wrote: > > On 18-12-2017 15:45, Takashi Iwai wrote: > > 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. > > I have reverted only one patch. > > >> 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... > > I have applied your patch on top of 4.14.7 without reverting anything > and I was planning to reply only after I had some result but building > failed (without any errors strangely). > > I took a second look at your patch and I have a (maybe silly/naive) > question, don't the second and third pr_info calls need an extra > argument? There are two %d in the string but only one variable (len). Yeah, sure, of course you need them :) I haven't tested the patch, but just to give you an idea. Sorry if you wasted your time due to that. Takashi From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: Mixer regression with usb soundcard From: Takashi Iwai Message-Id: Date: Mon, 18 Dec 2017 18:13:08 +0100 To: Mauro Santos Cc: Greg KH , Jaejoong Kim , alsa-devel@alsa-project.org, linux-usb@vger.kernel.org List-ID: T24gTW9uLCAxOCBEZWMgMjAxNyAxODowNToxOCArMDEwMCwKTWF1cm8gU2FudG9zIHdyb3RlOgo+ IAo+IE9uIDE4LTEyLTIwMTcgMTU6NDUsIFRha2FzaGkgSXdhaSB3cm90ZToKPiA+IE9uIE1vbiwg MTggRGVjIDIwMTcgMTY6MzA6MTMgKzAxMDAsCj4gPiBNYXVybyBTYW50b3Mgd3JvdGU6Cj4gPj4K PiA+PiBPbiAxOC0xMi0yMDE3IDEzOjUzLCBUYWthc2hpIEl3YWkgd3JvdGU6Cj4gPj4+IE9uIE1v biwgMTggRGVjIDIwMTcgMTQ6NDQ6NDQgKzAxMDAsCj4gPj4+IEdyZWcgS0ggd3JvdGU6Cj4gPj4+ Pgo+ID4+Pj4gT24gU3VuLCBEZWMgMTcsIDIwMTcgYXQgMDY6NTY6MDVQTSArMDAwMCwgTWF1cm8g U2FudG9zIHdyb3RlOgo+ID4+Pj4+IEkgYmVsaWV2ZSB0aGlzIGlzIHRoZSByaWdodCBwbGFjZSB0 byByZXBvcnQgdGhpcyBwcm9ibGVtLCBidXQgaWYgaXQKPiA+Pj4+PiBpc24ndCBwbGVhc2UgcG9p bnQgbWUgaW4gdGhlIHJpZ2h0IGRpcmVjdGlvbi4KPiA+Pj4+Cj4gPj4+PiBBZGRpbmcgdGhlIGRl dmVsb3BlciBvZiB0aGF0IHBhdGNoLCBhbmQgdGhlIHNvdW5kIG1haW50YWluZXIgYW5kCj4gPj4+ PiBkZXZlbG9wZXJzIHRvIHRoZSB0aHJlYWQuCj4gPj4+Pgo+ID4+Pj4+IEkgaGF2ZSBub3RpY2Vk IHRoYXQgYWZ0ZXIgdGhlIHVwZGF0ZSBmcm9tIGtlcm5lbCA0LjE0LjUgdG8gNC4xNC42Cj4gPj4+ Pj4gYWxzYW1peGVyIGRvZXMgbm90IHNob3cgdGhlIHVzdWFsIHZvbHVtZSBjb250cm9scyBmb3Ig bXkgdXNiIHNvdW5kY2FyZC4KPiA+Pj4+Pgo+ID4+Pj4+IFJldmVydGluZyAzODg0ZDEyZTE3YWI3 NzBhYTBmNWQ0YmM2NWJmYmZkMDA2ZjQxN2ZhIEFMU0E6IHVzYi1hdWRpbzogQWRkCj4gPj4+Pj4g Y2hlY2sgcmV0dXJuIHZhbHVlIGZvciB1c2Jfc3RyaW5nKCkgKGZyb20gbGludXgtc3RhYmxlKSBt YWtlcyB0aGUKPiA+Pj4+PiBjb250cm9scyBjb21lIGJhY2sgYWdhaW4gd2l0aCBrZXJuZWwgNC4x NC42Lgo+ID4+PiAoc25pcCkKPiA+Pj4+Cj4gPj4+PiBUaGlzIGlzIG9kZCwgVGFrYXNoaSwgSSB0 aG91Z2h0IHdlIGZpeGVkIHVwIHRoZSBwcm9ibGVtIHRoYXQgaWYgdGhlCj4gPj4+PiBzdHJpbmcg d2FzIGludmFsaWQsIHRoZSBjb2RlIHdvdWxkIGNvbnRpbnVlIHRvIGdvIG9uLCBpdCdzIG5vdCBh ICJyZWFsIgo+ID4+Pj4gZXJyb3IuICBEaWQgdGhhdCBub3QgZ2V0IG1hcmtlZCBmb3IgdGhlIHN0 YWJsZSB0cmVlcz8KPiA+Pj4KPiA+Pj4gWWVzLCBpdCB3YXMgbWFya2VkIGFzIHN0YWJsZSwgYW5k IGl0J3Mgb2RkIHRoYXQgdGhlIHJldmVydCBvZiB0aGUKPiA+Pj4gZ2l2ZW4gY29tbWl0IGNoYW5n ZXMgdGhlIGJlaGF2aW9yIGluIHRoYXQgd2F5Lgo+ID4+Pgo+ID4+PiBNYXVybywgY291bGQgeW91 IGRvdWJsZS1jaGVjayB3aGV0aGVyIHJldmVydGluZyB0aGUgY29tbWl0IHJlYWxseSBkb2VzCj4g Pj4+IGZpeCBpdCBhcyBleHBlY3RlZD8gIElmIHllcywgY291bGQgeW91IHB1dCBzb21lIGRlYnVn IHByaW50IGF0IHRoZQo+ID4+PiBwYXJ0IHRoZSBwYXRjaCB0b3VjaGVzLCBhbmQgY2hlY2sgd2hp Y2ggdW5pdCBpZCBnaXZlcyBsZW49MCBmcm9tCj4gPj4+IHNuZF91c2JfY29weV9zdHJpbmdfZGVz YygpPwo+ID4+Cj4gPj4gSSdtIHN1cmUgcmV2ZXJ0aW5nIHRoYXQgcGF0Y2ggbWFrZXMgdGhpbmdz IHdvcmsgYXMgZXhwZWN0ZWQuIEkgbm90aWNlZAo+ID4+IHRoZSBwcm9ibGVtIGFmdGVyIGFuIHVw ZGF0ZSBhbmQgdGhhdCBpcyB0aGUgb25seSB0aGluZyBJIHJldmVydCBvbiB0b3AKPiA+PiBvZiB0 aGUga2VybmVsIHByb3ZpZGVkIGJ5IHRoZSBkaXN0cm8gKEFyY2ggTGludXgpLgo+ID4gCj4gPiBE aWQgeW91IHJldmVydCBvbmx5IG9uZSBwYXRjaCwgbm90IGJvdGggcGF0Y2hlcz8KPiA+IEp1c3Qg dG8gYmUgc3VyZS4KPiAKPiBJIGhhdmUgcmV2ZXJ0ZWQgb25seSBvbmUgcGF0Y2guCj4gCj4gPj4g QWxzYW1peGVyIHdvcmtzIGZpbmUgZm9yIHRoZSBidWlsdCBpbiBzb3VuZGNhcmQgaW4gbXkgbGFw dG9wLCBidXQgdGhlCj4gPj4gbWl4ZXIgZm9yIHRoZSB1c2Igc291bmRjYXJkIHdhcyBub3Qgd29y a2luZyBzbyBpdCdzIHNwZWNpZmljIHRvIHVzYiBhbmQKPiA+PiBvbmx5IDIgcGF0Y2hlcyB0b3Vj aCB0aGUgbWl4ZXIuYyBmaWxlIGJldHdlZW4gNC4xNC41IGFuZCA0LjE0LjYuIEkndmUKPiA+PiB0 cmllZCByZXZlcnNpbmcgYm90aCwgb25lIGF0IGEgdGltZSwgYW5kIG9ubHkgcmV2ZXJ0aW5nIHRo aXMgb25lCj4gPj4gcmVzdG9yZWQgdGhlIGV4cGVjdGVkIGJlaGF2aW9yLgo+ID4+Cj4gPj4gUmVn YXJkaW5nIGFkZGluZyB0aGUgZGVidWcgcHJpbnQgSSdtIGdvaW5nIHRvIG5lZWQgZ3VpZGFuY2Uu IFdpdGhvdXQKPiA+PiByZXZlcnRpbmcgYW55dGhpbmcsIHdvdWxkIGFkZGluZyBhdCBsaW5lIDIx Nzggb2Ygc291bmQvdXNiL21peGVyLmMgdGhlCj4gPj4gZm9sbG93aW5nIGJlIGVub3VnaD8KPiA+ Pgo+ID4+IHByaW50aygidXNibWl4ZGJnOiBuYW1laWQ9JWQgbGVuPSVkIGlkLm5hbWU9JXNcbiIs bmFtZWlkLGxlbixrY3RsLT5pZC5uYW1lKTsKPiA+Pgo+ID4+IEl0IHdvdWxkIHRoZW4gbG9vayBs aWtlIHRoaXMgKG1pbnVzIHRoZSBsaW5lIHdyYXBwaW5nKToKPiA+PiBsZW4gPSBjaGVja19tYXBw ZWRfbmFtZShtYXAsIGtjdGwtPmlkLm5hbWUsIHNpemVvZihrY3RsLT5pZC5uYW1lKSk7Cj4gPj4g cHJpbnRrKCJ1c2JtaXhkYmc6IG5hbWVpZD0lZCBsZW49JWQgaWQubmFtZT0lc1xuIixuYW1laWQs bGVuLGtjdGwtPmlkLm5hbWUpOwo+ID4+IGlmIChsZW4pCj4gPiAKPiA+IFdlbGwsIGF0IHRoYXQg cG9pbnQsIHRoZXJlIHNob3VsZCBiZSBubyBkaWZmZXJlbmNlLgo+ID4gVGhlIGRpZmZlcmVuY2Ug aXMgYWZ0ZXIgdGhhdCwgc28gd2hhdCBJJ2QgbGlrZSB0byBzZWUgaXMgc29tZXRoaW5nCj4gPiBs aWtlOgo+ID4gCj4gPiAtLS0gYS9zb3VuZC91c2IvbWl4ZXIuYwo+ID4gKysrIGIvc291bmQvdXNi L21peGVyLmMKPiA+IEBAIC0yMTc1LDE0ICsyMTc1LDE4IEBAIHN0YXRpYyBpbnQgcGFyc2VfYXVk aW9fc2VsZWN0b3JfdW5pdChzdHJ1Y3QgbWl4ZXJfYnVpbGQgKnN0YXRlLCBpbnQgdW5pdGlkLAo+ ID4gIAo+ID4gIAluYW1laWQgPSB1YWNfc2VsZWN0b3JfdW5pdF9pU2VsZWN0b3IoZGVzYyk7Cj4g PiAgCWxlbiA9IGNoZWNrX21hcHBlZF9uYW1lKG1hcCwga2N0bC0+aWQubmFtZSwgc2l6ZW9mKGtj dGwtPmlkLm5hbWUpKTsKPiA+ICsJcHJfaW5mbygiWFhYIGlkPSVkLCBjaGVja19tYXBwZWRfbmFt ZT0lZFxuIiwgaWQsIGxlbik7Cj4gPiAgCWlmIChsZW4pCj4gPiAgCQk7Cj4gPiAtCWVsc2UgaWYg KG5hbWVpZCkKPiA+ICsJZWxzZSBpZiAobmFtZWlkKSB7Cj4gPiAgCQlsZW4gPSBzbmRfdXNiX2Nv cHlfc3RyaW5nX2Rlc2Moc3RhdGUsIG5hbWVpZCwga2N0bC0+aWQubmFtZSwKPiA+ICAJCQkJCSBz aXplb2Yoa2N0bC0+aWQubmFtZSkpOwo+ID4gLQllbHNlCj4gPiArCQlwcl9pbmZvKCJYWFggaWQ9 JWQsIGNvcHlfc3RyaW5nPSVkXG4iLCBsZW4pOwo+ID4gKwl9IGVsc2Ugewo+ID4gIAkJbGVuID0g Z2V0X3Rlcm1fbmFtZShzdGF0ZSwgJnN0YXRlLT5vdGVybSwKPiA+ICAJCQkJICAgIGtjdGwtPmlk Lm5hbWUsIHNpemVvZihrY3RsLT5pZC5uYW1lKSwgMCk7Cj4gPiArCQlwcl9pbmZvKCJYWFggaWQ9 JWQsIGdldF90ZXJtX25hbWU9JWRcbiIsIGxlbik7Cj4gPiArCX0KPiA+ICAKPiA+ICAJaWYgKCFs ZW4pIHsKPiA+ICAJCXN0cmxjcHkoa2N0bC0+aWQubmFtZSwgIlVTQiIsIHNpemVvZihrY3RsLT5p ZC5uYW1lKSk7Cj4gPiAKPiA+IAo+ID4gSWYgeW91IHNlZSBjb3B5X3N0cmluZz0wLCBpdCBtZWFu cyB0aGF0IHlvdXIgaGFyZHdhcmUgcmVwb3J0cyBhIGJvZ3VzCj4gPiBlbnRyeSwgYW5kIHRoZSBk cml2ZXIgZG9lcyBpdCBjb3JyZWN0bHkuICBJZiBpZ25vcmluZyB0aGF0IGVycm9yCj4gPiByZWFs bHkgcmVzdG9yZXMgdGhlIG9sZCBiZWhhdmlvciBiYWNrLCBpdCBlc3NlbnRpYWxseSBtZWFucyB0 aGF0IGl0Cj4gPiB3b3JrZWQgY2FzdWFsbHkgaW4gdGhlIHBhc3QuLi4KPiAKPiBJIGhhdmUgYXBw bGllZCB5b3VyIHBhdGNoIG9uIHRvcCBvZiA0LjE0Ljcgd2l0aG91dCByZXZlcnRpbmcgYW55dGhp bmcKPiBhbmQgSSB3YXMgcGxhbm5pbmcgdG8gcmVwbHkgb25seSBhZnRlciBJIGhhZCBzb21lIHJl c3VsdCBidXQgYnVpbGRpbmcKPiBmYWlsZWQgKHdpdGhvdXQgYW55IGVycm9ycyBzdHJhbmdlbHkp Lgo+IAo+IEkgdG9vayBhIHNlY29uZCBsb29rIGF0IHlvdXIgcGF0Y2ggYW5kIEkgaGF2ZSBhICht YXliZSBzaWxseS9uYWl2ZSkKPiBxdWVzdGlvbiwgZG9uJ3QgdGhlIHNlY29uZCBhbmQgdGhpcmQg cHJfaW5mbyBjYWxscyBuZWVkIGFuIGV4dHJhCj4gYXJndW1lbnQ/IFRoZXJlIGFyZSB0d28gJWQg aW4gdGhlIHN0cmluZyBidXQgb25seSBvbmUgdmFyaWFibGUgKGxlbikuCgpZZWFoLCBzdXJlLCBv ZiBjb3Vyc2UgeW91IG5lZWQgdGhlbSA6KQpJIGhhdmVuJ3QgdGVzdGVkIHRoZSBwYXRjaCwgYnV0 IGp1c3QgdG8gZ2l2ZSB5b3UgYW4gaWRlYS4KU29ycnkgaWYgeW91IHdhc3RlZCB5b3VyIHRpbWUg ZHVlIHRvIHRoYXQuCgoKVGFrYXNoaQotLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6 IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXVzYiIgaW4KdGhlIGJvZHkgb2YgYSBt ZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKTW9yZSBtYWpvcmRvbW8gaW5mbyBh dCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCg==