All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Santos <registo.mailling@gmail.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: Jaejoong Kim <climbbb.kim@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	alsa-devel@alsa-project.org, USB list <linux-usb@vger.kernel.org>
Subject: Mixer regression with usb soundcard
Date: Mon, 18 Dec 2017 21:56:07 +0000	[thread overview]
Message-ID: <ec393c6b-a960-5fb3-f98c-cb2120961d45@gmail.com> (raw)

On 18-12-2017 19:30, Takashi Iwai wrote:
> On Mon, 18 Dec 2017 20:10:44 +0100,
> Mauro Santos wrote:
>>
>> On 18-12-2017 17:50, Jaejoong Kim wrote:
>>> Mauro,
>>>
>>> Could you please try debug patch(I also attach the patch file)?
>>
>> With the attached patch I get the following when plugging in the usb dac
>> directly to a usb3 port:
>> [   54.391539] usb 1-2: new full-speed USB device number 7 using xhci_hcd
>> [   54.514996] usb 1-2: device descriptor read/64, error -71
>> [   54.849808] input: HiFimeDIY Audio HiFimeDIY DAC as
>> /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:1852:7022.0003/input/input20
>> [   54.850168] hid-generic 0003:1852:7022.0003: input,hidraw2: USB HID
>> v1.00 Device [HiFimeDIY Audio HiFimeDIY DAC] on usb-0000:00:14.0-2/input0
>> [   54.950421] usb 1-2: [DEBUG] nameid:0, len:0
>> [   54.950426] usb 1-2: [DEBUG] len:3, get_term_name:PCM
>> [   54.950429] usb 1-2: [11] SU [PCM] items = 2
>> [   54.950985] usbcore: registered new interface driver snd-usb-audio
> 
> Hmm, the driver get the supposedly correct name string here, so I see
> no flaw, so far.
> 
> Could you put the similar debug prints after reverting the commit and
> compare?  Or, at minimum, you can enable simply the kernel debug
> prints like below:
> 
>   % echo "file sound/usb/mixer.c +p" > /sys/kernel/debug/dynamic_debug_control
> 
> and re-plug the device.
> 
> Also, could you attach the output of "amixer contents" on both working
> and non-working kernels?
> 

I have compiled a new kernel where I have reverted the commit and I've
added the debug output based on your last debug patch. I attach the
patch that reverts the changes and adds the debug output just in case
anyone wants to do a sanity check on it (don't mind the indentation I
think I botched that).

With the debug patches I get no extra output when echoing to the
dynamic_debug/control file, I guess that's expected.

I attach the dmesg and amixer outputs for the case without revert plus
debug (bad) and revert plus debug (good).

One change does jump out:

bad:  usb 1-2: [11] SU [PCM] items = 2
good: usb 1-2: [11] SU [PCM Capture Source] items = 2

diff -ur a/sound/usb/mixer.c b/sound/usb/mixer.c
--- a/sound/usb/mixer.c	2017-12-18 19:47:02.748776502 +0000
+++ b/sound/usb/mixer.c	2017-12-18 20:18:30.023770892 +0000
@@ -595,7 +595,7 @@
 	while (snd_ctl_find_id(mixer->chip->card, &kctl->id))
 		kctl->id.index++;
 	if ((err = snd_ctl_add(mixer->chip->card, kctl)) < 0) {
-		usb_audio_dbg(mixer->chip, "cannot add control (err = %d)\n",
+		usb_audio_err(mixer->chip, "cannot add control (err = %d)\n",
 			      err);
 		return err;
 	}
@@ -656,10 +656,14 @@
 			 unsigned char *name, int maxlen, int term_only)
 {
 	struct iterm_name_combo *names;
+    int len;
 
-	if (iterm->name)
-		return snd_usb_copy_string_desc(state, iterm->name,
+	if (iterm->name) {
+		len = snd_usb_copy_string_desc(state, iterm->name,
 						name, maxlen);
+        usb_audio_err(state->chip, "[DEBUG] len:%d, iterm->name:%d\n", len, iterm->name);
+        return len;
+    }
 
 	/* virtual type - not a real terminal */
 	if (iterm->type >> 16) {
@@ -2175,17 +2179,24 @@
 
 	nameid = uac_selector_unit_iSelector(desc);
 	len = check_mapped_name(map, kctl->id.name, sizeof(kctl->id.name));
+
+    usb_audio_err(state->chip, "[DEBUG] nameid:%d, len:%d\n", nameid, 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
+        usb_audio_err(state->chip, "[DEBUG] len:%d, copy_string id.name:%s\n",
+                    len, (len > 0) ? kctl->id.name : " ");
+    }
+	else {
 		len = get_term_name(state, &state->oterm,
 				    kctl->id.name, sizeof(kctl->id.name), 0);
-
-	if (!len) {
-		strlcpy(kctl->id.name, "USB", sizeof(kctl->id.name));
+        usb_audio_err(state->chip, "[DEBUG] len:%d, get_term_name:%s\n",
+                    len, (len > 0) ? kctl->id.name : " ");
+		if (!len)
+			strlcpy(kctl->id.name, "USB", sizeof(kctl->id.name));
 
 		if (desc->bDescriptorSubtype == UAC2_CLOCK_SELECTOR)
 			append_ctl_name(kctl, " Clock Source");
@@ -2195,7 +2206,7 @@
 			append_ctl_name(kctl, " Playback Source");
 	}
 
-	usb_audio_dbg(state->chip, "[%d] SU [%s] items = %d\n",
+	usb_audio_err(state->chip, "[%d] SU [%s] items = %d\n",
 		    cval->head.id, kctl->id.name, desc->bNrInPins);
 	return snd_usb_mixer_add_control(&cval->head, kctl);
 }

WARNING: multiple messages have this Message-ID (diff)
From: Mauro Santos <registo.mailling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Takashi Iwai <tiwai-l3A5Bk7waGM@public.gmane.org>
Cc: Jaejoong Kim
	<climbbb.kim-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Greg KH
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org,
	USB list <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: Mixer regression with usb soundcard
Date: Mon, 18 Dec 2017 21:56:07 +0000	[thread overview]
Message-ID: <ec393c6b-a960-5fb3-f98c-cb2120961d45@gmail.com> (raw)
In-Reply-To: <s5htvwn6dg4.wl-tiwai-l3A5Bk7waGM@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2161 bytes --]

On 18-12-2017 19:30, Takashi Iwai wrote:
> On Mon, 18 Dec 2017 20:10:44 +0100,
> Mauro Santos wrote:
>>
>> On 18-12-2017 17:50, Jaejoong Kim wrote:
>>> Mauro,
>>>
>>> Could you please try debug patch(I also attach the patch file)?
>>
>> With the attached patch I get the following when plugging in the usb dac
>> directly to a usb3 port:
>> [   54.391539] usb 1-2: new full-speed USB device number 7 using xhci_hcd
>> [   54.514996] usb 1-2: device descriptor read/64, error -71
>> [   54.849808] input: HiFimeDIY Audio HiFimeDIY DAC as
>> /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:1852:7022.0003/input/input20
>> [   54.850168] hid-generic 0003:1852:7022.0003: input,hidraw2: USB HID
>> v1.00 Device [HiFimeDIY Audio HiFimeDIY DAC] on usb-0000:00:14.0-2/input0
>> [   54.950421] usb 1-2: [DEBUG] nameid:0, len:0
>> [   54.950426] usb 1-2: [DEBUG] len:3, get_term_name:PCM
>> [   54.950429] usb 1-2: [11] SU [PCM] items = 2
>> [   54.950985] usbcore: registered new interface driver snd-usb-audio
> 
> Hmm, the driver get the supposedly correct name string here, so I see
> no flaw, so far.
> 
> Could you put the similar debug prints after reverting the commit and
> compare?  Or, at minimum, you can enable simply the kernel debug
> prints like below:
> 
>   % echo "file sound/usb/mixer.c +p" > /sys/kernel/debug/dynamic_debug_control
> 
> and re-plug the device.
> 
> Also, could you attach the output of "amixer contents" on both working
> and non-working kernels?
> 

I have compiled a new kernel where I have reverted the commit and I've
added the debug output based on your last debug patch. I attach the
patch that reverts the changes and adds the debug output just in case
anyone wants to do a sanity check on it (don't mind the indentation I
think I botched that).

With the debug patches I get no extra output when echoing to the
dynamic_debug/control file, I guess that's expected.

I attach the dmesg and amixer outputs for the case without revert plus
debug (bad) and revert plus debug (good).

One change does jump out:

bad:  usb 1-2: [11] SU [PCM] items = 2
good: usb 1-2: [11] SU [PCM Capture Source] items = 2

-- 
Mauro Santos

[-- Attachment #2: amixer_good.txt --]
[-- Type: text/plain, Size: 600 bytes --]

Simple mixer control 'PCM',0
  Capabilities: pvolume pswitch pswitch-joined
  Playback channels: Front Left - Front Right
  Limits: Playback 0 - 110
  Mono:
  Front Left: Playback 0 [0%] [-55.00dB] [on]
  Front Right: Playback 0 [0%] [-55.00dB] [on]
Simple mixer control 'PCM Capture Source',0
  Capabilities: enum
  Items: 'Line' 'IEC958 In'
  Item0: 'Line'
Simple mixer control 'Line',0
  Capabilities: cvolume cswitch cswitch-joined
  Capture channels: Front Left - Front Right
  Limits: Capture 0 - 104
  Front Left: Capture 0 [0%] [-40.00dB] [off]
  Front Right: Capture 0 [0%] [-40.00dB] [off]

[-- Attachment #3: dmesg_good.txt --]
[-- Type: text/plain, Size: 657 bytes --]

[   63.600402] usb 1-2: new full-speed USB device number 7 using xhci_hcd
[   63.723739] usb 1-2: device descriptor read/64, error -71
[   64.057506] input: HiFimeDIY Audio HiFimeDIY DAC as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:1852:7022.0003/input/input20
[   64.057780] hid-generic 0003:1852:7022.0003: input,hidraw2: USB HID v1.00 Device [HiFimeDIY Audio HiFimeDIY DAC] on usb-0000:00:14.0-2/input0
[   64.154091] usb 1-2: [DEBUG] nameid:0, len:0
[   64.154095] usb 1-2: [DEBUG] len:3, get_term_name:PCM
[   64.154097] usb 1-2: [11] SU [PCM Capture Source] items = 2
[   64.154495] usbcore: registered new interface driver snd-usb-audio

[-- Attachment #4: revert_with_debug.patch --]
[-- Type: text/x-patch, Size: 2296 bytes --]

diff -ur a/sound/usb/mixer.c b/sound/usb/mixer.c
--- a/sound/usb/mixer.c	2017-12-18 19:47:02.748776502 +0000
+++ b/sound/usb/mixer.c	2017-12-18 20:18:30.023770892 +0000
@@ -595,7 +595,7 @@
 	while (snd_ctl_find_id(mixer->chip->card, &kctl->id))
 		kctl->id.index++;
 	if ((err = snd_ctl_add(mixer->chip->card, kctl)) < 0) {
-		usb_audio_dbg(mixer->chip, "cannot add control (err = %d)\n",
+		usb_audio_err(mixer->chip, "cannot add control (err = %d)\n",
 			      err);
 		return err;
 	}
@@ -656,10 +656,14 @@
 			 unsigned char *name, int maxlen, int term_only)
 {
 	struct iterm_name_combo *names;
+    int len;
 
-	if (iterm->name)
-		return snd_usb_copy_string_desc(state, iterm->name,
+	if (iterm->name) {
+		len = snd_usb_copy_string_desc(state, iterm->name,
 						name, maxlen);
+        usb_audio_err(state->chip, "[DEBUG] len:%d, iterm->name:%d\n", len, iterm->name);
+        return len;
+    }
 
 	/* virtual type - not a real terminal */
 	if (iterm->type >> 16) {
@@ -2175,17 +2179,24 @@
 
 	nameid = uac_selector_unit_iSelector(desc);
 	len = check_mapped_name(map, kctl->id.name, sizeof(kctl->id.name));
+
+    usb_audio_err(state->chip, "[DEBUG] nameid:%d, len:%d\n", nameid, 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
+        usb_audio_err(state->chip, "[DEBUG] len:%d, copy_string id.name:%s\n",
+                    len, (len > 0) ? kctl->id.name : " ");
+    }
+	else {
 		len = get_term_name(state, &state->oterm,
 				    kctl->id.name, sizeof(kctl->id.name), 0);
-
-	if (!len) {
-		strlcpy(kctl->id.name, "USB", sizeof(kctl->id.name));
+        usb_audio_err(state->chip, "[DEBUG] len:%d, get_term_name:%s\n",
+                    len, (len > 0) ? kctl->id.name : " ");
+		if (!len)
+			strlcpy(kctl->id.name, "USB", sizeof(kctl->id.name));
 
 		if (desc->bDescriptorSubtype == UAC2_CLOCK_SELECTOR)
 			append_ctl_name(kctl, " Clock Source");
@@ -2195,7 +2206,7 @@
 			append_ctl_name(kctl, " Playback Source");
 	}
 
-	usb_audio_dbg(state->chip, "[%d] SU [%s] items = %d\n",
+	usb_audio_err(state->chip, "[%d] SU [%s] items = %d\n",
 		    cval->head.id, kctl->id.name, desc->bNrInPins);
 	return snd_usb_mixer_add_control(&cval->head, kctl);
 }

[-- Attachment #5: amixer_bad.txt --]
[-- Type: text/plain, Size: 382 bytes --]

Simple mixer control 'PCM',0
  Capabilities: pvolume pswitch pswitch-joined enum
  Items: 'Line' 'IEC958 In'
  Item0: 'Line'
  Item1: 'Line'
Simple mixer control 'Line',0
  Capabilities: cvolume cswitch cswitch-joined
  Capture channels: Front Left - Front Right
  Limits: Capture 0 - 104
  Front Left: Capture 0 [0%] [-40.00dB] [off]
  Front Right: Capture 0 [0%] [-40.00dB] [off]

[-- Attachment #6: dmesg_bad.txt --]
[-- Type: text/plain, Size: 642 bytes --]

[   56.317038] usb 1-2: new full-speed USB device number 7 using xhci_hcd
[   56.440424] usb 1-2: device descriptor read/64, error -71
[   56.775327] input: HiFimeDIY Audio HiFimeDIY DAC as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:1852:7022.0003/input/input20
[   56.775689] hid-generic 0003:1852:7022.0003: input,hidraw2: USB HID v1.00 Device [HiFimeDIY Audio HiFimeDIY DAC] on usb-0000:00:14.0-2/input0
[   56.876892] usb 1-2: [DEBUG] nameid:0, len:0
[   56.876896] usb 1-2: [DEBUG] len:3, get_term_name:PCM
[   56.876900] usb 1-2: [11] SU [PCM] items = 2
[   56.877392] usbcore: registered new interface driver snd-usb-audio

             reply	other threads:[~2017-12-18 21:56 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-18 21:56 Mauro Santos [this message]
2017-12-18 21:56 ` Mixer regression with usb soundcard Mauro Santos
  -- strict thread matches above, loose matches on Subject: below --
2017-12-19  6:43 Takashi Iwai
2017-12-19  6:43 ` Takashi Iwai
2017-12-19  2:34 Jaejoong Kim
2017-12-19  2:34 ` Jaejoong Kim
2017-12-19  1:04 Mauro Santos
2017-12-19  1:04 ` Mauro Santos
2017-12-18 22:48 Takashi Iwai
2017-12-18 22:48 ` Takashi Iwai
2017-12-18 22:10 Mauro Santos
2017-12-18 22:10 ` Mauro Santos
2017-12-18 19:30 Takashi Iwai
2017-12-18 19:30 ` Takashi Iwai
2017-12-18 19:10 Mauro Santos
2017-12-18 19:10 ` Mauro Santos
2017-12-18 18:18 Mauro Santos
2017-12-18 18:18 ` Mauro Santos
2017-12-18 17:50 Jaejoong Kim
2017-12-18 17:50 ` Jaejoong Kim
2017-12-18 17:19 Jaejoong Kim
2017-12-18 17:19 ` Jaejoong Kim
2017-12-18 17:13 Takashi Iwai
2017-12-18 17:13 ` Takashi Iwai
2017-12-18 17:11 Takashi Iwai
2017-12-18 17:11 ` Takashi Iwai
2017-12-18 17:05 Mauro Santos
2017-12-18 17:05 ` Mauro Santos
2017-12-18 16:59 Jaejoong Kim
2017-12-18 16:59 ` Jaejoong Kim
2017-12-18 15:45 Takashi Iwai
2017-12-18 15:45 ` Takashi Iwai
     [not found] <0f95a1cc-c438-ca4e-cc5f-d311e33a496e@gmail.com>
     [not found] ` <0f95a1cc-c438-ca4e-cc5f-d311e33a496e-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-18 13:44   ` Greg KH
     [not found]     ` <20171218134444.GA18133-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2017-12-18 13:53       ` Takashi Iwai
     [not found]         ` <s5hy3m0dtw0.wl-tiwai-l3A5Bk7waGM@public.gmane.org>
2017-12-18 15:30           ` Mauro Santos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ec393c6b-a960-5fb3-f98c-cb2120961d45@gmail.com \
    --to=registo.mailling@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=climbbb.kim@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.